tls/subject: use byte array instead of string

TLS parsers use x509-parser crate which parses X.509 certificates that
use ASN.1 DER encoding that can allow arbitrary byte sequences. An
attacker could inject null byte in a certificate anywhere to stump the
common language parsers terminating the string at a null byte leading to
a bypass of a possibly malicious certificate.

So far, the rust TLS parser for "Subject" used a pattern that involved:
-> Get ASN.1 DER encoded raw data from the x509-parser crate
-> Convert this raw data to a decoded string (Rust)
-> Convert the Rust string to CString
-- The problem lies here. CString only accepts proper strings/byte
buffers and converts it into an owned C-compatible, null-terminated
string. However, if any null byte occurs in the string passed to the
CString then it panics.
In the rust TLS parser, this panic is handled by returning NULL.

This means that the parser will error out during the decoding of the
certificate. However, Suricata must be able to detect the null byte
injection attack being an IDS/IPS.

Hence, replace all such string patterns w.r.t. TLS Subject with a byte
array.

Bug 7887

rust/src/x509/mod.rs

@@ -1,4 +1,4 @@
-/* Copyright (C) 2019-2020 Open Information Security Foundation
+/* Copyright (C) 2019-2025 Open Information Security Foundation
  *
  * You can copy, redistribute or modify this Program under the terms of
  * the GNU General Public License version 2 as published by the Free
@@ -85,13 +85,17 @@ pub unsafe extern "C" fn SCX509Decode(
 }
 
 #[no_mangle]
-pub unsafe extern "C" fn SCX509GetSubject(ptr: *const X509) -> *mut c_char {
+pub unsafe extern "C" fn SCX509GetSubject(ptr: *const X509, subject_name: *mut *mut u8, subject_len: *mut u32) {
     if ptr.is_null() {
-        return std::ptr::null_mut();
+        *subject_len = 0;
+        *subject_name = std::ptr::null_mut();
+        return;
     }
     let x509 = cast_pointer! {ptr, X509};
-    let subject = x509.0.tbs_certificate.subject.to_string();
-    rust_string_to_c(subject)
+    let subject = x509.0.tbs_certificate.subject().to_string().into_bytes();
+
+    *subject_len = subject.len() as u32;
+    *subject_name = Box::into_raw(subject.into_boxed_slice()) as *mut u8;
 }
 
 #[no_mangle]
@@ -182,6 +186,14 @@ pub unsafe extern "C" fn SCX509Free(ptr: *mut X509) {
     drop(Box::from_raw(ptr));
 }
 
+#[no_mangle]
+pub unsafe extern "C" fn SCX509ArrayFree(ptr: *mut u8, len: u32) {
+    if ptr.is_null() {
+        return;
+    }
+    drop(Box::from_raw(std::ptr::slice_from_raw_parts_mut(ptr, len as usize)));
+}
+
 fn x509_parse_error_to_errcode(e: &Err<X509Error>) -> X509DecodeError {
     match e {
         Err::Incomplete(_) => X509DecodeError::InvalidLength,

src/app-layer-ssl.c

@@ -490,14 +490,13 @@ static int TlsDecodeHSCertificate(SSLState *ssl_state, SSLStateConnp *connp,
             goto next;
         }
 
-        char *str = SCX509GetSubject(x509);
-        if (str == NULL) {
+        SCX509GetSubject(x509, &connp->cert0_subject, &connp->cert0_subject_len);
+        if (connp->cert0_subject == NULL) {
             err_code = ERR_EXTRACT_SUBJECT;
             goto error;
         }
-        connp->cert0_subject = str;
 
-        str = SCX509GetIssuer(x509);
+        char *str = SCX509GetIssuer(x509);
         if (str == NULL) {
             err_code = ERR_EXTRACT_ISSUER;
             goto error;
@@ -2855,7 +2854,8 @@ static void SSLStateFree(void *p)
     SSLCertsChain *item;
 
     if (ssl_state->client_connp.cert0_subject)
-        SCRustCStringFree(ssl_state->client_connp.cert0_subject);
+        SCX509ArrayFree(
+                ssl_state->client_connp.cert0_subject, ssl_state->client_connp.cert0_subject_len);
     if (ssl_state->client_connp.cert0_issuerdn)
         SCRustCStringFree(ssl_state->client_connp.cert0_issuerdn);
     if (ssl_state->client_connp.cert0_serial)
@@ -2870,7 +2870,8 @@ static void SSLStateFree(void *p)
         SCFree(ssl_state->client_connp.hs_buffer);
 
     if (ssl_state->server_connp.cert0_subject)
-        SCRustCStringFree(ssl_state->server_connp.cert0_subject);
+        SCX509ArrayFree(
+                ssl_state->server_connp.cert0_subject, ssl_state->server_connp.cert0_subject_len);
     if (ssl_state->server_connp.cert0_issuerdn)
         SCRustCStringFree(ssl_state->server_connp.cert0_issuerdn);
     if (ssl_state->server_connp.cert0_serial)

src/app-layer-ssl.h

@@ -184,9 +184,10 @@ typedef struct SSLStateConnp_ {
     uint16_t session_id_length;
 
     uint8_t random[TLS_RANDOM_LEN];
-    char *cert0_subject;
     char *cert0_issuerdn;
     char *cert0_serial;
+    uint8_t *cert0_subject;
+    uint32_t cert0_subject_len;
     int64_t cert0_not_before;
     int64_t cert0_not_after;
     char *cert0_fingerprint;

src/detect-tls-cert-subject.c

@@ -138,8 +138,8 @@ static InspectionBuffer *GetData(DetectEngineThreadCtx *det_ctx,
             return NULL;
         }
 
-        const uint32_t data_len = (uint32_t)strlen(connp->cert0_subject);
-        const uint8_t *data = (uint8_t *)connp->cert0_subject;
+        const uint32_t data_len = connp->cert0_subject_len;
+        const uint8_t *data = connp->cert0_subject;
 
         InspectionBufferSetupAndApplyTransforms(
                 det_ctx, list_id, buffer, data, data_len, transforms);

src/detect-tls.c

@@ -187,7 +187,9 @@ static int DetectTlsSubjectMatch (DetectEngineThreadCtx *det_ctx,
         SCLogDebug("TLS: Subject is [%s], looking for [%s]\n",
                    connp->cert0_subject, tls_data->subject);
 
-        if (strstr(connp->cert0_subject, tls_data->subject) != NULL) {
+        if (SpmSearch(connp->cert0_subject, connp->cert0_subject_len,
+                    (const uint8_t *)tls_data->subject,
+                    (uint16_t)strlen(tls_data->subject)) != NULL) {
             if (tls_data->flags & DETECT_CONTENT_NEGATED) {
                 ret = 0;
             } else {

src/log-tlsstore.c

@@ -234,11 +234,14 @@ static void LogTlsLogPem(LogTlsStoreLogThread *aft, const Packet *p, SSLState *s
                 goto end_fwrite_fpmeta;
         }
 
-        if (fprintf(fpmeta,
-                    "TLS SUBJECT:       %s\n"
-                    "TLS ISSUERDN:      %s\n"
-                    "TLS FINGERPRINT:   %s\n",
-                    connp->cert0_subject, connp->cert0_issuerdn, connp->cert0_fingerprint) < 0)
+        char *subject = CreateStringFromByteArray(connp->cert0_subject, connp->cert0_subject_len);
+        int r = fprintf(fpmeta,
+                "TLS SUBJECT:       %s\n"
+                "TLS ISSUERDN:      %s\n"
+                "TLS FINGERPRINT:   %s\n",
+                subject ? subject : "<ERROR>", connp->cert0_issuerdn, connp->cert0_fingerprint);
+        SCFree(subject);
+        if (r < 0)
             goto end_fwrite_fpmeta;
 
         fclose(fpmeta);

src/output-json-tls.c

@@ -132,7 +132,12 @@ typedef struct JsonTlsLogThread_ {
 static void JsonTlsLogSubject(SCJsonBuilder *js, SSLState *ssl_state)
 {
     if (ssl_state->server_connp.cert0_subject) {
-        SCJbSetString(js, "subject", ssl_state->server_connp.cert0_subject);
+        if (ssl_state->server_connp.cert0_subject_len == 0) {
+            SCJbSetString(js, "subject", "");
+        } else {
+            SCJbSetStringFromBytes(js, "subject", ssl_state->server_connp.cert0_subject,
+                    ssl_state->server_connp.cert0_subject_len);
+        }
     }
 }
 
@@ -338,7 +343,11 @@ static void JsonTlsLogClientCert(
         SCJsonBuilder *js, SSLStateConnp *connp, const bool log_cert, const bool log_chain)
 {
     if (connp->cert0_subject != NULL) {
-        SCJbSetString(js, "subject", connp->cert0_subject);
+        if (connp->cert0_subject_len == 0) {
+            SCJbSetString(js, "subject", "");
+        } else {
+            SCJbSetStringFromBytes(js, "subject", connp->cert0_subject, connp->cert0_subject_len);
+        }
     }
     if (connp->cert0_issuerdn != NULL) {
         SCJbSetString(js, "issuerdn", connp->cert0_issuerdn);

src/util-lua-tls.c

@@ -174,7 +174,7 @@ static int GetCertInfo(lua_State *luastate, bool client, const SSLState *ssl_sta
     SSLVersionToString(ssl_state->server_connp.version, ssl_version);
 
     int r = LuaPushStringBuffer(luastate, (uint8_t *)ssl_version, strlen(ssl_version));
-    r += LuaPushStringBuffer(luastate, (uint8_t *)connp->cert0_subject, strlen(connp->cert0_subject));
+    r += LuaPushStringBuffer(luastate, connp->cert0_subject, connp->cert0_subject_len);
     r += LuaPushStringBuffer(luastate, (uint8_t *)connp->cert0_issuerdn, strlen(connp->cert0_issuerdn));
     r += LuaPushStringBuffer(luastate, (uint8_t *)connp->cert0_fingerprint, strlen(connp->cert0_fingerprint));
     return r;