Remove carry over of old buffers/sizes in EVP and X509 ext functions.

certs/test/catalog.txt

@@ -12,3 +12,9 @@ digsigku.pem:
   set.
 cert-ext-joi.pem:
   Simple certificate that includes OIDs for JurisdictionOfIncorporation
+cert-ext-oid-collide.der:
+  Certificate carrying a non-critical extension whose 4-byte OID hashes to the
+  same wc_oid_sum() as prime256v1, so it maps to a NID whose canonical OID is
+  longer. Used to test that wolfSSL_X509_set_ext() sizes the ASN1_OBJECT
+  buffer from the certificate's OID rather than the mapped one. The signature
+  is not valid; the certificate is only ever parsed, never verified.

certs/test/include.am

@@ -31,6 +31,7 @@ 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-ext-certs.sh \
          certs/test/server-duplicate-policy.pem \
          certs/test/cert-ext-joi.der \

src/x509.c

@@ -1115,12 +1115,12 @@ 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: the buffer is rewritten in full
+             * from oidBuf below, and ext->obj->objSz is the length of the
+             * canonical OID that OBJ_nid2obj() produced for the mapped NID,
+             * which may be longer than the OID in the certificate. */
             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.c

@@ -24,6 +24,7 @@
 #include <wolfssl/wolfcrypt/error-crypt.h>
 
 #include <wolfssl/openssl/evp.h>
+#include <wolfssl/openssl/pem.h>
 #include <tests/api/test_evp.h>
 
 /* Test functions for base64 encode/decode */

tests/api/test_evp_pkey.c

@@ -30,6 +30,7 @@
 
 #include <wolfssl/openssl/evp.h>
 #include <wolfssl/openssl/kdf.h>
+#include <wolfssl/openssl/pem.h>
 #include <tests/api/api.h>
 #include <tests/api/test_evp_pkey.h>
 
@@ -3112,3 +3113,126 @@ int test_wolfSSL_EVP_PKEY_encoded_public_key(void)
     return EXPECT_RESULT();
 }
 
