From 05cfeae3cea7ee00ca1f522df9b7fb19faeebea1 Mon Sep 17 00:00:00 2001 From: David Garske Date: Wed, 11 Jul 2018 12:32:49 -0700 Subject: [PATCH 1/3] Fix for handling max cert chain size. It was not accounting for the 3 byte header in max size calculation. --- src/ssl.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index bb5a99ea6..dad616904 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -4263,15 +4263,16 @@ static int ProcessUserChain(WOLFSSL_CTX* ctx, const unsigned char* buff, #endif byte* chainBuffer = staticBuffer; int dynamicBuffer = 0; - word32 bufferSz = sizeof(staticBuffer); + word32 bufferSz; long consumed = info->consumed; word32 idx = 0; int gotOne = 0; - if ( (sz - consumed) > (int)bufferSz) { + /* Calculate max possible size, including max headers */ + bufferSz = (word32)(sz - consumed) + (CERT_HEADER_SZ * MAX_CHAIN_DEPTH); + if (bufferSz > sizeof(staticBuffer)) { WOLFSSL_MSG("Growing Tmp Chain Buffer"); - bufferSz = (word32)(sz - consumed); - /* will shrink to actual size */ + /* will shrink to actual size */ chainBuffer = (byte*)XMALLOC(bufferSz, heap, DYNAMIC_TYPE_FILE); if (chainBuffer == NULL) { return MEMORY_E; @@ -4301,7 +4302,7 @@ static int ProcessUserChain(WOLFSSL_CTX* ctx, const unsigned char* buff, if (GetSequence(buff + consumed, &inOutIdx, &length, remain) < 0) { ret = ASN_NO_PEM_HEADER; } - length += inOutIdx; /* include leading squence */ + length += inOutIdx; /* include leading sequence */ } info->consumed = length; if (ret == 0) { @@ -4316,7 +4317,7 @@ static int ProcessUserChain(WOLFSSL_CTX* ctx, const unsigned char* buff, #ifdef WOLFSSL_TLS13 cnt++; #endif - if ((idx + part->length) > bufferSz) { + if ((idx + part->length + CERT_HEADER_SZ) > bufferSz) { WOLFSSL_MSG(" Cert Chain bigger than buffer"); ret = BUFFER_E; } @@ -4446,7 +4447,7 @@ int ProcessBuffer(WOLFSSL_CTX* ctx, const unsigned char* buff, if (GetSequence(buff, &inOutIdx, &length, (word32)sz) < 0) { ret = ASN_PARSE_E; } - length += inOutIdx; /* include leading squence */ + length += inOutIdx; /* include leading sequence */ } info->consumed = length; @@ -4465,6 +4466,10 @@ int ProcessBuffer(WOLFSSL_CTX* ctx, const unsigned char* buff, /* process user chain */ if (ret >= 0) { + /* First certificate in chain is loaded into ssl->buffers.certificate. + * Remainder are loaded into ssl->buffers.certChain. + * Chain should have server cert first, then intermediates, then root. + */ if (userChain) { ret = ProcessUserChain(ctx, buff, sz, format, type, ssl, used, info); } From 0ce6cbd4c495837e56fc35893d4d30352d0f16fd Mon Sep 17 00:00:00 2001 From: David Garske Date: Thu, 12 Jul 2018 13:22:21 -0700 Subject: [PATCH 2/3] Added API unit test for `wolfSSL_CTX_use_certificate_chain_file_format`. --- tests/api.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/api.c b/tests/api.c index 845a6be07..a0e354316 100644 --- a/tests/api.c +++ b/tests/api.c @@ -721,6 +721,25 @@ static void test_wolfSSL_CTX_load_verify_locations(void) #endif } + +static int test_wolfSSL_CTX_use_certificate_chain_file_format(void) +{ + int ret = 0; +#if !defined(NO_FILESYSTEM) && !defined(NO_CERTS) + const char* server_chain_der = "./certs/server-cert-chain.der"; + WOLFSSL_CTX* ctx; + + ctx = wolfSSL_CTX_new(wolfTLSv1_2_client_method()); + AssertNotNull(ctx); + + AssertIntEQ(wolfSSL_CTX_use_certificate_chain_file_format(ctx, + server_chain_der, WOLFSSL_FILETYPE_ASN1), WOLFSSL_SUCCESS); + + wolfSSL_CTX_free(ctx); +#endif + return ret; +} + static void test_wolfSSL_CTX_SetTmpDH_file(void) { #if !defined(NO_FILESYSTEM) && !defined(NO_CERTS) && !defined(NO_DH) && \ @@ -19894,6 +19913,7 @@ void ApiTest(void) AssertIntEQ(test_wolfSSL_CTX_use_certificate_buffer(), WOLFSSL_SUCCESS); test_wolfSSL_CTX_use_PrivateKey_file(); test_wolfSSL_CTX_load_verify_locations(); + test_wolfSSL_CTX_use_certificate_chain_file_format(); test_wolfSSL_CTX_trust_peer_cert(); test_wolfSSL_CTX_SetTmpDH_file(); test_wolfSSL_CTX_SetTmpDH_buffer(); From 0a19dc09405e04b9d1a106bf7776bd502a86d7c5 Mon Sep 17 00:00:00 2001 From: David Garske Date: Fri, 13 Jul 2018 11:41:06 -0700 Subject: [PATCH 3/3] Don't run new cert chain test if RSA is disabled (test chain contains RSA certs). --- tests/api.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/api.c b/tests/api.c index a0e354316..f1d744f75 100644 --- a/tests/api.c +++ b/tests/api.c @@ -725,7 +725,7 @@ static void test_wolfSSL_CTX_load_verify_locations(void) static int test_wolfSSL_CTX_use_certificate_chain_file_format(void) { int ret = 0; -#if !defined(NO_FILESYSTEM) && !defined(NO_CERTS) +#if !defined(NO_FILESYSTEM) && !defined(NO_CERTS) && !defined(NO_RSA) const char* server_chain_der = "./certs/server-cert-chain.der"; WOLFSSL_CTX* ctx;