Make GnuTLS parse_pkcs12() return extra certificates from the PKCS#12 too

Create a separate list, return them for the caller to do with as it sees fit.

This also cleans up the error handling a little. When this was a purely
internal GnuTLS function, it was fine to leave things (like *key) allocated
and return an error. If my intention is to make this exportable, then it
ought to clean up after itself when returning an error.

I think this actually fixes a potential memory leak for the GnuTLS internal
caller of this function, too.

Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>

diff --git a/gnutls.c b/gnutls.c
index 2c5891b..4df3a90 100644 (file)
--- a/gnutls.c
+++ b/gnutls.c
@@ -321,7 +321,9 @@ static int load_pkcs12_certificate(struct openconnect_info *vpninfo,
                                   gnutls_datum_t *datum,
                                   gnutls_x509_privkey_t *key,
                                   gnutls_x509_crt_t *cert,
-                                  gnutls_x509_crl_t * crl)
+                                  gnutls_x509_crt_t **extra_certs,
+                                  unsigned int *nr_extra_certs,
+                                  gnutls_x509_crl_t *crl)
 {
        gnutls_pkcs12_t p12;
        char *pass;
@@ -383,7 +385,8 @@ static int load_pkcs12_certificate(struct openconnect_info *vpninfo,
                return ret;
        }
 
-       err = parse_pkcs12(vpninfo->https_cred, p12, pass, key, cert, crl);
+       err = parse_pkcs12(vpninfo->https_cred, p12, pass, key, cert,
+                          extra_certs, nr_extra_certs, crl);
        gnutls_pkcs12_deinit(p12);
        if (err) {
                vpn_progress(vpninfo, PRG_ERR,
@@ -434,7 +437,8 @@ static int load_certificate(struct openconnect_info *vpninfo)
 
        if (vpninfo->cert_type == CERT_TYPE_PKCS12 ||
            vpninfo->cert_type == CERT_TYPE_UNKNOWN) {
-               err = load_pkcs12_certificate(vpninfo, &fdata, &key, &cert, &crl);
+               err = load_pkcs12_certificate(vpninfo, &fdata, &key, &cert,
+                                             NULL, NULL, &crl);
                if (!err)
                        goto got_cert;
                else if (err <= 0) {

diff --git a/gnutls_pkcs12.c b/gnutls_pkcs12.c
index 623f47c..393a838 100644 (file)
--- a/gnutls_pkcs12.c
+++ b/gnutls_pkcs12.c
@@ -44,9 +44,14 @@ parse_pkcs12 (gnutls_certificate_credentials_t res,
               gnutls_pkcs12_t p12,
               const char *password,
               gnutls_x509_privkey_t * key,
-              gnutls_x509_crt_t * cert, gnutls_x509_crl_t * crl)
+              gnutls_x509_crt_t * cert,
+              gnutls_x509_crt_t ** extra_certs_ret,
+              unsigned int * extra_certs_ret_len,
+              gnutls_x509_crl_t * crl)
 {
   gnutls_pkcs12_bag_t bag = NULL;
+  gnutls_x509_crt_t *extra_certs = NULL;
+  int extra_certs_len = 0;
   int idx = 0;
   int ret;
   size_t cert_id_size = 0;
@@ -237,6 +242,7 @@ parse_pkcs12 (gnutls_certificate_credentials_t res,
         {
           int type;
           gnutls_datum_t data;
+          gnutls_x509_crt_t this_cert;
 
           type = gnutls_pkcs12_bag_get_type (bag, i);
           if (type < 0)
@@ -255,13 +261,7 @@ parse_pkcs12 (gnutls_certificate_credentials_t res,
           switch (type)
             {
             case GNUTLS_BAG_CERTIFICATE:
-              if (*cert != NULL)        /* no need to set it again */
-                {
-                  gnutls_assert ();
-                  break;
-                }
-
-              ret = gnutls_x509_crt_init (cert);
+              ret = gnutls_x509_crt_init (&this_cert);
               if (ret < 0)
                 {
                   gnutls_assert ();
@@ -269,29 +269,56 @@ parse_pkcs12 (gnutls_certificate_credentials_t res,
                 }
 
               ret =
-                gnutls_x509_crt_import (*cert, &data, GNUTLS_X509_FMT_DER);
+                gnutls_x509_crt_import (this_cert, &data, GNUTLS_X509_FMT_DER);
               if (ret < 0)
                 {
                   gnutls_assert ();
-                  gnutls_x509_crt_deinit (*cert);
+                  gnutls_x509_crt_deinit (this_cert);
                   goto done;
                 }
 
               /* check if the key id match */
               cert_id_size = sizeof (cert_id);
               ret =
-                gnutls_x509_crt_get_key_id (*cert, 0, cert_id, &cert_id_size);
+                gnutls_x509_crt_get_key_id (this_cert, 0, cert_id, &cert_id_size);
               if (ret < 0)
                 {
                   gnutls_assert ();
-                  gnutls_x509_crt_deinit (*cert);
+                  gnutls_x509_crt_deinit (this_cert);
                   goto done;
                 }
 
               if (memcmp (cert_id, key_id, cert_id_size) != 0)
                 {               /* they don't match - skip the certificate */
-                  gnutls_x509_crt_deinit (*cert);
-                  *cert = NULL;
+                  if (extra_certs_ret)
+                    {
+                      extra_certs = gnutls_realloc (extra_certs,
+                                                    sizeof(extra_certs[0]) *
+                                                    ++extra_certs_len);
+                      if (!extra_certs)
+                        {
+                          gnutls_assert ();
+                          ret = GNUTLS_E_MEMORY_ERROR;
+                          goto done;
+                        }
+                      extra_certs[extra_certs_len - 1] = this_cert;
+                      this_cert = NULL;
+                    }
+                  else
+                    {
+                       gnutls_x509_crt_deinit (this_cert);
+                    }
+                  break;
+                }
+              else
+                {
+                   if (*cert != NULL)        /* no need to set it again */
+                     {
+                        gnutls_assert ();
+                        break;
+                     }
+                   *cert = this_cert;
+                   this_cert = NULL;
                 }
               break;
 
@@ -337,5 +364,25 @@ done:
   if (bag)
     gnutls_pkcs12_bag_deinit (bag);
 
+  if (ret)
+    {
+      if (*key)
+        gnutls_x509_privkey_deinit(*key);
+      if (*cert)
+        gnutls_x509_crt_deinit(*cert);
+      if (extra_certs_len)
+        {
+          int i;
+          for (i = 0; i < extra_certs_len; i++)
+            gnutls_x509_crt_deinit(extra_certs[i]);
+          gnutls_free(extra_certs);
+        }
+    }
+  else if (extra_certs_ret)
+    {
+      *extra_certs_ret = extra_certs;
+      *extra_certs_ret_len = extra_certs_len;
+    }
+
   return ret;
 }