Skip to content
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

Jump to bottom

TLSX_CA_Names_Parse: Verify the length of the extension #6644

Merged
merged 3 commits into from
Jul 31, 2023
Merged

TLSX_CA_Names_Parse: Verify the length of the extension #6644

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5947c9a
TLSX_CA_Names_Parse: Verify the length of the extension
julek-wolfssl Jul 21, 2023
a495bb4
TLSX_CA_Names_Parse: make sure to do cleanup when smallstack is on
julek-wolfssl Jul 26, 2023
854ae0d
Code review
julek-wolfssl Jul 31, 2023
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
31 changes: 20 additions & 11 deletions src/tls.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6634,6 +6634,9 @@ static int TLSX_CA_Names_Parse(WOLFSSL *ssl, const byte* input,
if (ssl->client_ca_names == NULL)
return MEMORY_ERROR;

if (length < OPAQUE16_LEN)
return BUFFER_ERROR;

ato16(input, &extLen);
input += OPAQUE16_LEN;
length -= OPAQUE16_LEN;
Expand All @@ -6644,6 +6647,7 @@ static int TLSX_CA_Names_Parse(WOLFSSL *ssl, const byte* input,
word32 idx = 0;
WOLFSSL_X509_NAME* name = NULL;
int ret = 0;
int didInit = FALSE;
/* Use a DecodedCert struct to get access to GetName to
* parse DN name */
#ifdef WOLFSSL_SMALL_STACK
Expand All @@ -6655,28 +6659,33 @@ static int TLSX_CA_Names_Parse(WOLFSSL *ssl, const byte* input,
DecodedCert cert[1];
#endif

if (length < OPAQUE16_LEN)
return BUFFER_ERROR;
ato16(input, &extLen);
idx += OPAQUE16_LEN;

if (extLen > length)
return BUFFER_ERROR;

InitDecodedCert(cert, input + idx, extLen, ssl->heap);
idx += extLen;
ret = BUFFER_ERROR;

ret = GetName(cert, SUBJECT, extLen);
if (ret == 0) {
InitDecodedCert(cert, input + idx, extLen, ssl->heap);
didInit = TRUE;
idx += extLen;
ret = GetName(cert, SUBJECT, extLen);
}

if (ret == 0 && (name = wolfSSL_X509_NAME_new()) == NULL)
ret = MEMORY_ERROR;

if (ret == 0)
if (ret == 0) {
CopyDecodedName(name, cert, SUBJECT);
if (wolfSSL_sk_X509_NAME_push(ssl->client_ca_names, name)
== WOLFSSL_FAILURE)
ret = MEMORY_ERROR;
}

if (ret == 0 && wolfSSL_sk_X509_NAME_push(ssl->client_ca_names, name)
== WOLFSSL_FAILURE)
ret = MEMORY_ERROR;

FreeDecodedCert(cert);
if (didInit)
FreeDecodedCert(cert);

#ifdef WOLFSSL_SMALL_STACK
XFREE(cert, ssl->heap, DYNAMIC_TYPE_DCERT);
Expand Down
50 changes: 50 additions & 0 deletions tests/api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62942,6 +62942,55 @@ static int test_dtls_no_extensions(void)
return EXPECT_RESULT();
}

static int test_TLSX_CA_NAMES_bad_extension(void)
{
EXPECT_DECLS;
#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13) && \
!defined(NO_CERTS) && !defined(WOLFSSL_NO_CA_NAMES) && \
defined(OPENSSL_EXTRA) && defined(WOLFSSL_SHA384) && \
defined(HAVE_NULL_CIPHER)
/* This test should only fail (with BUFFER_ERROR) when we actually try to
* parse the CA Names extension. Otherwise it will return other non-related
* errors. If CA Names will be parsed in more configurations, that should
* be reflected in the macro guard above. */
WOLFSSL *ssl_c = NULL;
WOLFSSL_CTX *ctx_c = NULL;
struct test_memio_ctx test_ctx;
/* HRR + SH using TLS_DHE_PSK_WITH_NULL_SHA384 */
const byte shBadCaNamesExt[] = {
0x16, 0x03, 0x04, 0x00, 0x3f, 0x02, 0x00, 0x00, 0x3b, 0x03, 0x03, 0xcf,
0x21, 0xad, 0x74, 0xe5, 0x9a, 0x61, 0x11, 0xbe, 0x1d, 0x8c, 0x02, 0x1e,
0x65, 0xb8, 0x91, 0xc2, 0xa2, 0x11, 0x16, 0x7a, 0xbb, 0x8c, 0x5e, 0x07,
0x9e, 0x09, 0xe2, 0xc8, 0xa8, 0x33, 0x9c, 0x00, 0x13, 0x03, 0x00, 0x00,
0x13, 0x94, 0x7e, 0x00, 0x03, 0x0b, 0xf7, 0x03, 0x00, 0x2b, 0x00, 0x02,
0x03, 0x04, 0x00, 0x33, 0x00, 0x02, 0x00, 0x19, 0x16, 0x03, 0x03, 0x00,
0x5c, 0x02, 0x00, 0x00, 0x3b, 0x03, 0x03, 0x03, 0xcf, 0x21, 0xad, 0x74,
0x00, 0x00, 0x83, 0x3f, 0x3b, 0x80, 0x01, 0xac, 0x65, 0x8c, 0x19, 0x2a,
0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x02, 0x00, 0x9e, 0x09, 0x1c, 0xe8,
0xa8, 0x09, 0x9c, 0x00, 0xc0, 0xb5, 0x00, 0x00, 0x11, 0x8f, 0x00, 0x00,
0x03, 0x3f, 0x00, 0x0c, 0x00, 0x2b, 0x00, 0x02, 0x03, 0x04, 0x13, 0x05,
0x00, 0x00, 0x08, 0x00, 0x00, 0x06, 0x00, 0x04, 0x00, 0x09, 0x00, 0x00,
0x0d, 0x00, 0x00, 0x11, 0x00, 0x00, 0x0d, 0x00, 0x2f, 0x00, 0x01, 0xff,
0xff, 0xff, 0xff, 0xfa, 0x0d, 0x00, 0x00, 0x00, 0xad, 0x02
};

XMEMSET(&test_ctx, 0, sizeof(test_ctx));

ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, NULL, &ssl_c, NULL,
wolfTLSv1_3_client_method, NULL), 0);

XMEMCPY(test_ctx.c_buff, shBadCaNamesExt, sizeof(shBadCaNamesExt));
test_ctx.c_len = sizeof(shBadCaNamesExt);

ExpectIntEQ(wolfSSL_connect(ssl_c), -1);
ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), BUFFER_ERROR);

wolfSSL_free(ssl_c);
wolfSSL_CTX_free(ctx_c);
#endif
return EXPECT_RESULT();
}

/*----------------------------------------------------------------------------*
| Main
*----------------------------------------------------------------------------*/
Expand Down Expand Up @@ -64192,6 +64241,7 @@ TEST_CASE testCases[] = {
TEST_DECL(test_dtls_ipv6_check),
TEST_DECL(test_wolfSSL_SCR_after_resumption),
TEST_DECL(test_dtls_no_extensions),
TEST_DECL(test_TLSX_CA_NAMES_bad_extension),
/* This test needs to stay at the end to clean up any caches allocated. */
TEST_DECL(test_wolfSSL_Cleanup)
};
Expand Down