[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