From 1aadf5c6064d7008e3b94c23c5565c843a5f7fa5 Mon Sep 17 00:00:00 2001 From: tiriplicamihai Date: Mon, 28 Jan 2019 13:49:16 +0200 Subject: [PATCH 1/6] Fix dnssnooper probe for multiple CNAMEs. --- probe/endpoint/dns_snooper.go | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/probe/endpoint/dns_snooper.go b/probe/endpoint/dns_snooper.go index 197b838fb..f0d73122e 100644 --- a/probe/endpoint/dns_snooper.go +++ b/probe/endpoint/dns_snooper.go @@ -250,15 +250,14 @@ func (s *DNSSnooper) processDNSMessage(dns *layers.DNS) { domainQueried = question.Name records = append(dns.Answers, dns.Additionals...) ips = map[string]struct{}{} - alias []byte + aliases = [][]byte{} ) - // Traverse records for a CNAME first since the DNS RFCs don't seem to guarantee it - // appearing before its A-records + // Traverse all the CNAME records and the get the aliases. There are cases when the A record is for only one of the + // aliases. We traverse CNAME records first because there is no guarantee that the A records will be the first ones for _, record := range records { - if record.Type == layers.DNSTypeCNAME && record.Class == layers.DNSClassIN && bytes.Equal(domainQueried, record.Name) { - alias = record.CNAME - break + if record.Type == layers.DNSTypeCNAME && record.Class == layers.DNSClassIN { + aliases = append(aliases, record.CNAME) } } @@ -267,8 +266,15 @@ func (s *DNSSnooper) processDNSMessage(dns *layers.DNS) { if record.Type != layers.DNSTypeA || record.Class != layers.DNSClassIN { continue } - if bytes.Equal(domainQueried, record.Name) || (alias != nil && bytes.Equal(alias, record.Name)) { + if bytes.Equal(domainQueried, record.Name) { ips[record.IP.String()] = struct{}{} + continue + } + for _, alias := range aliases { + if bytes.Equal(alias, record.Name) { + ips[record.IP.String()] = struct{}{} + continue + } } } From a6bc6b014827682cd842142493d392ed2a98b95f Mon Sep 17 00:00:00 2001 From: tiriplicamihai Date: Mon, 28 Jan 2019 23:06:18 +0200 Subject: [PATCH 2/6] Use break instead of continue. --- probe/endpoint/dns_snooper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/probe/endpoint/dns_snooper.go b/probe/endpoint/dns_snooper.go index f0d73122e..c1151fb09 100644 --- a/probe/endpoint/dns_snooper.go +++ b/probe/endpoint/dns_snooper.go @@ -273,7 +273,7 @@ func (s *DNSSnooper) processDNSMessage(dns *layers.DNS) { for _, alias := range aliases { if bytes.Equal(alias, record.Name) { ips[record.IP.String()] = struct{}{} - continue + break } } } From 364a7423a526d4fe5d81be9f35d30aa83f85dbfb Mon Sep 17 00:00:00 2001 From: tiriplicamihai Date: Tue, 30 Apr 2019 18:38:07 +0300 Subject: [PATCH 3/6] Add tests for dns snooper. --- probe/endpoint/dns_snooper_test.go | 74 ++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 probe/endpoint/dns_snooper_test.go diff --git a/probe/endpoint/dns_snooper_test.go b/probe/endpoint/dns_snooper_test.go new file mode 100644 index 000000000..0dc9de156 --- /dev/null +++ b/probe/endpoint/dns_snooper_test.go @@ -0,0 +1,74 @@ +// +build linux,amd64 linux,ppc64le +package endpoint + +import ( + "net" + "testing" + + "github.com/google/gopacket/layers" +) + +func TestprocessDNSMessageMultipleCNAME(t *testing.T) { + domain := "dummy.com" + question := layers.DNSQuestion{ + Name: []byte(domain), + Type: layers.DNSTypeA, + } + + ipAddressCNAME := "127.0.0.1" + ipAddress := "127.0.1.1" + answers := []layers.DNSResourceRecord{ + layers.DNSResourceRecord{ + Name: []byte("api.dummy.com"), + Type: layers.DNSTypeCNAME, + Class: layers.DNSClassIN, + }, + layers.DNSResourceRecord{ + Name: []byte("star.c10r.dummy.com"), + Type: layers.DNSTypeCNAME, + Class: layers.DNSClassIN, + }, + layers.DNSResourceRecord{ + Name: []byte("dummy.com"), + Type: layers.DNSTypeA, + Class: layers.DNSClassIN, + IP: net.ParseIP(ipAddress), + }, + layers.DNSResourceRecord{ + Name: []byte("star.c10r.dummy.com"), + Type: layers.DNSTypeA, + Class: layers.DNSClassIN, + IP: net.ParseIP(ipAddressCNAME), + }, + } + + dns := layers.DNS{ + ResponseCode: layers.DNSResponseCodeNoErr, + Questions: []layers.DNSQuestion{question}, + Answers: answers, + } + + snooper := &DNSSnooper{} + + snooper.processDNSMessage(&dns) + + existingDomains, err := snooper.reverseDNSCache.Get(ipAddressCNAME) + + if err != nil { + t.Errorf("A domain should have been inserted for the given CNAME IP:%v", err) + } + + if _, ok := existingDomains.(map[string]struct{})[domain]; !ok { + t.Errorf("Domain %s should have been inserted", domain) + } + + existingDomains, err = snooper.reverseDNSCache.Get(ipAddress) + + if err != nil { + t.Errorf("A domain should have been inserted for the given IP:%v", err) + } + + if _, ok := existingDomains.(map[string]struct{})[domain]; !ok { + t.Errorf("Domain %s should have been inserted", domain) + } +} From 1fbe648e82239672d84de940bc29450c52c08aa0 Mon Sep 17 00:00:00 2001 From: tiriplicamihai Date: Tue, 30 Apr 2019 18:41:16 +0300 Subject: [PATCH 4/6] Add newline. --- probe/endpoint/dns_snooper_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/probe/endpoint/dns_snooper_test.go b/probe/endpoint/dns_snooper_test.go index 0dc9de156..a63782477 100644 --- a/probe/endpoint/dns_snooper_test.go +++ b/probe/endpoint/dns_snooper_test.go @@ -1,4 +1,5 @@ // +build linux,amd64 linux,ppc64le + package endpoint import ( From 92d3c1d5e9329d3952773c4383ccd3f9206fc716 Mon Sep 17 00:00:00 2001 From: tiriplicamihai Date: Tue, 30 Apr 2019 19:09:02 +0300 Subject: [PATCH 5/6] Fix test data. --- probe/endpoint/dns_snooper_test.go | 32 ++++++++++-------------------- 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/probe/endpoint/dns_snooper_test.go b/probe/endpoint/dns_snooper_test.go index a63782477..32528b749 100644 --- a/probe/endpoint/dns_snooper_test.go +++ b/probe/endpoint/dns_snooper_test.go @@ -6,35 +6,32 @@ import ( "net" "testing" + "github.com/bluele/gcache" "github.com/google/gopacket/layers" ) -func TestprocessDNSMessageMultipleCNAME(t *testing.T) { +func TestProcessDNSMessageMultipleCNAME(t *testing.T) { domain := "dummy.com" question := layers.DNSQuestion{ - Name: []byte(domain), - Type: layers.DNSTypeA, + Name: []byte(domain), + Type: layers.DNSTypeA, + Class: layers.DNSClassIN, } ipAddressCNAME := "127.0.0.1" - ipAddress := "127.0.1.1" answers := []layers.DNSResourceRecord{ layers.DNSResourceRecord{ Name: []byte("api.dummy.com"), + CNAME: []byte("api.dummy.com"), Type: layers.DNSTypeCNAME, Class: layers.DNSClassIN, }, layers.DNSResourceRecord{ Name: []byte("star.c10r.dummy.com"), + CNAME: []byte("star.c10r.dummy.com"), Type: layers.DNSTypeCNAME, Class: layers.DNSClassIN, }, - layers.DNSResourceRecord{ - Name: []byte("dummy.com"), - Type: layers.DNSTypeA, - Class: layers.DNSClassIN, - IP: net.ParseIP(ipAddress), - }, layers.DNSResourceRecord{ Name: []byte("star.c10r.dummy.com"), Type: layers.DNSTypeA, @@ -44,12 +41,15 @@ func TestprocessDNSMessageMultipleCNAME(t *testing.T) { } dns := layers.DNS{ + QR: true, ResponseCode: layers.DNSResponseCodeNoErr, Questions: []layers.DNSQuestion{question}, Answers: answers, } - snooper := &DNSSnooper{} + snooper := &DNSSnooper{ + reverseDNSCache: gcache.New(4).LRU().Build(), + } snooper.processDNSMessage(&dns) @@ -62,14 +62,4 @@ func TestprocessDNSMessageMultipleCNAME(t *testing.T) { if _, ok := existingDomains.(map[string]struct{})[domain]; !ok { t.Errorf("Domain %s should have been inserted", domain) } - - existingDomains, err = snooper.reverseDNSCache.Get(ipAddress) - - if err != nil { - t.Errorf("A domain should have been inserted for the given IP:%v", err) - } - - if _, ok := existingDomains.(map[string]struct{})[domain]; !ok { - t.Errorf("Domain %s should have been inserted", domain) - } } From 0b4e26ed771dc53c0e6ead1c20a4225ba5667a4f Mon Sep 17 00:00:00 2001 From: tiriplicamihai Date: Tue, 30 Apr 2019 19:24:23 +0300 Subject: [PATCH 6/6] Fix formatting. --- probe/endpoint/dns_snooper_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/probe/endpoint/dns_snooper_test.go b/probe/endpoint/dns_snooper_test.go index 32528b749..ade8e0a6e 100644 --- a/probe/endpoint/dns_snooper_test.go +++ b/probe/endpoint/dns_snooper_test.go @@ -20,19 +20,19 @@ func TestProcessDNSMessageMultipleCNAME(t *testing.T) { ipAddressCNAME := "127.0.0.1" answers := []layers.DNSResourceRecord{ - layers.DNSResourceRecord{ + { Name: []byte("api.dummy.com"), CNAME: []byte("api.dummy.com"), Type: layers.DNSTypeCNAME, Class: layers.DNSClassIN, }, - layers.DNSResourceRecord{ + { Name: []byte("star.c10r.dummy.com"), CNAME: []byte("star.c10r.dummy.com"), Type: layers.DNSTypeCNAME, Class: layers.DNSClassIN, }, - layers.DNSResourceRecord{ + { Name: []byte("star.c10r.dummy.com"), Type: layers.DNSTypeA, Class: layers.DNSClassIN,