tls/issuerdn: 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 "issuerdn" 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 IssuerDN with a byte
array.

Bug 7887
pull/14564/head
Shivani Bhardwaj 6 months ago committed by Victor Julien
parent 77c21b05d2
commit f025e07191

@ -130,13 +130,17 @@ pub unsafe extern "C" fn SCX509GetSubjectAltNameAt(ptr: *const X509, idx: u16) -
}
#[no_mangle]
pub unsafe extern "C" fn SCX509GetIssuer(ptr: *const X509) -> *mut c_char {
pub unsafe extern "C" fn SCX509GetIssuer(ptr: *const X509, issuer_name: *mut *mut u8, issuer_len: *mut u32) {
if ptr.is_null() {
return std::ptr::null_mut();
*issuer_len = 0;
*issuer_name = std::ptr::null_mut();
return;
}
let x509 = cast_pointer! {ptr, X509};
let issuer = x509.0.tbs_certificate.issuer.to_string();
rust_string_to_c(issuer)
let issuer = x509.0.tbs_certificate.issuer.to_string().into_bytes();
*issuer_len = issuer.len() as u32;
*issuer_name = Box::into_raw(issuer.into_boxed_slice()) as *mut u8;
}
#[no_mangle]

@ -496,12 +496,11 @@ static int TlsDecodeHSCertificate(SSLState *ssl_state, SSLStateConnp *connp,
goto error;
}
char *str = SCX509GetIssuer(x509);
if (str == NULL) {
SCX509GetIssuer(x509, &connp->cert0_issuerdn, &connp->cert0_issuerdn_len);
if (connp->cert0_issuerdn == NULL) {
err_code = ERR_EXTRACT_ISSUER;
goto error;
}
connp->cert0_issuerdn = str;
connp->cert0_sans_len = SCX509GetSubjectAltNameLen(x509);
char **sans = SCCalloc(connp->cert0_sans_len, sizeof(char *));
@ -512,7 +511,7 @@ static int TlsDecodeHSCertificate(SSLState *ssl_state, SSLStateConnp *connp,
sans[i] = SCX509GetSubjectAltNameAt(x509, i);
}
connp->cert0_sans = sans;
str = SCX509GetSerial(x509);
char *str = SCX509GetSerial(x509);
if (str == NULL) {
err_code = ERR_INVALID_SERIAL;
goto error;
@ -2857,7 +2856,8 @@ static void SSLStateFree(void *p)
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);
SCX509ArrayFree(
ssl_state->client_connp.cert0_issuerdn, ssl_state->client_connp.cert0_issuerdn_len);
if (ssl_state->client_connp.cert0_serial)
SCRustCStringFree(ssl_state->client_connp.cert0_serial);
if (ssl_state->client_connp.cert0_fingerprint)
@ -2873,7 +2873,8 @@ static void SSLStateFree(void *p)
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);
SCX509ArrayFree(
ssl_state->server_connp.cert0_issuerdn, ssl_state->server_connp.cert0_issuerdn_len);
if (ssl_state->server_connp.cert0_serial)
SCRustCStringFree(ssl_state->server_connp.cert0_serial);
if (ssl_state->server_connp.cert0_fingerprint)

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

@ -1,4 +1,4 @@
/* Copyright (C) 2007-2022 Open Information Security Foundation
/* Copyright (C) 2007-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
@ -138,8 +138,8 @@ static InspectionBuffer *GetData(DetectEngineThreadCtx *det_ctx,
return NULL;
}
const uint32_t data_len = (uint32_t)strlen(connp->cert0_issuerdn);
const uint8_t *data = (uint8_t *)connp->cert0_issuerdn;
const uint32_t data_len = connp->cert0_issuerdn_len;
const uint8_t *data = connp->cert0_issuerdn;
InspectionBufferSetupAndApplyTransforms(
det_ctx, list_id, buffer, data, data_len, transforms);

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

@ -235,12 +235,16 @@ static void LogTlsLogPem(LogTlsStoreLogThread *aft, const Packet *p, SSLState *s
}
char *subject = CreateStringFromByteArray(connp->cert0_subject, connp->cert0_subject_len);
char *issuerdn =
CreateStringFromByteArray(connp->cert0_issuerdn, connp->cert0_issuerdn_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);
subject ? subject : "<ERROR>", issuerdn ? issuerdn : "<ERROR>",
connp->cert0_fingerprint);
SCFree(subject);
SCFree(issuerdn);
if (r < 0)
goto end_fwrite_fpmeta;

@ -144,7 +144,12 @@ static void JsonTlsLogSubject(SCJsonBuilder *js, SSLState *ssl_state)
static void JsonTlsLogIssuer(SCJsonBuilder *js, SSLState *ssl_state)
{
if (ssl_state->server_connp.cert0_issuerdn) {
SCJbSetString(js, "issuerdn", ssl_state->server_connp.cert0_issuerdn);
if (ssl_state->server_connp.cert0_issuerdn_len == 0) {
SCJbSetString(js, "issuerdn", "");
} else {
SCJbSetStringFromBytes(js, "issuerdn", ssl_state->server_connp.cert0_issuerdn,
ssl_state->server_connp.cert0_issuerdn_len);
}
}
}
@ -350,7 +355,12 @@ static void JsonTlsLogClientCert(
}
}
if (connp->cert0_issuerdn != NULL) {
SCJbSetString(js, "issuerdn", connp->cert0_issuerdn);
if (connp->cert0_issuerdn_len == 0) {
SCJbSetString(js, "issuerdn", "");
} else {
SCJbSetStringFromBytes(
js, "issuerdn", connp->cert0_issuerdn, connp->cert0_issuerdn_len);
}
}
if (connp->cert0_fingerprint) {
SCJbSetString(js, "fingerprint", connp->cert0_fingerprint);

@ -175,7 +175,7 @@ static int GetCertInfo(lua_State *luastate, bool client, const SSLState *ssl_sta
int r = LuaPushStringBuffer(luastate, (uint8_t *)ssl_version, strlen(ssl_version));
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, connp->cert0_issuerdn, connp->cert0_issuerdn_len);
r += LuaPushStringBuffer(luastate, (uint8_t *)connp->cert0_fingerprint, strlen(connp->cert0_fingerprint));
return r;
}

Loading…
Cancel
Save