From 47a52ac9d1ae2b7b963c82dff2040808eb511e99 Mon Sep 17 00:00:00 2001 From: Emma Stensland Date: Thu, 30 Jul 2026 10:00:19 -0600 Subject: [PATCH] fixed encode policy oid --- src/ssl_asn1.c | 4 +- tests/api/test_asn.c | 113 +++++++++++++++++++++++++++++++++++++++ tests/api/test_asn.h | 2 + wolfcrypt/src/asn.c | 112 +++++++++++++++++++++++--------------- wolfcrypt/src/asn_orig.c | 2 +- wolfssl/wolfcrypt/asn.h | 2 +- 6 files changed, 189 insertions(+), 46 deletions(-) diff --git a/src/ssl_asn1.c b/src/ssl_asn1.c index 9768eb4e0d6..8b7414a6361 100644 --- a/src/ssl_asn1.c +++ b/src/ssl_asn1.c @@ -6256,7 +6256,7 @@ int wc_OBJ_sn2nid(const char *sn) } #ifdef WOLFSSL_CERT_EXT - ret = EncodePolicyOID(out, &outSz, s, NULL); + ret = wc_EncodePolicyOID(out, &outSz, s, NULL); if (ret == 0) { /* sum OID */ sum = wc_oid_sum(out, outSz); @@ -6321,7 +6321,7 @@ int wc_OBJ_sn2nid(const char *sn) return NULL; /* If s is numerical value, try to sum oid */ - ret = EncodePolicyOID(out, &outSz, s, NULL); + ret = wc_EncodePolicyOID(out, &outSz, s, NULL); if (ret == 0 && outSz > 0) { /* If numerical encode succeeded then just * create object from that because sums are diff --git a/tests/api/test_asn.c b/tests/api/test_asn.c index c42124a458e..5c4e4d56287 100644 --- a/tests/api/test_asn.c +++ b/tests/api/test_asn.c @@ -2081,6 +2081,119 @@ int test_wc_DecodeObjectId(void) return EXPECT_RESULT(); } +int test_wc_EncodePolicyOID(void) +{ + EXPECT_DECLS; + +#if (defined(WOLFSSL_CERT_GEN) && defined(WOLFSSL_CERT_EXT)) \ + || defined(OPENSSL_EXTRA) + { + byte out[MAX_OID_SZ]; + word32 outSz; + + /* Test 1: combined first identifier fits in one byte. + * "1.2.3.4.5" -> 40*1+2=42 (0x2a), then 3, 4, 5 */ + outSz = sizeof(out); + ExpectIntEQ(wc_EncodePolicyOID(out, &outSz, "1.2.3.4.5", NULL), 0); + ExpectIntEQ((int)outSz, 4); + ExpectIntEQ(out[0], 0x2a); + ExpectIntEQ(out[1], 3); + ExpectIntEQ(out[2], 4); + ExpectIntEQ(out[3], 5); + + /* Test 2: combined first identifier needs multi-byte base-128 + * encoding. "2.100" -> 40*2+100=180 -> 0x81 0x34 */ + outSz = sizeof(out); + ExpectIntEQ(wc_EncodePolicyOID(out, &outSz, "2.100", NULL), 0); + ExpectIntEQ((int)outSz, 2); + ExpectIntEQ(out[0], 0x81); + ExpectIntEQ(out[1], 0x34); + + /* Test 3: X=2 combined identifier well past 127, still valid + * per X.690 since X=2 has no upper bound on Y. "2.999" -> + * 40*2+999=1079 -> 0x88 0x37 */ + outSz = sizeof(out); + ExpectIntEQ(wc_EncodePolicyOID(out, &outSz, "2.999", NULL), 0); + ExpectIntEQ((int)outSz, 2); + ExpectIntEQ(out[0], 0x88); + ExpectIntEQ(out[1], 0x37); + + /* Test 4: X=0/X=1 with Y>=40 cannot round-trip (combined value + * would collide with the X=2 range) and must be rejected. */ + outSz = sizeof(out); + ExpectIntEQ(wc_EncodePolicyOID(out, &outSz, "1.200", NULL), + WC_NO_ERR_TRACE(ASN_OBJECT_ID_E)); + outSz = sizeof(out); + ExpectIntEQ(wc_EncodePolicyOID(out, &outSz, "0.40", NULL), + WC_NO_ERR_TRACE(ASN_OBJECT_ID_E)); + + /* Test 5: X=0/X=1 with Y just under 40 is the valid boundary */ + outSz = sizeof(out); + ExpectIntEQ(wc_EncodePolicyOID(out, &outSz, "1.39", NULL), 0); + ExpectIntEQ((int)outSz, 1); + ExpectIntEQ(out[0], 40 + 39); + + /* Test 6: first arc out of range */ + outSz = sizeof(out); + ExpectIntEQ(wc_EncodePolicyOID(out, &outSz, "3.1", NULL), + WC_NO_ERR_TRACE(ASN_OBJECT_ID_E)); + + /* Test 7: negative arc components must not wrap to a huge word32 + * and slip past the range checks (e.g. "2.-1" wrapping firstArc + + * val back down to a small, valid-looking value). */ + outSz = sizeof(out); + ExpectIntEQ(wc_EncodePolicyOID(out, &outSz, "2.-1", NULL), + WC_NO_ERR_TRACE(ASN_OBJECT_ID_E)); + outSz = sizeof(out); + ExpectIntEQ(wc_EncodePolicyOID(out, &outSz, "1.-5", NULL), + WC_NO_ERR_TRACE(ASN_OBJECT_ID_E)); + outSz = sizeof(out); + ExpectIntEQ(wc_EncodePolicyOID(out, &outSz, "-1.2", NULL), + WC_NO_ERR_TRACE(ASN_OBJECT_ID_E)); + + /* Test 8: a later (non-combined) arc >= 128 encodes correctly + * with a large-enough buffer. "1.2.16384" -> combined id 42 + * (0x2a) + arc 16384 -> 0x81 0x80 0x00 (same multi-byte value + * as Test 9 below, but here with room to check the actual + * bytes at the later-arc call site, not just BUFFER_E). */ + outSz = sizeof(out); + ExpectIntEQ(wc_EncodePolicyOID(out, &outSz, "1.2.16384", NULL), 0); + ExpectIntEQ((int)outSz, 4); + ExpectIntEQ(out[0], 0x2a); + ExpectIntEQ(out[1], 0x81); + ExpectIntEQ(out[2], 0x80); + ExpectIntEQ(out[3], 0x00); + + /* Test 9: BUFFER_E when the output buffer is too small for the + * combined first identifier's multi-byte encoding. + * "2.16304" -> 40*2+16304=16384 -> needs 3 bytes (0x81 0x80 0x00), + * outSz=2 is one byte short. */ + outSz = 2; + ExpectIntEQ(wc_EncodePolicyOID(out, &outSz, "2.16304", NULL), + WC_NO_ERR_TRACE(BUFFER_E)); + + /* Test 10: BUFFER_E when the output buffer is too small for a + * later (non-combined) arc's multi-byte encoding. + * "1.2.16384" -> combined id 42 (1 byte) + arc 16384 (3 bytes, + * same as above) -> needs 4 bytes total, outSz=3 is one short. */ + outSz = 3; + ExpectIntEQ(wc_EncodePolicyOID(out, &outSz, "1.2.16384", NULL), + WC_NO_ERR_TRACE(BUFFER_E)); + + /* Test 11: NULL/bad args */ + outSz = sizeof(out); + ExpectIntEQ(wc_EncodePolicyOID(NULL, &outSz, "1.2.3", NULL), + WC_NO_ERR_TRACE(BAD_FUNC_ARG)); + ExpectIntEQ(wc_EncodePolicyOID(out, NULL, "1.2.3", NULL), + WC_NO_ERR_TRACE(BAD_FUNC_ARG)); + ExpectIntEQ(wc_EncodePolicyOID(out, &outSz, NULL, NULL), + WC_NO_ERR_TRACE(BAD_FUNC_ARG)); + } +#endif /* (WOLFSSL_CERT_GEN && WOLFSSL_CERT_EXT) || OPENSSL_EXTRA */ + + 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 8798bbc6aad..18163ad6e91 100644 --- a/tests/api/test_asn.h +++ b/tests/api/test_asn.h @@ -39,6 +39,7 @@ int test_DecodeCertExtensions_dup_certpol(void); int test_ParseCert_SM3wSM2_short_pubkey(void); int test_ParseCert_dnBufferBoundary(void); int test_wc_DecodeObjectId(void); +int test_wc_EncodePolicyOID(void); int test_ToTraditional_ex_handcrafted(void); int test_ToTraditional_ex_roundtrip(void); int test_ToTraditional_ex_negative(void); @@ -62,6 +63,7 @@ int test_wc_AsnFeatureCoverage(void); TEST_DECL_GROUP("asn", test_ParseCert_SM3wSM2_short_pubkey), \ TEST_DECL_GROUP("asn", test_ParseCert_dnBufferBoundary), \ TEST_DECL_GROUP("asn", test_wc_DecodeObjectId), \ + TEST_DECL_GROUP("asn", test_wc_EncodePolicyOID), \ 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 f1287ac709a..02a0edd9e9a 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -27758,7 +27758,7 @@ static int SetCertificatePolicies(byte *output, XMEMSET(oid, 0, oidSz); dataASN[POLICYINFOASN_IDX_QUALI].noOut = 1; - ret = EncodePolicyOID(oid, &oidSz, input[i], heap); + ret = wc_EncodePolicyOID(oid, &oidSz, input[i], heap); if (ret == 0) { XMEMSET(dataASN, 0, sizeof(dataASN)); SetASN_Buffer(&dataASN[POLICYINFOASN_IDX_ID], oid, oidSz); @@ -29087,9 +29087,9 @@ static int EncodeExtensions(Cert* cert, byte* output, word32 maxSz, int idx = CERTEXTSASN_IDX_START_CUSTOM + (i * 4); word32 encodedOidSz = MAX_OID_SZ; idx++; /* Skip one for for SEQ. */ - /* EncodePolicyOID() will never return error since we parsed this + /* wc_EncodePolicyOID() will never return error since we parsed this * OID when it was set. */ - EncodePolicyOID(&encodedOids[i * MAX_OID_SZ], &encodedOidSz, + wc_EncodePolicyOID(&encodedOids[i * MAX_OID_SZ], &encodedOidSz, cert->customCertExt[i].oid, NULL); SetASN_Buffer(&dataASN[idx], &encodedOids[i * MAX_OID_SZ], encodedOidSz); @@ -31922,7 +31922,7 @@ int wc_SetExtKeyUsageOID(Cert *cert, const char *in, word32 sz, byte idx, return BAD_FUNC_ARG; } - if (EncodePolicyOID(oid, &oidSz, in, heap) != 0) { + if (wc_EncodePolicyOID(oid, &oidSz, in, heap) != 0) { return BUFFER_E; } @@ -31955,7 +31955,7 @@ int wc_SetCustomExtension(Cert *cert, int critical, const char *oid, } /* Make sure we can properly parse the OID. */ - ret = EncodePolicyOID(encodedOid, &encodedOidSz, oid, NULL); + ret = wc_EncodePolicyOID(encodedOid, &encodedOidSz, oid, NULL); if (ret != 0) { return ret; } @@ -32514,12 +32514,41 @@ int wc_SetDatesBuffer(Cert* cert, const byte* der, int derSz) #if (defined(WOLFSSL_CERT_GEN) && defined(WOLFSSL_CERT_EXT)) \ || defined(OPENSSL_EXTRA) +/* Base-128 (DER) encode a single arc value at out[*idx], advancing *idx. + * Shared by the combined first identifier and every later arc, since both + * need the same continuation-bit encoding once the value reaches 128. */ +static int EncodeOidArc(byte *out, word32 *idx, word32 outSz, word32 val) +{ + word32 tb = 0; + int i = 0; + byte oid[MAX_OID_SZ]; + + while (val >= 128) { + word32 x = val % 128; + val /= 128; + oid[i++] = (byte) (((tb++) ? 0x80 : 0) | x); + } + + if ((*idx + (word32)i) >= outSz) + return BUFFER_E; + + oid[i] = (byte) (((tb++) ? 0x80 : 0) | val); + + /* push value in the right order */ + while (i >= 0) + out[(*idx)++] = oid[i--]; + + return 0; +} + /* Encode OID string representation to ITU-T X.690 format */ -int EncodePolicyOID(byte *out, word32 *outSz, const char *in, void* heap) +int wc_EncodePolicyOID(byte *out, word32 *outSz, const char *in, void* heap) { word32 idx = 0, nb_val; char *token, *str, *ptr; word32 len; + word32 firstArc = 0; + int ret = 0; (void)heap; @@ -32539,61 +32568,60 @@ int EncodePolicyOID(byte *out, word32 *outSz, const char *in, void* heap) token = XSTRTOK(str, ".", &ptr); while (token != NULL) { - word32 val = (word32)XATOI(token); + int ival = XATOI(token); + word32 val; + + /* reject negative arcs before they wrap to a huge word32 and + * defeat the range checks below (e.g. "2.-1" wrapping to a + * small-looking firstArc + val). */ + if (ival < 0) { + ret = ASN_OBJECT_ID_E; + break; + } + val = (word32)ival; if (nb_val == 0) { if (val > 2) { - XFREE(str, heap, DYNAMIC_TYPE_TMP_BUFFER); - return ASN_OBJECT_ID_E; + ret = ASN_OBJECT_ID_E; + break; } - out[idx] = (byte)(40 * val); + /* first two arcs combine into one X.690 identifier (40*X+Y); + * defer encoding until Y is known below. */ + firstArc = 40 * val; } else if (nb_val == 1) { - if (val > 127) { - XFREE(str, heap, DYNAMIC_TYPE_TMP_BUFFER); - return ASN_OBJECT_ID_E; - } - - if (idx > *outSz) { - XFREE(str, heap, DYNAMIC_TYPE_TMP_BUFFER); - return BUFFER_E; + /* when X (the first arc) is 0 or 1, the combined identifier + * 40*X+Y must stay under 80 for a decoder to recover X/Y + * (V<80 ? (V/40,V%40) : (2,V-80)), so Y must be < 40. X=2 + * has no such limit. */ + if (firstArc < 80 && val >= 40) { + ret = ASN_OBJECT_ID_E; + break; } - out[idx] = (byte)(out[idx] + val); - ++idx; + /* the combined identifier can exceed one byte (e.g. "2.100" + * -> 180), so it needs the same base-128 continuation + * encoding as any later arc, not a raw single byte. */ + ret = EncodeOidArc(out, &idx, *outSz, firstArc + val); + if (ret != 0) + break; } else { - word32 tb = 0; - int i = 0; - byte oid[MAX_OID_SZ]; - - while (val >= 128) { - word32 x = val % 128; - val /= 128; - oid[i++] = (byte) (((tb++) ? 0x80 : 0) | x); - } - - if ((idx+(word32)i) >= *outSz) { - XFREE(str, heap, DYNAMIC_TYPE_TMP_BUFFER); - return BUFFER_E; - } - - oid[i] = (byte) (((tb++) ? 0x80 : 0) | val); - - /* push value in the right order */ - while (i >= 0) - out[idx++] = oid[i--]; + ret = EncodeOidArc(out, &idx, *outSz, val); + if (ret != 0) + break; } token = XSTRTOK(NULL, ".", &ptr); nb_val++; } - *outSz = idx; + if (ret == 0) + *outSz = idx; XFREE(str, heap, DYNAMIC_TYPE_TMP_BUFFER); - return 0; + return ret; } #endif /* WOLFSSL_CERT_EXT || OPENSSL_EXTRA */ diff --git a/wolfcrypt/src/asn_orig.c b/wolfcrypt/src/asn_orig.c index cc0d3ddd7d7..fc6a80af427 100644 --- a/wolfcrypt/src/asn_orig.c +++ b/wolfcrypt/src/asn_orig.c @@ -5547,7 +5547,7 @@ static int SetCertificatePolicies(byte *output, oidSz = sizeof(oid); XMEMSET(oid, 0, oidSz); - ret = EncodePolicyOID(oid, &oidSz, input[i], heap); + ret = wc_EncodePolicyOID(oid, &oidSz, input[i], heap); if (ret != 0) return ret; diff --git a/wolfssl/wolfcrypt/asn.h b/wolfssl/wolfcrypt/asn.h index bd380e69ba4..0d00bfd4138 100644 --- a/wolfssl/wolfcrypt/asn.h +++ b/wolfssl/wolfcrypt/asn.h @@ -2509,7 +2509,7 @@ WOLFSSL_API int wc_SetUnknownExtCallbackEx(DecodedCert* cert, WOLFSSL_LOCAL int DecodePolicyOID(char *out, word32 outSz, const byte *in, word32 inSz); -WOLFSSL_LOCAL int EncodePolicyOID(byte *out, word32 *outSz, +WOLFSSL_TEST_VIS int wc_EncodePolicyOID(byte *out, word32 *outSz, const char *in, void* heap); WOLFSSL_TEST_VIS int DecodeExtensionType(const byte* input, word32 length, word32 oid, byte critical,