openconnect with Belgian EID

[David Woodhouse]

On Fri, 2013-11-15 at 20:08 +0100, Christof Haerens wrote:
> 
> Seems to work with this version. CAs are loaded on the fly::

Great, thanks!. I'll fix the memory leak I introduced, and commit it.

This was it, for reference.

diff --git a/configure.ac b/configure.ac
index 357b001..72012b2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -254,6 +254,8 @@ if test "$with_gnutls" = "yes"; then
     CFLAGS="$CFLAGS $GNUTLS_CFLAGS"
     AC_CHECK_FUNC(gnutls_dtls_set_data_mtu,
 		 [AC_DEFINE(HAVE_GNUTLS_DTLS_SET_DATA_MTU, 1)], [])
+    AC_CHECK_FUNC(gnutls_pkcs11_get_raw_issuer,
+		 [AC_DEFINE(HAVE_GNUTLS_PKCS11_GET_RAW_ISSUER, 1)], [])
     AC_CHECK_FUNC(gnutls_certificate_set_x509_system_trust,
 		 [AC_DEFINE(HAVE_GNUTLS_CERTIFICATE_SET_X509_SYSTEM_TRUST, 1)], [])
     if test "$ac_cv_func_gnutls_certificate_set_x509_system_trust" != "yes"; then
diff --git a/gnutls.c b/gnutls.c
index 2b3b45f..db1d0b1 100644
--- a/gnutls.c
+++ b/gnutls.c
@@ -397,7 +397,8 @@ static int load_pkcs12_certificate(struct openconnect_info *vpninfo,
 }
 
 /* Older versions of GnuTLS didn't actually bother to check this, so we'll
-   do it for them. */
+   do it for them. Is there a bug reference for this? Or just the git commit
+   reference (c1ef7efb in master, 5196786c in gnutls_3_0_x-2)? */
 static int check_issuer_sanity(gnutls_x509_crt_t cert, gnutls_x509_crt_t issuer)
 {
 #if GNUTLS_VERSION_NUMBER > 0x300014
@@ -1460,30 +1461,47 @@ static int load_certificate(struct openconnect_info *vpninfo)
 			/* 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 (check_issuer_sanity(last_cert, issuer)) {
-				/* Hm, is there a bug reference for this? Or just the git commit
-				   reference (c1ef7efb in master, 5196786c in gnutls_3_0_x-2)? */
+			if (!err && check_issuer_sanity(last_cert, issuer)) {
+				gnutls_x509_crt_deinit(issuer);
 				vpn_progress(vpninfo, PRG_ERR,
 					     _("WARNING: GnuTLS returned incorrect issuer certs; authentication may fail!\n"));
-				break;
+				err = GNUTLS_E_INTERNAL_ERROR;
 			}
-		}
 
-		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 (nr_supporting_certs > 1)
-				nr_supporting_certs--;
-			break;
+#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);
+				}
+				/// XXX We leak these certs. certs_to_free should be a bitmap or something
+				gnutls_free(t.data);
+			}
+#endif
+			if (err)
+				break;
+
 		}
 
 		/* OK, we found a new cert to add to our chain. */
@@ -1507,6 +1525,15 @@ static int load_certificate(struct openconnect_info *vpninfo)
 		/* Append the new one */
 		supporting_certs[nr_supporting_certs-1] = issuer;
 		last_cert = issuer;
+
+		if (gnutls_x509_crt_check_issuer(issuer, issuer)) {
+			/* 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 (nr_supporting_certs > 1)
+				nr_supporting_certs--;
+			break;
+		}
 	}
 	for (i = 1; i < nr_supporting_certs; i++) {
 		get_cert_name(supporting_certs[i], name, sizeof(name));

-- 
dwmw2

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 5745 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/openconnect-devel/attachments/20131115/46773e4f/attachment.bin>