From f020b0f24a3cadd604e05c79f899bedc4bfc660a Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Mon, 15 Jun 2020 14:41:05 -0600 Subject: [PATCH 1/4] add check on decode subtree return value --- wolfcrypt/src/asn.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 91cbab76e..38c853b4f 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -8408,7 +8408,10 @@ static int DecodeNameConstraints(const byte* input, int sz, DecodedCert* cert) return ASN_PARSE_E; } - DecodeSubtree(input + idx, length, subtree, cert->heap); + if (DecodeSubtree(input + idx, length, subtree, cert->heap) < 0) { + WOLFSSL_MSG("\terror parsing subtree"); + return ASN_PARSE_E; + } idx += length; } From f75659641aadcf89e4adf53d9c0d1f3f182f2b47 Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Wed, 17 Jun 2020 14:33:10 -0600 Subject: [PATCH 2/4] test on malformed name constraint --- tests/api.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/tests/api.c b/tests/api.c index f289dd513..79744d36a 100644 --- a/tests/api.c +++ b/tests/api.c @@ -1202,6 +1202,77 @@ static int test_wolfSSL_CertManagerSetVerify(void) return ret; } +static void test_wolfSSL_CertManagerNameConstraint(void) +{ +#if !defined(NO_FILESYSTEM) && !defined(NO_CERTS) && \ + !defined(NO_WOLFSSL_CM_VERIFY) && !defined(NO_RSA) && \ + defined(OPENSSL_EXTRA) && defined(WOLFSSL_CERT_GEN) && \ + defined(WOLFSSL_CERT_EXT) && defined(WOLFSSL_ALT_NAMES) + WOLFSSL_CERT_MANAGER* cm; + const char* ca_cert = "./certs/test/cert-ext-nc.der"; + const char* client_cert = "./certs/test/server-goodcn.der"; + int i = 0; + static const byte extNameConsOid[] = {85, 29, 30}; + + RsaKey key; + WC_RNG rng; + byte *der; + int derSz; + word32 idx = 0; + byte *pt; + WOLFSSL_X509 *x509; + + wc_InitRng(&rng); + + /* load in CA private key for signing */ + AssertIntEQ(wc_InitRsaKey_ex(&key, HEAP_HINT, devId), 0); + AssertIntEQ(wc_RsaPrivateKeyDecode(server_key_der_2048, &idx, &key, + sizeof_server_key_der_2048), 0); + + /* get ca certificate then alter it */ + AssertNotNull(der = + (byte*)XMALLOC(FOURK_BUF, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER)); + AssertNotNull(x509 = wolfSSL_X509_load_certificate_file(ca_cert, + WOLFSSL_FILETYPE_ASN1)); + AssertNotNull(pt = (byte*)wolfSSL_X509_get_tbs(x509, &derSz)); + XMEMCPY(der, pt, derSz); + + /* find the name constraint extension and alter it */ + pt = der; + for (i = 0; i < derSz - 3; i++) { + if (XMEMCMP(pt, extNameConsOid, 3) == 0) { + pt += 3; + break; + } + pt++; + } + AssertIntNE(i, derSz - 3); /* did not find OID if this case is hit */ + + /* go to the length value and set it to 0 */ + while (i < derSz && *pt != 0x81) { + pt++; + i++; + } + AssertIntNE(i, derSz); /* did not place to alter */ + pt++; + *pt = 0x00; + + /* resign the altered certificate */ + AssertIntGT((derSz = wc_SignCert(derSz, CTC_SHA256wRSA, der, + FOURK_BUF, &key, NULL, &rng)), 0); + + AssertNotNull(cm = wolfSSL_CertManagerNew()); + AssertIntEQ(wolfSSL_CertManagerLoadCABuffer(cm, der, derSz, + WOLFSSL_FILETYPE_ASN1), ASN_PARSE_E); + wolfSSL_CertManagerFree(cm); + + XFREE(der, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + wolfSSL_X509_free(x509); + wc_FreeRng(&rng); + wolfSSL_CertManagerFree(cm); +#endif +} + static void test_wolfSSL_CertManagerCRL(void) { #if !defined(NO_FILESYSTEM) && !defined(NO_CERTS) && defined(HAVE_CRL) && \ @@ -32081,6 +32152,7 @@ void ApiTest(void) test_wolfSSL_CertManagerLoadCABuffer(); test_wolfSSL_CertManagerGetCerts(); test_wolfSSL_CertManagerSetVerify(); + test_wolfSSL_CertManagerNameConstraint(); test_wolfSSL_CertManagerCRL(); test_wolfSSL_CTX_load_verify_locations_ex(); test_wolfSSL_CTX_load_verify_buffer_ex(); From dafd35e4c117488a848d488899bd291491266c3e Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Wed, 17 Jun 2020 15:55:08 -0600 Subject: [PATCH 3/4] remove unused variable --- tests/api.c | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/api.c b/tests/api.c index 79744d36a..8416860ee 100644 --- a/tests/api.c +++ b/tests/api.c @@ -1210,7 +1210,6 @@ static void test_wolfSSL_CertManagerNameConstraint(void) defined(WOLFSSL_CERT_EXT) && defined(WOLFSSL_ALT_NAMES) WOLFSSL_CERT_MANAGER* cm; const char* ca_cert = "./certs/test/cert-ext-nc.der"; - const char* client_cert = "./certs/test/server-goodcn.der"; int i = 0; static const byte extNameConsOid[] = {85, 29, 30}; From ae90119af4cd766459d2d1b39c57f60b1c67d6ab Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Tue, 23 Jun 2020 14:45:31 -0600 Subject: [PATCH 4/4] remove double free in test case --- tests/api.c | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/api.c b/tests/api.c index 8416860ee..2d30d26c9 100644 --- a/tests/api.c +++ b/tests/api.c @@ -1268,7 +1268,6 @@ static void test_wolfSSL_CertManagerNameConstraint(void) XFREE(der, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); wolfSSL_X509_free(x509); wc_FreeRng(&rng); - wolfSSL_CertManagerFree(cm); #endif }