From a5d67d1ce369e5646ad712c4dfe3698bf4e6dd0d Mon Sep 17 00:00:00 2001 From: Aidan Keefe Date: Fri, 12 Jun 2026 16:54:34 -0600 Subject: [PATCH 1/3] Updated size of OID represention to be word32 instead of word16 so OIDs will not be truncated. Added tests for updated OID handling. made change fips compatable removed tests for EncodeObjectId caused symbol error in CI/CD skoll review version fix test fix --- doc/dox_comments/header_files-ja/asn_public.h | 4 +- doc/dox_comments/header_files/asn.h | 20 +- doc/dox_comments/header_files/asn_public.h | 4 +- tests/api/test_asn.c | 290 ++++++++++++++++-- tests/api/test_asn.h | 8 +- wolfcrypt/src/asn.c | 270 +++++++++++----- wolfcrypt/src/wc_pkcs11.c | 2 +- wolfssl/wolfcrypt/asn.h | 20 +- 8 files changed, 498 insertions(+), 120 deletions(-) diff --git a/doc/dox_comments/header_files-ja/asn_public.h b/doc/dox_comments/header_files-ja/asn_public.h index c3f78987c17..c72c3672c9e 100644 --- a/doc/dox_comments/header_files-ja/asn_public.h +++ b/doc/dox_comments/header_files-ja/asn_public.h @@ -1934,7 +1934,7 @@ int wc_SetCustomExtension(Cert *cert, int critical, const char *oid, \code int ret = 0; // 不明な拡張コールバックのプロトタイプ - int myUnknownExtCallback(const word16* oid, word32 oidSz, int crit, + int myUnknownExtCallback(const word32* oid, word32 oidSz, int crit, const unsigned char* der, word32 derSz); // 登録する @@ -1948,7 +1948,7 @@ int wc_SetCustomExtension(Cert *cert, int critical, const char *oid, // crit: 拡張が重要としてマークされたかどうか。 // der: 拡張のコンテンツのderエンコーディング。 // derSz: derエンコーディングのサイズ（バイト単位）。 - int myCustomExtCallback(const word16* oid, word32 oidSz, int crit, + int myCustomExtCallback(const word32* oid, word32 oidSz, int crit, const unsigned char* der, word32 derSz) { // 拡張を解析するロジックがここに入ります。 diff --git a/doc/dox_comments/header_files/asn.h b/doc/dox_comments/header_files/asn.h index 03bbe38c038..c3aeb50325f 100644 --- a/doc/dox_comments/header_files/asn.h +++ b/doc/dox_comments/header_files/asn.h @@ -80,11 +80,16 @@ void FreeAltNames(DNS_entry* altNames, void* heap); \note This API is not public by default. Define WOLFSSL_PUBLIC_ASN to expose APIs marked WOLFSSL_ASN_API. + \note The oid argument passed to the callback is an array of word32 + elements, one per OID arc, so that arcs > 65535 are represented without + truncation. Callbacks must declare their oid parameter as const word32*. + The same applies to wc_SetUnknownExtCallback. + _Example_ \code DecodedCert cert; - int UnknownExtCallback(const byte* oid, word32 oidSz, int crit, + int UnknownExtCallback(const word32* oid, word32 oidSz, int crit, const byte* der, word32 derSz, void* ctx) { // handle unknown extension return 0; @@ -140,16 +145,19 @@ int wc_CheckCertSignature(const byte* cert, word32 certSz, void* heap, /*! \ingroup ASN - \brief This function encodes an array of word16 values into an ASN.1 + \brief This function encodes an array of word32 values into an ASN.1 Object Identifier (OID) in DER format. OIDs are used to identify algorithms, extensions, and other objects in certificates and cryptographic protocols. + Each OID arc is a word32, so OIDs containing arcs with values > 65535 are + represented without truncation. + \return 0 On success. \return BAD_FUNC_ARG If in, inSz, or outSz are invalid. \return BUFFER_E If out is not NULL and outSz is too small. - \param in pointer to array of word16 values representing OID components + \param in pointer to array of word32 values representing OID components \param inSz number of components in the OID \param out pointer to buffer to store encoded OID (can be NULL to calculate size) @@ -157,11 +165,11 @@ int wc_CheckCertSignature(const byte* cert, word32 certSz, void* heap, _Example_ \code - word16 oid[] = {1, 2, 840, 113549, 1, 1, 11}; // sha256WithRSAEncryption + word32 oid[] = {1, 2, 840, 113549, 1, 1, 11}; // sha256WithRSAEncryption byte encoded[32]; word32 encodedSz = sizeof(encoded); - int ret = wc_EncodeObjectId(oid, sizeof(oid)/sizeof(word16), + int ret = wc_EncodeObjectId(oid, sizeof(oid)/sizeof(*oid), encoded, &encodedSz); if (ret == 0) { // encoded contains DER encoded OID @@ -170,7 +178,7 @@ int wc_CheckCertSignature(const byte* cert, word32 certSz, void* heap, \sa wc_BerToDer */ -int wc_EncodeObjectId(const word16* in, word32 inSz, byte* out, +int wc_EncodeObjectId(const word32* in, word32 inSz, byte* out, word32* outSz); /*! diff --git a/doc/dox_comments/header_files/asn_public.h b/doc/dox_comments/header_files/asn_public.h index 5f9d06aafcf..38fb4b9d6cf 100644 --- a/doc/dox_comments/header_files/asn_public.h +++ b/doc/dox_comments/header_files/asn_public.h @@ -4108,7 +4108,7 @@ int wc_SetCustomExtension(Cert *cert, int critical, const char *oid, \code int ret = 0; // Unknown extension callback prototype - int myUnknownExtCallback(const word16* oid, word32 oidSz, int crit, + int myUnknownExtCallback(const word32* oid, word32 oidSz, int crit, const unsigned char* der, word32 derSz); // Register it @@ -4122,7 +4122,7 @@ int wc_SetCustomExtension(Cert *cert, int critical, const char *oid, // crit: Whether the extension was mark critical. // der: The der encoding of the content of the extension. // derSz: The size in bytes of the der encoding. - int myCustomExtCallback(const word16* oid, word32 oidSz, int crit, + int myCustomExtCallback(const word32* oid, word32 oidSz, int crit, const unsigned char* der, word32 derSz) { // Logic to parse extension goes here. diff --git a/tests/api/test_asn.c b/tests/api/test_asn.c index 7e74c03cca0..09139b16cfe 100644 --- a/tests/api/test_asn.c +++ b/tests/api/test_asn.c @@ -1704,55 +1704,188 @@ int test_SerialNumber0_RootCA(void) return EXPECT_RESULT(); } -int test_wc_DecodeObjectId(void) + +/* Test for word16 FIPS version of function */ +int test_wc_DecodeObjectId_FIPS16(void) +{ + EXPECT_DECLS; + +#if !defined(NO_ASN) && \ + (defined(HAVE_OID_DECODING) || defined(WOLFSSL_ASN_PRINT)) + { + word32 i; + static const word16 oid_dot_2[] = { + 2, 100, 4, 6 + }; + + static const byte oid_start_with_2[] = { + 0x81, 0x34, 0x04, 0x06 + }; + + + static const byte oid_sha256secp112r1[] = { + 0x2B, 0x81, 0x04, 0x00, 0x06 + }; + + static const word16 oid_dot_form[] = { + 1U, 3U, 132U, 0U, 6U + }; + + word16 out[MAX_OID_SZ]; + word32 outSz; + + word32 trueOutSz = sizeof(oid_dot_form) / sizeof(*oid_dot_form); + /* Test 1: Normal decode */ + outSz = MAX_OID_SZ; + ExpectIntEQ(DecodeObjectId(oid_sha256secp112r1, + sizeof(oid_sha256secp112r1), out, &outSz), 0); + ExpectIntEQ((int)outSz, trueOutSz); + for (i = 0; i < ((outSz <= trueOutSz) ? outSz : trueOutSz); i++) { + ExpectIntEQ(out[i], oid_dot_form[i]); + } + + /* Test 2: NULL args */ + outSz = MAX_OID_SZ; + ExpectIntEQ(DecodeObjectId(NULL, sizeof(oid_sha256secp112r1), + out, &outSz), + WC_NO_ERR_TRACE(BAD_FUNC_ARG)); + ExpectIntEQ(DecodeObjectId(oid_sha256secp112r1, + sizeof(oid_sha256secp112r1), out, NULL), + WC_NO_ERR_TRACE(BAD_FUNC_ARG)); + + /* Test 3 (Bug 1): outSz=1 must return BUFFER_E, not OOB write. + * The first OID byte decodes into two arcs, so outSz must be >= 2. */ + outSz = 1; + ExpectIntEQ(DecodeObjectId(oid_sha256secp112r1, + sizeof(oid_sha256secp112r1), out, &outSz), + WC_NO_ERR_TRACE(BUFFER_E)); + + /* Test 4: outSz=0 must also return BUFFER_E */ + outSz = 0; + ExpectIntEQ(DecodeObjectId(oid_sha256secp112r1, + sizeof(oid_sha256secp112r1), out, &outSz), + WC_NO_ERR_TRACE(BUFFER_E)); + + /* Test 5: outSz=2 is enough for a single-byte OID (two arcs) */ + { + static const byte oid_one_byte[] = { 0x2a }; /* 1.2 */ + outSz = 2; + ExpectIntEQ(DecodeObjectId(oid_one_byte, sizeof(oid_one_byte), + out, &outSz), 0); + ExpectIntEQ((int)outSz, 2); + ExpectIntEQ(out[0], 1); + ExpectIntEQ(out[1], 2); + } + + /* Test 6: Buffer too small for later arcs */ + outSz = 3; /* only room for 3 arcs, but OID has 7 */ + ExpectIntEQ(DecodeObjectId(oid_sha256secp112r1, + sizeof(oid_sha256secp112r1), out, &outSz), + WC_NO_ERR_TRACE(BUFFER_E)); + + /* Test 7: first Arc is 2 */ + { + word32 trueOutSz2 = sizeof(oid_dot_2) / sizeof(*oid_dot_2); + outSz = MAX_OID_SZ; + ExpectIntEQ(DecodeObjectId(oid_start_with_2, + sizeof(oid_start_with_2), + out, &outSz), 0); + ExpectIntEQ((int)outSz, trueOutSz2); + for (i = 0; i < ((outSz <= trueOutSz2) ? + outSz : trueOutSz2); i++) { + ExpectIntEQ(out[i], oid_dot_2[i]); + } + } + + /* Test 8: an OID with an arc that exceeds word16. Tests that wrong + * but unchangeable behavior is working correctly, + * + * word16 version is used in FIPS build + */ + { + static const byte oid_large_arc[] = { + 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x0b + }; + static const word16 oid_dot_large_arc[] = { + 1U, 2U, 840U, (word16)113549U, 1U, 1U, 11U + }; + word32 trueOutSz3 = sizeof(oid_dot_large_arc) / sizeof(word16); + + outSz = MAX_OID_SZ; + ExpectIntEQ(DecodeObjectId(oid_large_arc, sizeof(oid_large_arc), + out, &outSz), 0); + ExpectIntEQ((int)outSz, (int)trueOutSz3); + for (i = 0; i < ((outSz <= trueOutSz3) ? outSz : trueOutSz3); i++) { + ExpectIntEQ(out[i], oid_dot_large_arc[i]); + } + } + } +#endif /* !NO_ASN && (HAVE_OID_DECODING || WOLFSSL_ASN_PRINT) */ + + return EXPECT_RESULT(); +} + +int test_wc_DecodeObjectId_ex(void) { EXPECT_DECLS; #if !defined(NO_ASN) && \ (defined(HAVE_OID_DECODING) || defined(WOLFSSL_ASN_PRINT)) { - /* OID 1.2.840.113549.1.1.11 (sha256WithRSAEncryption) - * DER encoding: 2a 86 48 86 f7 0d 01 01 0b - * First byte 0x2a = 42 => arc0 = 42/40 = 1, arc1 = 42%40 = 2 - * Remaining arcs: 840, 113549, 1, 1, 11 + word32 i; + static const word32 oid_dot_2[] = { + 2, 100, 4, 6 + }; + + static const byte oid_start_with_2[] = { + 0x81, 0x34, 0x04, 0x06 + }; + + + /* OID 1.3.132.0.6 (secp112r1) + * DER encoding: 2b 81 04 00 06 + * First byte 0x2b = 43 => arc0 = 43/40 = 1, arc1 = 43%40 = 3 + * Remaining arcs: 132 0 6 */ static const byte oid_sha256rsa[] = { - 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x0b + 0x2B, 0x81, 0x04, 0x00, 0x06 }; - word16 out[MAX_OID_SZ]; + + static const word32 oid_dot_form[] = { + 1U, 3U, 132U, 0U, 6U + }; + + word32 out[MAX_OID_SZ]; word32 outSz; + word32 trueOutSz = sizeof(oid_dot_form) / sizeof(word32); /* Test 1: Normal decode */ outSz = MAX_OID_SZ; - ExpectIntEQ(DecodeObjectId(oid_sha256rsa, sizeof(oid_sha256rsa), + ExpectIntEQ(DecodeObjectId_ex(oid_sha256rsa, sizeof(oid_sha256rsa), out, &outSz), 0); - ExpectIntEQ((int)outSz, 7); - ExpectIntEQ(out[0], 1); - ExpectIntEQ(out[1], 2); - ExpectIntEQ(out[2], 840); - ExpectIntEQ(out[3], (word16)113549); /* truncated to word16 */ - ExpectIntEQ(out[4], 1); - ExpectIntEQ(out[5], 1); - ExpectIntEQ(out[6], 11); + ExpectIntEQ((int)outSz, trueOutSz); + for (i = 0; i < ((outSz <= trueOutSz) ? outSz : trueOutSz); i++) { + ExpectIntEQ(out[i], oid_dot_form[i]); + } /* Test 2: NULL args */ outSz = MAX_OID_SZ; - ExpectIntEQ(DecodeObjectId(NULL, sizeof(oid_sha256rsa), out, &outSz), + ExpectIntEQ(DecodeObjectId_ex(NULL, sizeof(oid_sha256rsa), out, &outSz), WC_NO_ERR_TRACE(BAD_FUNC_ARG)); - ExpectIntEQ(DecodeObjectId(oid_sha256rsa, sizeof(oid_sha256rsa), + ExpectIntEQ(DecodeObjectId_ex(oid_sha256rsa, sizeof(oid_sha256rsa), out, NULL), WC_NO_ERR_TRACE(BAD_FUNC_ARG)); /* Test 3 (Bug 1): outSz=1 must return BUFFER_E, not OOB write. * The first OID byte decodes into two arcs, so outSz must be >= 2. */ outSz = 1; - ExpectIntEQ(DecodeObjectId(oid_sha256rsa, sizeof(oid_sha256rsa), + ExpectIntEQ(DecodeObjectId_ex(oid_sha256rsa, sizeof(oid_sha256rsa), out, &outSz), WC_NO_ERR_TRACE(BUFFER_E)); /* Test 4: outSz=0 must also return BUFFER_E */ outSz = 0; - ExpectIntEQ(DecodeObjectId(oid_sha256rsa, sizeof(oid_sha256rsa), + ExpectIntEQ(DecodeObjectId_ex(oid_sha256rsa, sizeof(oid_sha256rsa), out, &outSz), WC_NO_ERR_TRACE(BUFFER_E)); @@ -1760,7 +1893,7 @@ int test_wc_DecodeObjectId(void) { static const byte oid_one_byte[] = { 0x2a }; /* 1.2 */ outSz = 2; - ExpectIntEQ(DecodeObjectId(oid_one_byte, sizeof(oid_one_byte), + ExpectIntEQ(DecodeObjectId_ex(oid_one_byte, sizeof(oid_one_byte), out, &outSz), 0); ExpectIntEQ((int)outSz, 2); ExpectIntEQ(out[0], 1); @@ -1769,15 +1902,128 @@ int test_wc_DecodeObjectId(void) /* Test 6: Buffer too small for later arcs */ outSz = 3; /* only room for 3 arcs, but OID has 7 */ - ExpectIntEQ(DecodeObjectId(oid_sha256rsa, sizeof(oid_sha256rsa), + ExpectIntEQ(DecodeObjectId_ex(oid_sha256rsa, sizeof(oid_sha256rsa), out, &outSz), WC_NO_ERR_TRACE(BUFFER_E)); + + /* Test 7: first Arc is 2 */ + { + word32 trueOutSz2 = sizeof(oid_dot_2) / sizeof(word32); + outSz = MAX_OID_SZ; + ExpectIntEQ(DecodeObjectId_ex(oid_start_with_2, + sizeof(oid_start_with_2), + out, &outSz), 0); + ExpectIntEQ((int)outSz, trueOutSz2); + for (i = 0; i < ((outSz <= trueOutSz2) ? + outSz : trueOutSz2); i++) { + ExpectIntEQ(out[i], oid_dot_2[i]); + } + } + + /* Test 8: an OID with an arc that exceeds word16. + */ + { + static const byte oid_large_arc[] = { + 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x0b + }; + static const word32 oid_dot_large_arc[] = { + 1U, 2U, 840U, 113549U, 1U, 1U, 11U + }; + word32 trueOutSz3 = sizeof(oid_dot_large_arc) / sizeof(word32); + + outSz = MAX_OID_SZ; + ExpectIntEQ(DecodeObjectId_ex(oid_large_arc, sizeof(oid_large_arc), + out, &outSz), 0); + ExpectIntEQ((int)outSz, (int)trueOutSz3); + for (i = 0; i < ((outSz <= trueOutSz3) ? outSz : trueOutSz3); i++) { + ExpectIntEQ(out[i], oid_dot_large_arc[i]); + } + } } #endif /* !NO_ASN && (HAVE_OID_DECODING || WOLFSSL_ASN_PRINT) */ return EXPECT_RESULT(); } +int test_wc_EncodeObjectId(void) +{ + EXPECT_DECLS; +#if defined(HAVE_OID_ENCODING) && !defined(NO_ASN) + { + /* 1.3.132.0.6 (secp112r1) -- every arc fits in word16, so this + * encodes identically in both build configs. */ + static const word32 oid_small[] = { 1U, 3U, 132U, 0U, 6U }; + static const byte oid_small_der[] = { + 0x2b, 0x81, 0x04, 0x00, 0x06 + }; + const word32 oid_small_cnt = sizeof(oid_small) / sizeof(word32); + byte out[MAX_OID_SZ]; + word32 outSz; + word32 i; + + /* Test 1: length-only query (out == NULL) */ + outSz = 0; + ExpectIntEQ(wc_EncodeObjectId(oid_small, oid_small_cnt, NULL, &outSz), + 0); + ExpectIntEQ((int)outSz, (int)sizeof(oid_small_der)); + + /* Test 2: normal encode matches expected DER */ + outSz = sizeof(out); + ExpectIntEQ(wc_EncodeObjectId(oid_small, oid_small_cnt, out, &outSz), 0); + ExpectIntEQ((int)outSz, (int)sizeof(oid_small_der)); + for (i = 0; i < outSz && i < sizeof(oid_small_der); i++) { + ExpectIntEQ(out[i], oid_small_der[i]); + } + + /* Test 3: NULL args */ + outSz = sizeof(out); + ExpectIntEQ(wc_EncodeObjectId(NULL, oid_small_cnt, out, &outSz), + WC_NO_ERR_TRACE(BAD_FUNC_ARG)); + ExpectIntEQ(wc_EncodeObjectId(oid_small, oid_small_cnt, out, NULL), + WC_NO_ERR_TRACE(BAD_FUNC_ARG)); + + /* Test 4: output buffer too small */ + outSz = 1; + ExpectIntEQ(wc_EncodeObjectId(oid_small, oid_small_cnt, out, &outSz), + WC_NO_ERR_TRACE(BUFFER_E)); + + /* Test 5 : arc greater that SHRT_MAX + */ + { + static const word32 oid_large[] = { + 1U, 2U, 840U, 113549U, 1U, 1U, 11U + }; + static const byte oid_large_der[] = { + 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x0b + }; + const word32 oid_large_cnt = sizeof(oid_large) / sizeof(word32); + + outSz = sizeof(out); + ExpectIntEQ(wc_EncodeObjectId(oid_large, oid_large_cnt, out, &outSz), + 0); + ExpectIntEQ((int)outSz, (int)sizeof(oid_large_der)); + for (i = 0; i < outSz && i < sizeof(oid_large_der); i++) { + ExpectIntEQ(out[i], oid_large_der[i]); + } + +#if defined(HAVE_OID_DECODING) || defined(WOLFSSL_ASN_PRINT) + { + word32 dec[MAX_OID_SZ]; + word32 decSz = MAX_OID_SZ; + ExpectIntEQ(DecodeObjectId_ex(out, outSz, dec, &decSz), 0); + ExpectIntEQ((int)decSz, (int)oid_large_cnt); + for (i = 0; i < decSz && i < oid_large_cnt; i++) { + ExpectIntEQ(dec[i], oid_large[i]); + } + } +#endif /* HAVE_OID_DECODING || WOLFSSL_ASN_PRINT */ + } + } +#endif /* HAVE_OID_ENCODING && !NO_ASN */ + + return EXPECT_RESULT(); +} + #if defined(HAVE_PKCS8) && !defined(NO_ASN) && \ (defined(WOLFSSL_TEST_CERT) || defined(OPENSSL_EXTRA) || \ defined(OPENSSL_EXTRA_X509_SMALL) || defined(WOLFSSL_PUBLIC_ASN)) && \ diff --git a/tests/api/test_asn.h b/tests/api/test_asn.h index d48aac7a6f5..6d8bb6e9f2b 100644 --- a/tests/api/test_asn.h +++ b/tests/api/test_asn.h @@ -37,7 +37,9 @@ int test_SerialNumber0_RootCA(void); int test_DecodeAltNames_length_underflow(void); int test_DecodeCertExtensions_dup_certpol(void); int test_ParseCert_SM3wSM2_short_pubkey(void); -int test_wc_DecodeObjectId(void); +int test_wc_DecodeObjectId_ex(void); +int test_wc_DecodeObjectId_FIPS16(void); +int test_wc_EncodeObjectId(void); int test_ToTraditional_ex_handcrafted(void); int test_ToTraditional_ex_roundtrip(void); int test_ToTraditional_ex_negative(void); @@ -57,7 +59,9 @@ int test_ToTraditional_ex_mldsa_bad_params(void); TEST_DECL_GROUP("asn", test_DecodeAltNames_length_underflow), \ TEST_DECL_GROUP("asn", test_DecodeCertExtensions_dup_certpol), \ TEST_DECL_GROUP("asn", test_ParseCert_SM3wSM2_short_pubkey), \ - TEST_DECL_GROUP("asn", test_wc_DecodeObjectId), \ + TEST_DECL_GROUP("asn", test_wc_DecodeObjectId_ex), \ + TEST_DECL_GROUP("asn", test_wc_EncodeObjectId), \ + TEST_DECL_GROUP("asn", test_wc_DecodeObjectId_FIPS16), \ TEST_DECL_GROUP("asn", test_ToTraditional_ex_handcrafted), \ TEST_DECL_GROUP("asn", test_ToTraditional_ex_roundtrip), \ TEST_DECL_GROUP("asn", test_ToTraditional_ex_negative), \ diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 5d961fc9aaf..40a3939ee01 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -7164,18 +7164,112 @@ static int CheckCurve(word32 oid) * @return BAD_FUNC_ARG when in or outSz is NULL. * @return BUFFER_E when buffer too small. */ -int wc_EncodeObjectId(const word16* in, word32 inSz, byte* out, word32* outSz) +int wc_EncodeObjectId(const word32* in, word32 inSz, + byte* out, word32* outSz) { - return EncodeObjectId(in, inSz, out, outSz); + return EncodeObjectId_ex(in, inSz, out, outSz); +} + +int EncodeObjectId_ex(const word32* in, word32 inSz, + byte* out, word32* outSz) +{ + int i, x, len; + word32 d, t; + + /* check args */ + if (in == NULL || outSz == NULL || inSz < 2 || in[0] > 2) { + return BAD_FUNC_ARG; + } + + /* compute length of encoded OID */ + d = ((word32)in[0] * 40) + in[1]; + len = 0; + for (i = 1; i < (int)inSz; i++) { + x = 0; + t = d; + while (t) { + x++; + t >>= 1; + } + len += (x / 7) + ((x % 7) ? 1 : 0) + (d == 0 ? 1 : 0); + + if (i < (int)inSz - 1) { + d = in[i + 1]; + } + } + + if (out) { + /* verify length */ + if ((int)*outSz < len) { + return BUFFER_E; /* buffer provided is not large enough */ + } + + /* calc first byte */ + d = ((word32)in[0] * 40) + in[1]; + + /* encode bytes */ + x = 0; + for (i = 1; i < (int)inSz; i++) { + if (d) { + int y = x, z; + byte mask = 0; + while (d) { + out[x++] = (byte)((d & 0x7F) | mask); + d >>= 7; + mask |= 0x80; /* upper bit is set on all but the last byte */ + } + /* now swap bytes y...x-1 */ + z = x - 1; + while (y < z) { + mask = out[y]; + out[y] = out[z]; + out[z] = mask; + ++y; + --z; + } + } + else { + out[x++] = 0x00; /* zero value */ + } + + /* next word */ + if (i < (int)inSz - 1) { + d = in[i + 1]; + } + } + } + + /* return length */ + *outSz = (word32)len; + + return 0; } -int EncodeObjectId(const word16* in, word32 inSz, byte* out, word32* outSz) +/* Encode dotted form of OID into byte array version. + * + * Uses word16 for OIDs and can cause OIDs to be truncated. + * Do not use this for new code unless necessary. (use wc_EncodeObjectId + * or EncodeObjectId_ex) + * We cannot change this function because FIPS code expects + * this signature. + * + * @param [in] in Dotted form of OID. + * @param [in] inSz Count of numbers in dotted form. + * @param [in] out Buffer to hold OID. + * @param [in, out] outSz On in, size of buffer. + * On out, number of bytes in buffer. + * @return 0 on success + * @return BAD_FUNC_ARG when in or outSz is NULL. + * @return BUFFER_E when buffer too small. + */ +int EncodeObjectId(const word16* in, word32 inSz, + byte* out, word32* outSz) { int i, x, len; word32 d, t; /* check args */ - if (in == NULL || outSz == NULL || inSz <= 0) { + if (in == NULL || outSz == NULL || inSz < 2) { return BAD_FUNC_ARG; } @@ -7256,7 +7350,8 @@ int EncodeObjectId(const word16* in, word32 inSz, byte* out, word32* outSz) * @return BAD_FUNC_ARG when in or outSz is NULL. * @return BUFFER_E when dotted form buffer too small. */ -int DecodeObjectId(const byte* in, word32 inSz, word16* out, word32* outSz) +int DecodeObjectId(const byte* in, word32 inSz, + word16* out, word32* outSz) { int x = 0, y = 0; word32 t = 0; @@ -7267,6 +7362,9 @@ int DecodeObjectId(const byte* in, word32 inSz, word16* out, word32* outSz) return BAD_FUNC_ARG; } + if (*outSz < 2) + return BUFFER_E; + /* decode bytes */ while (inSz--) { if (cnt == 4) @@ -7278,8 +7376,14 @@ int DecodeObjectId(const byte* in, word32 inSz, word16* out, word32* outSz) if ((int)*outSz < 2) { return BUFFER_E; } - out[0] = (word16)(t / 40); - out[1] = (word16)(t % 40); + if (t < 80) { + out[0] = (word16)(t / 40); + out[1] = (word16)(t % 40); + } + else { + out[0] = 2; + out[1] = (word16)(t - 80); + } y = 2; } else { @@ -7299,6 +7403,72 @@ int DecodeObjectId(const byte* in, word32 inSz, word16* out, word32* outSz) return 0; } + +/* Encode dotted form of OID into word32 array version. + * + * Same as DecodeObjectId() but outputs to a word32 array so that OID + * arc values larger than a word16 are not truncated. + * + * @param [in] in Byte array containing OID. + * @param [in] inSz Size of OID in bytes. + * @param [in] out Array to hold dotted form of OID. + * @param [in, out] outSz On in, number of elements in array. + * On out, count of numbers in dotted form. + * @return 0 on success + * @return BAD_FUNC_ARG when in or outSz is NULL. + * @return BUFFER_E when dotted form buffer too small. + */ +int DecodeObjectId_ex(const byte* in, word32 inSz, + word32* out, word32* outSz) +{ + int x = 0, y = 0; + word32 t = 0; + + /* check args */ + if (in == NULL || outSz == NULL) { + return BAD_FUNC_ARG; + } + + if (*outSz < 2) + return BUFFER_E; + + /* decode bytes */ + while (inSz--) { + if (t > 0xFFFFFFFFU >> 7) { + return ASN_OBJECT_ID_E; + } + t = (t << 7) | (in[x] & 0x7F); + if (!(in[x] & 0x80)) { + if (y == 0) { + if ((int)*outSz < 2) { + return BUFFER_E; + } + if (t < 80) { + out[0] = t / 40; + out[1] = t % 40; + } + else { + out[0] = 2; + out[1] = t - 80; + } + y = 2; + } + else { + if (y >= (int)*outSz) { + return BUFFER_E; + } + out[y++] = t; + } + t = 0; /* reset tmp */ + } + x++; + } + + /* return length */ + *outSz = (word32)y; + + return 0; +} #endif /* HAVE_OID_DECODING || WOLFSSL_ASN_PRINT */ /* Decode the header of a BER/DER encoded OBJECT ID. @@ -7381,14 +7551,14 @@ static int DumpOID(const byte* oidData, word32 oidSz, word32 oid, #ifdef HAVE_OID_DECODING { - word16 decOid[MAX_OID_SZ]; + word32 decOid[MAX_OID_SZ]; word32 decOidSz = MAX_OID_SZ; /* Decode the OID into dotted form. */ - ret = DecodeObjectId(oidData, oidSz, decOid, &decOidSz); + ret = DecodeObjectId_ex(oidData, oidSz, decOid, &decOidSz); if (ret == 0) { - printf(" Decoded (Sz %d): ", decOidSz); + printf(" Decoded (Sz %u): ", decOidSz); for (i=0; idp == NULL) { return BAD_FUNC_ARG; } #ifdef HAVE_OID_ENCODING + /* ecc_oid_t cannot be changed due to it being in the FIPS boundary so we + * have a work around of upsizing its representation*/ + /* Get the size of the encoded OID without having an encoded output */ ret = EncodeObjectId(key->dp->oid, key->dp->oidSz, NULL, &oidSz); if (ret != 0) { return ret; @@ -14335,7 +14507,7 @@ static int GenerateDNSEntryRIDString(DNS_entry* entry, void* heap) int tmpSize = MAX_OID_SZ; word32 oid = 0; word32 idx = 0; - word16 tmpName[MAX_OID_SZ]; + word32 tmpName[MAX_OID_SZ]; char oidName[MAX_OID_SZ]; char* finalName = NULL; @@ -14361,7 +14533,7 @@ static int GenerateDNSEntryRIDString(DNS_entry* entry, void* heap) #endif { /* Decode OBJECT_ID into dotted form array. */ - ret = DecodeObjectId((const byte*)(entry->name),(word32)entry->len, + ret = DecodeObjectId_ex((const byte*)(entry->name),(word32)entry->len, tmpName, (word32*)&tmpSize); if (ret == 0) { @@ -14374,14 +14546,14 @@ static int GenerateDNSEntryRIDString(DNS_entry* entry, void* heap) if (i < tmpSize - 1) { ret = XSNPRINTF(oidName + j, (word32)(MAX_OID_SZ - j), - "%d.", tmpName[i]); + "%u.", tmpName[i]); } else { ret = XSNPRINTF(oidName + j, (word32)(MAX_OID_SZ - j), - "%d", tmpName[i]); + "%u", tmpName[i]); } - if (ret >= 0) { + if (ret >= 0 && ret < MAX_OID_SZ - j) { j += ret; } else { @@ -21623,9 +21795,9 @@ static int DecodeCertExtensions(DecodedCert* cert) #ifdef WC_ASN_UNKNOWN_EXT_CB if (isUnknownExt && (cert->unknownExtCallback != NULL || cert->unknownExtCallbackEx != NULL)) { - word16 decOid[MAX_OID_SZ]; + word32 decOid[MAX_OID_SZ]; word32 decOidSz = MAX_OID_SZ; - ret = DecodeObjectId( + ret = DecodeObjectId_ex( dataASN[CERTEXTASN_IDX_OID].data.oid.data, dataASN[CERTEXTASN_IDX_OID].data.oid.length, decOid, &decOidSz); @@ -37976,64 +38148,6 @@ int wc_Asn1_SetOidToNameCb(Asn1* asn1, Asn1OidToNameCb nameCb) return ret; } -/* Encode dotted form of OID into byte array version. - * - * @param [in] in Byte array containing OID. - * @param [in] inSz Size of OID in bytes. - * @param [in] out Array to hold dotted form of OID. - * @param [in, out] outSz On in, number of elements in array. - * On out, count of numbers in dotted form. - * @return 0 on success - * @return BAD_FUNC_ARG when in or outSz is NULL. - * @return BUFFER_E when dotted form buffer too small. - */ -static int EncodedDottedForm(const byte* in, word32 inSz, word32* out, - word32* outSz) -{ - int x = 0, y = 0; - word32 t = 0; - - /* check args */ - if (in == NULL || outSz == NULL) { - return BAD_FUNC_ARG; - } - - if (*outSz < 2) { - return BUFFER_E; - } - - /* decode bytes */ - while (inSz--) { - t = (t << 7) | (in[x] & 0x7F); - if (!(in[x] & 0x80)) { - if (y >= (int)*outSz) { - return BUFFER_E; - } - if (y == 0) { - /* Special case for when first arc is 2 */ - if (t < 80) { - out[0] = t / 40; - out[1] = t % 40; - } - else { - out[0] = 2; - out[1] = t - 80; - } - y = 2; - } - else { - out[y++] = t; - } - t = 0; /* reset tmp */ - } - x++; - } - - /* return length */ - *outSz = (word32)y; - - return 0; -} /* Print OID in dotted form or as hex bytes. * * @param [in] file File pointer to write to. @@ -38047,7 +38161,7 @@ static void PrintObjectIdNum(XFILE file, unsigned char* oid, word32 len) word32 i; /* Decode OBJECT_ID into dotted form array. */ - if (EncodedDottedForm(oid, len, dotted_nums, &num) == 0) { + if (DecodeObjectId_ex(oid, len, dotted_nums, &num) == 0) { /* Print out each number of dotted form. */ for (i = 0; i < num; i++) { XFPRINTF(file, "%d", dotted_nums[i]); diff --git a/wolfcrypt/src/wc_pkcs11.c b/wolfcrypt/src/wc_pkcs11.c index 5a3b3cf5fee..d34dade36f2 100644 --- a/wolfcrypt/src/wc_pkcs11.c +++ b/wolfcrypt/src/wc_pkcs11.c @@ -1383,7 +1383,7 @@ static int Pkcs11EccSetParams(ecc_key* key, CK_ATTRIBUTE* tmpl, int idx) unsigned char* derParams = tmpl[idx].pValue; #if defined(HAVE_OID_ENCODING) word32 oidSz = ECC_MAX_OID_LEN - 2; - ret = wc_EncodeObjectId(key->dp->oid, key->dp->oidSz, derParams+2, &oidSz); + ret = EncodeObjectId(key->dp->oid, key->dp->oidSz, derParams+2, &oidSz); if (ret != 0) { return ret; } diff --git a/wolfssl/wolfcrypt/asn.h b/wolfssl/wolfcrypt/asn.h index 8b12a306b24..46b071253dd 100644 --- a/wolfssl/wolfcrypt/asn.h +++ b/wolfssl/wolfcrypt/asn.h @@ -1761,13 +1761,12 @@ typedef struct WOLFSSL_AIA_ENTRY { word32 uriSz; /* Length of URI data. */ } WOLFSSL_AIA_ENTRY; #endif /* WOLFSSL_AIA_ENTRY_DEFINED */ - #ifdef WC_ASN_UNKNOWN_EXT_CB -typedef int (*wc_UnknownExtCallback)(const word16* oid, word32 oidSz, int crit, - const unsigned char* der, word32 derSz); -typedef int (*wc_UnknownExtCallbackEx)(const word16* oid, word32 oidSz, - int crit, const unsigned char* der, - word32 derSz, void *ctx); + typedef int (*wc_UnknownExtCallback)(const word32* oid, word32 oidSz, + int crit, const unsigned char* der, word32 derSz); + typedef int (*wc_UnknownExtCallbackEx)(const word32* oid, word32 oidSz, + int crit, const unsigned char* der, + word32 derSz, void *ctx); #endif struct DecodedCert { @@ -2370,6 +2369,7 @@ typedef enum MimeStatus #if defined(HAVE_OID_DECODING) || defined(WOLFSSL_ASN_PRINT) || \ defined(OPENSSL_ALL) #define DecodeObjectId wc_DecodeObjectId + #define DecodeObjectId_ex wc_DecodeObjectId_ex #endif #if defined(WOLFSSL_AKID_NAME) && !defined(GetCAByAKID) /* GetCAByAKID() has two implementations, a full implementation in @@ -2613,15 +2613,21 @@ WOLFSSL_ASN_API int GetASNInt(const byte* input, word32* inOutIdx, int* len, WOLFSSL_LOCAL word32 wc_oid_sum(const byte* input, int length); #ifdef HAVE_OID_ENCODING - WOLFSSL_API int wc_EncodeObjectId(const word16* in, word32 inSz, + WOLFSSL_API int wc_EncodeObjectId(const word32* in, word32 inSz, + byte* out, word32* outSz); + WOLFSSL_LOCAL int EncodeObjectId_ex(const word32* in, word32 inSz, byte* out, word32* outSz); + /* Unchangeable due to being called from FIPS code expecting word16 in */ WOLFSSL_LOCAL int EncodeObjectId(const word16* in, word32 inSz, byte* out, word32* outSz); #endif #if defined(HAVE_OID_DECODING) || defined(WOLFSSL_ASN_PRINT) || \ defined(OPENSSL_ALL) + /* Unchangeable due to being called from FIPS code expecting word16 out */ WOLFSSL_TEST_VIS int DecodeObjectId(const byte* in, word32 inSz, word16* out, word32* outSz); + WOLFSSL_TEST_VIS int DecodeObjectId_ex(const byte* in, word32 inSz, + word32* out, word32* outSz); #endif WOLFSSL_LOCAL int GetASNObjectId(const byte* input, word32* inOutIdx, int* len, word32 maxIdx); From 2e06f51f20957bf7adc840a47c95229407190ef2 Mon Sep 17 00:00:00 2001 From: Aidan Keefe Date: Wed, 15 Jul 2026 11:22:53 -0600 Subject: [PATCH 2/3] fenrir review fixes --- tests/api/test_asn.c | 28 ++++++++++++++++++++++++++++ wolfcrypt/src/asn.c | 6 +++--- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/tests/api/test_asn.c b/tests/api/test_asn.c index 09139b16cfe..38bc4d509da 100644 --- a/tests/api/test_asn.c +++ b/tests/api/test_asn.c @@ -1939,6 +1939,20 @@ int test_wc_DecodeObjectId_ex(void) ExpectIntEQ(out[i], oid_dot_large_arc[i]); } } + + /* Test 9: an arc that does not fit in a word32 must be rejected by + * the overflow guard rather than silently wrapping. The first byte + * (0x2a) decodes to 1.2; the following five continuation bytes encode + * a single sub-identifier that needs more than 32 bits. */ + { + static const byte oid_overflow_arc[] = { + 0x2a, 0xFF, 0xFF, 0xFF, 0xFF, 0x7F + }; + outSz = MAX_OID_SZ; + ExpectIntEQ(DecodeObjectId_ex(oid_overflow_arc, + sizeof(oid_overflow_arc), out, &outSz), + WC_NO_ERR_TRACE(ASN_OBJECT_ID_E)); + } } #endif /* !NO_ASN && (HAVE_OID_DECODING || WOLFSSL_ASN_PRINT) */ @@ -2018,6 +2032,20 @@ int test_wc_EncodeObjectId(void) } #endif /* HAVE_OID_DECODING || WOLFSSL_ASN_PRINT */ } + + /* Test 6: first arc greater than 2 is invalid (in[0] > 2) */ + { + static const word32 oid_bad_first[] = { 3U, 1U }; + outSz = sizeof(out); + ExpectIntEQ(wc_EncodeObjectId(oid_bad_first, + sizeof(oid_bad_first) / sizeof(word32), out, &outSz), + WC_NO_ERR_TRACE(BAD_FUNC_ARG)); + } + + /* Test 7: fewer than two arcs is invalid (inSz < 2) */ + outSz = sizeof(out); + ExpectIntEQ(wc_EncodeObjectId(oid_small, 1, out, &outSz), + WC_NO_ERR_TRACE(BAD_FUNC_ARG)); } #endif /* HAVE_OID_ENCODING && !NO_ASN */ diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 40a3939ee01..a2753f7a1f4 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -7269,7 +7269,7 @@ int EncodeObjectId(const word16* in, word32 inSz, word32 d, t; /* check args */ - if (in == NULL || outSz == NULL || inSz < 2) { + if (in == NULL || outSz == NULL || inSz < 2 || in[0] > 2) { return BAD_FUNC_ARG; } @@ -7339,7 +7339,7 @@ int EncodeObjectId(const word16* in, word32 inSz, #endif /* HAVE_OID_ENCODING */ #if defined(HAVE_OID_DECODING) || defined(WOLFSSL_ASN_PRINT) -/* Encode dotted form of OID into byte array version. +/* Decode DER encoded form of OID into word16 OID dot seperated. * * @param [in] in Byte array containing OID. * @param [in] inSz Size of OID in bytes. @@ -7404,7 +7404,7 @@ int DecodeObjectId(const byte* in, word32 inSz, return 0; } -/* Encode dotted form of OID into word32 array version. +/* Decode DER encoded form of OID into word32 OID dot seperated. * * Same as DecodeObjectId() but outputs to a word32 array so that OID * arc values larger than a word16 are not truncated. From beee345a810f1869525d439c72ed2a81af7f81dd Mon Sep 17 00:00:00 2001 From: Aidan Keefe Date: Wed, 15 Jul 2026 11:47:38 -0600 Subject: [PATCH 3/3] hardening of OID Encoding --- wolfcrypt/src/asn.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index a2753f7a1f4..67df0be282d 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -7181,6 +7181,13 @@ int EncodeObjectId_ex(const word32* in, word32 inSz, return BAD_FUNC_ARG; } + /* guard against word32 overflow of the combined first arc: + * ((in[0] * 40) + in[1]) is computed below in both the length and + * encode passes, so rejecting it here covers both. */ + if (in[1] > 0xFFFFFFFFU - (in[0] * 40)) { + return BAD_FUNC_ARG; + } + /* compute length of encoded OID */ d = ((word32)in[0] * 40) + in[1]; len = 0; @@ -7339,7 +7346,7 @@ int EncodeObjectId(const word16* in, word32 inSz, #endif /* HAVE_OID_ENCODING */ #if defined(HAVE_OID_DECODING) || defined(WOLFSSL_ASN_PRINT) -/* Decode DER encoded form of OID into word16 OID dot seperated. +/* Decode DER encoded form of OID into word16 OID dot separated. * * @param [in] in Byte array containing OID. * @param [in] inSz Size of OID in bytes. @@ -7404,7 +7411,7 @@ int DecodeObjectId(const byte* in, word32 inSz, return 0; } -/* Decode DER encoded form of OID into word32 OID dot seperated. +/* Decode DER encoded form of OID into word32 OID dot separated. * * Same as DecodeObjectId() but outputs to a word32 array so that OID * arc values larger than a word16 are not truncated.