+
+
+int test_wolfSSL_EVP_PKEY_populate_shrink(void)
+{
+    EXPECT_DECLS;
+#if defined(OPENSSL_EXTRA) && defined(WOLFSSL_KEY_TO_DER) && \
+    !defined(NO_FILESYSTEM) && !defined(NO_BIO)
+#ifndef NO_RSA
+    {
+        /* 2048-bit key first, then a 1024-bit one into the same EVP_PKEY. */
+        EVP_PKEY* pkey = NULL;
+        RSA* big = NULL;
+        RSA* small = NULL;
+        BIO* bio = NULL;
+        int bigSz = 0;
+
+        ExpectNotNull(bio = BIO_new_file("./certs/server-key.pem", "rb"));
+        ExpectNotNull(big = PEM_read_bio_RSAPrivateKey(bio, NULL, NULL, NULL));
+        BIO_free(bio);
+        bio = NULL;
+        ExpectNotNull(bio = BIO_new_file("./certs/1024/server-key.pem", "rb"));
+        ExpectNotNull(small = PEM_read_bio_RSAPrivateKey(bio, NULL, NULL,
+            NULL));
+        BIO_free(bio);
+        bio = NULL;
+
+        ExpectNotNull(pkey = EVP_PKEY_new());
+        ExpectIntEQ(EVP_PKEY_set1_RSA(pkey, big), 1);
+        if (pkey != NULL)
+            bigSz = pkey->pkey_sz;
+        ExpectIntGT(bigSz, 0);
+
+        ExpectIntEQ(EVP_PKEY_set1_RSA(pkey, small), 1);
+        /* The cached DER must now describe the smaller key. */
+        if (pkey != NULL) {
+            ExpectIntGT(pkey->pkey_sz, 0);
+            ExpectIntLT(pkey->pkey_sz, bigSz);
+        }
+
+        RSA_free(big);
+        RSA_free(small);
+        EVP_PKEY_free(pkey);
+    }
+#if defined(WOLFSSL_KEY_GEN) && !defined(NO_TLS) && !defined(NO_WOLFSSL_CLIENT)
+    {
+        /* A PKCS#8-wrapped key: pkey_sz includes the wrapper, while
+         * PopulateRSAEvpPkeyDer() sizes its buffer from the unwrapped key. */
+        EVP_PKEY* pkey = NULL;
+        WOLFSSL_CTX* ctx = NULL;
+        BIO* bio = NULL;
+
+        ExpectNotNull(bio = BIO_new_file("./certs/server-keyPkcs8.pem", "rb"));
+        ExpectNotNull(pkey = PEM_read_bio_PrivateKey(bio, NULL, NULL, NULL));
+        ExpectNotNull(ctx = wolfSSL_CTX_new(wolfSSLv23_client_method()));
+        ExpectIntEQ(wolfSSL_CTX_use_PrivateKey(ctx, pkey), 1);
+        ExpectIntGT(pkey == NULL ? 0 : pkey->pkey_sz, 0);
+
+        wolfSSL_CTX_free(ctx);
+        EVP_PKEY_free(pkey);
+        BIO_free(bio);
+    }
+#endif /* WOLFSSL_KEY_GEN && !NO_TLS && !NO_WOLFSSL_CLIENT */
+#endif /* !NO_RSA */
+#if defined(HAVE_ECC) && \
+    (defined(HAVE_ECC521) || defined(HAVE_ALL_CURVES)) && \
+    ECC_MIN_KEY_SZ <= 521 && \
+    (defined(HAVE_ECC256) || defined(HAVE_ALL_CURVES))
+    {
+        /* A public-only EC key takes the ECC_PUBLICKEY branch.  The P-521
+         * private DER is longer than a P-256 SubjectPublicKeyInfo. */
+        EVP_PKEY* pkey = NULL;
+        EC_KEY* big = NULL;
+        EC_KEY* small = NULL;
+        unsigned char* der = NULL;
+        unsigned char* p;
+        const unsigned char* q;
+        int derSz = 0;
+        int bigSz = 0;
+
+        ExpectNotNull(pkey = EVP_PKEY_new());
+        ExpectNotNull(big = EC_KEY_new_by_curve_name(NID_secp521r1));
+        ExpectIntEQ(EC_KEY_generate_key(big), 1);
+        ExpectIntEQ(EVP_PKEY_set1_EC_KEY(pkey, big), 1);
+        if (pkey != NULL)
+            bigSz = pkey->pkey_sz;
+        ExpectIntGT(bigSz, 0);
+
+        /* Build a public-only P-256 key by round-tripping the point. */
+        ExpectNotNull(small = EC_KEY_new_by_curve_name(NID_X9_62_prime256v1));
+        ExpectIntEQ(EC_KEY_generate_key(small), 1);
+        ExpectIntGT((derSz = wolfSSL_i2o_ECPublicKey(small, NULL)), 0);
+        ExpectNotNull(der = (unsigned char*)XMALLOC((size_t)derSz, NULL,
+            DYNAMIC_TYPE_TMP_BUFFER));
+        if (der != NULL) {
+            p = der;
+            ExpectIntEQ(wolfSSL_i2o_ECPublicKey(small, &p), derSz);
+        }
+        EC_KEY_free(small);
+        small = NULL;
+        ExpectNotNull(small = EC_KEY_new_by_curve_name(NID_X9_62_prime256v1));
+        if (der != NULL) {
+            q = der;
+            ExpectNotNull(wolfSSL_o2i_ECPublicKey(&small, &q, derSz));
+        }
+        /* Push the decoded point into the internal key so that it is typed
+         * ECC_PUBLICKEY, which is the branch under test. */
+        ExpectIntEQ(EC_KEY_check_key(small), 1);
+
+        ExpectIntEQ(EVP_PKEY_set1_EC_KEY(pkey, small), 1);
+        if (pkey != NULL) {
+            ExpectIntGT(pkey->pkey_sz, 0);
+            ExpectIntLT(pkey->pkey_sz, bigSz);
+        }
+
+        XFREE(der, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+        EC_KEY_free(big);
+        EC_KEY_free(small);
+        EVP_PKEY_free(pkey);
+    }
+#endif /* HAVE_ECC && P-521 && P-256 */
+#endif /* OPENSSL_EXTRA && WOLFSSL_KEY_TO_DER && !NO_FILESYSTEM && !NO_BIO */
+    return EXPECT_RESULT();
+}

tests/api/test_evp_pkey.h

@@ -69,6 +69,7 @@ int test_wolfSSL_EVP_PKEY_ed448(void);
 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_EVP_PKEY_populate_shrink(void);
 
 #define TEST_EVP_PKEY_DECLS                                                    \
     TEST_DECL_GROUP("evp_pkey", test_wolfSSL_EVP_PKEY_CTX_new_id),             \
@@ -116,6 +117,7 @@ int test_wolfSSL_EVP_PKEY_encoded_public_key(void);
     TEST_DECL_GROUP("evp_pkey", test_wolfSSL_EVP_PKEY_ed448),                  \
     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_EVP_PKEY_encoded_public_key),     \
+    TEST_DECL_GROUP("evp_pkey", test_wolfSSL_EVP_PKEY_populate_shrink)
 
 #endif /* WOLFCRYPT_TEST_EVP_PKEY_H */

tests/api/test_ossl_x509_ext.c

@@ -2639,3 +2639,33 @@ int test_wolfSSL_NAME_CONSTRAINTS_excluded(void)
         * !IGNORE_NAME_CONSTRAINTS */
     return EXPECT_RESULT();
 }
+
+int test_wolfSSL_X509_set_ext_oid_collision(void)
+{
+    EXPECT_DECLS;
+#if defined(OPENSSL_EXTRA) && !defined(NO_FILESYSTEM) && !defined(NO_CERTS) && \
+    defined(HAVE_ECC)
+    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));
+    ExpectIntGT((count = X509_get_ext_count(x509)), 0);
+
+    /* The returned object's OID has to be the one in the certificate, not
+     * the longer canonical OID of the NID it happens to hash to. */
+    for (i = 0; i < count; i++) {
+        X509_EXTENSION* ext = NULL;
+        ASN1_OBJECT* obj = NULL;
+
+        ExpectNotNull(ext = X509_get_ext(x509, i));
+        ExpectNotNull(obj = X509_EXTENSION_get_object(ext));
+        /* 4 OID content bytes plus tag and length. */
+        ExpectIntEQ(obj == NULL ? -1 : (int)obj->objSz, 6);
+    }
+
+    X509_free(x509);
+#endif /* OPENSSL_EXTRA && !NO_FILESYSTEM && !NO_CERTS && HAVE_ECC */
+    return EXPECT_RESULT();
+}

tests/api/test_ossl_x509_ext.h

@@ -57,6 +57,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),   \
@@ -96,6 +97,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 */

wolfcrypt/src/evp.c

@@ -9282,9 +9282,13 @@ static int PopulateRSAEvpPkeyDer(WOLFSSL_EVP_PKEY *pkey)
 #ifdef WOLFSSL_NO_REALLOC
     derBuf = (byte*)XMALLOC((size_t)derSz, pkey->heap, DYNAMIC_TYPE_DER);
     if (derBuf != NULL) {
-        XMEMCPY(derBuf, pkey->pkey.ptr, (size_t)pkey->pkey_sz);
-        XFREE(pkey->pkey.ptr, pkey->heap, DYNAMIC_TYPE_DER);
-        pkey->pkey.ptr = NULL;
+        /* Don't carry the cached DER over: the whole buffer is re-serialized
+         * below, and pkey_sz describes the previous key, which may be longer
+         * than derSz. */
+        if (pkey->pkey.ptr != NULL) {
+            XFREE(pkey->pkey.ptr, pkey->heap, DYNAMIC_TYPE_DER);
+            pkey->pkey.ptr = NULL;
+        }
     }
 #else
     derBuf = (byte*)XREALLOC(pkey->pkey.ptr, (size_t)derSz,
@@ -9297,6 +9301,9 @@ static int PopulateRSAEvpPkeyDer(WOLFSSL_EVP_PKEY *pkey)
 
     /* Old pointer is invalid from this point on */
     pkey->pkey.ptr = (char*)derBuf;
+    /* pkey_sz still describes the old buffer; clear it so an error below
+     * cannot leave a length that overruns the new one. */
+    pkey->pkey_sz = 0;
 
     if (rsa->type == RSA_PRIVATE) {
         ret = wc_RsaKeyToDer(rsa, derBuf, (word32)derSz);
@@ -9840,25 +9847,17 @@ static int ECC_populate_EVP_PKEY(WOLFSSL_EVP_PKEY* pkey, WOLFSSL_EC_KEY *key)
     }
     else if (ecc->type == ECC_PUBLICKEY) {
         if ((derSz = wc_EccPublicKeyDerSize(ecc, 1)) > 0) {
-        #ifdef WOLFSSL_NO_REALLOC
-            derBuf = (byte*)XMALLOC((size_t)derSz, pkey->heap, DYNAMIC_TYPE_OPENSSL);
-            if (derBuf != NULL) {
-                XMEMCPY(derBuf, pkey->pkey.ptr, (size_t)pkey->pkey_sz);
-                XFREE(pkey->pkey.ptr, pkey->heap, DYNAMIC_TYPE_OPENSSL);
-                pkey->pkey.ptr = NULL;
-            }
-        #else
-            derBuf = (byte*)XREALLOC(pkey->pkey.ptr, (size_t)derSz, pkey->heap,
-                    DYNAMIC_TYPE_OPENSSL);
-        #endif
+            derBuf = (byte*)XMALLOC((size_t)derSz, pkey->heap,
+                DYNAMIC_TYPE_OPENSSL);
             if (derBuf != NULL) {
-                pkey->pkey.ptr = (char*)derBuf;
                 if ((derSz = wc_EccPublicKeyToDer(ecc, derBuf, (word32)derSz,
                         1)) < 0) {
                     XFREE(derBuf, pkey->heap, DYNAMIC_TYPE_OPENSSL);
                     derBuf = NULL;
-                    pkey->pkey.ptr = NULL;
-                    pkey->pkey_sz = 0;
+                }
+                else {
+                    XFREE(pkey->pkey.ptr, pkey->heap, DYNAMIC_TYPE_OPENSSL);
+                    pkey->pkey.ptr = (char*)derBuf;
                 }
             }
         }