X509: cache decoded public key, give get0_pubkey borrowed semantics

ChangeLog.md

@@ -84,6 +84,14 @@
   compile error; the ATECC508A/608A gating is identical, and on the TA100 the
   helper was declared without a definition, so that build failed at link.
 
+* **Behavioral change (`X509_get0_pubkey()` returns a borrowed key)**:
+  `wolfSSL_X509_get_pubkey()` now returns the public key cached on the
+  certificate with a new reference, and the new `wolfSSL_X509_get0_pubkey()`
+  (`X509_get0_pubkey()`, `X509_REQ_get0_pubkey()`) returns it without one, as
+  in OpenSSL.  Code that freed the result of `X509_get0_pubkey()` to avoid a
+  leak must stop doing so, and the returned key is shared with the
+  certificate, so it must be treated as read only.
+
 ## Post-Quantum Cryptography (PQC)
 
 * Added opt-in per-key Falcon signing caches (`--enable-falcon=cache-key`, `cache-basis`), roughly doubling signing speed with the default integer fpr backend. by @Frauschi

doc/dox_comments/header_files/ssl.h

@@ -5425,6 +5425,92 @@ long wolfSSL_BIO_get_mem_ptr(WOLFSSL_BIO *bio, WOLFSSL_BUF_MEM **m);
 */
 char*       wolfSSL_X509_NAME_oneline(WOLFSSL_X509_NAME* name, char* in, int sz);
 
+/*!
+    \ingroup CertsKeys
+
+    \brief This function returns the public key of the certificate with a new
+    reference. The key is decoded once and cached on the certificate, so the
+    same object is returned on every call. Treat it as read only. Free the
+    reference with wolfSSL_EVP_PKEY_free().
+
+    \return pointer to the WOLFSSL_EVP_PKEY on success.
+    \return NULL if x509 is NULL, has no public key, or the key cannot be
+    decoded.
+
+    \param x509 pointer to a WOLFSSL_X509 structure.
+
+    _Example_
+    \code
+    WOLFSSL_X509* x509;
+    WOLFSSL_EVP_PKEY* key;
+    ...
+    key = wolfSSL_X509_get_pubkey(x509);
+    if (key == NULL) {
+        // failed to get the public key
+    }
+    ...
+    wolfSSL_EVP_PKEY_free(key);
+    \endcode
+
+    \sa wolfSSL_X509_get0_pubkey
+    \sa wolfSSL_EVP_PKEY_free
+*/
+WOLFSSL_EVP_PKEY* wolfSSL_X509_get_pubkey(WOLFSSL_X509* x509);
+
+/*!
+    \ingroup CertsKeys
+
+    \brief This function returns the public key of the certificate without a
+    new reference. The key is owned by the certificate and must not be freed.
+    It stays valid until the certificate is freed or its public key is
+    changed. Treat it as read only.
+
+    \return pointer to the WOLFSSL_EVP_PKEY on success.
+    \return NULL if x509 is NULL, has no public key, or the key cannot be
+    decoded.
+
+    \param x509 pointer to a WOLFSSL_X509 structure.
+
+    _Example_
+    \code
+    WOLFSSL_X509* x509;
+    WOLFSSL_EVP_PKEY* key;
+    ...
+    key = wolfSSL_X509_get0_pubkey(x509);
+    if (key == NULL) {
+        // failed to get the public key
+    }
+    // do not free key
+    \endcode
+
+    \sa wolfSSL_X509_get_pubkey
+*/
+WOLFSSL_EVP_PKEY* wolfSSL_X509_get0_pubkey(const WOLFSSL_X509* x509);
+
+/*!
+    \ingroup CertsKeys
+
+    \brief This function returns the key held by a WOLFSSL_X509_PUBKEY without
+    a new reference. The key must not be freed.
+
+    \return pointer to the WOLFSSL_EVP_PKEY on success.
+    \return NULL if key is NULL or holds no key.
+
+    \param key pointer to a WOLFSSL_X509_PUBKEY structure.
+
+    _Example_
+    \code
+    WOLFSSL_X509* x509;
+    WOLFSSL_EVP_PKEY* pkey;
+    ...
+    pkey = wolfSSL_X509_PUBKEY_get0(wolfSSL_X509_get_X509_PUBKEY(x509));
+    \endcode
+
+    \sa wolfSSL_X509_PUBKEY_get
+    \sa wolfSSL_X509_get_X509_PUBKEY
+*/
+WOLFSSL_EVP_PKEY* wolfSSL_X509_PUBKEY_get0(const WOLFSSL_X509_PUBKEY* key);
+
 /*!
     \ingroup CertsKeys
 

src/internal.c

@@ -5828,19 +5828,27 @@ static void FreeX509Contents(WOLFSSL_X509* x509)
             wolfSSL_ASN1_OBJECT_free(x509->algor.algorithm);
             x509->algor.algorithm = NULL;
         }
+        if (x509->subjAltNameSrc != NULL) {
+            XFREE(x509->subjAltNameSrc, x509->heap, DYNAMIC_TYPE_X509_EXT);
+            x509->subjAltNameSrc= NULL;
+        }
+    #endif /* OPENSSL_ALL */
+    #if (defined(OPENSSL_EXTRA) || defined(OPENSSL_EXTRA_X509_SMALL)) && \
+        (defined(OPENSSL_ALL) || defined(OPENSSL_EXTRA) || \
+         defined(WOLFSSL_APACHE_HTTPD) || defined(WOLFSSL_HAPROXY) || \
+         defined(WOLFSSL_WPAS))
         if (x509->key.algor) {
             wolfSSL_X509_ALGOR_free(x509->key.algor);
             x509->key.algor = NULL;
         }
