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 <thomas2.kl...@uni-dortmund.de>
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 <thomas2.kl...@uni-dortmund.de>
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 <thomas2.kl...@uni-dortmund.de>
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
