Your message dated Tue, 19 May 2015 10:19:34 +0000
with message-id <[email protected]>
and subject line Bug#775909: fixed in mod-gnutls 0.6-1.4
has caused the Debian Bug report #775909,
regarding libapache2-mod-gnutls: segfaults with reverse proxy configuration
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact [email protected]
immediately.)


-- 
775909: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=775909
Debian Bug Tracking System
Contact [email protected] with problems
--- Begin Message ---
Package: libapache2-mod-gnutls
Version: 0.6-1.2
Severity: normal

I've configured mod_gnutls to handle client TLS connections for a
reverse proxy with HTTP back end connections. However, requests
handled by the proxy led to segfaults in the handler process and, after
I fixed the first issue, the TLS stack shutting down completely.

I found and fixed three bug in mod_gnutls related to the reverse proxy
configuration:

* NULL pointer dereference in ssl_engine_disable (removing nonexistent
  filters) [1]
* ssl_engine_disable disabled TLS globally instead of per connection [2]
* ssl_engine_disable called a global deinit function, leading to
  use-after-free segfaults [3]

The bugs exist in the upstream source as well, I have submitted a pull
request [4] on the mod_gnutls-devel mailing list. Until there is an
updated upstream version, I suggest adding the patches in Debian. The
attached patches fit on top of the existing quilt patch in
mod-gnutls_0.6-1.2, in the following order:

proxy-segfault-fix.patch
enable-tls-per-connection.patch
no-deinit-on-proxy-disable.patch


[1]
https://github.com/airtower-luna/mod_gnutls/commit/3d361b8e5d7c4c971d344658728979fe978dc759
[2]
https://github.com/airtower-luna/mod_gnutls/commit/e8acf058857eae21cde2fca0f4e97338075f5f60
[3]
https://github.com/airtower-luna/mod_gnutls/commit/c782c1f12c0ed4d5048eb52fd3ef51037c53f426
[4]
http://lists.gnupg.org/pipermail/mod_gnutls-devel/2015-January/000114.html

(Note: the "debsums: changed file" error below is a result of my
manually patched version being installed on top of the Debian package
on my development system. I've tested a package built with the patches
on a separate host.)

-- System Information:
Debian Release: 8.0
  APT prefers unstable
  APT policy: (500, 'unstable')
Architecture: amd64 (x86_64)

Kernel: Linux 3.16.0-4-amd64 (SMP w/1 CPU core)
Locale: LANG=de_DE.UTF-8, LC_CTYPE=de_DE.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)

Versions of packages libapache2-mod-gnutls depends on:
ii  apache2-bin [apache2-api-20120211]  2.4.10-9
ii  libapr-memcache0                    0.7.0-3
ii  libc6                               2.19-13
ii  libgnutls-deb0-28                   3.3.8-5
ii  libmsv1                             1.1-1

libapache2-mod-gnutls recommends no packages.

libapache2-mod-gnutls suggests no packages.

-- Configuration Files:
/etc/apache2/mods-available/gnutls.conf changed [not included]
/etc/apache2/sites-available/default-tls.conf changed [not included]

-- no debconf information

-- debsums errors found:
debsums: changed file /usr/lib/apache2/modules/mod_gnutls.so (from
libapache2-mod-gnutls package)
From 3d361b8e5d7c4c971d344658728979fe978dc759 Mon Sep 17 00:00:00 2001
From: Thomas Klute <[email protected]>
Date: Tue, 13 Jan 2015 17:04:38 +0100
Subject: [PATCH] Check if filters exist before removing them in
 ssl_engine_disable

Trying to remove filters that are NULL leads to a segfault in the worker
thread. Check if c->input_filters and c->output_filters are defined
before removing and remove only if set.

Also, output filters should be removed with the dedicated function.
---
 src/mod_gnutls.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

--- a/src/mod_gnutls.c
+++ b/src/mod_gnutls.c
@@ -80,8 +80,10 @@
     if(sc->enabled == GNUTLS_ENABLED_FALSE) {
         return 1;
     }
-    ap_remove_input_filter(c->input_filters);
-    ap_remove_input_filter(c->output_filters);
+    if (c->input_filters)
+        ap_remove_input_filter(c->input_filters);
+    if (c->output_filters)
+        ap_remove_output_filter(c->output_filters);
     mgs_cleanup_pre_config(c->pool);
     sc->enabled = 0;
     return 1;