+    #endif
+    #ifdef OPENSSL_EXTRA_X509_SMALL
+        /* Public key cached by wolfSSL_X509_get_pubkey() and friends. */
         if (x509->key.pkey) {
             wolfSSL_EVP_PKEY_free(x509->key.pkey);
             x509->key.pkey = NULL;
         }
-        if (x509->subjAltNameSrc != NULL) {
-            XFREE(x509->subjAltNameSrc, x509->heap, DYNAMIC_TYPE_X509_EXT);
-            x509->subjAltNameSrc= NULL;
-        }
-    #endif /* OPENSSL_ALL */
+    #endif
     #if defined(WOLFSSL_CERT_REQ) && defined(OPENSSL_ALL)
         if (x509->reqAttributes) {
             wolfSSL_sk_pop_free(x509->reqAttributes, NULL);
@@ -15615,6 +15623,11 @@ static void CopyDecodedSepFields(WOLFSSL_X509* x509, DecodedCert* dCert)
  * error, matching the original. */
 static int CopyDecodedPubKey(WOLFSSL_X509* x509, DecodedCert* dCert, int ret)
 {
+#ifdef OPENSSL_EXTRA_X509_SMALL
+    /* Drop the key decoded from a previous public key. */
+    wolfSSL_EVP_PKEY_free(x509->key.pkey);
+    x509->key.pkey = NULL;
+#endif
     if (dCert->publicKey != NULL && dCert->pubKeySize != 0) {
         x509->pubKey.buffer = (byte*)XMALLOC(
                         dCert->pubKeySize, x509->heap, DYNAMIC_TYPE_PUBLIC_KEY);
@@ -15644,9 +15657,6 @@ static int CopyDecodedPubKey(WOLFSSL_X509* x509, DecodedCert* dCert, int ret)
                 }
             }
 
-            wolfSSL_EVP_PKEY_free(x509->key.pkey);
-            x509->key.pkey = NULL;
-
             switch (dCert->keyOID) {
             #ifdef HAVE_ED25519
                 case ED25519k:
@@ -15675,8 +15685,9 @@ static int CopyDecodedPubKey(WOLFSSL_X509* x509, DecodedCert* dCert, int ret)
                 case ML_DSA_65k:
                 case ML_DSA_87k:
                     /* Raw key size does not distinguish draft from final, so
-                     * encode with the certificate's key OID */
-                    x509->key.pkey = wolfSSL_X509_get_pubkey(x509);
+                     * encode with the certificate's key OID. This fills
+                     * the x509->key.pkey cache without a new reference. */
+                    (void)wolfSSL_X509_get0_pubkey(x509);
                     break;
             #endif
                 default:

src/pk_ec.c

@@ -4416,6 +4416,11 @@ int SetECKeyExternal(WOLFSSL_EC_KEY* eckey)
                 WOLFSSL_MSG("SetECKeyExternal ec_point_external_set failed");
                 ret = WOLFSSL_FATAL_ERROR;
             }
+            /* Both sides of the point match now, so readers of a shared key
+             * do not rebuild the internal one. */
+            if (ret == 1) {
+                eckey->pub_key->inSet = 1;
+            }
         }
 
         /* set the external privkey */

