-
Notifications
You must be signed in to change notification settings - Fork 360
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New blacklist module. Fixes #48 #100
Conversation
mgo "gopkg.in/mgo.v2" | ||
) | ||
|
||
type resultsChan chan map[string][]blDB.BlacklistResult |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We chunk up the checks against RITA-BL. RITA-BL returns maps of (hosts or ip addresses or urls) to their results. The channel is used for communication.
analysis/blacklist/blacklist.go
Outdated
newList := lists.NewLineSeperatedList( | ||
entryType, | ||
path, | ||
86400, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cache time, could use a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comment would be very helpful :)
|
||
//provide a closure over path to read the file into a line separated blacklist | ||
func tryOpenFileThenURL(path string) func() (io.ReadCloser, error) { | ||
return func() (io.ReadCloser, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rita-bl expects a function which returns (io.ReadCloser, error) to get the data from.
ritaBL.SetRPCs(buildBlacklistRPCS(res)...) | ||
|
||
//update the lists | ||
ritaBL.Update() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line forces ritaBL to update its cache.
analysis/blacklist/blacklist.go
Outdated
//create the data | ||
//TODO: refactor these into modules | ||
bufferSize := 1000 | ||
buildBlacklistedIPs(uniqueSourceIter, res, ritaBL, bufferSize, true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
true is for source ip addresses, false if for destination ip addresses.
@@ -176,6 +183,12 @@ func expandConfig(reflected reflect.Value) { | |||
expandConfig(f) | |||
} else if f.Kind() == reflect.String { | |||
f.SetString(os.ExpandEnv(f.String())) | |||
} else if f.Kind() == reflect.Slice && f.Type().Elem().Kind() == reflect.String { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
expand lists
a862e7d
to
221487e
Compare
analysis/blacklist/blacklist.go
Outdated
package blacklist | ||
|
||
import ( | ||
bl "github.com/ocmdev/rita-blacklist2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At some point, we can probably just rename this rita-blacklist and get rid of the other one...
analysis/blacklist/blacklist.go
Outdated
//get our data sources | ||
ssn := res.DB.Session.Copy() | ||
defer ssn.Close() | ||
uniqueSourcesAggregation := getUniqueIPFromUconnPipeline("src") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a chance that the "src" and "dst" strings could ever change? Should they be variables?
analysis/blacklist/blacklist.go
Outdated
res.System.UrlsConfig.UrlsTable, | ||
).Find(nil).Iter() | ||
|
||
bufferSize := 1000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this something that could or should be in the config file?
analysis/blacklist/blacklist.go
Outdated
|
||
//create the data | ||
//TODO: refactor these into modules | ||
bufferSize := 1000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could or should this be a config option?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be a config option, but I don't see any reason why it should be changed. It is the amount of entries to check against rita-bl at a time. The chunking is mainly for rita-bl RPC's such as google safebrowsing. Some RPC's can take advantage of this chunked behavior when querying the internet. The proper thing to do here is probably change it to a constant in the blacklist package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, please make this a constant
analysis/blacklist/blacklist.go
Outdated
|
||
buildBlacklistedHostnames(hostnamesIter, res, ritaBL, bufferSize) | ||
|
||
buildBlacklistedURLs(urlIter, res, ritaBL, bufferSize, "http://") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens in the case of a different url prefix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So yeah. This is something I really struggled with. RITA-BL expects a prefix for its urls, and bro-ids does not, particularly since it places urls in files separated by protocol. Right now, we only pull urls from the http.log. The buildBlacklistedURLS function, however, is set up to take in a generic url iterator and protocol prefix. If we start reading the ftp log for instance, we can use buildBlacklistURLs(ftpIter, res, ritaBL, buffersize, "ftp://")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds good.
//So we can safely view buff as an array of strings using a | ||
//reinterpret cast. Then, we can dereference the pointer to the array | ||
//and use the variadic syntax to pass the array to CheckEntries. | ||
indexesArray := (*[]string)(unsafe.Pointer(&buff)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This structure is being treated as an array of strings, but if that ever changes, this will break. Will we ever need to add fields to this structure?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment above
for hostnames.Next(&buff[i]) { | ||
if i == bufferSize-1 { | ||
//see comment in checkRitaBlacklistIPs | ||
indexesArray := (*[]string)(unsafe.Pointer(&buff)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This structure is being treated as an array of strings, but if that ever changes, this will break. Will we ever need to add fields to this structure?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't foresee having to change this structure. In this piece of the code we are only checking hostnames. Perhaps hostnames may need to be represented by their individual labels for some reason in the future, but this line can be altered to match that.
Overall since this piece of code deals specifically with hostnames, I felt comfortable going with this solution.
(Same answer for ip addresses since RITA-BL treats both ip v4 and v6 addresses as strings)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds good.
CustomHostnameBlacklists: [] | ||
# URLs must each contain a protocol, a host, and a resource | ||
# Ex: http://google.com/ | ||
# Ex: ftp://myftpserver.com/a/file/over/here.txt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On this line, it looks like the url prefix is hardcoded to http... Will other prefixes still work and if so, how?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, we will never pull an ftp url out of the bro logs. In the future, we may read the bro ftp.log. We would prepend ftp:// to the urls in that file so as to match these blacklist entries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've got a few inline comments up, @Zalgo2462 could you please take a look and reply to my questions?
The previous blacklist module possessed some issues which involved mixing up ips, urls, and hostnames. Additionally, it had a hard time distinguishing between blacklisted hosts which initiated connections and received them. These kludges lead to difficulty in expanding the blacklisted module.
This new blacklist module accompanies a new rita blacklist backend. The new backend is meant to handle various types of blacklists, enforce the database abstraction, and provide support for blacklists that are managed by outside software. This has allowed the removal of the google safebrowsing code from RITA and the addition of the code to rita-bl. (Fixes issue #48)
The manner in which user defined lists has changed, as has the RITA config file.
User defined blacklists are expected to be simple line separated lists. These simple files are then registered with RITA in the config file.
Here is an example of a new blacklisted configuration section.
The new blacklist module brings with it four new commands.
The html output has been updated with a page for each of these commands.
Finally, crossref has been simplified to source and destination collections rather than internal/ external. The new blacklist information has been integrated into this new crossref configuration.