Skip to content

Preserve binary ALPN names via length-aware storage #11230

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
aidangarske wants to merge 1 commit into
base: master
Choose a base branch
from
+139 −69
Open

Preserve binary ALPN names via length-aware storage #11230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/internal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
102 changes: 44 additions & 58 deletions src/ssl_api_ext.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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);
}
}

Expand Down
11 changes: 6 additions & 5 deletions src/tls.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
80 changes: 80 additions & 0 deletions tests/api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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),
Expand Down
9 changes: 5 additions & 4 deletions wolfssl/internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading