Remove Need for Users to Specify Proxy Servers, Fix Filter Bugs (#665)

* Remove requirement to specify proxy servers, fix internal->internal filter bug

* Further fix filtering

* remove dstIP filter check for proxies

* update comment

* Remove unsued variable and function

config/static.go

@@ -115,7 +115,6 @@ type (
 		AlwaysInclude       []string `yaml:"AlwaysInclude" default:"[]"`
 		NeverInclude        []string `yaml:"NeverInclude" default:"[\"0.0.0.0/32\", \"127.0.0.0/8\", \"169.254.0.0/16\", \"224.0.0.0/4\", \"255.255.255.255/32\", \"::1/128\", \"fe80::/10\", \"ff00::/8\"]"`
 		InternalSubnets     []string `yaml:"InternalSubnets" default:"[\"10.0.0.0/8\", \"172.16.0.0/12\", \"192.168.0.0/16\"]"`
-		HTTPProxyServers    []string `yaml:"HTTPProxyServers" default:"[]"`
 		AlwaysIncludeDomain []string `yaml:"AlwaysIncludeDomain" default:"[]"`
 		NeverIncludeDomain  []string `yaml:"NeverIncludeDomain" default:"[]"`
 	}

config/static_test.go

@@ -102,7 +102,6 @@ var testConfigFullExp = StaticCfg{
 		AlwaysInclude:       []string{"8.8.8.8/32"},
 		NeverInclude:        []string{"8.8.4.4/32"},
 		InternalSubnets:     []string{"10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16"},
-		HTTPProxyServers:    []string{"1.1.1.1", "1.1.1.2/32", "1.2.0.0/16"},
 		AlwaysIncludeDomain: []string{"bad.com", "google.com", "*.myotherdomain.com"},
 		NeverIncludeDomain:  []string{"good.com", "google.com", "*.mydomain.com"},
 	},

etc/rita.yaml

@@ -83,20 +83,6 @@ Filtering:
     - 172.16.0.0/12 # Private-Use Networks  RFC 1918
     - 192.168.0.0/16 # Private-Use Networks  RFC 1918
 
-  # Example: HTTPProxyServers: ["192.168.0.1"]
-  # If the environment uses one or more HTTP proxy servers, list those IP
-  # addresses here. In some cases, we can perform additional analysis
-  # when internal systems are contacting external systems with
-  # a proxy server as an intermediary. The additional processing allows
-  # for analyzing the traffic from the internal system to the final destination
-  # (e.g., 192.168.0.5 to somedomain.com).
-  # Without this processing, we will likely see internal systems beaconing 
-  # to the proxy server rather than getting results showing the internal 
-  # systems beaconing to an external system. Note that the efficacy
-  # of the analysis depends on the proxy server configuration, network
-  # topology, and deployment network sensors
-  HTTPProxyServers: []
-
   # Example: AlwaysIncludeDomain: ["mydomain.com","*.mydomain.com"]
   # This functionality overrides the NeverIncludeDomain 
   # section, making sure that any connection records containing domains

parser/filter.go

@@ -101,7 +101,3 @@ func (fs *FSImporter) filterDomain(domain string) bool {
 	// default to not filter the connection pair
 	return false
 }
-
-func (fs *FSImporter) checkIfProxyServer(host net.IP) bool {
-	return util.ContainsIP(fs.httpProxyServers, host)
-}

parser/filter_test.go

@@ -21,46 +21,12 @@ type testCaseDomain struct {
 	msg    string
 }
 
-type testCaseIsProxyIP struct {
-	ip  string
-	out bool
-	msg string
-}
-
 type testCaseSingleIP struct {
 	ip  string
 	out bool
 	msg string
 }
 
-func TestCheckIfProxyServer(t *testing.T) {
-
-	fsTest := &FSImporter{
-		res:              nil,
-		indexingThreads:  1,
-		parseThreads:     1,
-		httpProxyServers: util.ParseSubnets([]string{"1.1.1.1", "1.1.1.2/32", "1.2.0.0/16"}),
-	}
-
-	// all permutations for possible IP matches/non-matches
-	singleIPNoCIDRFiltered := "1.1.1.1"
-	singleIPCIDRFiltered := "1.1.1.2"
-	cidrRangeFiltered := "1.2.1.1"
-	singleIPNotFiltered := "1.3.1.1"
-
-	testCases := []testCaseIsProxyIP{
-		{singleIPNoCIDRFiltered, true, "IP should match single, non-CIDR notation Proxy IP entry"},
-		{singleIPCIDRFiltered, true, "IP should match CIDR notation (/32) for single Proxy IP entry"},
-		{cidrRangeFiltered, true, "IP should match CIDR notation (/16) for Proxy IP range entry"},
-		{singleIPNotFiltered, false, "IP should not match any Proxy IP entries"},
-	}
-
-	for _, test := range testCases {
-		output := fsTest.checkIfProxyServer(net.ParseIP(test.ip))
-		assert.Equal(t, test.out, output, test.msg)
-	}
-}
-
 func TestFilterConnPairWithInternalSubnets(t *testing.T) {
 
 	fsTest := &FSImporter{

parser/fsimporter.go

@@ -44,7 +44,6 @@ type (
 		parseThreads         int
 		batchSizeBytes       int64
 		internal             []*net.IPNet
-		httpProxyServers     []*net.IPNet
 		alwaysIncluded       []*net.IPNet
 		neverIncluded        []*net.IPNet
 		alwaysIncludedDomain []string
@@ -71,7 +70,6 @@ func NewFSImporter(res *resources.Resources,
 		parseThreads:         parseThreads,
 		batchSizeBytes:       2 * (2 << 30), // 2 gigabytes (used to not run out of memory while importing)
 		internal:             util.ParseSubnets(res.Config.S.Filtering.InternalSubnets),
-		httpProxyServers:     util.ParseSubnets(res.Config.S.Filtering.HTTPProxyServers),
 		alwaysIncluded:       util.ParseSubnets(res.Config.S.Filtering.AlwaysInclude),
 		neverIncluded:        util.ParseSubnets(res.Config.S.Filtering.NeverInclude),
 		alwaysIncludedDomain: res.Config.S.Filtering.AlwaysIncludeDomain,
@@ -674,7 +672,30 @@ func (fs *FSImporter) parseFiles(indexedFiles []*fpt.IndexedFile, parsingThreads
 							// parse host
 							fqdn := parseHTTP.Host
 
-							if fs.filterDomain(fqdn) || fs.filterConnPair(srcIP, dstIP) {
+							// parse method type
+							method := parseHTTP.Method
+
+							// check if destination is a proxy server based on HTTP method
+							dstIsProxy := (method == "CONNECT")
+
+							// if the HTTP method is CONNECT, then the srcIP is communicating
+							// to an FQDN through the dstIP proxy. We need to handle that
+							// as a special case here so that we don't filter internal->internal
+							// connections if the dstIP is an internal IP because the dstIP
+							// is an intermediary and not the final destination.
+							//
+							// The dstIP filter check is not included for proxy connections either
+							// because it isn't really the destination and I don't think that it makes
+							// sense in this context to check for it. If the proxy IP is external,
+							// this will also allow a user to filter results from other modules
+							// (e.g., beacons), where false positives might arise due to the proxy IP
+							// appearing as a destination, while still allowing for processing that
+							// data for the proxy modules
+							if dstIsProxy {
+								if fs.filterDomain(fqdn) || fs.filterSingleIP(srcIP) {
+									continue
+								}
+							} else if fs.filterDomain(fqdn) || fs.filterConnPair(srcIP, dstIP) {
 								continue
 							}
 
@@ -686,15 +707,9 @@ func (fs *FSImporter) parseFiles(indexedFiles []*fpt.IndexedFile, parsingThreads
 							// get aggregation keys for ip addresses and connection pair
 							srcProxyFQDNKey := srcProxyFQDNTrio.MapKey()
 
-							// check if destination is a proxy server
-							dstIsProxy := fs.checkIfProxyServer(dstIP)
-
-							// parse method type
-							method := parseHTTP.Method
-
 							// check if internal IP is requesting a connection
 							// through a proxy
-							if method == "CONNECT" && dstIsProxy {
+							if dstIsProxy {
 
 								// Safely store the number of conns for this uconn
 								mutex.Lock()