From 007f9ea39dc6876ac6f08eb9826b51f176947d43 Mon Sep 17 00:00:00 2001 From: David Garske Date: Wed, 24 Jul 2024 08:28:25 -0700 Subject: [PATCH 1/5] Fix to restore `--enable-asn=original`. Fixes for building with ASN original (old). Add the new limit checks for alt names and subtree to the old ASN code. --- configure.ac | 6 +++--- wolfcrypt/src/asn.c | 19 +++++++++++++++++-- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/configure.ac b/configure.ac index 43aaa84b7f..222fcd6208 100644 --- a/configure.ac +++ b/configure.ac @@ -4762,10 +4762,10 @@ else fi if test "$ENABLED_ASN" = "yes"; then AM_CFLAGS="$AM_CFLAGS -DWOLFSSL_ASN_TEMPLATE" + elif test "$ENABLED_ASN" == "original"; then + AM_CFLAGS="$AM_CFLAGS -DWOLFSSL_ASN_ORIGINAL" else - if test "$ENABLED_ASN" != "original"; then - AC_MSG_ERROR([Invalid asn option. Valid are: template or original. Seen: $ENABLED_ASN.]) - fi + AC_MSG_ERROR([Invalid asn option. Valid are: template or original. Seen: $ENABLED_ASN.]) fi # turn off ASN if leanpsk on diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 16d773c5e7..80f98ab707 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -6920,7 +6920,7 @@ int ToTraditionalInline_ex2(const byte* input, word32* inOutIdx, word32 sz, if (tag == ASN_OBJECT_ID) { if ((*algId == ECDSAk) && (eccOid != NULL)) { - if (GetObjectId(input, &idx, eccOid, oidCurveType, maxIdx) < 0) + if (GetObjectId(input, &idx, eccOid, oidCurveType, sz) < 0) return ASN_PARSE_E; } else { @@ -18590,6 +18590,7 @@ static int DecodeAltNames(const byte* input, word32 sz, DecodedCert* cert) #ifndef WOLFSSL_ASN_TEMPLATE word32 idx = 0; int length = 0; + word32 numNames = 0; WOLFSSL_ENTER("DecodeAltNames"); @@ -18622,8 +18623,13 @@ static int DecodeAltNames(const byte* input, word32 sz, DecodedCert* cert) return BUFFER_E; } - current_byte = input[idx++]; + numNames++; + if (numNames > WOLFSSL_MAX_ALT_NAMES) { + WOLFSSL_MSG("\tToo many subject alternative names"); + return ASN_ALT_NAME_E; + } + current_byte = input[idx++]; length--; /* Save DNS Type names in the altNames list. */ @@ -20153,6 +20159,7 @@ static int DecodeSubtree(const byte* input, word32 sz, Base_entry** head, #ifndef WOLFSSL_ASN_TEMPLATE word32 idx = 0; int ret = 0; + word32 cnt = 0; (void)heap; @@ -20161,6 +20168,14 @@ static int DecodeSubtree(const byte* input, word32 sz, Base_entry** head, word32 nameIdx; byte b, bType; + if (limit > 0) { + cnt++; + if (cnt > limit) { + WOLFSSL_MSG("too many name constraints"); + return ASN_NAME_INVALID_E; + } + } + if (GetSequence(input, &idx, &seqLength, sz) < 0) { WOLFSSL_MSG("\tfail: should be a SEQUENCE"); return ASN_PARSE_E; From 3e2123f0b3abe6ce7b27712d94f24ca2a1ee9fee Mon Sep 17 00:00:00 2001 From: David Garske Date: Wed, 24 Jul 2024 08:45:19 -0700 Subject: [PATCH 2/5] Disable the ECC custom curve tests for original (old) ASN. --- tests/suites.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/suites.c b/tests/suites.c index 1604e18ea9..5c367fe9c9 100644 --- a/tests/suites.c +++ b/tests/suites.c @@ -1060,7 +1060,9 @@ int SuiteTest(int argc, char** argv) #if defined(HAVE_ECC) && !defined(NO_SHA256) && defined(WOLFSSL_CUSTOM_CURVES) && \ defined(HAVE_ECC_KOBLITZ) && defined(HAVE_ECC_BRAINPOOL) && \ /* Intel QuickAssist and Cavium Nitrox do not support custom curves */ \ - !defined(HAVE_INTEL_QA) && !defined(HAVE_CAVIUM_V) + !defined(HAVE_INTEL_QA) && !defined(HAVE_CAVIUM_V) && \ + /* only supported with newer ASN template code */ \ + defined(WOLFSSL_ASN_TEMPLATE) /* TLS non-NIST curves (Koblitz / Brainpool) */ XSTRLCPY(argv0[1], "tests/test-ecc-cust-curves.conf", sizeof(argv0[1])); From 4b9d89d387cb5f695aaf91116dd65f81a2ec3603 Mon Sep 17 00:00:00 2001 From: David Garske Date: Wed, 24 Jul 2024 09:10:25 -0700 Subject: [PATCH 3/5] Fix autoconf issue with `==` --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 222fcd6208..4578438607 100644 --- a/configure.ac +++ b/configure.ac @@ -4762,7 +4762,7 @@ else fi if test "$ENABLED_ASN" = "yes"; then AM_CFLAGS="$AM_CFLAGS -DWOLFSSL_ASN_TEMPLATE" - elif test "$ENABLED_ASN" == "original"; then + elif test "$ENABLED_ASN" = "original"; then AM_CFLAGS="$AM_CFLAGS -DWOLFSSL_ASN_ORIGINAL" else AC_MSG_ERROR([Invalid asn option. Valid are: template or original. Seen: $ENABLED_ASN.]) From 7f7d94abd5a101a23f96d885a69973d600522156 Mon Sep 17 00:00:00 2001 From: David Garske Date: Wed, 24 Jul 2024 12:35:37 -0700 Subject: [PATCH 4/5] Fixes for ASN original (old) to support checking int leading 0 and invalid OID. Disable invalid UTF8 test for old ASN (only supported with newer ASN template). --- wolfcrypt/src/asn.c | 34 ++++++++++++++++++++++++++-------- wolfcrypt/test/test.c | 4 ++-- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 80f98ab707..a79d41973b 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -2430,6 +2430,19 @@ static int GetASNHeader_ex(const byte* input, byte tag, word32* inOutIdx, if ((ret == 0) && (GetLength_ex(input, &idx, &length, maxIdx, check) < 0)) { ret = ASN_PARSE_E; } + if (ret == 0 && tag == ASN_OBJECT_ID) { + if (length < 3) { + /* OID data must be at least 3 bytes. */ + WOLFSSL_MSG("OID length less than 3"); + ret = ASN_PARSE_E; + } + else if ((input[(int)idx + length - 1] & 0x80) != 0x00) { + /* Last octet of a sub-identifier has bit 8 clear. Last octet must be + * last of a subidentifier. Ensure last octet hasn't got top bit set. */ + WOLFSSL_MSG("OID last octet has top bit set"); + ret = ASN_PARSE_E; + } + } if (ret == 0) { /* Return the length of data and index after header. */ *len = length; @@ -2691,14 +2704,15 @@ int GetASNInt(const byte* input, word32* inOutIdx, int* len, return ret; if (*len > 0) { - #ifndef WOLFSSL_ASN_INT_LEAD_0_ANY /* check for invalid padding on negative integer. * c.f. X.690 (ISO/IEC 8825-2:2003 (E)) 10.4.6; RFC 5280 4.1 */ if (*len > 1) { - if ((input[*inOutIdx] == 0xff) && (input[*inOutIdx + 1] & 0x80)) - return ASN_PARSE_E; + if ((input[*inOutIdx] == 0xff) && (input[*inOutIdx + 1] & 0x80)) { + WOLFSSL_MSG("Bad INTEGER encoding of negative"); + return ASN_EXPECT_0_E; + } } #endif @@ -2708,8 +2722,10 @@ int GetASNInt(const byte* input, word32* inOutIdx, int* len, (*len)--; #ifndef WOLFSSL_ASN_INT_LEAD_0_ANY - if (*len > 0 && (input[*inOutIdx] & 0x80) == 0) - return ASN_PARSE_E; + if (*len > 0 && (input[*inOutIdx] & 0x80) == 0) { + WOLFSSL_MSG("INTEGER is negative"); + return ASN_EXPECT_0_E; + } #endif } } @@ -11572,9 +11588,11 @@ static int GetCertHeader(DecodedCert* cert) cert->sigIndex) < 0) return ASN_PARSE_E; - if (wc_GetSerialNumber(cert->source, &cert->srcIdx, cert->serial, - &cert->serialSz, cert->sigIndex) < 0) - return ASN_PARSE_E; + ret = wc_GetSerialNumber(cert->source, &cert->srcIdx, cert->serial, + &cert->serialSz, cert->sigIndex); + if (ret < 0) { + return ret; + } return ret; } diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index 6f47de0fb6..c9e10ae5d2 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -18078,7 +18078,7 @@ WOLFSSL_TEST_SUBROUTINE wc_test_ret_t memory_test(void) #endif static const char* certBadOid = CERT_ROOT "test" CERT_PATH_SEP "cert-bad-oid.der"; -#ifndef WOLFSSL_NO_ASN_STRICT +#if defined(WOLFSSL_ASN_TEMPLATE) && !defined(WOLFSSL_NO_ASN_STRICT) static const char* certBadUtf8 = CERT_ROOT "test" CERT_PATH_SEP "cert-bad-utf8.der"; #endif @@ -18383,7 +18383,7 @@ static wc_test_ret_t cert_bad_asn1_test(void) /* Subject name OID: 55 04 f4. Last byte with top bit set invalid. */ ret = cert_load_bad(certBadOid, tmp, ASN_PARSE_E); } -#ifndef WOLFSSL_NO_ASN_STRICT +#if defined(WOLFSSL_ASN_TEMPLATE) && !defined(WOLFSSL_NO_ASN_STRICT) if (ret == 0) { /* Issuer name UTF8STRING: df 52 4e 44. Top bit of second byte not set. */ From c4f73f5955a8932e6a0f1d6ca0211ee07beb9aee Mon Sep 17 00:00:00 2001 From: David Garske Date: Wed, 24 Jul 2024 16:57:51 -0700 Subject: [PATCH 5/5] Peer review cleanups. --- wolfcrypt/src/asn.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index a79d41973b..59046cd33e 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -1210,7 +1210,7 @@ static int GetASN_ObjectId(const byte* input, word32 idx, int length) /* Last octet of a sub-identifier has bit 8 clear. Last octet must be last * of a subidentifier. Ensure last octet hasn't got top bit set. */ - else if ((input[(int)idx + length - 1] & 0x80) != 0x00) { + else if ((input[(int)idx + length - 1] & 0x80) == 0x80) { WOLFSSL_MSG("OID last octet has top bit set"); ret = ASN_PARSE_E; } @@ -2436,7 +2436,7 @@ static int GetASNHeader_ex(const byte* input, byte tag, word32* inOutIdx, WOLFSSL_MSG("OID length less than 3"); ret = ASN_PARSE_E; } - else if ((input[(int)idx + length - 1] & 0x80) != 0x00) { + else if ((input[(int)idx + length - 1] & 0x80) == 0x80) { /* Last octet of a sub-identifier has bit 8 clear. Last octet must be * last of a subidentifier. Ensure last octet hasn't got top bit set. */ WOLFSSL_MSG("OID last octet has top bit set"); @@ -3490,7 +3490,7 @@ int CheckBitString(const byte* input, word32* inOutIdx, int* len, } b = input[idx]; - if (zeroBits && b != 0x00) + if (zeroBits && (b != 0x00)) return ASN_EXPECT_0_E; if (b >= 0x08) return ASN_PARSE_E;