Changeset 2cde8111 in mod_gnutls

Timestamp:

Apr 5, 2015, 6:20:59 PM (4 years ago)

Author:

Thomas Klute <thomas2.klute@…>

Branches:

debian/master, debian/stretch-backports, jessie-backports, master, upstream

Children:

d04f7da

Parents:

351b51f

Message:

Workarounds for OpenPGP key handling

Commit 031acac9c6541034777f8917633164b51f6bd10a 'Use the new (3.1.3+)
GnuTLS APIs to obtain private keys' led to failed handshakes when using
OpenPGP keys for authentication. Debugging revealed two separate issues,
this commit adds workarounds for both.

The first problem was that the supported certificate types for the
session were not set correctly. This is a known bug in
gnutls_certificate_set_retrieve_function2 [1], the workaround comes from
[2]. The bug should be fixed in GnuTLS 3.3.12, hence the version guard.

After this problem was fixed, segfaults occurred during handshake. A
Valgrind trace showed attemts to access memory that had been free'd in
gnutls_privkey_import_openpgp_raw. I could work around the issue by
loading the key into a gnutls_openpgp_privkey_t structure first and then
importing it into the gnutls_privkey_t using
gnutls_privkey_import_openpgp afterwards.

Thank you to Nikos Mavrogiannopoulos for very fast help with debugging!

[1] ​https://lists.gnupg.org/pipermail/gnutls-devel/2015-January/007377.html
[2] ​https://github.com/vanrein/tlspool/commit/4938102d3d1b086491d147e6c8e4e2a02825fc12

Files:

• include/mod_gnutls.h.in (modified) (1 diff)
• src/gnutls_config.c (modified) (2 diffs)
• src/gnutls_hooks.c (modified) (1 diff)

include/mod_gnutls.h.in

@@ -170 +170 @@
         /* OpenPGP Certificate Private Key */
     gnutls_privkey_t privkey_pgp;
+    /* Internal structure for the OpenPGP private key. DO NOT USE
+     * outside key loading. */
+    gnutls_openpgp_privkey_t privkey_pgp_internal;
 
     /* Export full certificates to CGI environment: */

src/gnutls_config.c

@@ -422 +422 @@
         }
 
-        ret =
-            gnutls_privkey_import_openpgp_raw(sc->privkey_pgp, &data,
-                                              GNUTLS_OPENPGP_FMT_BASE64,
-                                              NULL, NULL);
+        /* Theoretically, this chain of gnutls_openpgp_privkey_init,
+         * gnutls_openpgp_privkey_import and
+         * gnutls_privkey_import_openpgp could be replaced with one
+         * call to gnutls_privkey_import_openpgp_raw as shown
+         * below. However, that led to a segfault during handshake
+         * which disappeared with the three step method.
+         *
+         * ret = gnutls_privkey_import_openpgp_raw(sc->privkey_pgp, &data,
+         *                                         GNUTLS_OPENPGP_FMT_BASE64,
+         *                                         NULL, NULL); */
+        ret = gnutls_openpgp_privkey_init(&sc->privkey_pgp_internal);
+        if (ret != 0) {
+            ap_log_error(APLOG_MARK, APLOG_STARTUP, 0, s,
+                         "GnuTLS: Failed to initialize "
+                         "PGP Private Key '%s': (%d) %s",
+                         sc->pgp_key_file, ret, gnutls_strerror(ret));
+            ret = -1;
+            goto cleanup;
+        }
+
+        ret = gnutls_openpgp_privkey_import(sc->privkey_pgp_internal, &data,
+                                            GNUTLS_OPENPGP_FMT_BASE64, NULL, 0);
         if (ret != 0) {
             ap_log_error(APLOG_MARK, APLOG_STARTUP, 0, s,
@@ -431 +449 @@
                          "PGP Private Key '%s': (%d) %s",
                          sc->pgp_key_file, ret, gnutls_strerror(ret));
+            ret = -1;
+            goto cleanup;
+        }
+
+        ret = gnutls_privkey_import_openpgp(sc->privkey_pgp,
+                                            sc->privkey_pgp_internal, 0);
+        if (ret != 0)
+        {
+            ap_log_error(APLOG_MARK, APLOG_STARTUP, 0, s,
+                         "GnuTLS: Failed to assign PGP Private Key '%s' "
+                         "to gnutls_privkey_t structure: (%d) %s",
+                         sc->pgp_key_file, ret, gnutls_strerror(ret));
             ret = -1;
             goto cleanup;

src/gnutls_hooks.c

@@ -360 +360 @@
             gnutls_anon_set_server_dh_params(sc->anon_creds, dh_params);
         }
+
+        /* The call after this comment is a workaround for bug in
+         * gnutls_certificate_set_retrieve_function2 that ignores
+         * supported certificate types. Should be fixed in GnuTLS
+         * 3.3.12.
+         *
+         * Details:
+         * https://lists.gnupg.org/pipermail/gnutls-devel/2015-January/007377.html
+         * Workaround from:
+         * https://github.com/vanrein/tlspool/commit/4938102d3d1b086491d147e6c8e4e2a02825fc12
+         *
+         * TODO: add appropriate version guards */
+#if GNUTLS_VERSION_NUMBER < 0x030312
+        gnutls_certificate_set_retrieve_function(sc->certs, (void *) exit);
+#endif
 
         gnutls_certificate_set_retrieve_function2(sc->certs, cert_retrieve_fn);