Skip to content
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

Added rrname_{ip,domain}, rdata_{ip,domain} fields #314

Merged
merged 3 commits into from May 3, 2021

Conversation

aaronkaplan
Copy link
Contributor

@aaronkaplan aaronkaplan commented May 2, 2021

... as discussed with @Rafiot this is unfortuantely needed for allowing correlation with other ip addresses or domains in other objects. See the detailed comment in the previous commit please for an explanation.

@adulau, I'd appreciate a quick merge so that I can proceed. Thx :)

BTW: no idea why validate.sh complains in git status --porcelain. I did a jq_all_the_things.sh before, and that works. It's valid JSON.

text only into MISP objects. Why? Because otherwise we can't use MISP's
correlation engine to correlate attributes (rrname, rdata) inside these
MISP objects with other events. Because "text" would not correlate with
other "ip-src" or "domain" types in other objects/attributes.

Kind of sucks to duplicate the rrname and rdata entries, but that's the
only solution we came up with.

The COF2MISP module will populate both the rrname,rdata as well as the
rrname_{domain,ip} and rdata_{domain,ip} attributes.

Checked with jq_all_the_things.sh.
Thanks for your consideration.
@adulau
Copy link
Member

adulau commented May 2, 2021

But the approach is to create a second object (ip/domain) and add a relationship. @Rafiot not sure I understand the issue.

@aaronkaplan
Copy link
Contributor Author

aaronkaplan commented May 2, 2021 via email

@Rafiot
Copy link
Member

Rafiot commented May 2, 2021

The issue is that the attribute type (on MISP) for rrname and rrdata is text. The correlation won't work if in an other event, we have the an attribute of type domain that would match an attribute of object_relation rrname in an object passive-dns.

@adulau
Copy link
Member

adulau commented May 2, 2021

Correlation works on the value. The type is disregarded.

@Rafiot
Copy link
Member

Rafiot commented May 2, 2021

Right. I thought it didn't. Never mind then, no need to change the template.

@aaronkaplan
Copy link
Contributor Author

aaronkaplan commented May 2, 2021 via email

@Rafiot
Copy link
Member

Rafiot commented May 2, 2021

it doesn't go down the drain, finding the type can simply be done on the consumer side: the MISP objects are created with the currently existing template, and when you want to use the data from MISP, you figure out the type.

Advantage: you can also use the objects created by other people.

When we discussed that, I understood that the correlation wasn't possible across different types on MISP, but correlation works. and as the passive DNS reference format doesn't enforce a type for rrname or rrdata, it is just plain confusing to add all the possible types in the template, especially since there are many different other types.

And I didn't realize, but the format gives the type: https://github.com/MISP/misp-modules/pull/491/files#diff-fd991138ae0c6034d46face71f07a15e814015d7fa8cd0ec5da32a40cff0a730R84
So when you get a MISPObject passive-dns you know the type to expect.

@aaronkaplan
Copy link
Contributor Author

aaronkaplan commented May 2, 2021 via email

aaronkaplan added a commit to aaronkaplan/misp-modules that referenced this pull request May 2, 2021
MISP#314

Removing *_ip and *_domain
Keeping bailiwick a domain type
@aaronkaplan
Copy link
Contributor Author

aaronkaplan commented May 2, 2021

Okay, so that's adapted to NOT need the rrname_ip, rrname_domain and rdata_ip, rdata_domain fields.
Bailiwick was changed to domain however, since it's always a domain.

please review, @adulau and @Rafiot if that fits you now.
If so, please merge and I adapted the other code in the misp_module to fit these definitions now.

Thx!!

PS: no idea why validate.sh fails. Since jq_all_the_things.sh works.

@adulau adulau merged commit b728ed3 into MISP:main May 3, 2021
@adulau
Copy link
Member

adulau commented May 3, 2021

Thanks a lot for the update!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants