Reduce the risk of misidentification of incoming traffic by verifying…

… destination hostname when resolving incoming internet traffic; Use `/proc/<pid>/root/etc/hostname` for IP to hostname resolution as an alternative to `/proc/<pid>/environ` (#244)

src/mapper/pkg/dnsintentspublisher/dns_intents_publisher.go

@@ -113,9 +113,11 @@ func (p *Publisher) updateResolvedIPs(ctx context.Context, clientIntents otteriz
 
 	updatedResolvedIPs := make([]otterizev2alpha1.ResolvedIPs, 0, len(resolvedIPsMap))
 	for dnsName, ips := range resolvedIPsMap {
+		ipSlice := lo.MapToSlice(ips, func(ip string, _ struct{}) string { return ip })
+		slices.Sort(ipSlice)
 		updatedResolvedIPs = append(updatedResolvedIPs, otterizev2alpha1.ResolvedIPs{
 			DNS: dnsName,
-			IPs: lo.MapToSlice(ips, func(ip string, _ struct{}) string { return ip }),
+			IPs: ipSlice,
 		})
 	}
 

src/mapper/pkg/dnsintentspublisher/dns_intents_publisher_test.go

@@ -298,8 +298,8 @@ func (s *PublisherTestSuite) TestAppendToOldIP() {
 		{
 			DNS: "my-blog.de",
 			IPs: []string{
-				oldIP,
 				IP1,
+				oldIP,
 			},
 		},
 	}

src/mapper/pkg/resolvers/resolver_test.go

@@ -1321,6 +1321,75 @@ func (s *ResolverTestSuite) TestTCPResultsFromExternalToPodSavedAsIncoming() {
 	s.Require().Equal("8.8.8.8", intents[0].Intent.IP)
 }
 
+func (s *ResolverTestSuite) TestTCPResultsFromExternalToPodSaveIfDestinationNameRight() {
+	// Create deployment
+	deploymentName := "coolz"
+	podIP := "1.1.1.3"
+	dep, pods := s.AddDeployment(deploymentName, []string{podIP}, map[string]string{"app": "coolz"})
+	s.Require().True(s.Mgr.GetCache().WaitForCacheSync(context.Background()))
+
+	// Report TCP results of traffic from external ip to pod
+	packetTime := time.Now().Add(time.Minute)
+	_, err := test_gql_client.ReportTCPCaptureResults(context.Background(), s.client, test_gql_client.CaptureTCPResults{
+		Results: []test_gql_client.RecordedDestinationsForSrc{
+			{
+				SrcIp: "8.8.8.8",
+				Destinations: []test_gql_client.Destination{
+					{
+						Destination:     pods[0].Name,
+						DestinationIP:   nilable.From(podIP),
+						DestinationPort: nilable.From(80),
+						LastSeen:        packetTime,
+					},
+				},
+			},
+		},
+	})
+	s.Require().NoError(err)
+
+	s.waitForCaptureResultsProcessed(10 * time.Second)
+
+	// Verify that the traffic from external ip to pod is saved as incoming
+	intents := s.resolver.incomingTrafficHolder.GetNewIntentsSinceLastGet()
+	s.Require().Len(intents, 1)
+	s.Require().Equal(dep.Name, intents[0].Intent.Server.Name)
+	s.Require().Equal(dep.Namespace, intents[0].Intent.Server.Namespace)
+	s.Require().Equal("8.8.8.8", intents[0].Intent.IP)
+}
+
+func (s *ResolverTestSuite) TestTCPResultsFromExternalToPodSkipIfDestinationNameWrong() {
+	// Create deployment
+	deploymentName := "coolz"
+	podIP := "1.1.1.3"
+	_, pods := s.AddDeployment(deploymentName, []string{podIP}, map[string]string{"app": "coolz"})
+	s.Require().True(s.Mgr.GetCache().WaitForCacheSync(context.Background()))
+
+	// Report TCP results of traffic from external ip to pod
+	packetTime := time.Now().Add(time.Minute)
+	_, err := test_gql_client.ReportTCPCaptureResults(context.Background(), s.client, test_gql_client.CaptureTCPResults{
+		Results: []test_gql_client.RecordedDestinationsForSrc{
+			{
+				SrcIp: "8.8.8.8",
+				Destinations: []test_gql_client.Destination{
+					{
+						Destination:     pods[0].Name + "-wrong",
+						DestinationIP:   nilable.From(podIP),
+						DestinationPort: nilable.From(80),
+						LastSeen:        packetTime,
+					},
+				},
+			},
+		},
+	})
+	s.Require().NoError(err)
+
+	s.waitForCaptureResultsProcessed(10 * time.Second)
+
+	// Verify that the traffic from external ip to pod is saved as incoming
+	intents := s.resolver.incomingTrafficHolder.GetNewIntentsSinceLastGet()
+	s.Require().Len(intents, 0)
+}
+
 func (s *ResolverTestSuite) TestTCPResultsFromExternalToLoadBalancerServiceUsingNodeIpAndPortSavedAsIncoming() {
 	// Create deployment
 	deploymentName := "coolz"
@@ -1392,34 +1461,6 @@ func (s *ResolverTestSuite) TestResolveOtterizeIdentityFilterSrcDestinationsByCr
 
 }
 
-func (s *ResolverTestSuite) TestPoop() {
-	//serviceIP := "10.0.0.10"
-	podIP := "1.1.1.3"
-
-	pod3 := s.AddPod("pod3", podIP, nil, nil)
-	//s.AddService(serviceName, map[string]string{"app": "test"}, serviceIP, []*v1.Pod{pod3})
-	s.Require().True(s.Mgr.GetCache().WaitForCacheSync(context.Background()))
-
-	pod := &v1.Pod{}
-	err := s.Mgr.GetClient().Get(context.Background(), types.NamespacedName{Name: pod3.Name, Namespace: pod3.Namespace}, pod)
-	s.Require().NoError(err)
-	podlist1 := &v1.PodList{}
-	err = s.Mgr.GetClient().List(context.Background(), podlist1, client.MatchingFields{"ip": pod.Status.PodIP})
-	s.Require().NoError(err)
-	s.Require().Len(podlist1.Items, 1)
-
-	err = s.Mgr.GetClient().Delete(context.Background(), pod)
-	s.Require().NoError(err)
-
-	s.Require().True(s.Mgr.GetCache().WaitForCacheSync(context.Background()))
-
-	podlist := &v1.PodList{}
-	err = s.Mgr.GetClient().List(context.Background(), podlist, client.MatchingFields{"ip": pod.Status.PodIP})
-	s.Require().NoError(err)
-	s.Require().Empty(podlist.Items)
-
-}
-
 func TestRunSuite(t *testing.T) {
 	suite.Run(t, new(ResolverTestSuite))
 }

src/mapper/pkg/resolvers/schema.helpers.resolvers.go

@@ -362,6 +362,18 @@ func (r *Resolver) resolveOtterizeIdentityForExternalAccessDestination(ctx conte
 			logrus.WithError(err).Debugf("Could not resolve %s to pod", destIP)
 			return model.OtterizeServiceIdentity{}, false, errors.Wrap(err)
 		}
+
+		if pod.CreationTimestamp.After(dest.LastSeen) {
+			logrus.Debugf("Pod %s was created after capture time %s, ignoring", pod.Name, dest.LastSeen)
+			return model.OtterizeServiceIdentity{}, false, nil
+		}
+
+		// If the sniffer sent a destination name, it should match the pod name.
+		if dest.Destination != "" && dest.Destination != destIP && dest.Destination != pod.Name {
+			logrus.Debugf("Destination %s (%s) has a different name than the pod %s, ignoring", destIP, dest.Destination, pod.Name)
+			return model.OtterizeServiceIdentity{}, false, nil
+		}
+
 		dstSvcIdentity, err := r.resolveInClusterIdentity(ctx, pod)
 		if err != nil {
 			return model.OtterizeServiceIdentity{}, false, errors.Wrap(err)

src/sniffer/pkg/collectors/dnssniffer.go

@@ -146,8 +146,8 @@ func (s *DNSSniffer) HandlePacket(packet gopacket.Packet) {
 					s.addCapturedRequest(ip.DstIP.String(), "", hostName, answer.IP.String(), captureTime, nilable.From(int(answer.TTL)), nil)
 					continue
 				}
-				hostname, err := s.resolver.ResolveIP(ip.DstIP.String())
-				if err != nil {
+				hostname, ok := s.resolver.ResolveIP(ip.DstIP.String())
+				if !ok {
 					logrus.Debugf("Can't resolve IP addr %s, skipping", ip.DstIP.String())
 				} else {
 					// Resolver cache could be outdated, verify same resolving result after next poll
@@ -194,8 +194,8 @@ func (s *DNSSniffer) RefreshHostsMapping() error {
 	}
 
 	for _, p := range s.pending {
-		hostname, err := s.resolver.ResolveIP(p.srcIp)
-		if err != nil {
+		hostname, ok := s.resolver.ResolveIP(p.srcIp)
+		if !ok {
 			logrus.WithError(err).Debugf("Could not to resolve %s, skipping packet", p.srcIp)
 			continue
 		}

src/sniffer/pkg/collectors/tcpsniffer.go

@@ -105,11 +105,15 @@ func (s *TCPSniffer) HandlePacket(packet gopacket.Packet) {
 		return
 	}
 
-	localHostname, err := s.resolver.ResolveIP(srcIP)
-	if err != nil {
-		logrus.Debugf("Can't resolve IP addr %s, sending IP only", srcIP)
+	localHostname, ok := s.resolver.ResolveIP(srcIP)
+	if !ok {
 		// This is still reported because might be ingress traffic, mapper would drop non-ingress captures with no src hostname
-		s.addCapturedRequest(srcIP, "", dstIP, dstIP, captureTime, nilable.FromPtr[int](nil), &dstPort)
+		destNameOrIP := dstIP
+		destHostname, ok := s.resolver.ResolveIP(dstIP)
+		if ok {
+			destNameOrIP = destHostname
+		}
+		s.addCapturedRequest(srcIP, "", destNameOrIP, dstIP, captureTime, nilable.FromPtr[int](nil), &dstPort)
 		return
 	}
 
@@ -161,8 +165,8 @@ func (s *TCPSniffer) RefreshHostsMapping() error {
 	}
 
 	for _, p := range s.pending {
-		hostname, err := s.resolver.ResolveIP(p.srcIp)
-		if err != nil {
+		hostname, ok := s.resolver.ResolveIP(p.srcIp)
+		if !ok {
 			logrus.WithError(err).Debugf("Could not to resolve %s, skipping packet", p.srcIp)
 			continue
 		}

src/sniffer/pkg/collectors/tcpsniffer_test.go

@@ -17,7 +17,7 @@ import (
 func TestTCPSniffer_TestHandlePacketAWS(t *testing.T) {
 	controller := gomock.NewController(t)
 	mockResolver := ipresolver.NewMockIPResolver(controller)
-	mockResolver.EXPECT().ResolveIP("10.0.2.48").Return("client-1", nil).Times(2) // once for the initial check, and then another for verification
+	mockResolver.EXPECT().ResolveIP("10.0.2.48").Return("client-1", true).Times(2) // once for the initial check, and then another for verification
 	mockResolver.EXPECT().Refresh().Return(nil).Times(1)
 
 	sniffer := NewTCPSniffer(mockResolver, true)

src/sniffer/pkg/config/config.go

@@ -16,6 +16,8 @@ const (
 	PacketsBufferLengthDefault         = 4096
 	HostsMappingRefreshIntervalKey     = "hosts-mapping-refresh-interval"
 	HostsMappingRefreshIntervalDefault = 500 * time.Millisecond
+	UseExtendedProcfsResolutionKey     = "use-extended-procfs-resolution"
+	UseExtendedProcfsResolutionDefault = false
 )
 
 func init() {
@@ -24,4 +26,5 @@ func init() {
 	viper.SetDefault(CallsTimeoutKey, CallsTimeoutDefault)
 	viper.SetDefault(HostProcDirKey, HostProcDirDefault)
 	viper.SetDefault(HostsMappingRefreshIntervalKey, HostsMappingRefreshIntervalDefault)
+	viper.SetDefault(UseExtendedProcfsResolutionKey, UseExtendedProcfsResolutionDefault)
 }

src/sniffer/pkg/ipresolver/ipresolver.go

@@ -7,7 +7,7 @@ import (
 
 type IPResolver interface {
 	Refresh() error
-	ResolveIP(ipaddr string) (hostname string, err error)
+	ResolveIP(ipaddr string) (hostname string, ok bool)
 }
 
 // NewMockIPResolver creates a new mock instance.
@@ -48,11 +48,11 @@ func (mr *MockIPResolverMockRecorder) Refresh() *gomock.Call {
 }
 
 // ResolveIP mocks base method.
-func (m *MockIPResolver) ResolveIP(ipaddr string) (string, error) {
+func (m *MockIPResolver) ResolveIP(ipaddr string) (string, bool) {
 	m.ctrl.T.Helper()
 	ret := m.ctrl.Call(m, "ResolveIP", ipaddr)
 	ret0, _ := ret[0].(string)
-	ret1, _ := ret[1].(error)
+	ret1, _ := ret[1].(bool)
 	return ret0, ret1
 }
 

src/sniffer/pkg/ipresolver/procfs_resolver.go

@@ -29,11 +29,11 @@ func NewProcFSIPResolver() *ProcFSIPResolver {
 	return &r
 }
 
-func (r *ProcFSIPResolver) ResolveIP(ipaddr string) (hostname string, err error) {
+func (r *ProcFSIPResolver) ResolveIP(ipaddr string) (hostname string, ok bool) {
 	if hostInfo, ok := r.byAddr[ipaddr]; ok {
-		return hostInfo.Hostname, nil
+		return hostInfo.Hostname, true
 	}
-	return "", errors.New("ip not found")
+	return "", false
 }
 
 func (r *ProcFSIPResolver) Refresh() error {

src/sniffer/pkg/ipresolver/procfs_resolver_test.go

@@ -101,78 +101,78 @@ func (s *ProcFSIPResolverTestSuite) TestResolverSimple() {
 	s.mockCreateProcess(10, "172.17.0.1", "service-1")
 	_ = s.resolver.Refresh()
 
-	hostname, err := s.resolver.ResolveIP("172.17.0.1")
-	s.Require().NoError(err)
+	hostname, ok := s.resolver.ResolveIP("172.17.0.1")
+	s.Require().True(ok)
 	s.Require().Equal("service-1", hostname)
 
 	s.mockKillProcess(10)
 	_ = s.resolver.Refresh()
 
-	hostname, err = s.resolver.ResolveIP("172.17.0.1")
-	s.Require().ErrorContains(err, "not found")
+	hostname, ok = s.resolver.ResolveIP("172.17.0.1")
+	s.Require().False(ok)
 	s.Require().Equal(hostname, "")
 }
 
 func (s *ProcFSIPResolverTestSuite) TestResolverRefCount() {
 	s.mockCreateProcess(20, "172.17.0.2", "service-2")
 	_ = s.resolver.Refresh()
 
-	hostname, err := s.resolver.ResolveIP("172.17.0.2")
-	s.Require().NoError(err)
+	hostname, ok := s.resolver.ResolveIP("172.17.0.2")
+	s.Require().True(ok)
 	s.Require().Equal("service-2", hostname)
 
 	s.mockCreateProcess(21, "172.17.0.2", "service-2")
 	s.mockCreateProcess(22, "172.17.0.2", "service-2")
 	_ = s.resolver.Refresh()
 
-	hostname, err = s.resolver.ResolveIP("172.17.0.2")
-	s.Require().NoError(err)
+	hostname, ok = s.resolver.ResolveIP("172.17.0.2")
+	s.Require().True(ok)
 	s.Require().Equal("service-2", hostname)
 
 	s.mockKillProcess(20)
 	s.mockKillProcess(22)
 	_ = s.resolver.Refresh()
 
-	hostname, err = s.resolver.ResolveIP("172.17.0.2")
-	s.Require().NoError(err)
+	hostname, ok = s.resolver.ResolveIP("172.17.0.2")
+	s.Require().True(ok)
 	s.Require().Equal("service-2", hostname)
 
 	s.mockKillProcess(21)
 	_ = s.resolver.Refresh()
 
-	hostname, err = s.resolver.ResolveIP("172.17.0.2")
-	s.Require().ErrorContains(err, "not found")
+	hostname, ok = s.resolver.ResolveIP("172.17.0.2")
+	s.Require().False(ok)
 	s.Require().Equal(hostname, "")
 }
 
 func (s *ProcFSIPResolverTestSuite) TestResolverCollision() {
 	s.mockCreateProcess(30, "172.17.0.3", "service-3")
 	_ = s.resolver.Refresh()
 
-	hostname, err := s.resolver.ResolveIP("172.17.0.3")
-	s.Require().NoError(err)
+	hostname, ok := s.resolver.ResolveIP("172.17.0.3")
+	s.Require().True(ok)
 	s.Require().Equal("service-3", hostname)
 
 	s.mockCreateProcess(31, "172.17.0.3", "service-3-new")
 	_ = s.resolver.Refresh()
 
 	// Newer hostname should override older one
-	hostname, err = s.resolver.ResolveIP("172.17.0.3")
-	s.Require().NoError(err)
+	hostname, ok = s.resolver.ResolveIP("172.17.0.3")
+	s.Require().True(ok)
 	s.Require().Equal("service-3-new", hostname)
 
 	s.mockKillProcess(31)
 	_ = s.resolver.Refresh()
 
 	// Older process isn't counted into ProcRefCount, the exit of the new one should be enough to remove the entry
-	hostname, err = s.resolver.ResolveIP("172.17.0.3")
-	s.Require().ErrorContains(err, "not found")
+	hostname, ok = s.resolver.ResolveIP("172.17.0.3")
+	s.Require().False(ok)
 	s.Require().Equal(hostname, "")
 
 	s.mockKillProcess(30)
 	_ = s.resolver.Refresh()
-	hostname, err = s.resolver.ResolveIP("172.17.0.3")
-	s.Require().ErrorContains(err, "not found")
+	hostname, ok = s.resolver.ResolveIP("172.17.0.3")
+	s.Require().False(ok)
 	s.Require().Equal(hostname, "")
 }