X509: cache decoded public key, give get0_pubkey borrowed semantics

ChangeLog.md

@@ -30,6 +30,14 @@
   --disable-tlsv12` and `--enable-dtls --enable-dtls13 --enable-dtlscid
   --enable-session-ticket --disable-tlsv12` now build and test cleanly.
 
+* **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.
+
 * **Behavioral change (`wc_PufReadSram` health tests the raw SRAM readout)**:
   the raw readout is now health tested before the context accepts it, and a
   readout that cannot be SRAM power-on noise is rejected with `PUF_READ_E`

doc/dox_comments/header_files/ssl.h

@@ -5295,6 +5295,91 @@ 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, stays valid for its
+    lifetime, and must not be freed. 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(WOLFSSL_X509_PUBKEY* key);
+
 /*!
     \ingroup CertsKeys
 

src/internal.c

@@ -5484,19 +5484,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);
@@ -15249,6 +15257,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);
@@ -15278,9 +15291,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:

src/pk_ec.c

@@ -4383,6 +4383,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

@@ -6397,19 +6397,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) {
@@ -6554,6 +6554,75 @@ 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. Concurrent first calls each
+ * decode; the first to publish wins and the others free their copy.
+ */
+static WOLFSSL_EVP_PKEY* X509CachedPubKey(WOLFSSL_X509* x509)
+{
+    WOLFSSL_EVP_PKEY* key;
+#ifdef WOLFSSL_ATOMIC_OPS
+    WOLFSSL_EVP_PKEY* current = NULL;
+#endif
+
+    if (x509 == NULL)
+        return NULL;
+    key = x509->key.pkey;
+    if (key == NULL) {
+        key = X509DecodePubKey(x509);
+        if (key != NULL) {
+            /* Set before publishing the pointer, so a reader that sees the
+             * key never sees an OID of 0 next to it. */
+            x509->key.pubKeyOID = x509->pubKeyOID;
+        #ifdef WOLFSSL_ATOMIC_OPS
+            if (!wolfSSL_Atomic_Ptr_CompareExchange(
+                    (void* volatile*)&x509->key.pkey, (void**)&current, key)) {
+                wolfSSL_EVP_PKEY_free(key);
+                key = current;
+            }
+        #else
+            x509->key.pkey = key;
+        #endif
+        }
+    }
+    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 is valid for the lifetime of x509 and must not be freed.
+ */
+WOLFSSL_EVP_PKEY* wolfSSL_X509_get0_pubkey(const WOLFSSL_X509* x509)
+{
+    WOLFSSL_ENTER("wolfSSL_X509_get0_pubkey");
+    /* The cache is the only thing written, like X509_get_X509_PUBKEY(). */
+    return X509CachedPubKey((WOLFSSL_X509*)x509);
+}
 #endif /* OPENSSL_EXTRA_X509_SMALL */
 
 /* End of smaller subset of X509 compatibility functions. Avoid increasing the
@@ -11494,6 +11563,9 @@ WOLFSSL_X509_PUBKEY* wolfSSL_X509_get_X509_PUBKEY(const WOLFSSL_X509* x509)
         return NULL;
     }
 
+    /* Decode the key so pkey is usable through the returned object. */
+    (void)X509CachedPubKey((WOLFSSL_X509*)x509);
+
     return (WOLFSSL_X509_PUBKEY*)&x509->key;
 }
 
@@ -11509,12 +11581,17 @@ 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())) {
             return WOLFSSL_FAILURE;
         }
-        pub->algor->algorithm = wolfSSL_OBJ_nid2obj(pub->pubKeyOID);
+        pub->algor->algorithm = wolfSSL_OBJ_nid2obj(
+            oid2nid((word32)pub->pubKeyOID, oidKeyType));
         if (pub->algor->algorithm == NULL) {
             WOLFSSL_MSG("Failed to create object from NID");
             return WOLFSSL_FAILURE;
@@ -11549,6 +11626,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(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;
@@ -16760,6 +16846,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
@@ -16877,6 +16964,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;
 }