-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Add keyword to match on 'raw' TLS certificates #3851
Conversation
Add keyword to do "raw" matching on the TLS certificate buffer. Example: alert tls any any -> any any (msg:"tls.cert test"; tls.cert; \ content:"|01 02 03 04|"; sid:1;)
return NULL; | ||
} | ||
|
||
const SSLCertsChain *cert = TAILQ_FIRST(&ssl_state->server_connp.certs); |
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.
are we inspecting only one cert?
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.
Ok I see this is mentioned in the PR. Would it make sense to inspect all? Or would there be a case for a separate keyword for all certs or something?
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.
There's not really a good reason to only inspect one certificate. I'll change it to inspect the entire certificate chain :)
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.
Ok cool. This will complicate the inspection code somewhat, because you can now have multiple buffers per tx. I would suggest checking the dns query code which has the same issue.
{ | ||
sigmatch_table[DETECT_AL_TLS_CERT].name = "tls.cert"; | ||
sigmatch_table[DETECT_AL_TLS_CERT].desc = "content modifier to match the TLS certificate buffer"; | ||
sigmatch_table[DETECT_AL_TLS_CERT].url = DOC_URL DOC_VERSION "/rules/tls-keywords.html#tls.cert"; |
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.
Hm, this should perhaps be changed to "tls-cert" instead of "tls.cert".
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.
Agreed.
#endif | ||
static InspectionBuffer *GetData(DetectEngineThreadCtx *det_ctx, | ||
const DetectEngineTransforms *transforms, | ||
Flow *_f, const uint8_t _flow_flags, |
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.
Should remove the underscores from the variables here as well.
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.
Yeah if they are used then you should
Can you also add suricata-verify tests? |
Replaced by #3855 |
Add new keyword
tls.cert
to match on the 'raw' TLS certificate. This keyword matches on the first certificate in the certificate chain.Example:
alert tls any any -> any any (msg:"test tls.cert"; tls.cert; content:"|01 02 03 04|"; sid:1;)
Updates:
https://redmine.openinfosecfoundation.org/issues/2670
Prscript: