Merge pull request #3566 from tiriplicamihai/fix-dns-snooper-multiple-cnames

Fix dnssnooper probe for multiple CNAMEs.

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{}{}
+				break
+			}
 		}
 	}
 

probe/endpoint/dns_snooper_test.go

@@ -0,0 +1,65 @@
+// +build linux,amd64 linux,ppc64le
+
+package endpoint
+
+import (
+	"net"
+	"testing"
+
+	"github.com/bluele/gcache"
+	"github.com/google/gopacket/layers"
+)
+
+func TestProcessDNSMessageMultipleCNAME(t *testing.T) {
+	domain := "dummy.com"
+	question := layers.DNSQuestion{
+		Name:  []byte(domain),
+		Type:  layers.DNSTypeA,
+		Class: layers.DNSClassIN,
+	}
+
+	ipAddressCNAME := "127.0.0.1"
+	answers := []layers.DNSResourceRecord{
+		{
+			Name:  []byte("api.dummy.com"),
+			CNAME: []byte("api.dummy.com"),
+			Type:  layers.DNSTypeCNAME,
+			Class: layers.DNSClassIN,
+		},
+		{
+			Name:  []byte("star.c10r.dummy.com"),
+			CNAME: []byte("star.c10r.dummy.com"),
+			Type:  layers.DNSTypeCNAME,
+			Class: layers.DNSClassIN,
+		},
+		{
+			Name:  []byte("star.c10r.dummy.com"),
+			Type:  layers.DNSTypeA,
+			Class: layers.DNSClassIN,
+			IP:    net.ParseIP(ipAddressCNAME),
+		},
+	}
+
+	dns := layers.DNS{
+		QR:           true,
+		ResponseCode: layers.DNSResponseCodeNoErr,
+		Questions:    []layers.DNSQuestion{question},
+		Answers:      answers,
+	}
+
+	snooper := &DNSSnooper{
+		reverseDNSCache: gcache.New(4).LRU().Build(),
+	}
+
+	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)
+	}
+}