From 3953a7ac7c0e38c7305233dadd664c9cba1adfff Mon Sep 17 00:00:00 2001 From: McStork Date: Tue, 5 Jan 2016 13:41:14 -0800 Subject: [PATCH] Address @urso review of DNS over TCP * Use printf format in all debug messages * Rename err in handleDecode * Remove a useless pointer assignement and move another one into a if statement --- packetbeat/protos/dns/dns.go | 10 +++++----- packetbeat/protos/dns/dns_tcp.go | 32 ++++++++++++++++---------------- packetbeat/protos/dns/dns_udp.go | 2 +- 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/packetbeat/protos/dns/dns.go b/packetbeat/protos/dns/dns.go index dad0fe864f4d..4c25bb901dc4 100644 --- a/packetbeat/protos/dns/dns.go +++ b/packetbeat/protos/dns/dns.go @@ -268,14 +268,14 @@ func (dns *Dns) ConnectionTimeout() time.Duration { } func (dns *Dns) receivedDnsRequest(tuple *DnsTuple, msg *DnsMessage) { - logp.Debug("dns", "Processing query. %s", tuple) + logp.Debug("dns", "Processing query. %s", tuple.String()) trans := dns.deleteTransaction(tuple.Hashable()) if trans != nil { // This happens if a client puts multiple requests in flight // with the same ID. trans.Notes = append(trans.Notes, DuplicateQueryMsg.Error()) - logp.Debug("dns", DuplicateQueryMsg.Error()+" %s", tuple) + logp.Debug("dns", "%s %s", DuplicateQueryMsg.Error(), tuple.String()) dns.publishTransaction(trans) dns.deleteTransaction(trans.tuple.Hashable()) } @@ -286,14 +286,14 @@ func (dns *Dns) receivedDnsRequest(tuple *DnsTuple, msg *DnsMessage) { } func (dns *Dns) receivedDnsResponse(tuple *DnsTuple, msg *DnsMessage) { - logp.Debug("dns", "Processing response. %s", tuple) + logp.Debug("dns", "Processing response. %s", tuple.String()) trans := dns.getTransaction(tuple.RevHashable()) if trans == nil { trans = newTransaction(msg.Ts, tuple.Reverse(), common.CmdlineTuple{ Src: msg.CmdlineTuple.Dst, Dst: msg.CmdlineTuple.Src}) trans.Notes = append(trans.Notes, OrphanedResponse.Error()) - logp.Debug("dns", OrphanedResponse.Error()+" %s", tuple) + logp.Debug("dns", "%s %s", OrphanedResponse.Error(), tuple.String()) } trans.Response = msg @@ -378,7 +378,7 @@ func (dns *Dns) publishTransaction(t *DnsTransaction) { func (dns *Dns) expireTransaction(t *DnsTransaction) { t.Notes = append(t.Notes, NoResponse.Error()) - logp.Debug("dns", NoResponse.Error()+" %s", t.tuple.String()) + logp.Debug("dns", "%s %s", NoResponse.Error(), t.tuple.String()) dns.publishTransaction(t) } diff --git a/packetbeat/protos/dns/dns_tcp.go b/packetbeat/protos/dns/dns_tcp.go index 291792f58d5a..55bb4048e10a 100644 --- a/packetbeat/protos/dns/dns_tcp.go +++ b/packetbeat/protos/dns/dns_tcp.go @@ -87,6 +87,7 @@ func (dns *Dns) doParse(conn *dnsConnectionData, pkt *protos.Packet, tcpTuple *c if stream == nil { stream = newStream(pkt, tcpTuple) + conn.Data[dir] = stream } else { if stream.message == nil { // nth message of the same stream stream.message = &DnsMessage{Ts: pkt.Ts, Tuple: pkt.Tuple} @@ -99,12 +100,9 @@ func (dns *Dns) doParse(conn *dnsConnectionData, pkt *protos.Packet, tcpTuple *c return conn } } - conn.Data[dir] = stream - decodedData, err := conn.Data[dir].handleTcpRawData() + decodedData, err := stream.handleTcpRawData() if err != nil { - logp.Debug("dns", err.Error()+" addresses %s, length %d", - tcpTuple.String(), len(stream.rawData)) if err == IncompleteMsg { logp.Debug("dns", "Waiting for more raw data") @@ -115,6 +113,9 @@ func (dns *Dns) doParse(conn *dnsConnectionData, pkt *protos.Packet, tcpTuple *c dns.publishResponseError(conn, err) } + logp.Debug("dns", "%s addresses %s, length %d", err.Error(), + tcpTuple.String(), len(stream.rawData)) + // This means that malformed requests or responses are being sent... // TODO: publish the situation also if Request conn.Data[dir] = nil @@ -122,7 +123,7 @@ func (dns *Dns) doParse(conn *dnsConnectionData, pkt *protos.Packet, tcpTuple *c } dns.messageComplete(conn, tcpTuple, dir, decodedData) - conn.Data[dir].PrepareForNewMessage() + stream.PrepareForNewMessage() return conn } @@ -175,20 +176,20 @@ func (dns *Dns) ReceivedFin(tcpTuple *common.TcpTuple, dir uint8, private protos return conn } - decodedData, err := conn.Data[dir].handleTcpRawData() + decodedData, err := stream.handleTcpRawData() if err == nil { dns.messageComplete(conn, tcpTuple, dir, decodedData) return conn } - logp.Debug("dns", err.Error()+" addresses %s, length %d", - tcpTuple.String(), len(stream.rawData)) - if dir == tcp.TcpDirectionReverse { dns.publishResponseError(conn, err) } + logp.Debug("dns", "%s addresses %s, length %d", err.Error(), + tcpTuple.String(), len(stream.rawData)) + return conn } @@ -206,7 +207,7 @@ func (dns *Dns) GapInStream(tcpTuple *common.TcpTuple, dir uint8, nbytes int, pr return private, false } - decodedData, err := conn.Data[dir].handleTcpRawData() + decodedData, err := stream.handleTcpRawData() if err == nil { dns.messageComplete(conn, tcpTuple, dir, decodedData) @@ -217,9 +218,8 @@ func (dns *Dns) GapInStream(tcpTuple *common.TcpTuple, dir uint8, nbytes int, pr dns.publishResponseError(conn, err) } - logp.Debug("dns", err.Error()+" addresses %s, length %d", + logp.Debug("dns", "%s addresses %s, length %d", err.Error(), tcpTuple.String(), len(stream.rawData)) - logp.Debug("dns", "Dropping the stream %s", tcpTuple.String()) // drop the stream because it is binary Data and it would be unexpected to have a decodable message later @@ -261,7 +261,7 @@ func (dns *Dns) publishResponseError(conn *dnsConnectionData, err error) { } // Manages data length prior to decoding the data and manages errors after decoding -func (stream *DnsStream) handleTcpRawData() (dns *layers.DNS, err error) { +func (stream *DnsStream) handleTcpRawData() (*layers.DNS, error) { rawData := stream.rawData messageLength := len(rawData) @@ -292,10 +292,10 @@ func (stream *DnsStream) handleTcpRawData() (dns *layers.DNS, err error) { return nil, IncompleteMsg } - decodedData, err_ := decodeDnsData(TransportTcp, rawData[:stream.parseOffset]) + decodedData, err := decodeDnsData(TransportTcp, rawData[:stream.parseOffset]) - if err_ != nil { - return nil, err_ + if err != nil { + return nil, err } return decodedData, nil diff --git a/packetbeat/protos/dns/dns_udp.go b/packetbeat/protos/dns/dns_udp.go index 4b693571f8af..c14c162f348a 100644 --- a/packetbeat/protos/dns/dns_udp.go +++ b/packetbeat/protos/dns/dns_udp.go @@ -18,7 +18,7 @@ func (dns *Dns) ParseUdp(pkt *protos.Packet) { // This means that malformed requests or responses are being sent or // that someone is attempting to the DNS port for non-DNS traffic. Both // are issues that a monitoring system should report. - logp.Debug("dns", err.Error()) + logp.Debug("dns", "%s", err.Error()) return }