tls/subjectaltname: use byte arr 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 "subjectaltname" 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 SAN with a byte
array.

Bug 7887

Author: inashivb

rust/src/x509/mod.rs

@@ -19,10 +19,8 @@
 
 // written by Pierre Chifflier  <chifflier@wzdftpd.net>
 
-use crate::common::rust_string_to_c;
 use nom7::Err;
 use std;
-use std::os::raw::c_char;
 use std::fmt;
 use x509_parser::prelude::*;
 use crate::x509::GeneralName;
@@ -115,18 +113,21 @@ pub unsafe extern "C" fn SCX509GetSubjectAltNameLen(ptr: *const X509) -> u16 {
 }
 
 #[no_mangle]
-pub unsafe extern "C" fn SCX509GetSubjectAltNameAt(ptr: *const X509, idx: u16) -> *mut c_char {
+pub unsafe extern "C" fn SCX509GetSubjectAltNameAt(ptr: *const X509, idx: u16, san: *mut *mut u8, san_len: *mut u32) {
     if ptr.is_null() {
-        return std::ptr::null_mut();
+        *san_len = 0;
+        *san = std::ptr::null_mut();
+        return;
     }
     let x509 = cast_pointer! {ptr, X509};
     let san_list = x509.0.tbs_certificate.subject_alternative_name();
     if let Ok(Some(sans)) = san_list {
         let general_name = &sans.value.general_names[idx as usize];
         let dns_name = SCGeneralName(general_name);
-        return rust_string_to_c(dns_name.to_string());
+        let dn = dns_name.to_string().into_bytes();
+        *san_len = dn.len() as u32;
+        *san = Box::into_raw(dn.into_boxed_slice()) as *mut u8;
     }
-    return std::ptr::null_mut();
 }
 
 #[no_mangle]

src/app-layer-ssl.c

@@ -502,15 +502,15 @@ static int TlsDecodeHSCertificate(SSLState *ssl_state, SSLStateConnp *connp,
             goto error;
         }
 
-        connp->cert0_sans_len = SCX509GetSubjectAltNameLen(x509);
-        char **sans = SCCalloc(connp->cert0_sans_len, sizeof(char *));
-        if (sans == NULL) {
+        connp->cert0_sans_num = SCX509GetSubjectAltNameLen(x509);
+        connp->cert0_sans = SCCalloc(connp->cert0_sans_num, sizeof(SSLSubjectAltName));
+        if (connp->cert0_sans == NULL) {
             goto error;
         }
-        for (uint16_t i = 0; i < connp->cert0_sans_len; i++) {
-            sans[i] = SCX509GetSubjectAltNameAt(x509, i);
+        for (uint16_t i = 0; i < connp->cert0_sans_num; i++) {
+            SCX509GetSubjectAltNameAt(
+                    x509, i, &connp->cert0_sans[i].san, &connp->cert0_sans[i].san_len);
         }
-        connp->cert0_sans = sans;
 
         SCX509GetSerial(x509, &connp->cert0_serial, &connp->cert0_serial_len);
         if (connp->cert0_serial == NULL) {
@@ -2834,8 +2834,8 @@ static void *SSLStateAlloc(void *orig_state, AppProto proto_orig)
 static void SSLStateCertSANFree(SSLStateConnp *connp)
 {
     if (connp->cert0_sans) {
-        for (uint16_t i = 0; i < connp->cert0_sans_len; i++) {
-            SCRustCStringFree(connp->cert0_sans[i]);
+        for (uint16_t i = 0; i < connp->cert0_sans_num; i++) {
+            SCX509ArrayFree(connp->cert0_sans[i].san, connp->cert0_sans[i].san_len);
         }
         SCFree(connp->cert0_sans);
     }

src/app-layer-ssl.h

@@ -164,6 +164,11 @@ typedef struct SSLCertsChain_ {
     TAILQ_ENTRY(SSLCertsChain_) next;
 } SSLCertsChain;
 
+typedef struct SSLSubjectAltName_ {
+    uint8_t *san;
+    uint32_t san_len;
+} SSLSubjectAltName;
+
 typedef struct SSLStateConnp_ {
     /* record length */
     uint32_t record_length;
@@ -194,8 +199,8 @@ typedef struct SSLStateConnp_ {
     int64_t cert0_not_after;
     char *cert0_fingerprint;
 
-    char **cert0_sans;
-    uint16_t cert0_sans_len;
+    SSLSubjectAltName *cert0_sans;
+    uint16_t cert0_sans_num;
     /* ssl server name indication extension */
     uint8_t *sni;
     uint16_t sni_len;

src/detect-tls-subjectaltname.c

@@ -63,13 +63,13 @@ static bool TlsSubjectAltNameGetData(DetectEngineThreadCtx *det_ctx, const void
 
     connp = &ssl_state->server_connp;
 
-    if (idx >= connp->cert0_sans_len) {
+    if (idx >= connp->cert0_sans_num) {
         return false;
     }
 
-    *buf = (const uint8_t *)connp->cert0_sans[idx];
+    *buf = (const uint8_t *)connp->cert0_sans[idx].san;
     if (*buf) {
-        *buf_len = (uint32_t)strlen(connp->cert0_sans[idx]);
+        *buf_len = connp->cert0_sans[idx].san_len;
     } else {
         // happens if the altname had a zero character in it
         *buf_len = 0;

src/detect-tls.c

@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2011-2012 ANSSI
- * Copyright (C) 2022 Open Information Security Foundation
+ * Copyright (C) 2022-2025 Open Information Security Foundation
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without

src/output-json-tls.c

@@ -155,10 +155,11 @@ static void JsonTlsLogIssuer(SCJsonBuilder *js, SSLState *ssl_state)
 
 static void JsonTlsLogSAN(SCJsonBuilder *js, SSLState *ssl_state)
 {
-    if (ssl_state->server_connp.cert0_sans_len > 0) {
+    if (ssl_state->server_connp.cert0_sans_num > 0) {
         SCJbOpenArray(js, "subjectaltname");
-        for (uint16_t i = 0; i < ssl_state->server_connp.cert0_sans_len; i++) {
-            SCJbAppendString(js, ssl_state->server_connp.cert0_sans[i]);
+        for (uint16_t i = 0; i < ssl_state->server_connp.cert0_sans_num; i++) {
+            SCJbAppendStringFromBytes(js, ssl_state->server_connp.cert0_sans[i].san,
+                    ssl_state->server_connp.cert0_sans[i].san_len);
         }
         SCJbClose(js);
     }