From b4e9ba4b3618fe49ceceb325d58b17c68aeb6fca Mon Sep 17 00:00:00 2001 From: Emmanuel Hocdet Date: Thu, 30 Mar 2017 19:25:07 +0200 Subject: [PATCH] MEDIUM: ssl: calculate the real min/max TLS version and find holes Plan is to add min-tlsxx max-tlsxx configuration, more consistent than no-tlsxx. Find the real min/max versions (openssl capabilities and haproxy configuration) and generate warning with bad versions range. 'no-tlsxx' can generate 'holes': "The list of protocols available can be further limited using the SSL_OP_NO_X options of the SSL_CTX_set_options or SSL_set_options functions. Clients should avoid creating 'holes' in the set of protocols they support, when disabling a protocol, make sure that you also disable either all previous or all subsequent protocol versions. In clients, when a protocol version is disabled without disabling all previous protocol versions, the effect is to also disable all subsequent protocol versions." To not break compatibility, "holes" is authorized with warning, because openssl 1.1.0 and boringssl deal with it (keep the upper or lower range depending the case and version). --- src/ssl_sock.c | 96 ++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 74 insertions(+), 22 deletions(-) diff --git a/src/ssl_sock.c b/src/ssl_sock.c index c43a33084..9cbdad93f 100644 --- a/src/ssl_sock.c +++ b/src/ssl_sock.c @@ -3271,27 +3271,53 @@ ssl_sock_initial_ctx(struct bind_conf *bind_conf) SSL_MODE_RELEASE_BUFFERS | SSL_MODE_SMALL_BUFFERS; struct tls_version_filter *conf_ssl_methods = &bind_conf->ssl_methods; - int i, min, max; + int i, min, max, hole; ctx = SSL_CTX_new(SSLv23_server_method()); - /* set options per default */ - /* XXX need check hole */ - for (i = CONF_TLSV_MIN; i <= CONF_TLSV_MAX; i++) - if (conf_ssl_methods->flags & methodVersions[i].flag) - options |= methodVersions[i].option; + /* Real min and max should be determinate with configuration and openssl's capabilities */ + if (conf_ssl_methods->min) + conf_ssl_methods->flags |= (methodVersions[conf_ssl_methods->min].flag - 1); + if (conf_ssl_methods->max) + conf_ssl_methods->flags |= ~((methodVersions[conf_ssl_methods->max].flag << 1) - 1); + + /* Find min, max and holds */ + min = max = CONF_TLSV_NONE; + hole = 0; + for (i = CONF_TLSV_MIN; i <= CONF_TLSV_MAX; i++) + /* version is in openssl && version not disable in configuration */ + if (methodVersions[i].option && !(conf_ssl_methods->flags & methodVersions[i].flag)) { + if (min) { + if (hole) { + Warning("Proxy '%s': SSL/TLS versions range not contiguous for bind '%s' at [%s:%d]. " + "Hole find for %s. Use only 'min-tlsvX' and 'max-tlsvY' to fix.\n", + bind_conf->frontend->id, bind_conf->arg, bind_conf->file, bind_conf->line, + methodVersions[hole].name); + hole = 0; + } + max = i; + } + else { + min = max = i; + } + } + else { + if (min) + hole = i; + /* set SSL_NO_VERSION per default */ + options |= methodVersions[i].option; + } + if (!min) + Warning("Proxy '%s': all SSL/TLS versions are disabled for bind '%s' at [%s:%d].\n", + bind_conf->frontend->id, bind_conf->arg, bind_conf->file, bind_conf->line); - /* XXX min and max can be CONF_TLSV_NONE or unavailable in openssl. - Real min and max should be determinate with conf + openssl's capabilities - */ - min = conf_ssl_methods->min; - max = conf_ssl_methods->max; #if (OPENSSL_VERSION_NUMBER < 0x1010000fL) && !defined(OPENSSL_IS_BORINGSSL) /* Keep force-xxx implementation as it is in older haproxy. It's a precautionary measure to avoid any suprise with older openssl version. */ if (min == max) methodVersions[min].set_version(ctx, 1 /* server */); #else /* openssl >= 1.1.0 */ + /* set the max_version is required to cap TLS version or activate new TLS (v1.3) */ methodVersions[min].set_version(ctx, 0); methodVersions[max].set_version(ctx, 1); #endif @@ -3657,7 +3683,7 @@ int ssl_sock_prepare_srv_ctx(struct server *srv) int verify = SSL_VERIFY_NONE; SSL_CTX *ctx = NULL; struct tls_version_filter *conf_ssl_methods = &srv->ssl_ctx.methods; - int i, min, max; + int i, min, max, hole; /* Make sure openssl opens /dev/urandom before the chroot */ if (!ssl_initialize_random()) { @@ -3684,23 +3710,49 @@ int ssl_sock_prepare_srv_ctx(struct server *srv) return cfgerr; } - /* set options per default */ - /* XXX need check hole */ - for (i = CONF_TLSV_MIN; i <= CONF_TLSV_MAX; i++) - if (conf_ssl_methods->flags & methodVersions[i].flag) - options |= methodVersions[i].option; + /* Real min and max should be determinate with configuration and openssl's capabilities */ + if (conf_ssl_methods->min) + conf_ssl_methods->flags |= (methodVersions[conf_ssl_methods->min].flag - 1); + if (conf_ssl_methods->max) + conf_ssl_methods->flags |= ~((methodVersions[conf_ssl_methods->max].flag << 1) - 1); + + /* find min, max and holds */ + min = max = CONF_TLSV_NONE; + hole = 0; + for (i = CONF_TLSV_MIN; i <= CONF_TLSV_MAX; i++) + /* version is in openssl && version not disable in configuration */ + if (methodVersions[i].option && !(conf_ssl_methods->flags & methodVersions[i].flag)) { + if (min) { + if (hole) { + Warning("config : %s '%s': SSL/TLS versions range not contiguous for server '%s'. " + "Hole find for %s. Use only 'min-tlsvX' and 'max-tlsvY' to fix.\n", + proxy_type_str(curproxy), curproxy->id, srv->id, + methodVersions[hole].name); + hole = 0; + } + max = i; + } + else { + min = max = i; + } + } + else { + if (min) + hole = i; + /* set SSL_NO_VERSION per default */ + options |= methodVersions[i].option; + } + if (!min) + Warning("config : %s '%s': all SSL/TLS versions are disabled for server '%s'.\n", + proxy_type_str(curproxy), curproxy->id, srv->id); - /* XXX min and max can be CONF_TLSV_NONE or unavailable in openssl. - Real min and max should be determinate with conf + openssl's capabilities - */ - min = conf_ssl_methods->min; - max = conf_ssl_methods->max; #if (OPENSSL_VERSION_NUMBER < 0x1010000fL) && !defined(OPENSSL_IS_BORINGSSL) /* Keep force-xxx implementation as it is in older haproxy. It's a precautionary measure to avoid any suprise with older openssl version. */ if (min == max) methodVersions[min].set_version(ctx, 0 /* client */); #else /* openssl >= 1.1.0 */ + /* set the max_version is required to cap TLS version or activate new TLS (v1.3) */ methodVersions[min].set_version(ctx, 0); methodVersions[max].set_version(ctx, 1); #endif