Package: libopenconnect2
Version: 5.03-1
Severity: important
Tags: patch upstream
X-Debbugs-CC: openconnect-de...@lists.infradead.org
The changes in gnutls.c from v5.01 to v5.02 concerning "support of CA
certificates from PKCS#11 tokens (with GnuTLS 3.2.7+)" break functionality
in openconnect at least if compiled with GnuTLS 2.12.x. Therefore, it also
affects libopenconnect2 (= 5.02-1) in Ubuntu 14.04LTS.
I have tried to investigate on this issue with GDB and have come as far as
to gnutls.c:1517 where err is not the return value of any call to
gnutls_pkcs11_get_raw_issuer() or gnutls_x509_crt_import() within the
code guarded by
#if defined(HAVE_P11KIT) && defined(HAVE_GNUTLS_PKCS11_GET_RAW_ISSUER)
if compiled with GnuTLS 2.12.x as in Debian and Ubuntu Linux.
So I thought to shift the lines 1517-1518 "if (err) break;" upwards to
its original position, but then it crashes in gnutls.c:1522 invoking
function gnutls_x509_crt_check_issuer(). Finally, I have given up and,
although I know this is far from being smart, I reverted all changes in
gnutls.c to v5.01 which works perfectly for me. The patch for reverting
changes in gnutls.c is attached.
Could you please find a smarter fix or at least apply the given patch
temporarily.
Thank you in advance!
Thomas Uhle
--- openconnect-5.03/gnutls.c 2014-02-03 14:11:19 +0100
+++ openconnect-5.01/gnutls.c 2014-04-10 14:10:52 +0200
@@ -499,7 +499,8 @@ static int assign_privkey(struct opencon
gnutls_privkey_t pkey,
gnutls_x509_crt_t *certs,
unsigned int nr_certs,
- uint8_t *free_certs)
+ gnutls_x509_crt_t *extra_certs,
+ unsigned int nr_extra_certs)
{
int i;
@@ -507,21 +508,28 @@ static int assign_privkey(struct opencon
if (!vpninfo->my_certs)
return GNUTLS_E_MEMORY_ERROR;
- vpninfo->free_my_certs = gnutls_malloc(nr_certs);
- if (vpninfo->free_my_certs) {
- gnutls_free(vpninfo->my_certs);
- vpninfo->my_certs = NULL;
- return GNUTLS_E_MEMORY_ERROR;
- }
-
- memcpy(vpninfo->free_my_certs, free_certs, nr_certs);
memcpy(vpninfo->my_certs, certs, nr_certs * sizeof(*certs));
vpninfo->nr_my_certs = nr_certs;
/* We are *keeping* the certs, unlike in GnuTLS 3 where our caller
can free them after gnutls_certificate_set_key() has been called.
- So wipe the 'free_certs' array. */
- memset(free_certs, 0, nr_certs);
+ So first wipe the 'certs' array (which is either '&cert' or
+ 'supporting_certs' in load_certificate())... */
+ memset(certs, 0, nr_certs * sizeof(*certs));
+
+ /* ... and then also zero out the entries in extra_certs[] that
+ correspond to the certs that we're stealing.
+ We know certs[0] was already stolen by the load_certificate()
+ function so we might as well start at certs[1]. */
+ for (i = 1; i < nr_certs; i++) {
+ int j;
+ for (j = 0; j < nr_extra_certs; j++) {
+ if (vpninfo->my_certs[i] == extra_certs[j]) {
+ extra_certs[j] = NULL;
+ break;
+ }
+ }
+ }
gnutls_certificate_set_retrieve_function(vpninfo->https_cred,
gtls_cert_cb);
@@ -539,7 +547,8 @@ static int assign_privkey(struct opencon
gnutls_privkey_t pkey,
gnutls_x509_crt_t *certs,
unsigned int nr_certs,
- uint8_t *free_certs)
+ gnutls_x509_crt_t *extra_certs,
+ unsigned int nr_extra_certs)
{
gnutls_pcert_st *pcerts = calloc(nr_certs, sizeof(*pcerts));
int i, err;
@@ -891,7 +900,7 @@ static int load_certificate(struct openc
gnutls_x509_crt_t last_cert, cert = NULL;
gnutls_x509_crt_t *extra_certs = NULL, *supporting_certs = NULL;
unsigned int nr_supporting_certs = 0, nr_extra_certs = 0;
- uint8_t *free_supporting_certs = NULL;
+ unsigned int certs_to_free = 0; /* How many of supporting_certs */
int err; /* GnuTLS error */
int ret;
int i;
@@ -1002,12 +1011,6 @@ static int load_certificate(struct openc
else if (!ret) {
if (nr_supporting_certs) {
cert = supporting_certs[0];
- free_supporting_certs = gnutls_malloc(nr_supporting_certs);
- if (!free_supporting_certs) {
- ret = -ENOMEM;
- goto out;
- }
- memset(free_supporting_certs, 1, nr_supporting_certs);
goto got_key;
}
vpn_progress(vpninfo, PRG_ERR,
@@ -1428,35 +1431,19 @@ static int load_certificate(struct openc
choose the _right_ one. (RT#1942)
Pick the right ones for ourselves and add them manually. */
- /* We may have already got a bunch of certs from PKCS#12
- file. Remember how many need to be freed when we're done,
- since we'll expand the supporting_certs array with more
- from the cafile and extra_certs[] array if we can, and
- those extra certs must not be freed (twice). */
- if (!nr_supporting_certs) {
- supporting_certs = gnutls_malloc(sizeof(*supporting_certs));
- if (!supporting_certs) {
- vpn_progress(vpninfo, PRG_ERR,
- _("Failed to allocate memory for certificate\n"));
- ret = -ENOMEM;
- goto out;
- }
- supporting_certs[0] = cert;
- nr_supporting_certs = 1;
-
- free_supporting_certs = gnutls_malloc(1);
- if (!free_supporting_certs) {
- vpn_progress(vpninfo, PRG_ERR,
- _("Failed to allocate memory for certificate\n"));
- ret = -ENOMEM;
- goto out;
- }
- free_supporting_certs[0] = 1;
+ if (nr_supporting_certs) {
+ /* We already got a bunch of certs from PKCS#12 file. Remember
+ how many need to be freed when we're done, since we'll
+ expand the supporting_certs array with more from the cafile
+ and extra_certs[] array if we can, and those extra certs
+ must not be freed (twice). */
+ last_cert = supporting_certs[nr_supporting_certs-1];
+ certs_to_free = nr_supporting_certs;
+ } else {
+ last_cert = cert;
+ certs_to_free = nr_supporting_certs = 1;
}
- last_cert = supporting_certs[nr_supporting_certs-1];
-
while (1) {
- uint8_t free_issuer;
gnutls_x509_crt_t issuer;
void *tmp;
@@ -1470,89 +1457,54 @@ static int load_certificate(struct openc
if (i < nr_extra_certs) {
/* We found the next cert in the chain in extra_certs[] */
issuer = extra_certs[i];
- extra_certs[i] = NULL;
- free_issuer = 1;
} else {
/* Look for it in the system trust cafile too. */
err = gnutls_certificate_get_issuer(vpninfo->https_cred,
last_cert, &issuer, 0);
+ if (err)
+ break;
+
/* The check_issuer_sanity() function works fine as a workaround where
it was used above, but when gnutls_certificate_get_issuer() returns
a bogus cert, there's nothing we can do to fix it up. We don't get
to iterate over all the available certs like we can over our own
list. */
- if (!err && check_issuer_sanity(last_cert, issuer)) {
+ if (check_issuer_sanity(last_cert, issuer)) {
vpn_progress(vpninfo, PRG_ERR,
_("WARNING: GnuTLS returned incorrect issuer certs; authentication may fail!\n"));
break;
}
- free_issuer = 0;
-
-#if defined(HAVE_P11KIT) && defined(HAVE_GNUTLS_PKCS11_GET_RAW_ISSUER)
- if (err && cert_is_p11) {
- gnutls_datum_t t;
-
- err = gnutls_pkcs11_get_raw_issuer(cert_url, last_cert, &t, GNUTLS_X509_FMT_DER, 0);
- if (!err) {
- err = gnutls_x509_crt_init(&issuer);
- if (!err) {
- err = gnutls_x509_crt_import(issuer, &t, GNUTLS_X509_FMT_DER);
- if (err)
- gnutls_x509_crt_deinit(issuer);
- }
- }
- if (err) {
- vpn_progress(vpninfo, PRG_ERR,
- "Got no issuer from PKCS#11\n");
- } else {
- get_cert_name(issuer, name, sizeof(name));
-
- vpn_progress(vpninfo, PRG_ERR,
- _("Got next CA '%s' from PKCS11\n"), name);
- }
- free_issuer = 1;
- gnutls_free(t.data);
- }
-#endif
- if (err)
- break;
-
}
- if (gnutls_x509_crt_check_issuer(issuer, issuer)) {
+ if (issuer == last_cert) {
/* Don't actually include the root CA. If they don't already trust it,
then handing it to them isn't going to help. But don't omit the
original certificate if it's self-signed. */
- if (free_issuer)
- gnutls_x509_crt_deinit(issuer);
+ if (nr_supporting_certs > 1)
+ nr_supporting_certs--;
break;
}
/* OK, we found a new cert to add to our chain. */
tmp = supporting_certs;
supporting_certs = gnutls_realloc(supporting_certs,
- sizeof(cert) * (nr_supporting_certs+1));
+ sizeof(cert) * (++nr_supporting_certs));
if (!supporting_certs) {
- supporting_certs = tmp;
- realloc_failed:
+ gnutls_free(tmp);
vpn_progress(vpninfo, PRG_ERR,
_("Failed to allocate memory for supporting certificates\n"));
- if (free_issuer)
- gnutls_x509_crt_deinit(issuer);
- break;
+ /* The world is probably about to end, but try without them anyway */
+ certs_to_free = 0;
+ ret = -ENOMEM;
+ goto out;
}
- tmp = free_supporting_certs;
- free_supporting_certs = gnutls_realloc(free_supporting_certs, nr_supporting_certs+1);
- if (!free_supporting_certs) {
- free_supporting_certs = tmp;
- goto realloc_failed;
- }
+ /* First time we actually allocated an array? Copy the first cert into it */
+ if (nr_supporting_certs == 2)
+ supporting_certs[0] = cert;
/* Append the new one */
- supporting_certs[nr_supporting_certs] = issuer;
- free_supporting_certs[nr_supporting_certs] = free_issuer;
- nr_supporting_certs++;
+ supporting_certs[nr_supporting_certs-1] = issuer;
last_cert = issuer;
}
for (i = 1; i < nr_supporting_certs; i++) {
@@ -1572,9 +1524,9 @@ static int load_certificate(struct openc
#if defined(HAVE_P11KIT) || defined(HAVE_TROUSERS)
if (pkey) {
err = assign_privkey(vpninfo, pkey,
- supporting_certs,
+ supporting_certs ? supporting_certs : &cert,
nr_supporting_certs,
- free_supporting_certs);
+ extra_certs, nr_extra_certs);
if (!err) {
pkey = NULL; /* we gave it away, and potentially also some
of extra_certs[] may have been zeroed. */
@@ -1582,7 +1534,7 @@ static int load_certificate(struct openc
} else
#endif /* P11KIT || TROUSERS */
err = gnutls_certificate_set_x509_key(vpninfo->https_cred,
- supporting_certs,
+ supporting_certs ? supporting_certs : &cert,
nr_supporting_certs, key);
if (err) {
@@ -1598,14 +1550,11 @@ static int load_certificate(struct openc
if (key)
gnutls_x509_privkey_deinit(key);
if (supporting_certs) {
- for (i = 0; i < nr_supporting_certs; i++) {
+ for (i = 0; i < certs_to_free; i++) {
- /* We get here in an error case with !free_supporting_certs
- and should free them all in that case */
- if (!free_supporting_certs || free_supporting_certs[i])
+ if (supporting_certs[i])
gnutls_x509_crt_deinit(supporting_certs[i]);
}
gnutls_free(supporting_certs);
- gnutls_free(free_supporting_certs);
} else if (cert) {
/* Not if supporting_certs. It's supporting_certs[0] then and
was already freed. */
@@ -2056,12 +2005,9 @@ void openconnect_close_https(struct open
if (vpninfo->my_certs) {
int i;
for (i = 0; i < vpninfo->nr_my_certs; i++)
- if (vpninfo->free_my_certs[i])
- gnutls_x509_crt_deinit(vpninfo->my_certs[i]);
+ gnutls_x509_crt_deinit(vpninfo->my_certs[i]);
gnutls_free(vpninfo->my_certs);
- gnutls_free(vpninfo->free_my_certs);
vpninfo->my_certs = NULL;
- vpninfo->free_my_certs = NULL;
}
#endif
}