src/x509.c

@@ -6781,19 +6781,19 @@ int wolfSSL_X509_NAME_get_text_by_NID(WOLFSSL_X509_NAME* name,
     return (textSz - 1); /* do not include null character in size */
 }
 
-/* Creates a new WOLFSSL_EVP_PKEY structure that has the public key from x509
+/* Decodes the public key of x509 into a new WOLFSSL_EVP_PKEY.
  *
  * returns a pointer to the created WOLFSSL_EVP_PKEY on success and NULL on fail
  */
-WOLFSSL_EVP_PKEY* wolfSSL_X509_get_pubkey(WOLFSSL_X509* x509)
+static WOLFSSL_EVP_PKEY* X509DecodePubKey(WOLFSSL_X509* x509)
 {
     WOLFSSL_EVP_PKEY* key = NULL;
     int ret = 0;
 
     (void)ret;
 
-    WOLFSSL_ENTER("wolfSSL_X509_get_pubkey");
-    if (x509 != NULL) {
+    if (x509 != NULL && x509->pubKey.buffer != NULL &&
+            x509->pubKey.length > 0) {
         key = wolfSSL_EVP_PKEY_new_ex(x509->heap);
         if (key != NULL) {
             if (x509->pubKeyOID == RSAk) {
@@ -6963,6 +6963,60 @@ WOLFSSL_EVP_PKEY* wolfSSL_X509_get_pubkey(WOLFSSL_X509* x509)
     }
     return key;
 }
+
+/* Returns the public key cached in x509, decoding it on first use.
+ * The key is owned by x509 and freed with it. */
+static WOLFSSL_EVP_PKEY* X509CachedPubKey(WOLFSSL_X509* x509)
+{
+    WOLFSSL_EVP_PKEY* key;
+
+    if (x509 == NULL)
+        return NULL;
+    key = x509->key.pkey;
+    if (key == NULL) {
+        key = X509DecodePubKey(x509);
+        if (key != NULL) {
+            x509->key.pubKeyOID = x509->pubKeyOID;
+            x509->key.pkey = key;
+        }
+    }
+    return key;
+}
+
+/* Returns the public key of x509 with a new reference.
+ *
+ * returns a pointer to the WOLFSSL_EVP_PKEY on success and NULL on fail.
+ * The caller frees it with wolfSSL_EVP_PKEY_free().
+ */
+WOLFSSL_EVP_PKEY* wolfSSL_X509_get_pubkey(WOLFSSL_X509* x509)
+{
+    WOLFSSL_EVP_PKEY* key;
+
+    WOLFSSL_ENTER("wolfSSL_X509_get_pubkey");
+    key = X509CachedPubKey(x509);
+    if (key != NULL) {
+        int ret;
+        wolfSSL_RefInc(&key->ref, &ret);
+        if (ret != 0) {
+            WOLFSSL_MSG("Failed to lock pkey mutex");
+            key = NULL;
+        }
+    }
+    return key;
+}
+
+/* Returns the public key of x509 without a new reference.
+ *
+ * returns a pointer to the WOLFSSL_EVP_PKEY on success and NULL on fail.
+ * The key must not be freed. It is valid until x509 is freed or its public
+ * key is changed.
+ */
+WOLFSSL_EVP_PKEY* wolfSSL_X509_get0_pubkey(const WOLFSSL_X509* x509)
+{
+    WOLFSSL_ENTER("wolfSSL_X509_get0_pubkey");
+    /* Only the lazily decoded key cache is written. */
+    return X509CachedPubKey((WOLFSSL_X509*)x509);
+}
 #endif /* OPENSSL_EXTRA_X509_SMALL */
 
 /* End of smaller subset of X509 compatibility functions. Avoid increasing the
@@ -12200,6 +12254,10 @@ WOLFSSL_X509_PUBKEY* wolfSSL_X509_get_X509_PUBKEY(const WOLFSSL_X509* x509)
         return NULL;
     }
 
+    /* OpenSSL also takes a const X509 and returns a mutable X509_PUBKEY.
+     * Only the lazily decoded key cache is written here. */
+    (void)X509CachedPubKey((WOLFSSL_X509*)x509);
+
     return (WOLFSSL_X509_PUBKEY*)&x509->key;
 }
 
@@ -12260,16 +12318,26 @@ int wolfSSL_X509_PUBKEY_get0_param(WOLFSSL_ASN1_OBJECT **ppkalg,
         WOLFSSL_MSG("X509_PUBKEY struct not populated");
         return WOLFSSL_FAILURE;
     }
+    if ((pk || ppklen) && !pub->pkey) {
+        WOLFSSL_MSG("X509_PUBKEY has no decoded key");
+        return WOLFSSL_FAILURE;
+    }
 
     if (!pub->algor) {
-        if (!(pub->algor = wolfSSL_X509_ALGOR_new())) {
+        /* Build the algorithm fully before storing it, so a failure
+         * never leaves a half built one behind. */
+        WOLFSSL_X509_ALGOR* algor = wolfSSL_X509_ALGOR_new();
+        if (algor == NULL) {
             return WOLFSSL_FAILURE;
         }
-        pub->algor->algorithm = wolfSSL_OBJ_nid2obj(pub->pubKeyOID);
-        if (pub->algor->algorithm == NULL) {
+        algor->algorithm = wolfSSL_OBJ_nid2obj(
+            oid2nid((word32)pub->pubKeyOID, oidKeyType));
+        if (algor->algorithm == NULL) {
             WOLFSSL_MSG("Failed to create object from NID");
+            wolfSSL_X509_ALGOR_free(algor);
             return WOLFSSL_FAILURE;
         }
+        pub->algor = algor;
     }
 
     if (pa)
@@ -12312,6 +12380,15 @@ WOLFSSL_EVP_PKEY* wolfSSL_X509_PUBKEY_get(WOLFSSL_X509_PUBKEY* key)
     return key->pkey;
 }
 
+/* Returns the pkey without a new reference. */
+WOLFSSL_EVP_PKEY* wolfSSL_X509_PUBKEY_get0(const WOLFSSL_X509_PUBKEY* key)
+{
+    WOLFSSL_ENTER("wolfSSL_X509_PUBKEY_get0");
+    if (key == NULL)
+        return NULL;
+    return key->pkey;
+}
+
 int wolfSSL_X509_PUBKEY_set(WOLFSSL_X509_PUBKEY **x, WOLFSSL_EVP_PKEY *key)
 {
     WOLFSSL_X509_PUBKEY *pk = NULL;
@@ -17535,6 +17612,7 @@ int wolfSSL_X509_set_pubkey(WOLFSSL_X509 *cert, WOLFSSL_EVP_PKEY *pkey)
                 return WOLFSSL_FAILURE;
             }
             cert->pubKeyOID = ECDSAk;
+            cert->pkCurveOID = ecc->dp->oidSum;
         }
         break;
 #endif
@@ -17661,6 +17739,19 @@ int wolfSSL_X509_set_pubkey(WOLFSSL_X509 *cert, WOLFSSL_EVP_PKEY *pkey)
     XFREE(cert->pubKey.buffer, cert->heap, DYNAMIC_TYPE_PUBLIC_KEY);
     cert->pubKey.buffer = p;
     cert->pubKey.length = (unsigned int)derSz;
+    /* Drop what was decoded from the previous public key, unless the caller
+     * passed that very key. */
+    if (cert->key.pkey != pkey) {
+        wolfSSL_EVP_PKEY_free(cert->key.pkey);
+        cert->key.pkey = NULL;
+    }
+    cert->key.pubKeyOID = cert->pubKeyOID;
+#if defined(OPENSSL_ALL) || defined(OPENSSL_EXTRA) || \
+    defined(WOLFSSL_APACHE_HTTPD) || defined(WOLFSSL_HAPROXY) || \
+    defined(WOLFSSL_WPAS)
+    wolfSSL_X509_ALGOR_free(cert->key.algor);
+    cert->key.algor = NULL;
+#endif
 
     return WOLFSSL_SUCCESS;
 }