From e3153f3997d8a5c4c95ea4a8efefb199ada4590f Mon Sep 17 00:00:00 2001 From: jordan Date: Mon, 24 Oct 2022 18:09:44 -0500 Subject: [PATCH 1/6] Fix X509 subject and issuer name_hash mismatch --- src/x509.c | 100 +++++++++++++++++++++++++++++++++++++--------------- tests/api.c | 46 +++++++++++++++++------- 2 files changed, 105 insertions(+), 41 deletions(-) diff --git a/src/x509.c b/src/x509.c index 9a40ca74f..93371a2fc 100644 --- a/src/x509.c +++ b/src/x509.c @@ -4767,67 +4767,109 @@ WOLFSSL_X509_NAME* wolfSSL_X509_get_subject_name(WOLFSSL_X509* cert) #if defined(OPENSSL_EXTRA) && (!defined(NO_SHA) || !defined(NO_SHA256)) /****************************************************************************** -* wolfSSL_X509_subject_name_hash - compute the hash digest of the raw subject name -* This function prefers SHA-1 (if available) for compatibility +* wolfSSL_X509_subject_name_hash +* wolfSSL_X509_issuer_name_hash +* Compute the hash digest of the subject / issuer name. +* These functions prefers SHA-1 (if available) for compatibility. * * RETURNS: -* The beginning of the hash digest. Otherwise, returns zero. +* The first 4 bytes of SHA hash in little endian order as unsigned long. +* Otherwise, returns zero. * Note: -* Returns a different hash value from OpenSSL's X509_subject_name_hash() API -* depending on the subject name. +* Returns the same hash value as OpenSSL's X509_X_name_hash() API +* if SHA-1 support is compiled in. */ unsigned long wolfSSL_X509_subject_name_hash(const WOLFSSL_X509* x509) { unsigned long ret = 0; - int retHash = NOT_COMPILED_IN; WOLFSSL_X509_NAME *subjectName = NULL; + unsigned char* canonName = NULL; byte digest[WC_MAX_DIGEST_SIZE]; + int size = 0; if (x509 == NULL) { return ret; } subjectName = wolfSSL_X509_get_subject_name((WOLFSSL_X509*)x509); - if (subjectName != NULL) { - #ifndef NO_SHA - retHash = wc_ShaHash((const byte*)subjectName->name, - (word32)subjectName->sz, digest); - #elif !defined(NO_SHA256) - retHash = wc_Sha256Hash((const byte*)subjectName->name, - (word32)subjectName->sz, digest); - #endif - if (retHash == 0) { - ret = (unsigned long)MakeWordFromHash(digest); - } + + if (subjectName == NULL) { + return ret; } + size = wolfSSL_i2d_X509_NAME_canon(subjectName, &canonName); + + if (size <= 0){ + WOLFSSL_MSG("wolfSSL_i2d_X509_NAME_canon error"); + return ret; + } + + #ifndef NO_SHA + if (wc_ShaHash((const byte*)canonName, (word32)size, digest) != 0) { + WOLFSSL_MSG("wc_ShaHash error"); + return ret; + } + #elif !defined(NO_SHA256) + if (wc_Sha256Hash((const byte*)canonName, (word32)size, digest) != 0) { + WOLFSSL_MSG("wc_Sha256Hash error"); + return ret; + } + #endif + + ret = (unsigned long) digest[0]; + ret |= ((unsigned long) digest[1]) << 8; + ret |= ((unsigned long) digest[2]) << 16; + ret |= ((unsigned long) digest[3]) << 24; + + XFREE(canonName, NULL, DYNAMIC_TYPE_OPENSSL); + return ret; } unsigned long wolfSSL_X509_issuer_name_hash(const WOLFSSL_X509* x509) { unsigned long ret = 0; - int retHash = NOT_COMPILED_IN; WOLFSSL_X509_NAME *issuerName = NULL; + unsigned char* canonName = NULL; byte digest[WC_MAX_DIGEST_SIZE]; + int size = 0; if (x509 == NULL) { return ret; } issuerName = wolfSSL_X509_get_issuer_name((WOLFSSL_X509*)x509); - if (issuerName != NULL) { - #ifndef NO_SHA - retHash = wc_ShaHash((const byte*)issuerName->name, - (word32)issuerName->sz, digest); - #elif !defined(NO_SHA256) - retHash = wc_Sha256Hash((const byte*)issuerName->name, - (word32)issuerName->sz, digest); - #endif - if (retHash == 0) { - ret = (unsigned long)MakeWordFromHash(digest); - } + + if (issuerName == NULL) { + return ret; } + + size = wolfSSL_i2d_X509_NAME_canon(issuerName, &canonName); + + if (size <= 0){ + WOLFSSL_MSG("wolfSSL_i2d_X509_NAME_canon error"); + return ret; + } + + #ifndef NO_SHA + if (wc_ShaHash((const byte*)canonName, (word32)size, digest) != 0) { + WOLFSSL_MSG("wc_ShaHash error"); + return ret; + } + #elif !defined(NO_SHA256) + if (wc_Sha256Hash((const byte*)canonName, (word32)size, digest) != 0) { + WOLFSSL_MSG("wc_ShaHash error"); + return ret; + } + #endif + + ret = (unsigned long) digest[0]; + ret |= ((unsigned long) digest[1]) << 8; + ret |= ((unsigned long) digest[2]) << 16; + ret |= ((unsigned long) digest[3]) << 24; + + XFREE(canonName, NULL, DYNAMIC_TYPE_OPENSSL); + return ret; } #endif /* OPENSSL_EXTRA && (!NO_SHA || !NO_SHA256) */ diff --git a/tests/api.c b/tests/api.c index 3981e4242..f50058ab3 100644 --- a/tests/api.c +++ b/tests/api.c @@ -31387,25 +31387,36 @@ static int test_wolfSSL_X509_subject_name_hash(void) { #if defined(OPENSSL_EXTRA) && !defined(NO_CERTS) && !defined(NO_FILESYSTEM) \ && !defined(NO_RSA) && (!defined(NO_SHA) || !defined(NO_SHA256)) - X509* x509; X509_NAME* subjectName = NULL; - unsigned long ret = 0; + unsigned long ret1 = 0; + unsigned long ret2 = 0; printf(testingFmt, "wolfSSL_X509_subject_name_hash()"); AssertNotNull(x509 = wolfSSL_X509_load_certificate_file(cliCertFile, SSL_FILETYPE_PEM)); - AssertNotNull(subjectName = wolfSSL_X509_get_subject_name(x509)); - ret = X509_subject_name_hash(x509); - AssertIntNE(ret, 0); + + /* These two + * - X509_subject_name_hash(x509) + * - X509_NAME_hash(X509_get_subject_name(x509)) + * should give the same hash, if !defined(NO_SHA) is true. */ + + ret1 = X509_subject_name_hash(x509); + AssertIntNE(ret1, 0); + + ret2 = X509_NAME_hash(X509_get_subject_name(x509)); + AssertIntNE(ret2, 0); + +#if !defined(NO_SHA) + AssertIntEQ(ret1, ret2); +#endif X509_free(x509); printf(resultFmt, passed); #endif - return 0; } @@ -31413,25 +31424,36 @@ static int test_wolfSSL_X509_issuer_name_hash(void) { #if defined(OPENSSL_EXTRA) && !defined(NO_CERTS) && !defined(NO_FILESYSTEM) \ && !defined(NO_RSA) && (!defined(NO_SHA) || !defined(NO_SHA256)) - X509* x509; X509_NAME* issuertName = NULL; - unsigned long ret = 0; + unsigned long ret1 = 0; + unsigned long ret2 = 0; printf(testingFmt, "wolfSSL_X509_issuer_name_hash()"); AssertNotNull(x509 = wolfSSL_X509_load_certificate_file(cliCertFile, SSL_FILETYPE_PEM)); - AssertNotNull(issuertName = wolfSSL_X509_get_issuer_name(x509)); - ret = X509_issuer_name_hash(x509); - AssertIntNE(ret, 0); + + /* These two + * - X509_issuer_name_hash(x509) + * - X509_NAME_hash(X509_get_issuer_name(x509)) + * should give the same hash, if !defined(NO_SHA) is true. */ + + ret1 = X509_issuer_name_hash(x509); + AssertIntNE(ret1, 0); + + ret2 = X509_NAME_hash(X509_get_issuer_name(x509)); + AssertIntNE(ret2, 0); + +#if !defined(NO_SHA) + AssertIntEQ(ret1, ret2); +#endif X509_free(x509); printf(resultFmt, passed); #endif - return 0; } From 8b7668f77114a4fff118e2d4755122e3321f1813 Mon Sep 17 00:00:00 2001 From: jordan Date: Tue, 25 Oct 2022 13:00:22 -0500 Subject: [PATCH 2/6] Fix X509 subject and issuer name_hash mismatch Fix api test and cleanup. --- src/x509.c | 66 +++++++++++++++++++++++++++-------------------------- tests/api.c | 8 +++++-- 2 files changed, 40 insertions(+), 34 deletions(-) diff --git a/src/x509.c b/src/x509.c index 93371a2fc..695f23222 100644 --- a/src/x509.c +++ b/src/x509.c @@ -4781,96 +4781,98 @@ WOLFSSL_X509_NAME* wolfSSL_X509_get_subject_name(WOLFSSL_X509* cert) */ unsigned long wolfSSL_X509_subject_name_hash(const WOLFSSL_X509* x509) { - unsigned long ret = 0; - WOLFSSL_X509_NAME *subjectName = NULL; - unsigned char* canonName = NULL; - byte digest[WC_MAX_DIGEST_SIZE]; - int size = 0; + unsigned long hash = 0; + WOLFSSL_X509_NAME* subjectName = NULL; + unsigned char* canonName = NULL; + byte digest[WC_MAX_DIGEST_SIZE]; + int size = 0; if (x509 == NULL) { - return ret; + return 0; } subjectName = wolfSSL_X509_get_subject_name((WOLFSSL_X509*)x509); if (subjectName == NULL) { - return ret; + WOLFSSL_MSG("wolfSSL_X509_get_subject_name error"); + return 0; } size = wolfSSL_i2d_X509_NAME_canon(subjectName, &canonName); - if (size <= 0){ + if (size <= 0 || canonName == NULL){ WOLFSSL_MSG("wolfSSL_i2d_X509_NAME_canon error"); - return ret; + return 0; } #ifndef NO_SHA if (wc_ShaHash((const byte*)canonName, (word32)size, digest) != 0) { WOLFSSL_MSG("wc_ShaHash error"); - return ret; + return 0; } #elif !defined(NO_SHA256) if (wc_Sha256Hash((const byte*)canonName, (word32)size, digest) != 0) { WOLFSSL_MSG("wc_Sha256Hash error"); - return ret; + return 0; } #endif - ret = (unsigned long) digest[0]; - ret |= ((unsigned long) digest[1]) << 8; - ret |= ((unsigned long) digest[2]) << 16; - ret |= ((unsigned long) digest[3]) << 24; + hash = (unsigned long) digest[0]; + hash |= ((unsigned long) digest[1]) << 8; + hash |= ((unsigned long) digest[2]) << 16; + hash |= ((unsigned long) digest[3]) << 24; XFREE(canonName, NULL, DYNAMIC_TYPE_OPENSSL); - return ret; + return hash; } unsigned long wolfSSL_X509_issuer_name_hash(const WOLFSSL_X509* x509) { - unsigned long ret = 0; - WOLFSSL_X509_NAME *issuerName = NULL; - unsigned char* canonName = NULL; - byte digest[WC_MAX_DIGEST_SIZE]; - int size = 0; + unsigned long hash = 0; + WOLFSSL_X509_NAME* issuerName = NULL; + unsigned char* canonName = NULL; + byte digest[WC_MAX_DIGEST_SIZE]; + int size = 0; if (x509 == NULL) { - return ret; + return 0; } issuerName = wolfSSL_X509_get_issuer_name((WOLFSSL_X509*)x509); if (issuerName == NULL) { - return ret; + WOLFSSL_MSG("wolfSSL_X509_get_issuer_name error"); + return 0; } size = wolfSSL_i2d_X509_NAME_canon(issuerName, &canonName); - if (size <= 0){ + if (size <= 0 || canonName == NULL){ WOLFSSL_MSG("wolfSSL_i2d_X509_NAME_canon error"); - return ret; + return 0; } #ifndef NO_SHA if (wc_ShaHash((const byte*)canonName, (word32)size, digest) != 0) { WOLFSSL_MSG("wc_ShaHash error"); - return ret; + return 0; } #elif !defined(NO_SHA256) if (wc_Sha256Hash((const byte*)canonName, (word32)size, digest) != 0) { WOLFSSL_MSG("wc_ShaHash error"); - return ret; + return 0; } #endif - ret = (unsigned long) digest[0]; - ret |= ((unsigned long) digest[1]) << 8; - ret |= ((unsigned long) digest[2]) << 16; - ret |= ((unsigned long) digest[3]) << 24; + hash = (unsigned long) digest[0]; + hash |= ((unsigned long) digest[1]) << 8; + hash |= ((unsigned long) digest[2]) << 16; + hash |= ((unsigned long) digest[3]) << 24; XFREE(canonName, NULL, DYNAMIC_TYPE_OPENSSL); - return ret; + return hash; } #endif /* OPENSSL_EXTRA && (!NO_SHA || !NO_SHA256) */ diff --git a/tests/api.c b/tests/api.c index f50058ab3..a4f74c59a 100644 --- a/tests/api.c +++ b/tests/api.c @@ -31406,11 +31406,13 @@ static int test_wolfSSL_X509_subject_name_hash(void) ret1 = X509_subject_name_hash(x509); AssertIntNE(ret1, 0); +#if !defined(NO_SHA) ret2 = X509_NAME_hash(X509_get_subject_name(x509)); AssertIntNE(ret2, 0); -#if !defined(NO_SHA) AssertIntEQ(ret1, ret2); +#else + (void) ret2; #endif X509_free(x509); @@ -31443,11 +31445,13 @@ static int test_wolfSSL_X509_issuer_name_hash(void) ret1 = X509_issuer_name_hash(x509); AssertIntNE(ret1, 0); +#if !defined(NO_SHA) ret2 = X509_NAME_hash(X509_get_issuer_name(x509)); AssertIntNE(ret2, 0); -#if !defined(NO_SHA) AssertIntEQ(ret1, ret2); +#else + (void) ret2; #endif X509_free(x509); From be074018566639e06fe6de03aeafa3f55c68cf7a Mon Sep 17 00:00:00 2001 From: jordan Date: Wed, 26 Oct 2022 10:25:05 -0500 Subject: [PATCH 3/6] Fix X509 subject and issuer name_hash mismatch Use WOLFSSL_ERROR_MSG for errors, don't leak canonName, and better error handling and comments. --- src/x509.c | 127 ++++++++++++++++++++++++++++++++--------------------- 1 file changed, 76 insertions(+), 51 deletions(-) diff --git a/src/x509.c b/src/x509.c index 695f23222..afc238294 100644 --- a/src/x509.c +++ b/src/x509.c @@ -2993,43 +2993,44 @@ char* wolfSSL_X509_NAME_oneline(WOLFSSL_X509_NAME* name, char* in, int sz) unsigned long wolfSSL_X509_NAME_hash(WOLFSSL_X509_NAME* name) { #ifndef NO_SHA - byte digest[WC_SHA_DIGEST_SIZE]; - unsigned long ret = 0; - unsigned char* canon_name = NULL; - int size = 0; + unsigned long hash = 0; + unsigned char* canonName = NULL; + byte digest[WC_SHA_DIGEST_SIZE]; + int size = 0; WOLFSSL_ENTER("wolfSSL_X509_NAME_hash"); if (name == NULL) { - WOLFSSL_MSG("WOLFSSL_X509_NAME pointer was NULL"); + WOLFSSL_ERROR_MSG("WOLFSSL_X509_NAME pointer was NULL"); return 0; } if (name->sz == 0) { - WOLFSSL_MSG("nothing to hash in WOLFSSL_X509_NAME"); + WOLFSSL_ERROR_MSG("nothing to hash in WOLFSSL_X509_NAME"); return 0; } - size = wolfSSL_i2d_X509_NAME_canon(name, &canon_name); + size = wolfSSL_i2d_X509_NAME_canon(name, &canonName); - if (size <= 0){ - WOLFSSL_MSG("wolfSSL_i2d_X509_NAME_canon error"); + if (size <= 0 || canonName == NULL) { + WOLFSSL_ERROR_MSG("wolfSSL_i2d_X509_NAME_canon error"); return 0; } - if (wc_ShaHash((byte*)canon_name, size, digest) != 0) { - WOLFSSL_MSG("wc_ShaHash error"); - return 0; + if (wc_ShaHash((byte*)canonName, size, digest) == 0) { + /* 4 bytes in little endian as unsigned long */ + hash = (((unsigned long)digest[3] << 24) | + ((unsigned long)digest[2] << 16) | + ((unsigned long)digest[1] << 8) | + ((unsigned long)digest[0])); + } + else { + WOLFSSL_ERROR_MSG("wc_ShaHash error"); } - XFREE(canon_name, NULL, DYNAMIC_TYPE_OPENSSL); - - ret = (unsigned long) digest[0]; - ret |= ((unsigned long) digest[1]) << 8; - ret |= ((unsigned long) digest[2]) << 16; - ret |= ((unsigned long) digest[3]) << 24; - return ret; + XFREE(canonName, NULL, DYNAMIC_TYPE_OPENSSL); + return hash; #else (void)name; - WOLFSSL_MSG("wolfSSL_X509_NAME_hash sha support not compiled in"); + WOLFSSL_ERROR_MSG("wolfSSL_X509_NAME_hash sha support not compiled in"); return 0; #endif } @@ -4770,14 +4771,18 @@ WOLFSSL_X509_NAME* wolfSSL_X509_get_subject_name(WOLFSSL_X509* cert) * wolfSSL_X509_subject_name_hash * wolfSSL_X509_issuer_name_hash * Compute the hash digest of the subject / issuer name. -* These functions prefers SHA-1 (if available) for compatibility. +* These functions prefer SHA-1 (if available) for compatibility. Otherwise +* they use SHA-256. * * RETURNS: -* The first 4 bytes of SHA hash in little endian order as unsigned long. +* The first 4 bytes of SHA-1 (or SHA-256) hash in little endian order as +* unsigned long. * Otherwise, returns zero. +* * Note: * Returns the same hash value as OpenSSL's X509_X_name_hash() API -* if SHA-1 support is compiled in. +* if SHA-1 support is compiled in. SHA-256 will be used if SHA-1 is +* not available. */ unsigned long wolfSSL_X509_subject_name_hash(const WOLFSSL_X509* x509) { @@ -4786,41 +4791,51 @@ unsigned long wolfSSL_X509_subject_name_hash(const WOLFSSL_X509* x509) unsigned char* canonName = NULL; byte digest[WC_MAX_DIGEST_SIZE]; int size = 0; + int retHash = NOT_COMPILED_IN; if (x509 == NULL) { + WOLFSSL_ERROR_MSG("WOLFSSL_X509 pointer was NULL"); return 0; } subjectName = wolfSSL_X509_get_subject_name((WOLFSSL_X509*)x509); if (subjectName == NULL) { - WOLFSSL_MSG("wolfSSL_X509_get_subject_name error"); + WOLFSSL_ERROR_MSG("wolfSSL_X509_get_subject_name error"); return 0; } size = wolfSSL_i2d_X509_NAME_canon(subjectName, &canonName); if (size <= 0 || canonName == NULL){ - WOLFSSL_MSG("wolfSSL_i2d_X509_NAME_canon error"); + WOLFSSL_ERROR_MSG("wolfSSL_i2d_X509_NAME_canon error"); return 0; } #ifndef NO_SHA - if (wc_ShaHash((const byte*)canonName, (word32)size, digest) != 0) { - WOLFSSL_MSG("wc_ShaHash error"); - return 0; - } + retHash = wc_ShaHash((const byte*)canonName, (word32)size, digest); #elif !defined(NO_SHA256) - if (wc_Sha256Hash((const byte*)canonName, (word32)size, digest) != 0) { - WOLFSSL_MSG("wc_Sha256Hash error"); - return 0; - } + retHash = wc_Sha256Hash((const byte*)canonName, (word32)size, digest); + #else + retHash = NOT_COMPILED_IN; #endif - hash = (unsigned long) digest[0]; - hash |= ((unsigned long) digest[1]) << 8; - hash |= ((unsigned long) digest[2]) << 16; - hash |= ((unsigned long) digest[3]) << 24; + if (retHash == 0) { + /* 4 bytes in little endian as unsigned long */ + hash = (((unsigned long)digest[3] << 24) | + ((unsigned long)digest[2] << 16) | + ((unsigned long)digest[1] << 8) | + ((unsigned long)digest[0])); + } + else { + #ifndef NO_SHA + WOLFSSL_ERROR_MSG("wc_ShaHash error"); + #elif !defined(NO_SHA256) + WOLFSSL_ERROR_MSG("wc_Sha256Hash error"); + #else + WOLFSSL_ERROR_MSG("wolfSSL_X509_subject_name_hash not compiled in"); + #endif + } XFREE(canonName, NULL, DYNAMIC_TYPE_OPENSSL); @@ -4834,41 +4849,51 @@ unsigned long wolfSSL_X509_issuer_name_hash(const WOLFSSL_X509* x509) unsigned char* canonName = NULL; byte digest[WC_MAX_DIGEST_SIZE]; int size = 0; + int retHash = NOT_COMPILED_IN; if (x509 == NULL) { + WOLFSSL_ERROR_MSG("WOLFSSL_X509 pointer was NULL"); return 0; } issuerName = wolfSSL_X509_get_issuer_name((WOLFSSL_X509*)x509); if (issuerName == NULL) { - WOLFSSL_MSG("wolfSSL_X509_get_issuer_name error"); + WOLFSSL_ERROR_MSG("wolfSSL_X509_get_issuer_name error"); return 0; } size = wolfSSL_i2d_X509_NAME_canon(issuerName, &canonName); if (size <= 0 || canonName == NULL){ - WOLFSSL_MSG("wolfSSL_i2d_X509_NAME_canon error"); + WOLFSSL_ERROR_MSG("wolfSSL_i2d_X509_NAME_canon error"); return 0; } #ifndef NO_SHA - if (wc_ShaHash((const byte*)canonName, (word32)size, digest) != 0) { - WOLFSSL_MSG("wc_ShaHash error"); - return 0; - } + retHash = wc_ShaHash((const byte*)canonName, (word32)size, digest); #elif !defined(NO_SHA256) - if (wc_Sha256Hash((const byte*)canonName, (word32)size, digest) != 0) { - WOLFSSL_MSG("wc_ShaHash error"); - return 0; - } + retHash = wc_Sha256Hash((const byte*)canonName, (word32)size, digest); + #else + retHash = NOT_COMPILED_IN; #endif - hash = (unsigned long) digest[0]; - hash |= ((unsigned long) digest[1]) << 8; - hash |= ((unsigned long) digest[2]) << 16; - hash |= ((unsigned long) digest[3]) << 24; + if (retHash == 0) { + /* 4 bytes in little endian as unsigned long */ + hash = (((unsigned long)digest[3] << 24) | + ((unsigned long)digest[2] << 16) | + ((unsigned long)digest[1] << 8) | + ((unsigned long)digest[0])); + } + else { + #ifndef NO_SHA + WOLFSSL_ERROR_MSG("wc_ShaHash error"); + #elif !defined(NO_SHA256) + WOLFSSL_ERROR_MSG("wc_Sha256Hash error"); + #else + WOLFSSL_ERROR_MSG("wolfSSL_X509_issuer_name_hash not compiled in"); + #endif + } XFREE(canonName, NULL, DYNAMIC_TYPE_OPENSSL); From b31567e752b5c328d9ede0c458cb58f03d6cbbda Mon Sep 17 00:00:00 2001 From: jordan Date: Wed, 26 Oct 2022 15:27:15 -0500 Subject: [PATCH 4/6] Fix X509 subject and issuer name_hash mismatch Refactor duplicate code, and better error messages. --- src/x509.c | 145 ++++++++++++++++++++--------------------------------- 1 file changed, 55 insertions(+), 90 deletions(-) diff --git a/src/x509.c b/src/x509.c index afc238294..ce42e95d9 100644 --- a/src/x509.c +++ b/src/x509.c @@ -2999,10 +2999,12 @@ unsigned long wolfSSL_X509_NAME_hash(WOLFSSL_X509_NAME* name) int size = 0; WOLFSSL_ENTER("wolfSSL_X509_NAME_hash"); + if (name == NULL) { WOLFSSL_ERROR_MSG("WOLFSSL_X509_NAME pointer was NULL"); return 0; } + if (name->sz == 0) { WOLFSSL_ERROR_MSG("nothing to hash in WOLFSSL_X509_NAME"); return 0; @@ -3024,6 +3026,7 @@ unsigned long wolfSSL_X509_NAME_hash(WOLFSSL_X509_NAME* name) } else { WOLFSSL_ERROR_MSG("wc_ShaHash error"); + hash = 0; } XFREE(canonName, NULL, DYNAMIC_TYPE_OPENSSL); @@ -4766,6 +4769,50 @@ WOLFSSL_X509_NAME* wolfSSL_X509_get_subject_name(WOLFSSL_X509* cert) return NULL; } +#if defined(OPENSSL_EXTRA) && defined(NO_SHA) && !defined(NO_SHA256) +static unsigned long wolfSSL_X509_name_sha256hash(WOLFSSL_X509_NAME* name) +{ + unsigned long hash = 0; + unsigned char* canonName = NULL; + byte digest[WC_MAX_DIGEST_SIZE]; + int size = 0; + + WOLFSSL_ENTER("wolfSSL_X509_name_sha256hash"); + + if (name == NULL) { + WOLFSSL_ERROR_MSG("WOLFSSL_X509_NAME pointer was NULL"); + return 0; + } + + if (name->sz == 0) { + WOLFSSL_ERROR_MSG("nothing to hash in WOLFSSL_X509_NAME"); + return 0; + } + + size = wolfSSL_i2d_X509_NAME_canon(name, &canonName); + + if (size <= 0 || canonName == NULL){ + WOLFSSL_ERROR_MSG("wolfSSL_i2d_X509_NAME_canon error"); + return 0; + } + + if (wc_Sha256Hash((const byte*)canonName, (word32)size, digest) == 0) { + /* 4 bytes in little endian as unsigned long */ + hash = (((unsigned long)digest[3] << 24) | + ((unsigned long)digest[2] << 16) | + ((unsigned long)digest[1] << 8) | + ((unsigned long)digest[0])); + } + else { + WOLFSSL_ERROR_MSG("wc_Sha256Hash error"); + hash = 0; + } + + XFREE(canonName, NULL, DYNAMIC_TYPE_OPENSSL); + return hash; +} +#endif /* OPENSSL_EXTRA && NO_SHA && !NO_SHA256 */ + #if defined(OPENSSL_EXTRA) && (!defined(NO_SHA) || !defined(NO_SHA256)) /****************************************************************************** * wolfSSL_X509_subject_name_hash @@ -4786,118 +4833,36 @@ WOLFSSL_X509_NAME* wolfSSL_X509_get_subject_name(WOLFSSL_X509* cert) */ unsigned long wolfSSL_X509_subject_name_hash(const WOLFSSL_X509* x509) { - unsigned long hash = 0; - WOLFSSL_X509_NAME* subjectName = NULL; - unsigned char* canonName = NULL; - byte digest[WC_MAX_DIGEST_SIZE]; - int size = 0; - int retHash = NOT_COMPILED_IN; - if (x509 == NULL) { WOLFSSL_ERROR_MSG("WOLFSSL_X509 pointer was NULL"); return 0; } - subjectName = wolfSSL_X509_get_subject_name((WOLFSSL_X509*)x509); - - if (subjectName == NULL) { - WOLFSSL_ERROR_MSG("wolfSSL_X509_get_subject_name error"); - return 0; - } - - size = wolfSSL_i2d_X509_NAME_canon(subjectName, &canonName); - - if (size <= 0 || canonName == NULL){ - WOLFSSL_ERROR_MSG("wolfSSL_i2d_X509_NAME_canon error"); - return 0; - } - #ifndef NO_SHA - retHash = wc_ShaHash((const byte*)canonName, (word32)size, digest); + return wolfSSL_X509_NAME_hash((WOLFSSL_X509_NAME*) &x509->subject); #elif !defined(NO_SHA256) - retHash = wc_Sha256Hash((const byte*)canonName, (word32)size, digest); + return wolfSSL_X509_name_sha256hash((WOLFSSL_X509_NAME*) &x509->subject); #else - retHash = NOT_COMPILED_IN; + WOLFSSL_ERROR_MSG("hash function not compiled in"); + return 0; #endif - - if (retHash == 0) { - /* 4 bytes in little endian as unsigned long */ - hash = (((unsigned long)digest[3] << 24) | - ((unsigned long)digest[2] << 16) | - ((unsigned long)digest[1] << 8) | - ((unsigned long)digest[0])); - } - else { - #ifndef NO_SHA - WOLFSSL_ERROR_MSG("wc_ShaHash error"); - #elif !defined(NO_SHA256) - WOLFSSL_ERROR_MSG("wc_Sha256Hash error"); - #else - WOLFSSL_ERROR_MSG("wolfSSL_X509_subject_name_hash not compiled in"); - #endif - } - - XFREE(canonName, NULL, DYNAMIC_TYPE_OPENSSL); - - return hash; } unsigned long wolfSSL_X509_issuer_name_hash(const WOLFSSL_X509* x509) { - unsigned long hash = 0; - WOLFSSL_X509_NAME* issuerName = NULL; - unsigned char* canonName = NULL; - byte digest[WC_MAX_DIGEST_SIZE]; - int size = 0; - int retHash = NOT_COMPILED_IN; - if (x509 == NULL) { WOLFSSL_ERROR_MSG("WOLFSSL_X509 pointer was NULL"); return 0; } - issuerName = wolfSSL_X509_get_issuer_name((WOLFSSL_X509*)x509); - - if (issuerName == NULL) { - WOLFSSL_ERROR_MSG("wolfSSL_X509_get_issuer_name error"); - return 0; - } - - size = wolfSSL_i2d_X509_NAME_canon(issuerName, &canonName); - - if (size <= 0 || canonName == NULL){ - WOLFSSL_ERROR_MSG("wolfSSL_i2d_X509_NAME_canon error"); - return 0; - } - #ifndef NO_SHA - retHash = wc_ShaHash((const byte*)canonName, (word32)size, digest); + return wolfSSL_X509_NAME_hash((WOLFSSL_X509_NAME*) &x509->issuer); #elif !defined(NO_SHA256) - retHash = wc_Sha256Hash((const byte*)canonName, (word32)size, digest); + return wolfSSL_X509_name_sha256hash((WOLFSSL_X509_NAME*) &x509->issuer); #else - retHash = NOT_COMPILED_IN; + WOLFSSL_ERROR_MSG("hash function not compiled in"); + return 0; #endif - - if (retHash == 0) { - /* 4 bytes in little endian as unsigned long */ - hash = (((unsigned long)digest[3] << 24) | - ((unsigned long)digest[2] << 16) | - ((unsigned long)digest[1] << 8) | - ((unsigned long)digest[0])); - } - else { - #ifndef NO_SHA - WOLFSSL_ERROR_MSG("wc_ShaHash error"); - #elif !defined(NO_SHA256) - WOLFSSL_ERROR_MSG("wc_Sha256Hash error"); - #else - WOLFSSL_ERROR_MSG("wolfSSL_X509_issuer_name_hash not compiled in"); - #endif - } - - XFREE(canonName, NULL, DYNAMIC_TYPE_OPENSSL); - - return hash; } #endif /* OPENSSL_EXTRA && (!NO_SHA || !NO_SHA256) */ From 822f11d1a1a02c85463f8b312b53808594a1fc87 Mon Sep 17 00:00:00 2001 From: jordan Date: Thu, 27 Oct 2022 15:15:55 -0500 Subject: [PATCH 5/6] Fix X509 subject and issuer name_hash mismatch Refactor duplicate code a bit more. --- src/x509.c | 183 +++++++++++++++++++++-------------------------------- 1 file changed, 73 insertions(+), 110 deletions(-) diff --git a/src/x509.c b/src/x509.c index ce42e95d9..7d85083ea 100644 --- a/src/x509.c +++ b/src/x509.c @@ -2990,15 +2990,18 @@ char* wolfSSL_X509_NAME_oneline(WOLFSSL_X509_NAME* name, char* in, int sz) } #ifdef OPENSSL_EXTRA -unsigned long wolfSSL_X509_NAME_hash(WOLFSSL_X509_NAME* name) +/* Given an X509_NAME, convert it to canonical form and then hash + * with the provided hash type. Returns the first 4 bytes of the hash + * as unsigned long on success, and 0 otherwise. */ +static unsigned long X509NameHash(WOLFSSL_X509_NAME* name, int hashType) { -#ifndef NO_SHA unsigned long hash = 0; unsigned char* canonName = NULL; - byte digest[WC_SHA_DIGEST_SIZE]; + byte digest[WC_MAX_DIGEST_SIZE]; int size = 0; + int rc; - WOLFSSL_ENTER("wolfSSL_X509_NAME_hash"); + WOLFSSL_ENTER("wolfSSL_X509_name_sha256hash"); if (name == NULL) { WOLFSSL_ERROR_MSG("WOLFSSL_X509_NAME pointer was NULL"); @@ -3006,7 +3009,7 @@ unsigned long wolfSSL_X509_NAME_hash(WOLFSSL_X509_NAME* name) } if (name->sz == 0) { - WOLFSSL_ERROR_MSG("nothing to hash in WOLFSSL_X509_NAME"); + WOLFSSL_ERROR_MSG("Nothing to hash in WOLFSSL_X509_NAME"); return 0; } @@ -3017,25 +3020,82 @@ unsigned long wolfSSL_X509_NAME_hash(WOLFSSL_X509_NAME* name) return 0; } - if (wc_ShaHash((byte*)canonName, size, digest) == 0) { - /* 4 bytes in little endian as unsigned long */ + rc = wc_Hash(hashType, (const byte*)canonName,(word32)size, digest, + sizeof(digest)); + + if (rc == 0) { hash = (((unsigned long)digest[3] << 24) | ((unsigned long)digest[2] << 16) | ((unsigned long)digest[1] << 8) | ((unsigned long)digest[0])); } + else if (rc == HASH_TYPE_E) { + WOLFSSL_ERROR_MSG("Hash function not compiled in"); + } else { - WOLFSSL_ERROR_MSG("wc_ShaHash error"); - hash = 0; + WOLFSSL_ERROR_MSG("Error hashing name"); } XFREE(canonName, NULL, DYNAMIC_TYPE_OPENSSL); return hash; -#else - (void)name; - WOLFSSL_ERROR_MSG("wolfSSL_X509_NAME_hash sha support not compiled in"); +} + +unsigned long wolfSSL_X509_NAME_hash(WOLFSSL_X509_NAME* name) +{ + return X509NameHash(name, WC_HASH_TYPE_SHA); +} + +/****************************************************************************** +* wolfSSL_X509_subject_name_hash +* wolfSSL_X509_issuer_name_hash +* Compute the hash digest of the subject / issuer name. +* These functions prefer SHA-1 (if available) for compatibility. Otherwise +* they use SHA-256. +* +* RETURNS: +* The first 4 bytes of SHA-1 (or SHA-256) hash in little endian order as +* unsigned long. +* Otherwise, returns zero. +* +* Note: +* Returns the same hash value as OpenSSL's X509_X_name_hash() API +* if SHA-1 support is compiled in. SHA-256 will be used if SHA-1 is +* not available. +*/ +unsigned long wolfSSL_X509_subject_name_hash(const WOLFSSL_X509* x509) +{ + if (x509 == NULL) { + WOLFSSL_ERROR_MSG("WOLFSSL_X509 pointer was NULL"); + return 0; + } + + #ifndef NO_SHA + return X509NameHash((WOLFSSL_X509_NAME*) &x509->subject, WC_HASH_TYPE_SHA); + #elif !defined(NO_SHA256) + return X509NameHash((WOLFSSL_X509_NAME*) &x509->subject, + WC_HASH_TYPE_SHA256); + #else + WOLFSSL_ERROR_MSG("Hash function not compiled in"); return 0; -#endif + #endif +} + +unsigned long wolfSSL_X509_issuer_name_hash(const WOLFSSL_X509* x509) +{ + if (x509 == NULL) { + WOLFSSL_ERROR_MSG("WOLFSSL_X509 pointer was NULL"); + return 0; + } + + #ifndef NO_SHA + return X509NameHash((WOLFSSL_X509_NAME*) &x509->issuer, WC_HASH_TYPE_SHA); + #elif !defined(NO_SHA256) + return X509NameHash((WOLFSSL_X509_NAME*) &x509->issuer, + WC_HASH_TYPE_SHA256); + #else + WOLFSSL_ERROR_MSG("Hash function not compiled in"); + return 0; + #endif } #endif /* OPENSSL_EXTRA */ @@ -4769,103 +4829,6 @@ WOLFSSL_X509_NAME* wolfSSL_X509_get_subject_name(WOLFSSL_X509* cert) return NULL; } -#if defined(OPENSSL_EXTRA) && defined(NO_SHA) && !defined(NO_SHA256) -static unsigned long wolfSSL_X509_name_sha256hash(WOLFSSL_X509_NAME* name) -{ - unsigned long hash = 0; - unsigned char* canonName = NULL; - byte digest[WC_MAX_DIGEST_SIZE]; - int size = 0; - - WOLFSSL_ENTER("wolfSSL_X509_name_sha256hash"); - - if (name == NULL) { - WOLFSSL_ERROR_MSG("WOLFSSL_X509_NAME pointer was NULL"); - return 0; - } - - if (name->sz == 0) { - WOLFSSL_ERROR_MSG("nothing to hash in WOLFSSL_X509_NAME"); - return 0; - } - - size = wolfSSL_i2d_X509_NAME_canon(name, &canonName); - - if (size <= 0 || canonName == NULL){ - WOLFSSL_ERROR_MSG("wolfSSL_i2d_X509_NAME_canon error"); - return 0; - } - - if (wc_Sha256Hash((const byte*)canonName, (word32)size, digest) == 0) { - /* 4 bytes in little endian as unsigned long */ - hash = (((unsigned long)digest[3] << 24) | - ((unsigned long)digest[2] << 16) | - ((unsigned long)digest[1] << 8) | - ((unsigned long)digest[0])); - } - else { - WOLFSSL_ERROR_MSG("wc_Sha256Hash error"); - hash = 0; - } - - XFREE(canonName, NULL, DYNAMIC_TYPE_OPENSSL); - return hash; -} -#endif /* OPENSSL_EXTRA && NO_SHA && !NO_SHA256 */ - -#if defined(OPENSSL_EXTRA) && (!defined(NO_SHA) || !defined(NO_SHA256)) -/****************************************************************************** -* wolfSSL_X509_subject_name_hash -* wolfSSL_X509_issuer_name_hash -* Compute the hash digest of the subject / issuer name. -* These functions prefer SHA-1 (if available) for compatibility. Otherwise -* they use SHA-256. -* -* RETURNS: -* The first 4 bytes of SHA-1 (or SHA-256) hash in little endian order as -* unsigned long. -* Otherwise, returns zero. -* -* Note: -* Returns the same hash value as OpenSSL's X509_X_name_hash() API -* if SHA-1 support is compiled in. SHA-256 will be used if SHA-1 is -* not available. -*/ -unsigned long wolfSSL_X509_subject_name_hash(const WOLFSSL_X509* x509) -{ - if (x509 == NULL) { - WOLFSSL_ERROR_MSG("WOLFSSL_X509 pointer was NULL"); - return 0; - } - - #ifndef NO_SHA - return wolfSSL_X509_NAME_hash((WOLFSSL_X509_NAME*) &x509->subject); - #elif !defined(NO_SHA256) - return wolfSSL_X509_name_sha256hash((WOLFSSL_X509_NAME*) &x509->subject); - #else - WOLFSSL_ERROR_MSG("hash function not compiled in"); - return 0; - #endif -} - -unsigned long wolfSSL_X509_issuer_name_hash(const WOLFSSL_X509* x509) -{ - if (x509 == NULL) { - WOLFSSL_ERROR_MSG("WOLFSSL_X509 pointer was NULL"); - return 0; - } - - #ifndef NO_SHA - return wolfSSL_X509_NAME_hash((WOLFSSL_X509_NAME*) &x509->issuer); - #elif !defined(NO_SHA256) - return wolfSSL_X509_name_sha256hash((WOLFSSL_X509_NAME*) &x509->issuer); - #else - WOLFSSL_ERROR_MSG("hash function not compiled in"); - return 0; - #endif -} -#endif /* OPENSSL_EXTRA && (!NO_SHA || !NO_SHA256) */ - WOLFSSL_ABI WOLFSSL_X509_NAME* wolfSSL_X509_get_issuer_name(WOLFSSL_X509* cert) { From c4e758dda5177822a99d62f35c61536728ca2c5e Mon Sep 17 00:00:00 2001 From: jordan Date: Thu, 27 Oct 2022 19:31:30 -0500 Subject: [PATCH 6/6] Fix X509 subject and issuer name_hash mismatch Fix logging message and g++ invalid conversion error. --- src/x509.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/x509.c b/src/x509.c index 7d85083ea..2f56834e4 100644 --- a/src/x509.c +++ b/src/x509.c @@ -2993,7 +2993,8 @@ char* wolfSSL_X509_NAME_oneline(WOLFSSL_X509_NAME* name, char* in, int sz) /* Given an X509_NAME, convert it to canonical form and then hash * with the provided hash type. Returns the first 4 bytes of the hash * as unsigned long on success, and 0 otherwise. */ -static unsigned long X509NameHash(WOLFSSL_X509_NAME* name, int hashType) +static unsigned long X509NameHash(WOLFSSL_X509_NAME* name, + enum wc_HashType hashType) { unsigned long hash = 0; unsigned char* canonName = NULL; @@ -3001,7 +3002,7 @@ static unsigned long X509NameHash(WOLFSSL_X509_NAME* name, int hashType) int size = 0; int rc; - WOLFSSL_ENTER("wolfSSL_X509_name_sha256hash"); + WOLFSSL_ENTER("X509NameHash"); if (name == NULL) { WOLFSSL_ERROR_MSG("WOLFSSL_X509_NAME pointer was NULL");