Add keyword to match on 'raw' TLS certificates #3545

thus · 2018-11-15T16:37:54Z

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;)

https://redmine.openinfosecfoundation.org/issues/2670

Prscript:

PR thus-pcap: https://buildbot.openinfosecfoundation.org/builders/thus-pcap/builds/177
PR thus: https://buildbot.openinfosecfoundation.org/builders/thus/builds/177

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;)

Rename buffer type for the old TLS keywords to avoid a name conflict with the 'tls_cert' keyword.

norg · 2018-11-18T00:21:15Z

doc/userguide/rules/tls-keywords.rst

+  alert tls any any -> any any (msg:"match bytes in TLS cert"; tls_cert; \
+    content:"|06 09 2a 86|"; sid:200070;)
+
+``tls_cert`` is a 'Sticky buffer'.


sticky instead of Sticky? :) but in general doc looks good

It is written as 'Sticky' on all the other keywords in the same file. Do you want me to lowercase the existing keywords as well? :)

I like the lowercase one better as well. If you want to update it elsewhere, please do.

victorjulien · 2019-02-08T15:24:14Z

src/detect-tls-cert.c

+}
+
+static InspectionBuffer *GetData(DetectEngineThreadCtx *det_ctx,
+        const DetectEngineTransforms *transforms, Flow *_f,


_f is a rust-ism, indicating that the variable isn't used. So seeing it being used as _f is kind of weird :)

I agree! I just copied that from some other code in the repo ;)
It's like that for the other mpm TLS keywords. Do you want me to change this for the other keywords as well?

Yes please.

victorjulien · 2019-02-08T15:27:19Z

src/detect-tls-cert.c

+    SigGroupBuild(de_ctx);
+    DetectEngineThreadCtxInit(&tv, (void *)de_ctx, (void *)&det_ctx);
+
+    FLOWLOCK_WRLOCK(&f);


you can remove the locks from these tests

Sure. Do you want me to remove them from the other mpm TLS keywords too? :)

Yeah I think that would be good.

victorjulien · 2019-02-08T15:27:34Z

src/detect-tls-cert.c

+    de_ctx->mpm_matcher = mpm_default_matcher;
+    de_ctx->flags |= DE_QUIET;
+
+    s = DetectEngineAppendSig(de_ctx, "alert tls any any -> any any "


declare s here

victorjulien · 2019-02-08T15:27:59Z

src/detect-tls-cert.c

+    FAIL_IF_NOT(PacketAlertCheck(p3, 1));
+    AppLayerParserThreadCtxFree(alp_tctx);
+    DetectEngineThreadCtxDeinit(&tv, det_ctx);
+    SigGroupCleanup(de_ctx);


this is called as well by DetectEngineCtxFree

I'll remove it then.

victorjulien · 2019-02-08T15:28:22Z

src/detect-tls.c

@@ -159,9 +159,9 @@ void DetectTlsRegister (void)
    DetectSetupParseRegexes(PARSE_REGEX_FINGERPRINT,
            &fingerprint_parse_regex, &fingerprint_parse_regex_study);

-    g_tls_cert_list_id = DetectBufferTypeRegister("tls_cert");
+    g_tls_cert_list_id = DetectBufferTypeRegister("tls_keywords_old");


I would suggest tls_legacy_keywords

I agree, that's a better name!

victorjulien · 2019-02-08T15:29:22Z

src/detect-tls-cert.c

+    return buffer;
+}
+
+#ifdef UNITTESTS


can you move the tests into their own file in tests/ ?

victorjulien · 2019-02-08T15:29:34Z

src/detect-tls-cert.c

+    InspectionBuffer *buffer = &det_ctx->inspect_buffers[list_id];
+
+    if (buffer->inspect == NULL) {
+        SSLState *ssl_state = (SSLState *)_f->alstate;


can this be const?

I'll change it.

victorjulien · 2019-02-08T15:29:40Z

src/detect-tls-cert.c

+            return NULL;
+        }
+
+        SSLCertsChain *cert = TAILQ_FIRST(&ssl_state->server_connp.certs);


I'll change it.

victorjulien · 2019-02-08T15:30:54Z

src/detect-tls-cert.c

+static int DetectTlsCertSetup(DetectEngineCtx *de_ctx, Signature *s,
+                              const char *str)
+{
+    DetectBufferSetActiveList(s, g_tls_cert_buffer_id);


check the return code here as well

This is probably something that is missing from a lot of other places too. So maybe you can check at least all the TLS ones for this?

victorjulien · 2019-02-08T15:31:26Z

src/detect-tls-cert.c

+    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";
+    sigmatch_table[DETECT_AL_TLS_CERT].Match = NULL;


remove the NULL settings please, they are redundant

thus · 2019-05-13T14:04:20Z

Replaced by #3851

thus added 3 commits November 15, 2018 17:17

detect: add tls_cert keyword

6bbb2b2

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;)

userguide: add documentation for tls_cert keyword

93d982f

detect-tls: rename buffer type to avoid conflict

b04fa57

Rename buffer type for the old TLS keywords to avoid a name conflict with the 'tls_cert' keyword.

thus requested review from norg and a team as code owners November 15, 2018 16:37

thus mentioned this pull request Nov 15, 2018

Add keyword to match on 'raw' TLS certificates #3542

Closed

norg reviewed Nov 18, 2018

View reviewed changes

victorjulien added this to the 5.0 milestone Nov 21, 2018

victorjulien requested changes Feb 8, 2019

View reviewed changes

victorjulien closed this Feb 22, 2019

This was referenced May 13, 2019

Add JA3S fingerprint support #3850

Closed

Add keyword to match on 'raw' TLS certificates #3851

Closed

thus mentioned this pull request May 14, 2019

TLS keyword cleanup #3853

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add keyword to match on 'raw' TLS certificates #3545

Add keyword to match on 'raw' TLS certificates #3545

thus commented Nov 15, 2018

norg Nov 18, 2018

thus Feb 17, 2019

victorjulien Feb 17, 2019

victorjulien Feb 8, 2019

thus Feb 17, 2019

victorjulien Feb 17, 2019

victorjulien Feb 8, 2019

thus Feb 17, 2019

victorjulien Feb 17, 2019

victorjulien Feb 8, 2019

thus Feb 17, 2019

victorjulien Feb 8, 2019

thus Feb 17, 2019

victorjulien Feb 8, 2019

thus Feb 17, 2019

victorjulien Feb 8, 2019

thus Feb 17, 2019

victorjulien Feb 8, 2019

thus Feb 17, 2019

victorjulien Feb 8, 2019

thus Feb 17, 2019

victorjulien Feb 8, 2019

thus Feb 17, 2019

victorjulien Feb 17, 2019

victorjulien Feb 8, 2019

thus Feb 17, 2019

thus commented May 13, 2019

Add keyword to match on 'raw' TLS certificates #3545

Add keyword to match on 'raw' TLS certificates #3545

Conversation

thus commented Nov 15, 2018

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

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

thus commented May 13, 2019