From 26f3924c93fc4d658e2c9198a203062220088826 Mon Sep 17 00:00:00 2001 From: David Garske Date: Tue, 4 Apr 2017 10:55:22 -0700 Subject: [PATCH 1/6] Fix for return code checking on ConfirmSignature, so it returns actual error codes or 0 on success. --- wolfcrypt/src/asn.c | 76 ++++++++++++++++++++++++--------------------- 1 file changed, 40 insertions(+), 36 deletions(-) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index d68a4220b..4760a4896 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -4164,13 +4164,13 @@ int wc_GetCTC_HashOID(int type) }; } -/* return true (1) or false (0) for Confirmation */ +/* return 0=success, else failure */ static int ConfirmSignature(const byte* buf, word32 bufSz, const byte* key, word32 keySz, word32 keyOID, const byte* sig, word32 sigSz, word32 sigOID, void* heap) { - int typeH = 0, digestSz = 0, ret = 0; + int typeH = 0, digestSz = 0, ret = -1; #ifdef WOLFSSL_SMALL_STACK byte* digest; #else @@ -4180,7 +4180,7 @@ static int ConfirmSignature(const byte* buf, word32 bufSz, #ifdef WOLFSSL_SMALL_STACK digest = (byte*)XMALLOC(WC_MAX_DIGEST_SIZE, NULL, DYNAMIC_TYPE_TMP_BUFFER); if (digest == NULL) - return 0; /* not confirmed */ + return MEMORY_E; #endif (void)key; @@ -4260,7 +4260,7 @@ static int ConfirmSignature(const byte* buf, word32 bufSz, #ifdef WOLFSSL_SMALL_STACK XFREE(digest, NULL, DYNAMIC_TYPE_TMP_BUFFER); #endif - return 0; /* not confirmed */ + return ALGO_ID_E; } switch (keyOID) { @@ -4328,21 +4328,26 @@ static int ConfirmSignature(const byte* buf, word32 bufSz, WOLFSSL_MSG("Rsa SSL verify error"); } else { + #ifdef WOLFSSL_DEBUG_ENCODING + int x; + #endif verifySz = ret; - /* make sure we're right justified */ - encodedSigSz = - wc_EncodeSignature(encodedSig, digest, digestSz, typeH); - if (encodedSigSz != verifySz || - XMEMCMP(out, encodedSig, encodedSigSz) != 0) { - WOLFSSL_MSG("Rsa SSL verify match encode error"); - } - else - ret = 1; /* match */ + + ret = wc_EncodeSignature(encodedSig, digest, digestSz, typeH); + if (ret > 0) { + encodedSigSz = ret; + + /* check length to make sure we're right justified */ + if (encodedSigSz == verifySz && + XMEMCMP(out, encodedSig, encodedSigSz) == 0) { + ret = 0; /* match */ + } + else { + WOLFSSL_MSG("Rsa SSL verify match encode error"); + ret = SIG_VERIFY_E; + } #ifdef WOLFSSL_DEBUG_ENCODING - { - int x; - printf("wolfssl encodedSig:\n"); for (x = 0; x < encodedSigSz; x++) { @@ -4361,11 +4366,9 @@ static int ConfirmSignature(const byte* buf, word32 bufSz, } printf("\n"); - } #endif /* WOLFSSL_DEBUG_ENCODING */ - + } } - } wc_FreeRsaKey(pubKey); @@ -4406,15 +4409,17 @@ static int ConfirmSignature(const byte* buf, word32 bufSz, WOLFSSL_MSG("ASN Key import error ECC"); } else { - if (wc_ecc_verify_hash(sig, sigSz, digest, digestSz, &verify, - pubKey) != 0) { + ret = wc_ecc_verify_hash(sig, sigSz, digest, digestSz, &verify, + pubKey); + if (ret != 0) { WOLFSSL_MSG("ECC verify hash error"); } - else if (1 != verify) { + else if (verify != 1) { WOLFSSL_MSG("ECC Verify didn't match"); - } else - ret = 1; /* match */ - + ret = SIG_VERIFY_E; + } else { + ret = 0; /* match */ + } } wc_ecc_free(pubKey); @@ -5802,11 +5807,11 @@ int ParseCertRelative(DecodedCert* cert, int type, int verify, void* cm) if (verify == VERIFY) { /* try to confirm/verify signature */ - if (!ConfirmSignature(cert->source + cert->certBegin, + if (ConfirmSignature(cert->source + cert->certBegin, cert->sigIndex - cert->certBegin, ca->publicKey, ca->pubKeySize, ca->keyOID, cert->signature, cert->sigLength, cert->signatureOID, - cert->heap)) { + cert->heap) != 0) { WOLFSSL_MSG("Confirm signature failed"); return ASN_SIG_CONFIRM_E; } @@ -10059,8 +10064,7 @@ static int DecodeBasicOcspResponse(byte* source, word32* ioIndex, resp->sig, resp->sigSz, resp->sigOID, NULL); FreeDecodedCert(&cert); - if (ret == 0) - { + if (ret != 0) { WOLFSSL_MSG("\tOCSP Confirm signature failed"); return ASN_OCSP_CONFIRM_E; } @@ -10076,9 +10080,9 @@ static int DecodeBasicOcspResponse(byte* source, word32* ioIndex, ca = GetCA(cm, resp->issuerHash); #endif - if (!ca || !ConfirmSignature(resp->response, resp->responseSz, - ca->publicKey, ca->pubKeySize, ca->keyOID, - resp->sig, resp->sigSz, resp->sigOID, NULL)) { + if (!ca || ConfirmSignature(resp->response, resp->responseSz, + ca->publicKey, ca->pubKeySize, ca->keyOID, + resp->sig, resp->sigSz, resp->sigOID, NULL) != 0) { WOLFSSL_MSG("\tOCSP Confirm signature failed"); return ASN_OCSP_CONFIRM_E; } @@ -10694,10 +10698,10 @@ int ParseCRL(DecodedCRL* dcrl, const byte* buff, word32 sz, void* cm) return ASN_CRL_NO_SIGNER_E; } #endif /* IGNORE_KEY_EXTENSIONS */ - if (!ConfirmSignature(buff + dcrl->certBegin, - dcrl->sigIndex - dcrl->certBegin, - ca->publicKey, ca->pubKeySize, ca->keyOID, - dcrl->signature, dcrl->sigLength, dcrl->signatureOID, NULL)) { + if (ConfirmSignature(buff + dcrl->certBegin, + dcrl->sigIndex - dcrl->certBegin, ca->publicKey, + ca->pubKeySize, ca->keyOID, dcrl->signature, dcrl->sigLength, + dcrl->signatureOID, NULL) != 0) { WOLFSSL_MSG("CRL Confirm signature failed"); return ASN_CRL_CONFIRM_E; } From 2aa523e0ea2f89bcd3fcde8a48e6f3da96069ca5 Mon Sep 17 00:00:00 2001 From: David Garske Date: Tue, 4 Apr 2017 11:15:32 -0700 Subject: [PATCH 2/6] Added API unit test for certificate validation failure using corrupted signature in cert. --- tests/api.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++++- wolfssl/test.h | 56 +++++++++++++++++------ 2 files changed, 162 insertions(+), 15 deletions(-) diff --git a/tests/api.c b/tests/api.c index 78a3a4d68..d6d623406 100644 --- a/tests/api.c +++ b/tests/api.c @@ -77,7 +77,7 @@ static const char* passed = "passed"; static const char* failed = "failed"; #if !defined(NO_FILESYSTEM) && !defined(NO_CERTS) - static const char* bogusFile = + static const char* bogusFile = #ifdef _WIN32 "NUL" #else @@ -86,6 +86,12 @@ static const char* failed = "failed"; ; #endif +enum { + TESTING_RSA = 1, + TESTING_ECC = 2 +}; + + /*----------------------------------------------------------------------------* | Setup *----------------------------------------------------------------------------*/ @@ -541,7 +547,7 @@ static void test_wolfSSL_SetTmpDH_buffer(void) /* Test function for wolfSSL_SetMinVersion. Sets the minimum downgrade version - * allowed. + * allowed. * POST: return 1 on success. */ static int test_wolfSSL_SetMinVersion(void) @@ -3128,6 +3134,111 @@ static void test_wc_ecc_get_curve_id_from_name(void) } +/*----------------------------------------------------------------------------* + | Certficate Failure Checks + *----------------------------------------------------------------------------*/ +#ifndef NO_CERTS + /* Use the Cert Manager(CM) API to generate the error ASN_SIG_CONFIRM_E */ + static int verify_sig_cm(const char* ca, byte* cert_buf, size_t cert_sz, + int type) + { + int ret; + WOLFSSL_CERT_MANAGER* cm = NULL; + + switch (type) { + case TESTING_RSA: + #ifdef NO_RSA + printf("RSA disabled, skipping test\n"); + return ASN_SIG_CONFIRM_E; + #else + break; + #endif + case TESTING_ECC: + #ifndef HAVE_ECC + printf("ECC disabled, skipping test\n"); + return ASN_SIG_CONFIRM_E; + #else + break; + #endif + default: + printf("Bad function argument\n"); + return BAD_FUNC_ARG; + } + cm = wolfSSL_CertManagerNew(); + if (cm == NULL) { + printf("wolfSSL_CertManagerNew failed\n"); + return -1; + } + + ret = wolfSSL_CertManagerLoadCA(cm, ca, 0); + if (ret != SSL_SUCCESS) { + printf("wolfSSL_CertManagerLoadCA failed\n"); + wolfSSL_CertManagerFree(cm); + return ret; + } + + ret = wolfSSL_CertManagerVerifyBuffer(cm, cert_buf, cert_sz, SSL_FILETYPE_ASN1); + /* Let AssertIntEQ handle return code */ + + wolfSSL_CertManagerFree(cm); + + return ret; + } + + static int test_RsaConfirmSig_cm(void) + { + int ret = 0; + const char* ca_cert = "./certs/ca-cert.pem"; + const char* server_cert = "./certs/server-cert.der"; + byte* cert_buf = NULL; + size_t cert_sz = 0; + + ret = load_file(server_cert, &cert_buf, &cert_sz); + if (ret == 0) { + /* corrupt DER - invert last byte, which is signature */ + cert_buf[cert_sz-1] = ~cert_buf[cert_sz-1]; + + /* test bad cert */ + ret = verify_sig_cm(ca_cert, cert_buf, cert_sz, TESTING_RSA); + } + + printf("Verify signature exception test: RSA: Ret %d\n", ret); + + if (cert_buf) + free(cert_buf); + + return ret; + } + + static int test_EccConfirmSig_cm(void) + { + int ret = 0; + /* self-signed ECC cert, so use server cert as CA */ + const char* ca_cert = "./certs/server-ecc.pem"; + const char* server_cert = "./certs/server-ecc.der"; + byte* cert_buf = NULL; + size_t cert_sz = 0; + + ret = load_file(server_cert, &cert_buf, &cert_sz); + if (ret == 0) { + /* corrupt DER - invert last byte, which is signature */ + cert_buf[cert_sz-1] = ~cert_buf[cert_sz-1]; + + /* test bad cert */ + ret = verify_sig_cm(ca_cert, cert_buf, cert_sz, TESTING_ECC); + } + + printf("Verify signature exception test: ECC: Ret %d\n", ret); + + if (cert_buf) + free(cert_buf); + + return ret; + } + +#endif /* NO_CERTS */ + + /*----------------------------------------------------------------------------* | Main *----------------------------------------------------------------------------*/ @@ -3196,6 +3307,12 @@ void ApiTest(void) test_wc_ecc_get_curve_size_from_name(); test_wc_ecc_get_curve_id_from_name(); +#ifndef NO_CERTS + /* Bad certificate tests */ + AssertIntEQ(test_EccConfirmSig_cm(), ASN_SIG_CONFIRM_E); + AssertIntEQ(test_RsaConfirmSig_cm(), ASN_SIG_CONFIRM_E); +#endif /* NO_CERTS */ + printf(" End API Tests\n"); } diff --git a/wolfssl/test.h b/wolfssl/test.h index db73f406c..4e5e7446a 100644 --- a/wolfssl/test.h +++ b/wolfssl/test.h @@ -1099,8 +1099,41 @@ static INLINE unsigned int my_psk_server_cb(WOLFSSL* ssl, const char* identity, #endif /* USE_WINDOWS_API */ -#if defined(NO_FILESYSTEM) && !defined(NO_CERTS) && defined(FORCE_BUFFER_TEST) +#if !defined(NO_CERTS) + /* reads file size, allocates buffer, reads into buffer, returns buffer */ + static INLINE int load_file(const char* fname, byte** buf, size_t* bufLen) + { + int ret = 0; + FILE* file = fopen(fname, "rb"); + + if (!file) { + printf("Error loading %s\n", fname); + return -1; + } + + fseek(file, 0, SEEK_END); + *bufLen = ftell(file); + rewind(file); + if (*bufLen > 0) { + *buf = (byte*)malloc(*bufLen); + if (*buf == NULL) { + ret = MEMORY_E; + printf("Error allocating %lu bytes\n", *bufLen); + } + else { + size_t readLen = fread(*buf, *bufLen, 1, file); + + /* check response code */ + ret = (readLen > 0) ? 0 : -1; + } + } + fclose(file); + + return ret; + } + + #if defined(NO_FILESYSTEM) && defined(FORCE_BUFFER_TEST) enum { WOLFSSL_CA = 1, WOLFSSL_CERT = 2, @@ -1111,19 +1144,14 @@ static INLINE unsigned int my_psk_server_cb(WOLFSSL* ssl, const char* identity, static INLINE void load_buffer(WOLFSSL_CTX* ctx, const char* fname, int type) { int format = SSL_FILETYPE_PEM; + byte* buff = NULL; + size_t sz = 0; - /* test buffer load */ - long sz = 0; - byte buff[10000]; - FILE* file = fopen(fname, "rb"); - - if (!file) + ret = load_file(fname, &buff, &sz); + if (ret != 0) { err_sys("can't open file for buffer load " "Please run from wolfSSL home directory if not"); - fseek(file, 0, SEEK_END); - sz = ftell(file); - rewind(file); - fread(buff, sizeof(buff), 1, file); + } /* determine format */ if (strstr(fname, ".der")) @@ -1150,10 +1178,12 @@ static INLINE unsigned int my_psk_server_cb(WOLFSSL* ssl, const char* identity, err_sys("can't load cert chain buffer"); } - fclose(file); + if (buff) + free(buff); } + #endif /* NO_FILESYSTEM && FORCE_BUFFER_TEST */ -#endif /* NO_FILESYSTEM */ +#endif /* !NO_CERTS */ #ifdef VERIFY_CALLBACK From e0a4758c0f469f522270defc4e2265675d95b00e Mon Sep 17 00:00:00 2001 From: David Garske Date: Tue, 4 Apr 2017 11:40:54 -0700 Subject: [PATCH 3/6] Fixes with load_file helper to make sure return code is set correctly and args are initialized. --- wolfssl/test.h | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/wolfssl/test.h b/wolfssl/test.h index 4e5e7446a..7b93e7fc9 100644 --- a/wolfssl/test.h +++ b/wolfssl/test.h @@ -1104,12 +1104,21 @@ static INLINE unsigned int my_psk_server_cb(WOLFSSL* ssl, const char* identity, /* reads file size, allocates buffer, reads into buffer, returns buffer */ static INLINE int load_file(const char* fname, byte** buf, size_t* bufLen) { - int ret = 0; - FILE* file = fopen(fname, "rb"); + int ret; + FILE* file; + if (fname == NULL || buf == NULL || bufLen == NULL) + return BAD_FUNC_ARG; + + /* set defaults */ + *buf = NULL; + *bufLen = 0; + + /* open file (read-only binary) */ + file = fopen(fname, "rb"); if (!file) { printf("Error loading %s\n", fname); - return -1; + return BAD_PATH_ERROR; } fseek(file, 0, SEEK_END); @@ -1128,6 +1137,9 @@ static INLINE unsigned int my_psk_server_cb(WOLFSSL* ssl, const char* identity, ret = (readLen > 0) ? 0 : -1; } } + else { + ret = BUFFER_E; + } fclose(file); return ret; From deb80e5ddbea318f28f7d1ac0072b2e560f79ce8 Mon Sep 17 00:00:00 2001 From: David Garske Date: Tue, 4 Apr 2017 11:45:55 -0700 Subject: [PATCH 4/6] =?UTF-8?q?Fix=20load=5Ffile/load=5Fbuffer=20enables.?= =?UTF-8?q?=20Fix=20error=20in=20load=5Fbuffer=20with=20=E2=80=9Cret?= =?UTF-8?q?=E2=80=9D.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- wolfssl/test.h | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/wolfssl/test.h b/wolfssl/test.h index 7b93e7fc9..19b486673 100644 --- a/wolfssl/test.h +++ b/wolfssl/test.h @@ -1100,6 +1100,8 @@ static INLINE unsigned int my_psk_server_cb(WOLFSSL* ssl, const char* identity, #if !defined(NO_CERTS) + #if !defined(NO_FILESYSTEM) || \ + (defined(NO_FILESYSTEM) && defined(FORCE_BUFFER_TEST)) /* reads file size, allocates buffer, reads into buffer, returns buffer */ static INLINE int load_file(const char* fname, byte** buf, size_t* bufLen) @@ -1145,7 +1147,6 @@ static INLINE unsigned int my_psk_server_cb(WOLFSSL* ssl, const char* identity, return ret; } - #if defined(NO_FILESYSTEM) && defined(FORCE_BUFFER_TEST) enum { WOLFSSL_CA = 1, WOLFSSL_CERT = 2, @@ -1159,8 +1160,7 @@ static INLINE unsigned int my_psk_server_cb(WOLFSSL* ssl, const char* identity, byte* buff = NULL; size_t sz = 0; - ret = load_file(fname, &buff, &sz); - if (ret != 0) { + if (load_file(fname, &buff, &sz) != 0) { err_sys("can't open file for buffer load " "Please run from wolfSSL home directory if not"); } @@ -1193,8 +1193,7 @@ static INLINE unsigned int my_psk_server_cb(WOLFSSL* ssl, const char* identity, if (buff) free(buff); } - #endif /* NO_FILESYSTEM && FORCE_BUFFER_TEST */ - + #endif /* !NO_FILESYSTEM || (NO_FILESYSTEM && FORCE_BUFFER_TEST) */ #endif /* !NO_CERTS */ #ifdef VERIFY_CALLBACK From 15091675fee855fda4dfa82322426f49724e2865 Mon Sep 17 00:00:00 2001 From: David Garske Date: Wed, 5 Apr 2017 07:23:53 -0700 Subject: [PATCH 5/6] Fix VS warning about long to size_t conversion in load_buffer. --- wolfssl/test.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/wolfssl/test.h b/wolfssl/test.h index 19b486673..bed19d9c5 100644 --- a/wolfssl/test.h +++ b/wolfssl/test.h @@ -1170,23 +1170,23 @@ static INLINE unsigned int my_psk_server_cb(WOLFSSL* ssl, const char* identity, format = SSL_FILETYPE_ASN1; if (type == WOLFSSL_CA) { - if (wolfSSL_CTX_load_verify_buffer(ctx, buff, sz, format) + if (wolfSSL_CTX_load_verify_buffer(ctx, buff, (long)sz, format) != SSL_SUCCESS) err_sys("can't load buffer ca file"); } else if (type == WOLFSSL_CERT) { - if (wolfSSL_CTX_use_certificate_buffer(ctx, buff, sz, + if (wolfSSL_CTX_use_certificate_buffer(ctx, buff, (long)sz, format) != SSL_SUCCESS) err_sys("can't load buffer cert file"); } else if (type == WOLFSSL_KEY) { - if (wolfSSL_CTX_use_PrivateKey_buffer(ctx, buff, sz, + if (wolfSSL_CTX_use_PrivateKey_buffer(ctx, buff, (long)sz, format) != SSL_SUCCESS) err_sys("can't load buffer key file"); } else if (type == WOLFSSL_CERT_CHAIN) { - if (wolfSSL_CTX_use_certificate_chain_buffer_format(ctx, buff, sz, - format) != SSL_SUCCESS) + if (wolfSSL_CTX_use_certificate_chain_buffer_format(ctx, buff, + (long)sz, format) != SSL_SUCCESS) err_sys("can't load cert chain buffer"); } From 3328b4d38bda36edee5ec12e370838fd2df882c3 Mon Sep 17 00:00:00 2001 From: David Garske Date: Wed, 5 Apr 2017 11:24:22 -0700 Subject: [PATCH 6/6] Cleanup the unit test naming for new signature failure tests. --- tests/api.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/api.c b/tests/api.c index d6d623406..18aea0834 100644 --- a/tests/api.c +++ b/tests/api.c @@ -3185,7 +3185,7 @@ static void test_wc_ecc_get_curve_id_from_name(void) return ret; } - static int test_RsaConfirmSig_cm(void) + static int test_RsaSigFailure_cm(void) { int ret = 0; const char* ca_cert = "./certs/ca-cert.pem"; @@ -3202,7 +3202,7 @@ static void test_wc_ecc_get_curve_id_from_name(void) ret = verify_sig_cm(ca_cert, cert_buf, cert_sz, TESTING_RSA); } - printf("Verify signature exception test: RSA: Ret %d\n", ret); + printf("Signature failure test: RSA: Ret %d\n", ret); if (cert_buf) free(cert_buf); @@ -3210,7 +3210,7 @@ static void test_wc_ecc_get_curve_id_from_name(void) return ret; } - static int test_EccConfirmSig_cm(void) + static int test_EccSigFailure_cm(void) { int ret = 0; /* self-signed ECC cert, so use server cert as CA */ @@ -3228,7 +3228,7 @@ static void test_wc_ecc_get_curve_id_from_name(void) ret = verify_sig_cm(ca_cert, cert_buf, cert_sz, TESTING_ECC); } - printf("Verify signature exception test: ECC: Ret %d\n", ret); + printf("Signature failure test: ECC: Ret %d\n", ret); if (cert_buf) free(cert_buf); @@ -3308,9 +3308,9 @@ void ApiTest(void) test_wc_ecc_get_curve_id_from_name(); #ifndef NO_CERTS - /* Bad certificate tests */ - AssertIntEQ(test_EccConfirmSig_cm(), ASN_SIG_CONFIRM_E); - AssertIntEQ(test_RsaConfirmSig_cm(), ASN_SIG_CONFIRM_E); + /* Bad certificate signature tests */ + AssertIntEQ(test_EccSigFailure_cm(), ASN_SIG_CONFIRM_E); + AssertIntEQ(test_RsaSigFailure_cm(), ASN_SIG_CONFIRM_E); #endif /* NO_CERTS */ printf(" End API Tests\n");