[x509] Ensure certificate remains valid during x509_append()

The allocation of memory for the certificate chain link may cause the
certificate itself to be freed by the cache discarder, if the only
current reference to the certificate is held by the certificate store
and the system runs out of memory during the call to malloc().

Ensure that this cannot happen by taking out a temporary additional
reference to the certificate within x509_append(), rather than
requiring the caller to do so.

Signed-off-by: Michael Brown <mcb30@ipxe.org>

diff --git a/src/crypto/x509.c b/src/crypto/x509.c
index acb27411a47df2bb9ab0c2fb7dbc47e05ec4a83a..10bc6369a313c140b2b57a08692763c9f19a104d 100644 (file)
--- a/src/crypto/x509.c
+++ b/src/crypto/x509.c
@@ -1634,11 +1634,17 @@ struct x509_chain * x509_alloc_chain ( void ) {
  */
 int x509_append ( struct x509_chain *chain, struct x509_certificate *cert ) {
        struct x509_link *link;
+       int rc;
+
+       /* Ensure allocation of link cannot invalidate certificate */
+       x509_get ( cert );
 
        /* Allocate link */
        link = zalloc ( sizeof ( *link ) );
-       if ( ! link )
-               return -ENOMEM;
+       if ( ! link ) {
+               rc = -ENOMEM;
+               goto err_alloc;
+       }
 
        /* Add link to chain */
        link->cert = x509_get ( cert );
@@ -1646,7 +1652,12 @@ int x509_append ( struct x509_chain *chain, struct x509_certificate *cert ) {
        DBGC ( chain, "X509 chain %p added X509 %p \"%s\"\n",
               chain, cert, x509_name ( cert ) );
 
-       return 0;
+       /* Success */
+       rc = 0;
+
+       x509_put ( cert );
+ err_alloc:
+       return rc;
 }
 
 /**

diff --git a/src/net/tls.c b/src/net/tls.c
index 4c135f0901c02a72aac1c0c96720d0401a91aeff..643b9292d4e6dcc419e1e614fa33cc63d053308a 100644 (file)
--- a/src/net/tls.c
+++ b/src/net/tls.c
@@ -2470,9 +2470,6 @@ static int tls_new_certificate_request ( struct tls_connection *tls,
        /* Determine client certificate to be sent, if any */
        cert = x509_find_key ( NULL, tls->client.key );
        if ( cert ) {
-
-               /* Get temporary reference to certificate */
-               x509_get ( cert );
                DBGC ( tls, "TLS %p selected client certificate %s\n",
                       tls, x509_name ( cert ) );
 
@@ -2491,14 +2488,10 @@ static int tls_new_certificate_request ( struct tls_connection *tls,
                       "to private key\n", tls );
        }
 
-       /* Drop local reference (if any) to client certificate */
-       x509_put ( cert );
-
        return 0;
 
  err_auto_append:
  err_append:
-       x509_put ( cert );
        x509_chain_put ( tls->client.chain );
        tls->client.chain = NULL;
  err_alloc: