From 71bef1d10aa06df456cf916aaa68d91948cbaedf Mon Sep 17 00:00:00 2001 From: Aidan Garske Date: Thu, 20 Aug 2026 18:31:14 -0700 Subject: [PATCH] F-10620 - Preserve binary ALPN names via length-aware storage --- src/internal.c | 6 ++- src/ssl_api_ext.c | 102 +++++++++++++++++++-------------------------- src/tls.c | 11 ++--- tests/api.c | 80 +++++++++++++++++++++++++++++++++++ wolfssl/internal.h | 9 ++-- 5 files changed, 139 insertions(+), 69 deletions(-) diff --git a/src/internal.c b/src/internal.c index afa472608b..490eda5000 100644 --- a/src/internal.c +++ b/src/internal.c @@ -8704,7 +8704,9 @@ int InitSSL(WOLFSSL* ssl, WOLFSSL_CTX* ctx, int writeDup) if (!ret) { #endif WOLFSSL_MSG("failed to set alpn protos to ssl object"); - return ret; + /* Map the non-negative public API failure to a negative error + * so wolfSSL_new frees the object instead of returning it. */ + return BAD_FUNC_ARG; } } #endif @@ -42330,7 +42332,7 @@ static int AddPSKtoPreMasterSecret(WOLFSSL* ssl) alpn = (ALPN*)extension->data; if (alpn != NULL && alpn->negotiated == 1 && alpn->protocol_name != NULL) { - word32 protoLen = (word32)XSTRLEN(alpn->protocol_name); + word32 protoLen = (word32)alpn->protocol_nameSz; if (protoLen > 0) { return wc_Hash(TICKET_BINDING_HASH_TYPE, (const byte*)alpn->protocol_name, diff --git a/src/ssl_api_ext.c b/src/ssl_api_ext.c index 2be0da0375..d02803d490 100644 --- a/src/ssl_api_ext.c +++ b/src/ssl_api_ext.c @@ -2714,59 +2714,15 @@ int wolfSSL_CTX_set_alpn_protos(WOLFSSL_CTX *ctx, const unsigned char *p, #ifdef HAVE_ALPN #ifndef NO_BIO -/* Convert a wire-format ALPN protocol list into a comma-separated string. - * - * The wire format is a sequence of entries, each a length byte followed by - * that many protocol-name bytes. - * - * @param [in] p ALPN protocol list in wire format. - * @param [in] p_len Length of the protocol list in bytes. - * @param [out] pt Buffer to hold the comma-separated list. Must hold at - * least p_len bytes. - * @param [out] ptLen Length of the comma-separated list written. - * @return 1 on success. - * @return 0 when the wire format is invalid. - */ -static int wolfssl_alpn_protos_to_list(const unsigned char* p, - unsigned int p_len, char* pt, unsigned int* ptLen) -{ - unsigned int idx = 0; - unsigned int ptIdx = 0; - unsigned int sz; - int ret = 1; - - /* Convert into a comma separated list. */ - while (idx < p_len - 1) { - unsigned int i; - - sz = p[idx++]; - if (idx + sz > p_len) { - WOLFSSL_MSG("Bad list format"); - ret = 0; - break; - } - if (sz > 0) { - for (i = 0; i < sz; i++) { - pt[ptIdx++] = p[idx++]; - } - if (idx < p_len - 1) { - pt[ptIdx++] = ','; - } - } - } - - if (ret == 1) { - *ptLen = ptIdx; - } - - return ret; -} - /* Set the ALPN protocol list, in wire format, on the object. * * The list is length-prefixed, e.g. * unsigned char p[] = { 8, 'h','t','t','p','/','1','.','1' }; * + * Each length-prefixed entry is added directly, so protocol names that contain + * any byte value, including a comma or a NUL, are preserved. Zero-length + * entries are rejected and the whole list must be consumed exactly. + * * @param [in] ssl SSL/TLS object. * @param [in] p ALPN protocol list in wire format (length-prefixed). * @param [in] p_len Length of the protocol list in bytes. @@ -2776,12 +2732,18 @@ static int wolfssl_alpn_protos_to_list(const unsigned char* p, int wolfSSL_set_alpn_protos(WOLFSSL* ssl, const unsigned char* p, unsigned int p_len) { - char* pt = NULL; - unsigned int ptIdx = 0; + unsigned int idx = 0; + unsigned int count = 0; + unsigned int i; + unsigned int sz; + int valid = 1; + int ok = 1; /* RFC 7301: a server that does not select any of the client's offered * protocols MUST send no_application_protocol. Match that contract on * the OpenSSL-compat surface rather than silently continuing. */ int alpn_opt = WOLFSSL_ALPN_FAILED_ON_MISMATCH; + /* One pointer per entry, each pointing at that entry's length byte. */ + const unsigned char** entries = NULL; #if defined(WOLFSSL_ERROR_CODE_OPENSSL) int ret = 1; #else @@ -2791,18 +2753,42 @@ int wolfSSL_set_alpn_protos(WOLFSSL* ssl, WOLFSSL_ENTER("wolfSSL_set_alpn_protos"); if ((ssl != NULL) && (p_len > 1) && (p != NULL)) { - /* Replacing leading number with trailing ',' and adding '\0'. */ - pt = (char*)XMALLOC(p_len + 1, ssl->heap, DYNAMIC_TYPE_OPENSSL); - if (pt != NULL) { - if (wolfssl_alpn_protos_to_list(p, p_len, pt, &ptIdx)) { - pt[ptIdx++] = '\0'; + entries = (const unsigned char**)XMALLOC( + sizeof(*entries) * WOLFSSL_MAX_ALPN_NUMBER, ssl->heap, + DYNAMIC_TYPE_OPENSSL); + if (entries != NULL) { + /* Record each length-prefixed entry, rejecting zero-length names + * and any entry that runs past the end of the list. */ + while (idx < p_len) { + sz = p[idx++]; + if ((sz == 0) || (idx + sz > p_len) || + (count >= (unsigned int)WOLFSSL_MAX_ALPN_NUMBER)) { + valid = 0; + break; + } + entries[count++] = p + idx - 1; + idx += sz; + } + /* Require the whole list to be consumed exactly. */ + if (valid && (idx == p_len) && (count > 0)) { /* Clear out all currently set ALPN extensions. */ TLSX_Remove(&ssl->extensions, TLSX_APPLICATION_LAYER_PROTOCOL, ssl->heap); - if (wolfSSL_UseALPN(ssl, pt, ptIdx, (byte)alpn_opt) == - WOLFSSL_SUCCESS) { + /* Add in reverse so the offered order matches the input. */ + i = count; + while (i > 0) { + i--; + if (TLSX_UseALPN(&ssl->extensions, entries[i] + 1, + entries[i][0], (byte)alpn_opt, ssl->heap) != + WOLFSSL_SUCCESS) { + ok = 0; + break; + } + } + + if (ok) { #if defined(WOLFSSL_ERROR_CODE_OPENSSL) ret = 0; #else @@ -2811,7 +2797,7 @@ int wolfSSL_set_alpn_protos(WOLFSSL* ssl, } } - XFREE(pt, ssl->heap, DYNAMIC_TYPE_OPENSSL); + XFREE(entries, ssl->heap, DYNAMIC_TYPE_OPENSSL); } } diff --git a/src/tls.c b/src/tls.c index 3fed04bdec..85c9a666ad 100644 --- a/src/tls.c +++ b/src/tls.c @@ -1809,6 +1809,7 @@ static ALPN* TLSX_ALPN_New(char *protocol_name, word16 protocol_nameSz, XMEMCPY(alpn->protocol_name, protocol_name, protocol_nameSz); alpn->protocol_name[protocol_nameSz] = 0; + alpn->protocol_nameSz = protocol_nameSz; (void)heap; @@ -1848,7 +1849,7 @@ static word16 TLSX_ALPN_GetSize(ALPN *list) list = alpn->next; length++; /* protocol name length is on one byte */ - length += (word32)XSTRLEN(alpn->protocol_name); + length += (word32)alpn->protocol_nameSz; if (length > WOLFSSL_MAX_16BIT) { return 0; @@ -1868,7 +1869,7 @@ static word16 TLSX_ALPN_Write(ALPN *list, byte *output) while ((alpn = list)) { list = alpn->next; - length = (word16)XSTRLEN(alpn->protocol_name); + length = alpn->protocol_nameSz; /* protocol name length */ output[offset++] = (byte)length; @@ -1895,8 +1896,8 @@ static ALPN* TLSX_ALPN_Find(ALPN *list, char *protocol_name, word16 size) alpn = list; while (alpn != NULL && ( - (word16)XSTRLEN(alpn->protocol_name) != size || - XSTRNCMP(alpn->protocol_name, protocol_name, size))) + alpn->protocol_nameSz != size || + XMEMCMP(alpn->protocol_name, protocol_name, size))) alpn = alpn->next; return alpn; @@ -2220,7 +2221,7 @@ int TLSX_ALPN_GetRequest(TLSX* extensions, void** data, word16 *dataSz) } *data = alpn->protocol_name; - *dataSz = (word16)XSTRLEN((char*)*data); + *dataSz = alpn->protocol_nameSz; return WOLFSSL_SUCCESS; } diff --git a/tests/api.c b/tests/api.c index 2304f24f86..7e87d15ac2 100644 --- a/tests/api.c +++ b/tests/api.c @@ -2854,6 +2854,85 @@ static int test_wolfSSL_set_alpn_protos_default_fails(void) return EXPECT_RESULT(); } +static int test_wolfSSL_set_alpn_protos_binary_safe(void) +{ + EXPECT_DECLS; +#if defined(HAVE_ALPN) && defined(OPENSSL_EXTRA) && !defined(NO_BIO) && \ + !defined(NO_WOLFSSL_CLIENT) + WOLFSSL_CTX* ctx = NULL; + WOLFSSL* ssl = NULL; + /* one 3-byte protocol name that contains a comma */ + unsigned char comma[] = { 3, 'a', ',', 'b' }; + /* a valid entry followed by a zero-length entry */ + unsigned char empty[] = { 1, 'a', 0 }; + /* a non-empty name containing a NUL byte */ + unsigned char embeddedNul[] = { 3, 'a', 0, 'b' }; + TLSX* ext = NULL; + ALPN* alpn = NULL; + + ExpectNotNull(ctx = wolfSSL_CTX_new(wolfSSLv23_client_method())); + ExpectNotNull(ssl = wolfSSL_new(ctx)); + + /* A comma inside a name must not split it into two protocols. */ +#ifdef WOLFSSL_ERROR_CODE_OPENSSL + ExpectIntEQ(wolfSSL_set_alpn_protos(ssl, comma, sizeof(comma)), 0); +#else + ExpectIntEQ(wolfSSL_set_alpn_protos(ssl, comma, sizeof(comma)), + WOLFSSL_SUCCESS); +#endif + if (ssl != NULL) { + ext = TLSX_Find(ssl->extensions, TLSX_APPLICATION_LAYER_PROTOCOL); + ExpectNotNull(ext); + if (ext != NULL) { + alpn = (ALPN*)ext->data; + ExpectNotNull(alpn); + if (alpn != NULL) { + /* Exactly one protocol, named "a,b". */ + ExpectNull(alpn->next); + ExpectNotNull(alpn->protocol_name); + ExpectStrEQ(alpn->protocol_name, "a,b"); + } + } + } + + /* A zero-length entry is malformed and must be rejected. */ +#ifdef WOLFSSL_ERROR_CODE_OPENSSL + ExpectIntNE(wolfSSL_set_alpn_protos(ssl, empty, sizeof(empty)), 0); +#else + ExpectIntNE(wolfSSL_set_alpn_protos(ssl, empty, sizeof(empty)), + WOLFSSL_SUCCESS); +#endif + + /* A NUL byte inside a non-empty name is a valid ALPN identifier and must + * round-trip intact: length preserved, all bytes unchanged. */ +#ifdef WOLFSSL_ERROR_CODE_OPENSSL + ExpectIntEQ(wolfSSL_set_alpn_protos(ssl, embeddedNul, + sizeof(embeddedNul)), 0); +#else + ExpectIntEQ(wolfSSL_set_alpn_protos(ssl, embeddedNul, + sizeof(embeddedNul)), WOLFSSL_SUCCESS); +#endif + if (ssl != NULL) { + ext = TLSX_Find(ssl->extensions, TLSX_APPLICATION_LAYER_PROTOCOL); + ExpectNotNull(ext); + if (ext != NULL) { + alpn = (ALPN*)ext->data; + ExpectNotNull(alpn); + if (alpn != NULL) { + ExpectNull(alpn->next); + ExpectIntEQ(alpn->protocol_nameSz, 3); + ExpectNotNull(alpn->protocol_name); + ExpectBufEQ(alpn->protocol_name, embeddedNul + 1, 3); + } + } + } + + wolfSSL_free(ssl); + wolfSSL_CTX_free(ctx); +#endif + return EXPECT_RESULT(); +} + static int test_wolfSSL_CTX_use_certificate(void) { EXPECT_DECLS; @@ -40336,6 +40415,7 @@ TEST_CASE testCases[] = { TEST_DECL(test_wolfSSL_set_cipher_list_tls13_with_version), TEST_DECL(test_wolfSSL_set_cipher_list_exclusions), TEST_DECL(test_wolfSSL_set_alpn_protos_default_fails), + TEST_DECL(test_wolfSSL_set_alpn_protos_binary_safe), TEST_DECL(test_wolfSSL_CTX_use_certificate), TEST_DECL(test_wolfSSL_CTX_use_certificate_file), TEST_DECL(test_wolfSSL_CTX_use_certificate_buffer), diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 7569a5cb2a..e68a5db196 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -3510,10 +3510,11 @@ WOLFSSL_LOCAL int TLSX_UseTrustedCA(TLSX** extensions, byte type, /* Application-Layer Protocol Negotiation - RFC 7301 */ #ifdef HAVE_ALPN typedef struct ALPN { - char* protocol_name; /* ALPN protocol name */ - struct ALPN* next; /* List Behavior */ - byte options; /* Behavior options */ - byte negotiated; /* ALPN protocol negotiated or not */ + char* protocol_name; /* ALPN protocol name */ + struct ALPN* next; /* List Behavior */ + word16 protocol_nameSz; /* length of protocol_name in bytes */ + byte options; /* Behavior options */ + byte negotiated; /* ALPN protocol negotiated or not */ } ALPN; WOLFSSL_LOCAL int TLSX_ALPN_GetRequest(TLSX* extensions,