Merge pull request #11021 from kareem-wolfssl/zd22223

Remove carry over of old buffers/sizes in X509 ext function.

certs/test/catalog.txt

@@ -12,3 +12,10 @@ digsigku.pem:
   set.
 cert-ext-joi.pem:
   Simple certificate that includes OIDs for JurisdictionOfIncorporation
+cert-ext-oid-collide.der:
+  Non-critical extension whose 4-byte OID has the same wc_oid_sum() as
+  prime256v1, so it maps to a NID with a longer canonical OID. Tests that
+  wolfSSL_X509_set_ext() sizes the ASN1_OBJECT buffer from the certificate's
+  OID, not the mapped one. Does not collide under WOLFSSL_OLD_OID_SUM.
+  Signature is invalid; only ever parsed, never verified. Regenerate with
+  certs/test/gen-oid-collide-cert.sh.

certs/test/gen-oid-collide-cert.sh

@@ -0,0 +1,112 @@
+#!/bin/sh
+#
+# Regenerates certs/test/cert-ext-oid-collide.der, whose extension OID maps via
+# wc_oid_sum() to a NID with a longer canonical OID. That shrink is what
+# test_wolfSSL_X509_set_ext_oid_collision() exercises.
+#
+# wc_oid_sum() (wolfcrypt/src/asn.c) is an invertible four lane XOR fold, so a
+# four byte OID exists for any sum. This one collides with prime256v1:
+#
+#     2A 86 48 CE 3D 03 01 07  (prime256v1)  -> 0x49498517
+#     E8 85 B6 49                            -> 0x49498517
+#
+# Those bytes are a single subidentifier, which openssl will not emit from a
+# dotted OID, hence the DER surgery instead of `openssl req`. The signature is
+# left stale; the test parses without verifying.
+#
+# Run from the wolfssl root directory.
+
+set -e
+
+BASE=certs/ca-ecc-cert.der
+OUT=certs/test/cert-ext-oid-collide.der
+OID_HEX="E8 85 B6 49"
+
+if [ ! -f "$BASE" ]; then
+    echo "Run from the wolfssl root directory (missing $BASE)" >&2
+    exit 1
+fi
+
+# Re-check the collision in case wc_oid_sum() changed.
+python3 - "$OID_HEX" <<'EOF'
+import sys
+
+def oid_sum(data):
+    oid, shift = 0, 0
+    for b in data:
+        oid ^= (~b & 0xFFFFFFFF) << shift
+        oid &= 0xFFFFFFFF
+        shift = (shift + 8) & 0x1f
+    return oid & 0x7fffffff
+
+prime256v1 = bytes([0x2A, 0x86, 0x48, 0xCE, 0x3D, 0x03, 0x01, 0x07])
+chosen = bytes.fromhex(sys.argv[1].replace(' ', ''))
+print("prime256v1 sum = 0x%08X" % oid_sum(prime256v1))
+print("chosen     sum = 0x%08X" % oid_sum(chosen))
+if oid_sum(prime256v1) != oid_sum(chosen):
+    sys.exit("OID no longer collides with prime256v1; pick a new one")
+EOF
+
+python3 - "$BASE" "$OUT" "$OID_HEX" <<'EOF'
+import sys
+
+src, dst, oid_hex = sys.argv[1], sys.argv[2], sys.argv[3]
+oid_content = bytes.fromhex(oid_hex.replace(' ', ''))
+
+
+def tlv(buf, i=0):
+    """(tag, header len, content len, content start) at i."""
+    tag = buf[i]
+    j = i + 1
+    n = buf[j]
+    j += 1
+    if n & 0x80:
+        cnt = n & 0x7F
+        n = int.from_bytes(buf[j:j + cnt], 'big')
+        j += cnt
+    return tag, j - i, n, j
+
+
+def children(buf):
+    """Split constructed content into raw TLV slices."""
+    out, i = [], 0
+    while i < len(buf):
+        _, _, cl, cs = tlv(buf, i)
+        out.append(buf[i:cs + cl])
+        i = cs + cl
+    return out
+
+
+def enc_len(n):
+    if n < 0x80:
+        return bytes([n])
+    b = n.to_bytes((n.bit_length() + 7) // 8, 'big')
+    return bytes([0x80 | len(b)]) + b
+
+
+def wrap(tag, content):
+    return bytes([tag]) + enc_len(len(content)) + content
+
+
+def content_of(raw):
+    _, _, cl, cs = tlv(raw)
+    return raw[cs:cs + cl]
+
+
+cert = open(src, 'rb').read()
+tbs, sigalg, sig = children(content_of(cert))
+
+fields = children(content_of(tbs))
+# Replace the [3] extensions element with our own.
+fields = [f for f in fields if f[0] != 0xA3]
+
+# SEQUENCE { extnID OID, extnValue OCTET STRING }. critical is omitted so the
+# parser does not reject an unknown critical extension.
+ext = wrap(0x30, wrap(0x06, oid_content) + wrap(0x04, b'\x01\x02\x03\x04'))
+fields.append(wrap(0xA3, wrap(0x30, ext)))
+
+out = wrap(0x30, wrap(0x30, b''.join(fields)) + sigalg + sig)
+open(dst, 'wb').write(out)
+print("Created: %s (%d bytes), extension OID content %s"
+      % (dst, len(out), oid_content.hex(' ')))
+EOF

certs/test/include.am

@@ -31,6 +31,8 @@ EXTRA_DIST += \
          certs/test/cert-ext-ndir-exc.cfg \
          certs/test/cert-ext-ndir-exc.der \
          certs/test/cert-ext-ndir-exc.pem \
+         certs/test/cert-ext-oid-collide.der \
+         certs/test/gen-oid-collide-cert.sh \
          certs/test/gen-ext-certs.sh \
          certs/test/server-duplicate-policy.pem \
          certs/test/cert-ext-joi.der \

src/x509.c

@@ -1115,12 +1115,11 @@ WOLFSSL_X509_EXTENSION* wolfSSL_X509_set_ext(WOLFSSL_X509* x509, int loc)
             (ext->obj->obj == NULL)) {
             byte* tmp;
         #ifdef WOLFSSL_NO_REALLOC
+            /* Don't carry the old OID over: objSz is the mapped NID's
+             * canonical length, which can exceed the certificate's OID. The
+             * buffer is rewritten from oidBuf below. */
             tmp = (byte*)XMALLOC(objSz, NULL, DYNAMIC_TYPE_ASN1);
-            if (tmp != NULL && ext->obj->obj != NULL) {
-                XMEMCPY(tmp, ext->obj->obj, ext->obj->objSz);
-                XFREE((byte*)ext->obj->obj, NULL, DYNAMIC_TYPE_ASN1);
-            }
-            else if (tmp == NULL) {
+            if (ext->obj->obj != NULL) {
                 XFREE((byte*)ext->obj->obj, NULL, DYNAMIC_TYPE_ASN1);
             }
             ext->obj->obj = tmp;

tests/api/test_evp_pkey.c

@@ -3793,3 +3793,48 @@ int test_wolfSSL_d2i_PrivateKey_reuse_resets_state(void)
 #endif
     return EXPECT_RESULT();
 }
+
+/* wolfSSL_CTX_use_PrivateKey() re-encodes the key already held by the EVP_PKEY
+ * through PopulateRSAEvpPkeyDer(), which sizes its buffer from the unwrapped
+ * PKCS#1 encoding while a PKCS#8 loaded key's cached DER carries the wrapper.
+ * Under WOLFSSL_NO_REALLOC the old contents were copied into that smaller
+ * buffer, so one call on a wrapped key overran it, with no key replacement.
+ *
+ * test_wolfSSL_EVP_PKEY_set1_shrinking_der() covers the same sink through
+ * EVP_PKEY_set1_*; this covers the only other caller. The overrun needs a
+ * sanitizer on a WOLFSSL_NO_REALLOC build to see; the sizes below hold either
+ * way. WOLFSSL_KEY_GEN is required. */
+int test_wolfSSL_CTX_use_PrivateKey_pkcs8_repopulate(void)
+{
+    EXPECT_DECLS;
+#if defined(OPENSSL_EXTRA) && defined(WOLFSSL_KEY_TO_DER) && \
+    defined(WOLFSSL_KEY_GEN) && defined(HAVE_PKCS8) && !defined(NO_RSA) && \
+    !defined(NO_FILESYSTEM) && !defined(NO_CERTS) && !defined(NO_TLS) && \
+    !defined(NO_WOLFSSL_CLIENT)
+    WOLFSSL_CTX*      ctx = NULL;
+    WOLFSSL_EVP_PKEY* pkey = NULL;
+    const unsigned char* in;
+    byte*  buf = NULL;
+    size_t bufSz = 0;
+    int    wrappedSz = 0;
+
+    ExpectIntEQ(load_file("./certs/server-keyPkcs8.der", &buf, &bufSz), 0);
+    in = buf;
+    ExpectNotNull(pkey = wolfSSL_d2i_PrivateKey(EVP_PKEY_RSA, NULL, &in,
+        (long)bufSz));
+    /* Premise: without the wrapper the re-encode is the same size and the
+     * shrink is never exercised. */
+    ExpectIntGT((int)pkey->pkcs8HeaderSz, 0);
+    ExpectIntGT(wrappedSz = wolfSSL_i2d_PrivateKey(pkey, NULL), 0);
+
+    ExpectNotNull(ctx = wolfSSL_CTX_new(wolfSSLv23_client_method()));
+    ExpectIntEQ(wolfSSL_CTX_use_PrivateKey(ctx, pkey), WOLFSSL_SUCCESS);
+    /* Exact: a dropped or stale wrapper shows up as a size mismatch. */
+    ExpectIntEQ(wolfSSL_i2d_PrivateKey(pkey, NULL), wrappedSz);
+
+    XFREE(buf, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+    wolfSSL_CTX_free(ctx);
+    wolfSSL_EVP_PKEY_free(pkey);
+#endif
+    return EXPECT_RESULT();
+}

tests/api/test_evp_pkey.h

@@ -77,6 +77,7 @@ int test_wolfSSL_EVP_PKEY_x25519(void);
 int test_wolfSSL_EVP_PKEY_x448(void);
 int test_wolfSSL_EVP_PKEY_encoded_public_key(void);
 int test_wolfSSL_d2i_PrivateKey_reuse_resets_state(void);
+int test_wolfSSL_CTX_use_PrivateKey_pkcs8_repopulate(void);
 
 #define TEST_EVP_PKEY_DECLS                                                    \
     TEST_DECL_GROUP("evp_pkey", test_wolfSSL_EVP_PKEY_CTX_new_id),             \
@@ -132,6 +133,8 @@ int test_wolfSSL_d2i_PrivateKey_reuse_resets_state(void);
     TEST_DECL_GROUP("evp_pkey", test_wolfSSL_EVP_PKEY_x25519),                 \
     TEST_DECL_GROUP("evp_pkey", test_wolfSSL_EVP_PKEY_x448),                   \
     TEST_DECL_GROUP("evp_pkey", test_wolfSSL_EVP_PKEY_encoded_public_key),     \
-    TEST_DECL_GROUP("evp_pkey", test_wolfSSL_d2i_PrivateKey_reuse_resets_state)
+    TEST_DECL_GROUP("evp_pkey", test_wolfSSL_d2i_PrivateKey_reuse_resets_state),\
+    TEST_DECL_GROUP("evp_pkey",                                                \
+        test_wolfSSL_CTX_use_PrivateKey_pkcs8_repopulate)
 
 #endif /* WOLFCRYPT_TEST_EVP_PKEY_H */

tests/api/test_ossl_x509_ext.c

@@ -2685,3 +2685,49 @@ int test_wolfSSL_NAME_CONSTRAINTS_excluded(void)
         * !IGNORE_NAME_CONSTRAINTS */
     return EXPECT_RESULT();
 }
+
+int test_wolfSSL_X509_set_ext_oid_collision(void)
+{
+    EXPECT_DECLS;
+/* The fixture's OID collides under wc_oid_sum() only. With WOLFSSL_OLD_OID_SUM
+ * it maps elsewhere, no canonical OID is cached, and there is nothing to
+ * shrink. */
+#if defined(OPENSSL_EXTRA) && !defined(NO_FILESYSTEM) && !defined(NO_CERTS) && \
+    defined(HAVE_ECC) && !defined(WOLFSSL_OLD_OID_SUM)
+    /* The fixture's extension OID, tag and length included. */
+    static const unsigned char certOid[] = {
+        0x06, 0x04, 0xE8, 0x85, 0xB6, 0x49
+    };
+    X509* x509 = NULL;
+    int count = 0;
+    int i;
+
+    ExpectNotNull(x509 = wolfSSL_X509_load_certificate_file(
+        "./certs/test/cert-ext-oid-collide.der", WOLFSSL_FILETYPE_ASN1));
+    ExpectIntEQ((count = X509_get_ext_count(x509)), 1);
+
+    for (i = 0; i < count; i++) {
+        X509_EXTENSION* ext = NULL;
+        ASN1_OBJECT* obj = NULL;
+        ASN1_OBJECT* canonical = NULL;
+
+        ExpectNotNull(ext = X509_get_ext(x509, i));
+        ExpectNotNull(obj = X509_EXTENSION_get_object(ext));
+
+        /* Must be the certificate's OID, byte for byte. */
+        ExpectIntEQ((int)obj->objSz, (int)sizeof(certOid));
+        ExpectBufEQ(obj->obj, certOid, sizeof(certOid));
+
+        /* Premise: the mapped NID's canonical OID is still longer, so the
+         * shrink is exercised. If this fails, regenerate the fixture with
+         * certs/test/gen-oid-collide-cert.sh. */
+        ExpectNotNull(canonical = wolfSSL_OBJ_nid2obj(obj->nid));
+        ExpectIntGT((int)canonical->objSz, (int)obj->objSz);
+        ASN1_OBJECT_free(canonical);
+    }
+
+    X509_free(x509);
+#endif /* OPENSSL_EXTRA && !NO_FILESYSTEM && !NO_CERTS && HAVE_ECC &&
+        * !WOLFSSL_OLD_OID_SUM */
+    return EXPECT_RESULT();
+}

tests/api/test_ossl_x509_ext.h

@@ -58,6 +58,7 @@ int test_wolfSSL_NAME_CONSTRAINTS_check_name(void);
 int test_wolfSSL_NAME_CONSTRAINTS_manual_paths(void);
 int test_wolfSSL_NAME_CONSTRAINTS_dns(void);
 int test_wolfSSL_NAME_CONSTRAINTS_excluded(void);
+int test_wolfSSL_X509_set_ext_oid_collision(void);
 
 #define TEST_OSSL_X509_EXT_DECLS                                               \
     TEST_DECL_GROUP("ossl_x509_ext", test_wolfSSL_X509_get_extension_flags),   \
@@ -98,6 +99,8 @@ int test_wolfSSL_NAME_CONSTRAINTS_excluded(void);
     TEST_DECL_GROUP("ossl_x509_ext", test_wolfSSL_NAME_CONSTRAINTS_check_name),\
     TEST_DECL_GROUP("ossl_x509_ext", test_wolfSSL_NAME_CONSTRAINTS_manual_paths),\
     TEST_DECL_GROUP("ossl_x509_ext", test_wolfSSL_NAME_CONSTRAINTS_dns),       \
-    TEST_DECL_GROUP("ossl_x509_ext", test_wolfSSL_NAME_CONSTRAINTS_excluded)
+    TEST_DECL_GROUP("ossl_x509_ext", test_wolfSSL_NAME_CONSTRAINTS_excluded),  \
+    TEST_DECL_GROUP("ossl_x509_ext",                                           \
+                                  test_wolfSSL_X509_set_ext_oid_collision)
 
 #endif /* WOLFCRYPT_TEST_OSSL_X509_EXT_H */