From 39d0b032e833de0123d07852d731d187ccb25ee1 Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Tue, 3 Nov 2020 19:30:56 +0700 Subject: [PATCH 1/4] strict certificate version allowed from client --- src/internal.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/internal.c b/src/internal.c index a2e856542..703a5e3aa 100644 --- a/src/internal.c +++ b/src/internal.c @@ -11041,6 +11041,19 @@ int ProcessPeerCerts(WOLFSSL* ssl, byte* input, word32* inOutIdx, } #endif /* SESSION_CERTS && WOLFSSL_ALT_CERT_CHAINS */ + /* Check peer's certificate version number. TLS 1.2 / 1.3 + * requires the clients certificate be version 3 unless a + * different version has been negotiated using RFC 7250 */ + if ((ret == 0) && + (ssl->options.side == WOLFSSL_SERVER_END)) { + if (args->dCert->version != 2) { + WOLFSSL_MSG("Peers certificate was not version 3!"); + args->lastErr = ASN_VERSION_E; + /* setting last error but not considering it fatal + * giving the user a chance to override */ + } + } + /* check if fatal error */ if (args->verifyErr) { args->fatal = 1; From 979216d5955c55f2da25544fb44929c9e3beaf4a Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Wed, 11 Nov 2020 18:57:09 +0700 Subject: [PATCH 2/4] add test case for rejecting version 2 x509 --- src/internal.c | 2 +- src/ssl.c | 10 +- tests/api.c | 199 ++++++++++++++++++++++++++++++++++++++++ wolfssl/wolfcrypt/asn.h | 3 + 4 files changed, 209 insertions(+), 5 deletions(-) diff --git a/src/internal.c b/src/internal.c index 703a5e3aa..cda188704 100644 --- a/src/internal.c +++ b/src/internal.c @@ -11046,7 +11046,7 @@ int ProcessPeerCerts(WOLFSSL* ssl, byte* input, word32* inOutIdx, * different version has been negotiated using RFC 7250 */ if ((ret == 0) && (ssl->options.side == WOLFSSL_SERVER_END)) { - if (args->dCert->version != 2) { + if (args->dCert->version != WOLFSSL_X509_V3) { WOLFSSL_MSG("Peers certificate was not version 3!"); args->lastErr = ASN_VERSION_E; /* setting last error but not considering it fatal diff --git a/src/ssl.c b/src/ssl.c index b1c0a9871..b8db83754 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -37389,11 +37389,11 @@ void* wolfSSL_GetDhAgreeCtx(WOLFSSL* ssl) } out[0] = t->type; - sz = SetLength(t->length - 1, out + 1) + 1; /* gen tag */ - for (i = 0; i < t->length - 1; i++) { + sz = SetLength(t->length, out + 1) + 1; /* gen tag */ + for (i = 0; i < t->length; i++) { out[sz + i] = t->data[i]; } - return t->length - 1 + sz; + return t->length + sz; } #endif /* WOLFSSL_ALT_NAMES */ @@ -37674,8 +37674,10 @@ void* wolfSSL_GetDhAgreeCtx(WOLFSSL* ssl) key = (void*)&ecc; } #endif - if (key == NULL) + if (key == NULL) { + WOLFSSL_MSG("No public key found for certificate"); return WOLFSSL_FAILURE; + } /* Make the body of the certificate request. */ #ifdef WOLFSSL_CERT_REQ diff --git a/tests/api.c b/tests/api.c index 6d064c537..90f99886c 100644 --- a/tests/api.c +++ b/tests/api.c @@ -5630,6 +5630,204 @@ static void test_wolfSSL_X509_verify(void) printf(resultFmt, passed); #endif } + + +#if !defined(NO_FILESYSTEM) && !defined(NO_CERTS) && !defined(NO_RSA) && \ + !defined(NO_WOLFSSL_CLIENT) && !defined(NO_DH) && !defined(NO_AES) && \ + defined(HAVE_IO_TESTS_DEPENDENCIES) && !defined(SINGLE_THREADED) && \ + defined(OPENSSL_EXTRA) +/* create certificate with version 2 */ +static void test_set_x509_badversion(WOLFSSL_CTX* ctx) +{ + WOLFSSL_X509 *x509, *x509v2; + WOLFSSL_EVP_PKEY* priv; + unsigned char *der = NULL, *key = NULL; + char *header, *name; + int derSz; + long keySz; + XFILE fp; + WOLFSSL_ASN1_TIME *notBefore, *notAfter; + time_t t; + + AssertNotNull(x509 = wolfSSL_X509_load_certificate_file(cliCertFile, + WOLFSSL_FILETYPE_PEM)); + + fp = XFOPEN(cliKeyFile, "rb"); + AssertIntEQ(wolfSSL_PEM_read(fp, &name, &header, &key, &keySz), + WOLFSSL_SUCCESS); + AssertNotNull(priv = wolfSSL_d2i_PrivateKey(EVP_PKEY_RSA, NULL, + (const unsigned char**)&key, keySz)); + + + /* create the version 2 certificate */ + AssertNotNull(x509v2 = X509_new()); + AssertIntEQ(wolfSSL_X509_set_version(x509v2, 1), WOLFSSL_SUCCESS); + + AssertIntEQ(wolfSSL_X509_set_subject_name(x509v2, + wolfSSL_X509_get_subject_name(x509)), WOLFSSL_SUCCESS); + AssertIntEQ(wolfSSL_X509_set_issuer_name(x509v2, + wolfSSL_X509_get_issuer_name(x509)), WOLFSSL_SUCCESS); + AssertIntEQ(X509_set_pubkey(x509v2, + wolfSSL_X509_get_pubkey(x509)), WOLFSSL_SUCCESS); + + t = time(NULL); + AssertNotNull(notBefore = wolfSSL_ASN1_TIME_adj(NULL, t, 0, 0)); + AssertNotNull(notAfter = wolfSSL_ASN1_TIME_adj(NULL, t, 365, 0)); + AssertTrue(wolfSSL_X509_set_notBefore(x509v2, notBefore)); + AssertTrue(wolfSSL_X509_set_notAfter(x509v2, notAfter)); + + AssertIntGT(wolfSSL_X509_sign(x509v2, priv, EVP_sha256()), 0); + derSz = wolfSSL_i2d_X509(x509v2, &der); + AssertIntGT(derSz, 0); + AssertIntEQ(wolfSSL_CTX_use_certificate_buffer(ctx, der, derSz, + WOLFSSL_FILETYPE_ASN1), WOLFSSL_SUCCESS); + free(der); + wolfSSL_X509_free(x509); + wolfSSL_X509_free(x509v2); + wolfSSL_EVP_PKEY_free(priv); +} + + +/* override certificate version error */ +static int test_override_x509(int preverify, WOLFSSL_X509_STORE_CTX* store) +{ + AssertIntEQ(store->error, ASN_VERSION_E); + AssertIntEQ((int)wolfSSL_X509_get_version(store->current_cert), 1); + (void)preverify; + return 1; +} + + +/* set verify callback that will override bad certificate version */ +static void test_set_override_x509(WOLFSSL_CTX* ctx) +{ + wolfSSL_CTX_set_verify(ctx, WOLFSSL_VERIFY_PEER, test_override_x509); +} +#endif + + +static void test_wolfSSL_X509_TLS_version(void) +{ +#if !defined(NO_FILESYSTEM) && !defined(NO_CERTS) && !defined(NO_RSA) && \ + !defined(NO_WOLFSSL_CLIENT) && !defined(NO_DH) && !defined(NO_AES) && \ + defined(HAVE_IO_TESTS_DEPENDENCIES) && !defined(SINGLE_THREADED) && \ + defined(OPENSSL_EXTRA) + tcp_ready ready; + func_args server_args; + func_args client_args; + THREAD_TYPE serverThread; + callback_functions func_cb_client; + callback_functions func_cb_server; + + printf(testingFmt, "test_wolfSSL_X509_TLS_version"); + + /* test server rejects a client certificate that is not version 3 */ +#ifdef WOLFSSL_TIRTOS + fdOpenSession(Task_self()); +#endif + XMEMSET(&server_args, 0, sizeof(func_args)); + XMEMSET(&client_args, 0, sizeof(func_args)); + XMEMSET(&func_cb_client, 0, sizeof(callback_functions)); + XMEMSET(&func_cb_server, 0, sizeof(callback_functions)); + + StartTCP(); + InitTcpReady(&ready); + +#if defined(USE_WINDOWS_API) + /* use RNG to get random port if using windows */ + ready.port = GetRandomPort(); +#endif + + server_args.signal = &ready; + client_args.signal = &ready; + server_args.return_code = TEST_FAIL; + client_args.return_code = TEST_FAIL; + + func_cb_client.ctx_ready = &test_set_x509_badversion; +#ifndef WOLFSSL_NO_TLS12 + func_cb_client.method = wolfTLSv1_2_client_method; +#else + func_cb_client.method = wolfTLSv1_3_client_method; +#endif + client_args.callbacks = &func_cb_client; + +#ifndef WOLFSSL_NO_TLS12 + func_cb_server.method = wolfTLSv1_2_server_method; +#else + func_cb_server.method = wolfTLSv1_3_server_method; +#endif + server_args.callbacks = &func_cb_server; + + start_thread(test_server_nofail, &server_args, &serverThread); + wait_tcp_ready(&server_args); + test_client_nofail(&client_args, NULL); + join_thread(serverThread); + + AssertIntEQ(client_args.return_code, TEST_FAIL); + AssertIntEQ(server_args.return_code, TEST_FAIL); + + FreeTcpReady(&ready); + +#ifdef WOLFSSL_TIRTOS + fdCloseSession(Task_self()); +#endif + + /* Now re run but override the bad X509 version */ +#ifdef WOLFSSL_TIRTOS + fdOpenSession(Task_self()); +#endif + XMEMSET(&server_args, 0, sizeof(func_args)); + XMEMSET(&client_args, 0, sizeof(func_args)); + XMEMSET(&func_cb_client, 0, sizeof(callback_functions)); + XMEMSET(&func_cb_server, 0, sizeof(callback_functions)); + + StartTCP(); + InitTcpReady(&ready); + +#if defined(USE_WINDOWS_API) + /* use RNG to get random port if using windows */ + ready.port = GetRandomPort(); +#endif + + server_args.signal = &ready; + client_args.signal = &ready; + server_args.return_code = TEST_FAIL; + client_args.return_code = TEST_FAIL; + + func_cb_client.ctx_ready = &test_set_x509_badversion; + func_cb_server.ctx_ready = &test_set_override_x509; +#ifndef WOLFSSL_NO_TLS12 + func_cb_client.method = wolfTLSv1_2_client_method; +#else + func_cb_client.method = wolfTLSv1_3_client_method; +#endif + client_args.callbacks = &func_cb_client; + +#ifndef WOLFSSL_NO_TLS12 + func_cb_server.method = wolfTLSv1_2_server_method; +#else + func_cb_server.method = wolfTLSv1_3_server_method; +#endif + server_args.callbacks = &func_cb_server; + + start_thread(test_server_nofail, &server_args, &serverThread); + wait_tcp_ready(&server_args); + test_client_nofail(&client_args, NULL); + join_thread(serverThread); + + AssertIntEQ(client_args.return_code, TEST_SUCCESS); + AssertIntEQ(server_args.return_code, TEST_SUCCESS); + + FreeTcpReady(&ready); + +#ifdef WOLFSSL_TIRTOS + fdCloseSession(Task_self()); +#endif + + printf(resultFmt, passed); +#endif +} + /* Testing function wolfSSL_CTX_SetMinVersion; sets the minimum downgrade * version allowed. * POST: 1 on success. @@ -37738,6 +37936,7 @@ void ApiTest(void) test_wolfSSL_URI(); test_wolfSSL_TBS(); test_wolfSSL_X509_verify(); + test_wolfSSL_X509_TLS_version(); test_wc_PemToDer(); test_wc_AllocDer(); diff --git a/wolfssl/wolfcrypt/asn.h b/wolfssl/wolfcrypt/asn.h index 6ea6b6882..eff31fa7d 100644 --- a/wolfssl/wolfcrypt/asn.h +++ b/wolfssl/wolfcrypt/asn.h @@ -382,6 +382,9 @@ enum Misc_ASN { MIN_VERSION_SZ = 3, /* Min bytes needed for GetMyVersion */ MAX_X509_VERSION = 3, /* Max X509 version allowed */ MIN_X509_VERSION = 0, /* Min X509 version allowed */ + WOLFSSL_X509_V1 = 0, + WOLFSSL_X509_V2 = 1, + WOLFSSL_X509_V3 = 2, #if defined(OPENSSL_ALL) || defined(WOLFSSL_MYSQL_COMPATIBLE) || \ defined(WOLFSSL_NGINX) || defined(WOLFSSL_HAPROXY) || \ defined(OPENSSL_EXTRA) || defined(HAVE_PKCS7) From 4705ebde884e351012ba3855c840527ec3abe3aa Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Wed, 11 Nov 2020 21:53:52 +0700 Subject: [PATCH 3/4] add guard on test case for cert gen --- tests/api.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/api.c b/tests/api.c index 90f99886c..ffe6e1fea 100644 --- a/tests/api.c +++ b/tests/api.c @@ -5635,7 +5635,7 @@ static void test_wolfSSL_X509_verify(void) #if !defined(NO_FILESYSTEM) && !defined(NO_CERTS) && !defined(NO_RSA) && \ !defined(NO_WOLFSSL_CLIENT) && !defined(NO_DH) && !defined(NO_AES) && \ defined(HAVE_IO_TESTS_DEPENDENCIES) && !defined(SINGLE_THREADED) && \ - defined(OPENSSL_EXTRA) + defined(OPENSSL_EXTRA) && defined(WOLFSSL_CERT_GEN) /* create certificate with version 2 */ static void test_set_x509_badversion(WOLFSSL_CTX* ctx) { @@ -5711,7 +5711,7 @@ static void test_wolfSSL_X509_TLS_version(void) #if !defined(NO_FILESYSTEM) && !defined(NO_CERTS) && !defined(NO_RSA) && \ !defined(NO_WOLFSSL_CLIENT) && !defined(NO_DH) && !defined(NO_AES) && \ defined(HAVE_IO_TESTS_DEPENDENCIES) && !defined(SINGLE_THREADED) && \ - defined(OPENSSL_EXTRA) + defined(OPENSSL_EXTRA) && defined(WOLFSSL_CERT_GEN) tcp_ready ready; func_args server_args; func_args client_args; From a8333b09a04d66d3609d068cb5908e41254aef50 Mon Sep 17 00:00:00 2001 From: JacobBarthelmeh Date: Thu, 12 Nov 2020 20:24:47 -0800 Subject: [PATCH 4/4] memory cleanup with test case --- tests/api.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/tests/api.c b/tests/api.c index ffe6e1fea..ac64ddbca 100644 --- a/tests/api.c +++ b/tests/api.c @@ -5640,8 +5640,8 @@ static void test_wolfSSL_X509_verify(void) static void test_set_x509_badversion(WOLFSSL_CTX* ctx) { WOLFSSL_X509 *x509, *x509v2; - WOLFSSL_EVP_PKEY* priv; - unsigned char *der = NULL, *key = NULL; + WOLFSSL_EVP_PKEY *priv, *pub; + unsigned char *der = NULL, *key = NULL, *pt; char *header, *name; int derSz; long keySz; @@ -5655,8 +5655,10 @@ static void test_set_x509_badversion(WOLFSSL_CTX* ctx) fp = XFOPEN(cliKeyFile, "rb"); AssertIntEQ(wolfSSL_PEM_read(fp, &name, &header, &key, &keySz), WOLFSSL_SUCCESS); + XFCLOSE(fp); + pt = key; AssertNotNull(priv = wolfSSL_d2i_PrivateKey(EVP_PKEY_RSA, NULL, - (const unsigned char**)&key, keySz)); + (const unsigned char**)&pt, keySz)); /* create the version 2 certificate */ @@ -5667,8 +5669,8 @@ static void test_set_x509_badversion(WOLFSSL_CTX* ctx) wolfSSL_X509_get_subject_name(x509)), WOLFSSL_SUCCESS); AssertIntEQ(wolfSSL_X509_set_issuer_name(x509v2, wolfSSL_X509_get_issuer_name(x509)), WOLFSSL_SUCCESS); - AssertIntEQ(X509_set_pubkey(x509v2, - wolfSSL_X509_get_pubkey(x509)), WOLFSSL_SUCCESS); + AssertNotNull(pub = wolfSSL_X509_get_pubkey(x509)); + AssertIntEQ(X509_set_pubkey(x509v2, pub), WOLFSSL_SUCCESS); t = time(NULL); AssertNotNull(notBefore = wolfSSL_ASN1_TIME_adj(NULL, t, 0, 0)); @@ -5682,9 +5684,18 @@ static void test_set_x509_badversion(WOLFSSL_CTX* ctx) AssertIntEQ(wolfSSL_CTX_use_certificate_buffer(ctx, der, derSz, WOLFSSL_FILETYPE_ASN1), WOLFSSL_SUCCESS); free(der); + if (key != NULL) + free(key); + if (name != NULL) + free(name); + if (header != NULL) + free(header); wolfSSL_X509_free(x509); wolfSSL_X509_free(x509v2); wolfSSL_EVP_PKEY_free(priv); + wolfSSL_EVP_PKEY_free(pub); + wolfSSL_ASN1_TIME_free(notBefore); + wolfSSL_ASN1_TIME_free(notAfter); }