[mod_gnutls-devel] [PATCH 2/2] implement GnuTLSExportCertificates control over max exported cert size

Daniel Kahn Gillmor dkg at fifthhorseman.net
Sun Feb 23 19:15:23 CET 2014


This patchset implements the proposed modification to
GnuTLSExportCertificates, allowing server administrators to choose the
maximum size of the exported certs.

Some advantages:

 * avoids large buffers on the stack
 * more configurable for server admins who expect to use larger certs
 * better visibilty for users when a too-large-cert is encountered

This also increases the default maximum exported size from 10KiB to
16KiB.
---
 include/mod_gnutls.h.in |  4 +--
 src/gnutls_config.c     | 23 ++++++++----
 src/gnutls_hooks.c      | 94 +++++++++++++++++++++++++++++++------------------
 src/mod_gnutls.c        |  4 +--
 4 files changed, 80 insertions(+), 45 deletions(-)

diff --git a/include/mod_gnutls.h.in b/include/mod_gnutls.h.in
index 57aa52e..8aa4162 100644
--- a/include/mod_gnutls.h.in
+++ b/include/mod_gnutls.h.in
@@ -126,7 +126,7 @@ typedef struct {
 	/* Is the module enabled? */
     int enabled;
     /* Export full certificates to CGI environment: */
-    int export_certificates_enabled;
+    int export_certificates_size;
 	/* GnuTLS Priorities */
     gnutls_priority_t priorities;
 	/* GnuTLS DH Parameters */
@@ -359,7 +359,7 @@ const char *mgs_set_keyring_file(cmd_parms * parms, void *dummy,
 
 const char *mgs_set_enabled(cmd_parms * parms, void *dummy,
                             const char *arg);
-const char *mgs_set_export_certificates_enabled(cmd_parms * parms, void *dummy,
+const char *mgs_set_export_certificates_size(cmd_parms * parms, void *dummy,
                             const char *arg);
 const char *mgs_set_priorities(cmd_parms * parms, void *dummy,
                             const char *arg);
diff --git a/src/gnutls_config.c b/src/gnutls_config.c
index bff6b4c..3300854 100644
--- a/src/gnutls_config.c
+++ b/src/gnutls_config.c
@@ -18,6 +18,7 @@
  */
 
 #include "mod_gnutls.h"
+#include "apr_lib.h"
 
 #ifdef APLOG_USE_MODULE
 APLOG_USE_MODULE(gnutls);
@@ -553,15 +554,23 @@ const char *mgs_set_enabled(cmd_parms * parms, void *dummy,
     return NULL;
 }
 
-const char *mgs_set_export_certificates_enabled(cmd_parms * parms, void *dummy, const char *arg) {
+const char *mgs_set_export_certificates_size(cmd_parms * parms, void *dummy, const char *arg) {
     mgs_srvconf_rec *sc = (mgs_srvconf_rec *) ap_get_module_config(parms->server->module_config, &gnutls_module);
     if (!strcasecmp(arg, "On")) {
-        sc->export_certificates_enabled = GNUTLS_ENABLED_TRUE;
+        sc->export_certificates_size = 16 * 1024;
     } else if (!strcasecmp(arg, "Off")) {
-        sc->export_certificates_enabled = GNUTLS_ENABLED_FALSE;
+        sc->export_certificates_size = 0;
     } else {
-        return
-        "GnuTLSExportCertificates must be set to 'On' or 'Off'";
+        char* endptr;
+        sc->export_certificates_size = strtol(arg, &endptr, 10);
+        while (apr_isspace(*endptr)) endptr++;
+        if (*endptr == '\0' || *endptr == 'b' || *endptr == 'B') {
+            ;
+        } else if (*endptr == 'k' || *endptr == 'K') {
+            sc->export_certificates_size *= 1024;
+        } else {
+            return "GnuTLSExportCertificates must be set to a size (in bytes) or 'On' or 'Off'";
+        }
     }
 
     return NULL;
@@ -637,7 +646,7 @@ static mgs_srvconf_rec *_mgs_config_server_create(apr_pool_t * p, char** err) {
     sc->priorities = NULL;
     sc->dh_params = NULL;
     sc->proxy_enabled = GNUTLS_ENABLED_UNSET;
-    sc->export_certificates_enabled = GNUTLS_ENABLED_UNSET;
+    sc->export_certificates_size = -1;
     sc->client_verify_method = mgs_cvm_unset;
 
 /* this relies on GnuTLS never changing the gnutls_certificate_request_t enum to define -1 */
@@ -666,7 +675,7 @@ void *mgs_config_server_merge(apr_pool_t *p, void *BASE, void *ADD) {
     gnutls_srvconf_merge(enabled, GNUTLS_ENABLED_UNSET);
     gnutls_srvconf_merge(tickets, GNUTLS_ENABLED_UNSET);
     gnutls_srvconf_merge(proxy_enabled, GNUTLS_ENABLED_UNSET);
-    gnutls_srvconf_merge(export_certificates_enabled, GNUTLS_ENABLED_UNSET);
+    gnutls_srvconf_merge(export_certificates_size, -1);
     gnutls_srvconf_merge(client_verify_method, mgs_cvm_unset);
     gnutls_srvconf_merge(client_verify_mode, -1);
     gnutls_srvconf_merge(srp_tpasswd_file, NULL);
diff --git a/src/gnutls_hooks.c b/src/gnutls_hooks.c
index aee7b86..6f15f28 100644
--- a/src/gnutls_hooks.c
+++ b/src/gnutls_hooks.c
@@ -42,8 +42,8 @@ static gnutls_datum_t session_ticket_key = {NULL, 0};
 
 static int mgs_cert_verify(request_rec * r, mgs_handle_t * ctxt);
 /* use side==0 for server and side==1 for client */
-static void mgs_add_common_cert_vars(request_rec * r, gnutls_x509_crt_t cert, int side, int export_full_cert);
-static void mgs_add_common_pgpcert_vars(request_rec * r, gnutls_openpgp_crt_t cert, int side, int export_full_cert);
+static void mgs_add_common_cert_vars(request_rec * r, gnutls_x509_crt_t cert, int side, int export_cert_size);
+static void mgs_add_common_pgpcert_vars(request_rec * r, gnutls_openpgp_crt_t cert, int side, int export_cert_size);
 static const char* mgs_x509_construct_uid(request_rec * pool, gnutls_x509_crt_t cert);
 static int mgs_status_hook(request_rec *r, int flags);
 
@@ -372,8 +372,8 @@ int mgs_hook_post_config(apr_pool_t * p, apr_pool_t * plog, apr_pool_t * ptemp,
             sc->enabled = GNUTLS_ENABLED_FALSE;
         if (sc->tickets == GNUTLS_ENABLED_UNSET)
             sc->tickets = GNUTLS_ENABLED_TRUE;
-        if (sc->export_certificates_enabled == GNUTLS_ENABLED_UNSET)
-            sc->export_certificates_enabled = GNUTLS_ENABLED_FALSE;
+        if (sc->export_certificates_size < 0)
+            sc->export_certificates_size = 0;
         if (sc->client_verify_mode ==  -1)
             sc->client_verify_mode = GNUTLS_CERT_IGNORE;
         if (sc->client_verify_method ==  mgs_cvm_unset)
@@ -817,9 +817,9 @@ int mgs_hook_fixups(request_rec * r) {
     apr_table_setn(env, "SSL_SESSION_ID", apr_pstrdup(r->pool, tmp));
 
     if (gnutls_certificate_type_get(ctxt->session) == GNUTLS_CRT_X509) {
-		mgs_add_common_cert_vars(r, ctxt->sc->certs_x509_chain[0], 0, ctxt->sc->export_certificates_enabled);
+		mgs_add_common_cert_vars(r, ctxt->sc->certs_x509_chain[0], 0, ctxt->sc->export_certificates_size);
 	} else if (gnutls_certificate_type_get(ctxt->session) == GNUTLS_CRT_OPENPGP) {
-        mgs_add_common_pgpcert_vars(r, ctxt->sc->cert_pgp, 0, ctxt->sc->export_certificates_enabled);
+        mgs_add_common_pgpcert_vars(r, ctxt->sc->cert_pgp, 0, ctxt->sc->export_certificates_size);
 	}
 
     return rv;
@@ -893,12 +893,12 @@ int mgs_hook_authz(request_rec * r) {
 
 /* @param side is either 0 for SERVER or 1 for CLIENT
  *
- * @param export_full_cert (boolean) export the PEM-encoded
- * certificate in full as an environment variable.
+ * @param export_cert_size (int) maximum size for environment variable
+ * to use for the PEM-encoded certificate (0 means do not export)
  */
 #define MGS_SIDE ((side==0)?"SSL_SERVER":"SSL_CLIENT")
 
-static void mgs_add_common_cert_vars(request_rec * r, gnutls_x509_crt_t cert, int side, int export_full_cert) {
+static void mgs_add_common_cert_vars(request_rec * r, gnutls_x509_crt_t cert, int side, int export_cert_size) {
     unsigned char sbuf[64]; /* buffer to hold serials */
     char buf[AP_IOBUFSIZE];
     const char *tmp;
@@ -912,16 +912,29 @@ static void mgs_add_common_cert_vars(request_rec * r, gnutls_x509_crt_t cert, in
     apr_table_t *env = r->subprocess_env;
 
     _gnutls_log(debug_log_fp, "%s: %d\n", __func__, __LINE__);
-    if (export_full_cert != 0) {
-        char cert_buf[10 * 1024];
-        len = sizeof (cert_buf);
-
-        if (gnutls_x509_crt_export(cert, GNUTLS_X509_FMT_PEM, cert_buf, &len) >= 0)
-            apr_table_setn(env, apr_pstrcat(r->pool, MGS_SIDE, "_CERT", NULL),
-                           apr_pstrmemdup(r->pool, cert_buf, len));
-        else
-            ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r,
-                          "GnuTLS: Failed to export X.509 certificate to environment");
+    if (export_cert_size > 0) {
+        len = 0;
+        ret = gnutls_x509_crt_export(cert, GNUTLS_X509_FMT_PEM, NULL, &len);
+        if (ret == GNUTLS_E_SHORT_MEMORY_BUFFER) {
+            if (len >= export_cert_size) {
+                apr_table_setn(env, apr_pstrcat(r->pool, MGS_SIDE, "_CERT", NULL),
+                               "GNUTLS_CERTIFICATE_SIZE_LIMIT_EXCEEDED");
+                ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r,
+                              "GnuTLS: Failed to export too-large X.509 certificate to environment");
+            } else {
+                char* cert_buf = apr_palloc(r->pool, len + 1);
+                if (cert_buf != NULL && gnutls_x509_crt_export(cert, GNUTLS_X509_FMT_PEM, cert_buf, &len) >= 0) {
+                    cert_buf[len] = 0;
+                    apr_table_setn(env, apr_pstrcat(r->pool, MGS_SIDE, "_CERT", NULL), cert_buf);
+                } else {
+                    ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r,
+                                  "GnuTLS: failed to export X.509 certificate");
+                }
+            }
+        } else {
+            ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r,
+                          "GnuTLS: dazed and confused about X.509 certificate size");
+        }
     }
 
     len = sizeof (buf);
@@ -1036,10 +1049,10 @@ static void mgs_add_common_cert_vars(request_rec * r, gnutls_x509_crt_t cert, in
 
 /* @param side 0: server, 1: client
  *
- * @param export_full_cert (boolean) export the PEM-encoded
- * certificate in full as an environment variable.
+ * @param export_cert_size (int) maximum size for environment variable
+ * to use for the PEM-encoded certificate (0 means do not export)
  */
-static void mgs_add_common_pgpcert_vars(request_rec * r, gnutls_openpgp_crt_t cert, int side, int export_full_cert) {
+static void mgs_add_common_pgpcert_vars(request_rec * r, gnutls_openpgp_crt_t cert, int side, int export_cert_size) {
 
 	unsigned char sbuf[64]; /* buffer to hold serials */
     char buf[AP_IOBUFSIZE];
@@ -1053,16 +1066,29 @@ static void mgs_add_common_pgpcert_vars(request_rec * r, gnutls_openpgp_crt_t ce
     _gnutls_log(debug_log_fp, "%s: %d\n", __func__, __LINE__);
     apr_table_t *env = r->subprocess_env;
 
-    if (export_full_cert != 0) {
-        char cert_buf[10 * 1024];
-        len = sizeof (cert_buf);
-
-        if (gnutls_openpgp_crt_export(cert, GNUTLS_OPENPGP_FMT_BASE64, cert_buf, &len) >= 0)
-            apr_table_setn(env, apr_pstrcat(r->pool, MGS_SIDE, "_CERT", NULL),
-                           apr_pstrmemdup(r->pool, cert_buf, len));
-        else
-            ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r,
-                          "GnuTLS: Failed to export OpenPGP certificate to environment");
+    if (export_cert_size > 0) {
+        len = 0;
+        ret = gnutls_openpgp_crt_export(cert, GNUTLS_OPENPGP_FMT_BASE64, NULL, &len);
+        if (ret == GNUTLS_E_SHORT_MEMORY_BUFFER) {
+            if (len >= export_cert_size) {
+                apr_table_setn(env, apr_pstrcat(r->pool, MGS_SIDE, "_CERT", NULL),
+                               "GNUTLS_CERTIFICATE_SIZE_LIMIT_EXCEEDED");
+                ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r,
+                              "GnuTLS: Failed to export too-large OpenPGP certificate to environment");
+            } else {
+                char* cert_buf = apr_palloc(r->pool, len + 1);
+                if (cert_buf != NULL && gnutls_openpgp_crt_export(cert, GNUTLS_OPENPGP_FMT_BASE64, cert_buf, &len) >= 0) {
+                    cert_buf[len] = 0;
+                    apr_table_setn(env, apr_pstrcat(r->pool, MGS_SIDE, "_CERT", NULL), cert_buf);
+                } else {
+                    ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r,
+                                  "GnuTLS: failed to export OpenPGP certificate");
+                }
+            }
+        } else {
+            ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r,
+                          "GnuTLS: dazed and confused about OpenPGP certificate size");
+        }
     }
 
     len = sizeof (buf);
@@ -1327,9 +1353,9 @@ static int mgs_cert_verify(request_rec * r, mgs_handle_t * ctxt) {
     }
 
     if (gnutls_certificate_type_get(ctxt->session) == GNUTLS_CRT_X509)
-        mgs_add_common_cert_vars(r, cert.x509[0], 1, ctxt->sc->export_certificates_enabled);
+        mgs_add_common_cert_vars(r, cert.x509[0], 1, ctxt->sc->export_certificates_size);
     else if (gnutls_certificate_type_get(ctxt->session) == GNUTLS_CRT_OPENPGP)
-        mgs_add_common_pgpcert_vars(r, cert.pgp, 1, ctxt->sc->export_certificates_enabled);
+        mgs_add_common_pgpcert_vars(r, cert.pgp, 1, ctxt->sc->export_certificates_size);
 
     {
         /* days remaining */
diff --git a/src/mod_gnutls.c b/src/mod_gnutls.c
index e576fb8..14e8320 100644
--- a/src/mod_gnutls.c
+++ b/src/mod_gnutls.c
@@ -180,10 +180,10 @@ static const command_rec mgs_config_cmds[] = {
     RSRC_CONF,
     "Whether this server has GnuTLS Enabled. Default: Off"),
     AP_INIT_TAKE1("GnuTLSExportCertificates",
-    mgs_set_export_certificates_enabled,
+    mgs_set_export_certificates_size,
     NULL,
     RSRC_CONF,
-    "Whether to export PEM encoded certificates to CGIs. Default: Off"),
+    "Max size to export PEM encoded certificates to CGIs (or off to disable). Default: off"),
     { NULL },
 };
 
-- 
1.8.5.3




More information about the mod_gnutls-devel mailing list