From e8acf058857eae21cde2fca0f4e97338075f5f60 Mon Sep 17 00:00:00 2001
From: Thomas Klute <[email protected]>
Date: Tue, 20 Jan 2015 16:30:36 +0100
Subject: [PATCH] Enable/disable TLS per connection in ssl_engine_disable

Previously, ssl_engine_disable set the server wide variable sc->enabled
to GNUTLS_ENABLED_FALSE, leading to mod_gnutls refusing to serve any
connection, including incoming client connections. The general HTTP
handler cannot process raw TLS traffic, so all further requests using
TLS failed.

This commit adds a new element "enabled" to struct mgs_handle_t, which
is used to disable TLS per connection, making it possible to disable TLS
for proxy back end connections while continuing to serve TLS clients.
---
 include/mod_gnutls.h.in |  2 ++
 src/gnutls_hooks.c      | 50 +++++++++++++++++++++++++++++++------------------
 src/mod_gnutls.c        | 23 +++++++++++++++++++----
 3 files changed, 53 insertions(+), 22 deletions(-)

Index: mod-gnutls-0.6/include/mod_gnutls.h.in
===================================================================
--- mod-gnutls-0.6.orig/include/mod_gnutls.h.in
+++ mod-gnutls-0.6/include/mod_gnutls.h.in
@@ -170,6 +170,8 @@ typedef struct {
     mgs_srvconf_rec *sc;
 	/* Connection record */
     conn_rec* c;
+	/* Is TLS enabled for this connection? */
+    int enabled;
 	/* GnuTLS Session handle */
     gnutls_session_t session;
 	/* module input status */
Index: mod-gnutls-0.6/src/gnutls_hooks.c
===================================================================
--- mod-gnutls-0.6.orig/src/gnutls_hooks.c
+++ mod-gnutls-0.6/src/gnutls_hooks.c
@@ -674,14 +674,23 @@ mgs_srvconf_rec *mgs_find_sni_server(gnu
     return NULL;
 }
 
-static void create_gnutls_handle(conn_rec * c) {
-    mgs_handle_t *ctxt;
-    /* Get mod_gnutls Configuration Record */
-    mgs_srvconf_rec *sc =(mgs_srvconf_rec *)
-            ap_get_module_config(c->base_server->module_config,&gnutls_module);
+static void create_gnutls_handle(conn_rec * c)
+{
+    /* Get mod_gnutls server configuration */
+    mgs_srvconf_rec *sc = (mgs_srvconf_rec *)
+            ap_get_module_config(c->base_server->module_config, &gnutls_module);
 
     _gnutls_log(debug_log_fp, "%s: %d\n", __func__, __LINE__);
-    ctxt = apr_pcalloc(c->pool, sizeof (*ctxt));
+
+    /* Get connection specific configuration */
+    mgs_handle_t *ctxt = (mgs_handle_t *) ap_get_module_config(c->conn_config, &gnutls_module);
+    if (ctxt == NULL)
+    {
+        ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c, "%s: allocating connection memory", __func__);
+        ctxt = apr_pcalloc(c->pool, sizeof (*ctxt));
+        ap_set_module_config(c->conn_config, &gnutls_module, ctxt);
+    }
+    ctxt->enabled = GNUTLS_ENABLED_TRUE;
     ctxt->c = c;
     ctxt->sc = sc;
     ctxt->status = 0;
@@ -692,6 +701,7 @@ static void create_gnutls_handle(conn_re
     ctxt->output_bb = apr_brigade_create(c->pool, c->bucket_alloc);
     ctxt->output_blen = 0;
     ctxt->output_length = 0;
+
     /* Initialize GnuTLS Library */
     gnutls_init(&ctxt->session, GNUTLS_SERVER);
     /* Initialize Session Tickets */
@@ -707,8 +717,6 @@ static void create_gnutls_handle(conn_re
     /* Initialize Session Cache */
     mgs_cache_session_init(ctxt);
 
-    /* Set this config for this connection */
-    ap_set_module_config(c->conn_config, &gnutls_module, ctxt);
     /* Set pull, push & ptr functions */
     gnutls_transport_set_pull_function(ctxt->session,
             mgs_transport_read);
@@ -722,15 +730,20 @@ static void create_gnutls_handle(conn_re
             ctxt, NULL, c);
 }
 
-int mgs_hook_pre_connection(conn_rec * c, void *csd) {
-    mgs_srvconf_rec *sc;
-
+int mgs_hook_pre_connection(conn_rec * c, void *csd)
+{
     _gnutls_log(debug_log_fp, "%s: %d\n", __func__, __LINE__);
 
-    sc = (mgs_srvconf_rec *) ap_get_module_config(c->base_server->module_config,
-            &gnutls_module);
-
-    if (sc && (!sc->enabled || sc->proxy_enabled == GNUTLS_ENABLED_TRUE)) {
+    mgs_srvconf_rec *sc = (mgs_srvconf_rec *)
+        ap_get_module_config(c->base_server->module_config, &gnutls_module);
+    mgs_handle_t *ctxt = (mgs_handle_t *)
+        ap_get_module_config(c->conn_config, &gnutls_module);
+
+    if ((sc && (!sc->enabled || sc->proxy_enabled == GNUTLS_ENABLED_TRUE))
+        || (ctxt && ctxt->enabled == GNUTLS_ENABLED_FALSE))
+    {
+        ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c, "%s declined connection",
+                      __func__);
         return DECLINED;
     }
 
@@ -752,11 +765,12 @@ int mgs_hook_fixups(request_rec * r) {
     _gnutls_log(debug_log_fp, "%s: %d\n", __func__, __LINE__);
     apr_table_t *env = r->subprocess_env;
 
-    ctxt =
-            ap_get_module_config(r->connection->conn_config,
-            &gnutls_module);
+    ctxt = ap_get_module_config(r->connection->conn_config,
+                                &gnutls_module);
 
-    if (!ctxt || ctxt->session == NULL) {
+    if (!ctxt || ctxt->enabled != GNUTLS_ENABLED_TRUE || ctxt->session == NULL)
+    {
+        ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, "request declined in %s", __func__);
         return DECLINED;
     }
 
Index: mod-gnutls-0.6/src/mod_gnutls.c
===================================================================
--- mod-gnutls-0.6.orig/src/mod_gnutls.c
+++ mod-gnutls-0.6/src/mod_gnutls.c
@@ -19,8 +19,12 @@
 
 #include "mod_gnutls.h"
 
-static void gnutls_hooks(apr_pool_t * p) {
+#ifdef APLOG_USE_MODULE
+APLOG_USE_MODULE(gnutls);
+#endif
 
+static void gnutls_hooks(apr_pool_t * p)
+{
     /* Try Run Post-Config Hook After mod_proxy */
     static const char * const aszPre[] = { "mod_proxy.c", NULL };
     ap_hook_post_config(mgs_hook_post_config, aszPre, NULL,APR_HOOK_REALLY_LAST);
@@ -74,18 +78,29 @@ int ssl_is_https(conn_rec *c) {
     return 1;
 }
 
-int ssl_engine_disable(conn_rec *c) {
+int ssl_engine_disable(conn_rec *c)
+{
     mgs_srvconf_rec *sc = (mgs_srvconf_rec *)
-            ap_get_module_config(c->base_server->module_config, &gnutls_module);
+        ap_get_module_config(c->base_server->module_config, &gnutls_module);
     if(sc->enabled == GNUTLS_ENABLED_FALSE) {
         return 1;
     }
+
+    /* disable TLS for this connection */
+    mgs_handle_t *ctxt = (mgs_handle_t *) ap_get_module_config(c->conn_config, &gnutls_module);
+    if (ctxt == NULL)
+    {
+        ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c, "%s: allocating connection memory", __func__);
+        ctxt = apr_pcalloc(c->pool, sizeof (*ctxt));
+        ap_set_module_config(c->conn_config, &gnutls_module, ctxt);
+    }
+    ctxt->enabled = GNUTLS_ENABLED_FALSE;
+
     if (c->input_filters)
         ap_remove_input_filter(c->input_filters);
     if (c->output_filters)
         ap_remove_output_filter(c->output_filters);
     mgs_cleanup_pre_config(c->pool);
-    sc->enabled = 0;
     return 1;
 }
 
From c782c1f12c0ed4d5048eb52fd3ef51037c53f426 Mon Sep 17 00:00:00 2001
From: Thomas Klute <[email protected]>
Date: Wed, 21 Jan 2015 09:41:51 +0100
Subject: [PATCH] Don't do global deinit when disabling TLS for a proxy back
 end connection

Prior to this commit, ssl_engine_disable called mgs_cleanup_pre_config
on the connection pool before returning:

mgs_cleanup_pre_config(c->pool);

mgs_cleanup_pre_config does not even touch the argument, as its
signature shows.

apr_status_t mgs_cleanup_pre_config(void *data __attribute__((unused)));

It does, however, deinitialize the global session cache and, more
importantly, the global GnuTLS data structures. Trying to use those
deinitialized data structures led to segmentation faults during TLS
handshake.

Since there is no reason to globally deinitialize GnuTLS when disabling
TLS for one specific proxy back end connection, the solution is to
simply remove the call to mgs_cleanup_pre_config from
ssl_engine_disable.
---
 src/mod_gnutls.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mod_gnutls.c b/src/mod_gnutls.c
index a77364d..2d0e6ea 100644
--- a/src/mod_gnutls.c
+++ b/src/mod_gnutls.c
@@ -100,7 +100,7 @@ int ssl_engine_disable(conn_rec *c)
         ap_remove_input_filter(c->input_filters);
     if (c->output_filters)
         ap_remove_output_filter(c->output_filters);
-    mgs_cleanup_pre_config(c->pool);
+
     return 1;
 }
 
-- 
2.1.4


--- End Message ---
--- Begin Message ---
Source: mod-gnutls
Source-Version: 0.6-1.4

We believe that the bug you reported is fixed in the latest version of
mod-gnutls, which is due to be installed in the Debian FTP archive.

A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed.  If you
have further comments please address them to [email protected],
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Thomas Klute <[email protected]> (supplier of updated mod-gnutls 
package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing [email protected])


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Format: 1.8
Date: Wed, 15 Apr 2015 11:34:37 +0200
Source: mod-gnutls
Binary: libapache2-mod-gnutls
Architecture: source
Version: 0.6-1.4
Distribution: unstable
Urgency: low
Maintainer: Daniel Kahn Gillmor <[email protected]>
Changed-By: Thomas Klute <[email protected]>
Description:
 libapache2-mod-gnutls - Apache module for SSL and TLS encryption with GnuTLS
Closes: 775909
Changes:
 mod-gnutls (0.6-1.4) unstable; urgency=low
 .
   * Non-maintainer upload.
   * Fix segfaults with reverse proxy configuration (Closes: #775909)
   * Upgrade Standards-Version to 3.9.6, change DocumentRoot in
     default-tls.conf to /var/www/html accordingly.
Checksums-Sha1:
 9cf1df0b5fee113679b0c25402b1e4082dccf0f7 1639 mod-gnutls_0.6-1.4.dsc
 1faf95fb219aaa7cfe4fbc2efb0a76aac34d66c2 21616 mod-gnutls_0.6-1.4.debian.tar.xz
Checksums-Sha256:
 ae52d6938f9e832145fad5b01c36b4b6fb654f0dd53bf538a21cacf66a2bfe9b 1639 
mod-gnutls_0.6-1.4.dsc
 77369932215b506124fb413fd9bfceb7655a7b82090ec809297b47b8beddc7ca 21616 
mod-gnutls_0.6-1.4.debian.tar.xz
Files:
 41717293794668a17661332914236da6 1639 httpd extra mod-gnutls_0.6-1.4.dsc
 74f47865decda55dc8121e1134ec65fb 21616 httpd extra 
mod-gnutls_0.6-1.4.debian.tar.xz

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQEcBAEBCAAGBQJVWwruAAoJEIYLf7sy+BGdHosIAK0XAnqCjPNhAVzAFMDCFe+G
X3Tmc1kk5NxJFId3bNTrzvV61yCIxBF+lVUv5OlErqsHhmY0tvPLm9lQ+C+872Gm
DURpymNPzOJMHiOh5xtP0dckKxB2rOJDACXn/CMB8esF8zjSNA+OpxH6gfdTXjBR
9WbEL1BN/dOgEU1Bw67qrnN4MPrvCIRm9qkVIQUNYRJuPqDN419uHbYrVyLAllhw
AbMbtOaubzIZ4nKIR3oznTdGfecDO7NAPZLO4yIVx6xd2MZh8sHBMF7pAN3Fzo0n
xmMy7xOWAmXoyQrxgbiYVOcJKXwiA8FupFT1cnRmAFYyp0SJ7UmD1kYrIuzu/oQ=
=v2qN
-----END PGP SIGNATURE-----

--- End Message ---

Reply via email to