-
Notifications
You must be signed in to change notification settings - Fork 171
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
TESTING: Enrich dns indicators #112
Conversation
So I think the only issue now is that we need to be a little more forgiving on the dates - probably going back in time an extra 24 hours. I'm really wishing I'd already done #35 now. :P |
I'm going to spend some time on this as soon as #121 is stable |
man I really need to set up a smaller data set for this. |
test data set created, and there are some improvements already. the real issue now is adding the data correctly to the main result set. |
@alexcpsec this is ready for your testing! 🎉 |
as_num, as_name = org_by_addr(address) | ||
country = geo_data.country_code_by_addr('%s' % address) | ||
if dnsdb: | ||
hostname = maxhits(dnsdb.query_rdata_ip('%s' % address)) | ||
rhost = maxhits(dnsdb.query_rdata_ip('%s' % address)) |
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 is the first "logical issue" - we should be gathering the PTR records here, so it should actually be a dnsdb.query_rrset
using the inaddr.arpa
address of the IP address. I have changed the other of hostname
and rhost
variables to reflect what they are on the CSV so it can be clearer what is expected of this function.
We are going for maxhits
all right. It should be rare that there is more than 1 PTR record associated with an in-addr.arpa
. We might get a CNAME that points to a PTR, and we should just let the CNAME go in for now.
@alexcpsec I think this follows your logic correctly now and it seems to return the correct data in the test cases I examined. But you should make sure. |
👍 / 👎 ? |
Getting there |
This is 100% correct now. We still have query speed challenges on DNSDB, but that is another problem altogether. |
TESTING: Enrich dns indicators
🎉 |
Close #36