Gather FQDN IPs using DNS lookups from previous chunks #676

Zalgo2462 · 2021-08-02T22:29:29Z

Fixes #669 by having the FQDN beaconing module extract all of the external hosts we saw in the parse results from the hostMap and query the hostnames collection to obtain the list of FQDNs we need to update along with all of the associated IPs.

Note: old FQDN -> IP mappings are removed from the hostnames collection by the remover package in the function pkg/remover/mongodb.go:removeOutdatedCIDs(cid int) error

…hecking which hostnames are associated with the external IPs we saw in the current import run. Move fqdnInput out of hostnames package and into beaconfqdn package. Add ./.vscode to gitignore (vscode dlv debugger creates launch profiles here)

Zalgo2462 · 2021-08-10T21:37:44Z

.gitignore

@@ -4,6 +4,7 @@
 *.swp
 *.swo
 *.exe
+/.vscode/


I used dlv while developing this PR. VSCode stores dlv configurations in .vscode. VSCode is free to use for both open source and commercial development.

Zalgo2462 · 2021-08-10T21:38:22Z

go.mod

@@ -12,14 +12,14 @@ require (
 	github.com/activecm/mgosec v0.1.1
 	github.com/activecm/rita-bl v0.0.0-20200806232046-0db4a39fcf49
 	github.com/blang/semver v3.5.1+incompatible
+	github.com/briandowns/spinner v1.16.0


While running the query which gathers the hostnames associated with the newly parsed in host IPs, we display a busy spinner.

Zalgo2462 · 2021-08-10T21:38:59Z

parser/fsimporter.go

@@ -199,7 +199,7 @@ func (fs *FSImporter) Run(indexedFiles []*fpt.IndexedFile) {
 		fs.buildBeacons(uconnMap)

 		// build or update the FQDN Beacons Table
-		fs.buildFQDNBeacons(hostnameMap)
+		fs.buildFQDNBeacons(hostMap)


FQDNBeacons now takes in the list of hosts in the current parse set rather than the list of DNS queries.

Zalgo2462 · 2021-08-10T21:39:39Z

pkg/beaconfqdn/analyzer.go

+		conf             *config.Config  // contains details needed to access MongoDB
+		analyzedCallback func(*update)   // called on each analyzed result
+		closedCallback   func()          // called when .close() is called and no more calls to analyzedCallback will be made
+		analysisChannel  chan *fqdnInput // holds unanalyzed data


fqdnInput has been moved out of the hostname package and into the beaconfqdn package since it is never used in the hostname package.

…mes methods

fullmetalcache

Just a few minor comments/discussion ideas and a small bug on the first pass. Tested on gcat set and it seems to produce the same FQDN results as before, which is good!

To further test, I broke the gcat set out into two folders. In the first folder, I removed all of the conn logs. This meant that the first import would get all of the DNS entries but wouldn't get any of the conn info. We will call the imported dataset "gcat-test". The second folder had just the conn logs. I imported the conn logs into the gcat-test set that was created with the first import. This meant that the second import would have to rely solely on the DNS entries from the first import for beacon FQDN analysis. After the first import, there were no beaconsFQDN results. After the second import, the number of beaconsFQDN results matched the number we had previously for the gcat sample-data set. I think this is good evidence that it's working.

Might want to do some quick tests on the amount of time that this adds on to the beacons FQDN processing. I know we've talked about it and have each looked at it, but at some point we will probably want to give everything in beacons FQDN yet another look over for any more tricks we can do to speed everything up.

fullmetalcache · 2021-08-23T18:29:45Z

pkg/beaconfqdn/mongodb.go

+// roughly less than 200,000 external hosts.
+func (r *repo) affectedHostnameIPsSimple(hostMap map[string]*host.Input) ([]hostnameIPs, error) {
+	// preallocate externalHosts slice assuming at least half of the observed hosts are external
+	var externalHosts []data.UniqueIP = make([]data.UniqueIP, 0, len(hostMap)/2)


Would there be a major disadvantage to just pre-allocating all of the space other than the additional space requirements?

Looking at the gcat dataset and using dlv, I see that len(hostMap) == 5211. The length of externalHosts == 5189 after going through the isLocal check.

On a large data set, I see len(hostMap) == 15121 and len(externalHosts) == 8490. That is a bit closer to the less than half mark, but will still result in externalHosts needed to grow.

The only disadvantage is RAM usage as you said.

I went with /2 because I took a guess at how slice allocation works and assumed that it doubled the size of the backing array when reallocation happens. It looks like that Go does actually double the size of the underlying slice when you append more items than the slice has capacity for: https://github.com/golang/go/blob/0a7f00ae239570d664488085f9c395919bc69066/src/runtime/slice.go#L164.

So, with /2, we guarantee that we will only need to re-allocate once in the worst case. I could add that in as a comment if you'd like.

I'm happy to change it to len(hostMap) however if you feel that best suits our needs.

Oh nope, if there is just one re-allocation then I think it's worth it to go minimal. I was just concerned if it was going to be re-allocating with every new entry. This way is great though if it just does the one.

Chill, I'll add the explanation above as a comment.

fullmetalcache · 2021-08-23T19:05:08Z

pkg/beaconfqdn/mongodb.go

@@ -117,9 +122,31 @@ func (r *repo) Upsert(hostnameMap map[string]*hostname.Input, minTimestamp, maxT
 		writerWorker.start()
 	}

+	s := spinner.New(spinner.CharSets[36], 200*time.Millisecond)
+	s.Prefix = "\t[-] Gathering FQDNs for Beacon Analysis ...\t"
+	s.Start()


Something seems to be just a bit off with moving from this spinner to the next spinner. I tested twice and see something like this each time:

I cannot reproduce this issue in my terminal. In my terminal, the second spinner overwrites the first one.
I'll try adding a println call after s.Stop(), but I'll need you to let me know whether it helped or not.

It could be an ESXi thing. I've had other weird terminal things happen too that nobody else experienced.

fullmetalcache · 2021-08-23T19:14:57Z

parser/fsimporter.go

@@ -575,7 +575,7 @@ func (fs *FSImporter) buildFQDNBeacons(hostnameMap map[string]*hostname.Input, m
 			}

 			// send uconns to beacon analysis


I know this comment is from before, but it needs updated to read "hostMap" instead of uconns

fullmetalcache · 2021-08-23T19:29:19Z

pkg/beaconfqdn/mongodb.go

+func (r *repo) affectedHostnameIPsChunked(hostMap map[string]*host.Input) ([]hostnameIPs, error) {
+	// preallocate externalHosts slice assuming at least half of the observed hosts are external
+	// util.Min ensures that we don't preallocate more than the maximum allowed number of hosts in a 16MB BSON document
+	var externalHosts []data.UniqueIP = make([]data.UniqueIP, 0, util.Min(200000, len(hostMap)/2))


Same comment here as above

fullmetalcache · 2021-08-23T19:34:07Z

pkg/beaconfqdn/mongodb.go

+UniqueIPs. Additionally, all of the IPs associated with each hostname are returned.
+*/
+func reverseDNSQueryWithIPs(uniqueIPs []data.UniqueIP) []bson.M {
+	var uniqueIPBsonSelectors []bson.M = make([]bson.M, 0, len(uniqueIPs))


Not a big deal and will just comment in this one place, but I don't think the "var" and data type aren't needed on the left with the "make" on the right.

fullmetalcache · 2021-08-23T19:35:43Z

pkg/beaconfqdn/mongodb.go

+	// and consuming more RAM
+
+	// affectedHostnameIPMap maps hostnames to their respective ResolvedIPs
+	var affectedHostnameIPMap map[string][]data.UniqueIP = make(map[string][]data.UniqueIP, len(hostMap)/10)


Maybe quick comment here explaining the /10 factor.

This assumes that there would be roughly 10 IPs per observed hostname in the dataset. I'm not sure that's a good assumption to be honest. I didn't want to pre-allocate with len(hostMap) since that would obviously be too big. But, I also didn't want to start from 0. Thoughts?

I think 10 is reasonable. Maybe just add a comment mentioning that?

Oh hang on though, this wouldn't be 10 IPs per host. This would be len(hostMap)/10 IPs per host.

len(hostMap)/10 per hostname* or rather #IPs/10 : #FQDNs.

So, this assumes that for every 10 IPs in the parsed in set of logs, there would be 1 FQDN.

Got it, was reading the make statement wrong!

Zalgo2462 · 2021-08-23T20:38:24Z

I've addressed the changes by adding comments, using short variable assignments, and adding a call to fmt.Println after stopping the spinner. Please let me know if you run into any further issues.

Might want to do some quick tests on the amount of time that this adds on to the beacons FQDN processing. I know we've talked about it and have each looked at it, but at some point we will probably want to give everything in beacons FQDN yet
another look over for any more tricks we can do to speed everything up.

In my testing, on small datasets, the additional query performed here cancelled out the performance gains in the FQDN Beaconing analysis made by PR #668. On large datasets, the time savings made in #668 outweighed the additional processing time required by this PR. However, FQDN beaconing still seems rather slow on large datasets. I believe that addressing issue #686 may help speed up the process as well. In general, I agree that in the future, time will need to be devoted towards speeding up the FQDN analysis specifically.

fullmetalcache · 2021-08-23T21:33:44Z

The println seems to help. That chart didn't seem to finish for me in either case. I am not sure that's it's worth holding things up for though.

fullmetalcache · 2021-08-23T21:35:25Z

Approving. The progress bar could very well be an issue just on my system. Up to you if you want to fix it. Otherwise, merge when you are ready!

Zalgo2462 · 2021-08-23T21:43:25Z

The println seems to help. That chart didn't seem to finish for me in either case. I am not sure that's it's worth holding things up for though.

In general, that chart won't finish out. Since we don't know know how long it will take, the animation just repeats itself until its done and when you call Stop() the animation stops where it was at.

Logan L added 2 commits August 2, 2021 16:19

linting fixes

de21a2a

Zalgo2462 mentioned this pull request Aug 10, 2021

Speed up RITA parser #654

Merged

Zalgo2462 commented Aug 10, 2021

View reviewed changes

Logan L added 5 commits August 10, 2021 19:13

Simplify affectedHostnameIPsChunked, fix a preallocation issue

6f04b39

added comment on preallocation of externalHosts in the affectedHostna…

70b3b63

…mes methods

comment formatting on reverseDNSQueryWithIPs

8ed2a81

Allow disk use for large reverseDNSQuery aggregation

e1e45b4

Merge branch 'master' into 669-fqdn-beacon-chunk-fix

2c37686

fullmetalcache requested changes Aug 23, 2021

View reviewed changes

Logan L and others added 3 commits August 23, 2021 14:28

Add requested comments, use short variable assignments

6cc6f0f

Add println after stopping the first fqdn spinner

f805641

Merge branch 'master' into 669-fqdn-beacon-chunk-fix

af44368

fullmetalcache approved these changes Aug 23, 2021

View reviewed changes

Zalgo2462 merged commit 8baf8e5 into master Aug 23, 2021

Zalgo2462 deleted the 669-fqdn-beacon-chunk-fix branch August 23, 2021 21:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gather FQDN IPs using DNS lookups from previous chunks #676

Gather FQDN IPs using DNS lookups from previous chunks #676

Zalgo2462 commented Aug 2, 2021

Zalgo2462 Aug 10, 2021

Zalgo2462 Aug 10, 2021

Zalgo2462 Aug 10, 2021

Zalgo2462 Aug 10, 2021

fullmetalcache left a comment

fullmetalcache Aug 23, 2021

Zalgo2462 Aug 23, 2021 •

edited

Loading

fullmetalcache Aug 23, 2021

Zalgo2462 Aug 23, 2021

fullmetalcache Aug 23, 2021

Zalgo2462 Aug 23, 2021

fullmetalcache Aug 23, 2021

fullmetalcache Aug 23, 2021

fullmetalcache Aug 23, 2021

fullmetalcache Aug 23, 2021

fullmetalcache Aug 23, 2021

Zalgo2462 Aug 23, 2021

fullmetalcache Aug 23, 2021

fullmetalcache Aug 23, 2021

Zalgo2462 Aug 23, 2021

fullmetalcache Aug 23, 2021

Zalgo2462 commented Aug 23, 2021

fullmetalcache commented Aug 23, 2021 •

edited

Loading

fullmetalcache commented Aug 23, 2021

Zalgo2462 commented Aug 23, 2021

		@@ -575,7 +575,7 @@ func (fs FSImporter) buildFQDNBeacons(hostnameMap map[string]hostname.Input, m
		}

		// send uconns to beacon analysis

Gather FQDN IPs using DNS lookups from previous chunks #676

Gather FQDN IPs using DNS lookups from previous chunks #676

Conversation

Zalgo2462 commented Aug 2, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fullmetalcache left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Zalgo2462 Aug 23, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Zalgo2462 commented Aug 23, 2021

fullmetalcache commented Aug 23, 2021 • edited Loading

fullmetalcache commented Aug 23, 2021

Zalgo2462 commented Aug 23, 2021

Zalgo2462 Aug 23, 2021 •

edited

Loading

fullmetalcache commented Aug 23, 2021 •

edited

Loading