BUG/MEDIUM: ssl: in bind line, ssl-options after 'crt' are ignored.

Bug introduced with "removes SSL_CTX_set_ssl_version call and cleanup CTX
creation": ssl_sock_new_ctx is called before all the bind line is parsed.
The fix consists of separating the use of default_ctx as the initialization
context of the SSL connection via bind_conf->initial_ctx. Initial_ctx contains
all the necessary parameters before performing the selection of the CTX:
default_ctx is processed as others ctx without unnecessary parameters.
This commit is contained in:
Emmanuel Hocdet 2017-03-06 15:34:44 +01:00 committed by Willy Tarreau
parent 4608ed9511
commit f6b37c67be
2 changed files with 45 additions and 31 deletions

View File

@ -141,6 +141,7 @@ struct bind_conf {
struct ssl_bind_conf ssl_conf; /* ssl conf for ctx setting */ struct ssl_bind_conf ssl_conf; /* ssl conf for ctx setting */
unsigned long long ca_ignerr; /* ignored verify errors in handshake if depth > 0 */ unsigned long long ca_ignerr; /* ignored verify errors in handshake if depth > 0 */
unsigned long long crt_ignerr; /* ignored verify errors in handshake if depth == 0 */ unsigned long long crt_ignerr; /* ignored verify errors in handshake if depth == 0 */
SSL_CTX *initial_ctx; /* SSL context for initial negotiation */
SSL_CTX *default_ctx; /* SSL context of first/default certificate */ SSL_CTX *default_ctx; /* SSL context of first/default certificate */
struct ssl_bind_conf *default_ssl_conf; /* custom SSL conf of default_ctx */ struct ssl_bind_conf *default_ssl_conf; /* custom SSL conf of default_ctx */
int strict_sni; /* refuse negotiation if sni doesn't match a certificate */ int strict_sni; /* refuse negotiation if sni doesn't match a certificate */

View File

@ -120,8 +120,6 @@
#define HASH_FUNCT EVP_sha256 #define HASH_FUNCT EVP_sha256
#endif /* OPENSSL_NO_SHA256 */ #endif /* OPENSSL_NO_SHA256 */
static SSL_CTX *ssl_sock_new_ctx(struct bind_conf *bind_conf);
/* server and bind verify method, it uses a global value as default */ /* server and bind verify method, it uses a global value as default */
enum { enum {
SSL_SOCK_VERIFY_DEFAULT = 0, SSL_SOCK_VERIFY_DEFAULT = 0,
@ -1628,8 +1626,10 @@ static int ssl_sock_switchctx_cbk(const struct ssl_early_callback_ctx *ctx)
} }
} else { } else {
/* without SNI extension, is the default_ctx (need SSL_TLSEXT_ERR_NOACK) */ /* without SNI extension, is the default_ctx (need SSL_TLSEXT_ERR_NOACK) */
if (!s->strict_sni) if (!s->strict_sni) {
ssl_sock_switchctx_set(ctx->ssl, s->default_ctx);
return 1; return 1;
}
goto abort; goto abort;
} }
@ -1753,9 +1753,11 @@ static int ssl_sock_switchctx_cbk(const struct ssl_early_callback_ctx *ctx)
ssl_sock_switchctx_set(ctx->ssl, container_of(node, struct sni_ctx, name)->ctx); ssl_sock_switchctx_set(ctx->ssl, container_of(node, struct sni_ctx, name)->ctx);
return 1; return 1;
} }
if (!s->strict_sni) if (!s->strict_sni) {
/* no certificate match, is the default_ctx */ /* no certificate match, is the default_ctx */
ssl_sock_switchctx_set(ctx->ssl, s->default_ctx);
return 1; return 1;
}
abort: abort:
/* abort handshake (was SSL_TLSEXT_ERR_ALERT_FATAL) */ /* abort handshake (was SSL_TLSEXT_ERR_ALERT_FATAL) */
conn->err_code = CO_ER_SSL_HANDSHAKE; conn->err_code = CO_ER_SSL_HANDSHAKE;
@ -1797,9 +1799,10 @@ static int ssl_sock_switchctx_cbk(SSL *ssl, int *al, void *priv)
} }
} }
#endif #endif
return (s->strict_sni ? if (s->strict_sni)
SSL_TLSEXT_ERR_ALERT_FATAL : return SSL_TLSEXT_ERR_ALERT_FATAL;
SSL_TLSEXT_ERR_NOACK); ssl_sock_switchctx_set(ssl, s->default_ctx);
return SSL_TLSEXT_ERR_NOACK;
} }
for (i = 0; i < trash.size; i++) { for (i = 0; i < trash.size; i++) {
@ -1834,9 +1837,10 @@ static int ssl_sock_switchctx_cbk(SSL *ssl, int *al, void *priv)
return SSL_TLSEXT_ERR_OK; return SSL_TLSEXT_ERR_OK;
} }
#endif #endif
return (s->strict_sni ? if (s->strict_sni)
SSL_TLSEXT_ERR_ALERT_FATAL : return SSL_TLSEXT_ERR_ALERT_FATAL;
SSL_TLSEXT_ERR_OK); ssl_sock_switchctx_set(ssl, s->default_ctx);
return SSL_TLSEXT_ERR_OK;
} }
/* switch ctx */ /* switch ctx */
@ -2536,7 +2540,7 @@ static int ssl_sock_load_multi_cert(const char *path, struct bind_conf *bind_con
if (cur_ctx == NULL) { if (cur_ctx == NULL) {
/* need to create SSL_CTX */ /* need to create SSL_CTX */
cur_ctx = ssl_sock_new_ctx(bind_conf); cur_ctx = SSL_CTX_new(SSLv23_server_method());
if (cur_ctx == NULL) { if (cur_ctx == NULL) {
memprintf(err, "%sunable to allocate SSL context.\n", memprintf(err, "%sunable to allocate SSL context.\n",
err && *err ? *err : ""); err && *err ? *err : "");
@ -2762,7 +2766,7 @@ static int ssl_sock_load_cert_file(const char *path, struct bind_conf *bind_conf
int ret; int ret;
SSL_CTX *ctx; SSL_CTX *ctx;
ctx = ssl_sock_new_ctx(bind_conf); ctx = SSL_CTX_new(SSLv23_server_method());
if (!ctx) { if (!ctx) {
memprintf(err, "%sunable to allocate SSL context for cert '%s'.\n", memprintf(err, "%sunable to allocate SSL context for cert '%s'.\n",
err && *err ? *err : "", path); err && *err ? *err : "", path);
@ -3176,9 +3180,9 @@ int ssl_sock_load_cert_list_file(char *file, struct bind_conf *bind_conf, struct
#endif #endif
/* create an SSL_CTX according method wanted */ /* Create an initial CTX used to start the SSL connection before switchctx */
static SSL_CTX * static SSL_CTX *
ssl_sock_new_ctx(struct bind_conf *bind_conf) ssl_sock_initial_ctx(struct bind_conf *bind_conf)
{ {
SSL_CTX *ctx = NULL; SSL_CTX *ctx = NULL;
long ssloptions = long ssloptions =
@ -3227,6 +3231,16 @@ ssl_sock_new_ctx(struct bind_conf *bind_conf)
SSL_CTX_set_mode(ctx, sslmode); SSL_CTX_set_mode(ctx, sslmode);
if (global_ssl.life_time) if (global_ssl.life_time)
SSL_CTX_set_timeout(ctx, global_ssl.life_time); SSL_CTX_set_timeout(ctx, global_ssl.life_time);
#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
#ifdef OPENSSL_IS_BORINGSSL
SSL_CTX_set_select_certificate_cb(ctx, ssl_sock_switchctx_cbk);
SSL_CTX_set_tlsext_servername_callback(ctx, ssl_sock_switchctx_err_cbk);
#else
SSL_CTX_set_tlsext_servername_callback(ctx, ssl_sock_switchctx_cbk);
SSL_CTX_set_tlsext_servername_arg(ctx, bind_conf);
#endif
#endif
return ctx; return ctx;
} }
@ -3239,11 +3253,6 @@ int ssl_sock_prepare_ctx(struct bind_conf *bind_conf, struct ssl_bind_conf *ssl_
const char *conf_ciphers; const char *conf_ciphers;
const char *conf_curves = NULL; const char *conf_curves = NULL;
/* Make sure openssl opens /dev/urandom before the chroot */
if (!ssl_initialize_random()) {
Alert("OpenSSL random data generator initialization failed.\n");
cfgerr++;
}
switch ((ssl_conf && ssl_conf->verify) ? ssl_conf->verify : bind_conf->ssl_conf.verify) { switch ((ssl_conf && ssl_conf->verify) ? ssl_conf->verify : bind_conf->ssl_conf.verify) {
case SSL_SOCK_VERIFY_NONE: case SSL_SOCK_VERIFY_NONE:
verify = SSL_VERIFY_NONE; verify = SSL_VERIFY_NONE;
@ -3397,16 +3406,6 @@ int ssl_sock_prepare_ctx(struct bind_conf *bind_conf, struct ssl_bind_conf *ssl_
if (ssl_conf_cur) if (ssl_conf_cur)
SSL_CTX_set_alpn_select_cb(ctx, ssl_sock_advertise_alpn_protos, ssl_conf_cur); SSL_CTX_set_alpn_select_cb(ctx, ssl_sock_advertise_alpn_protos, ssl_conf_cur);
#endif #endif
#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
#ifdef OPENSSL_IS_BORINGSSL
SSL_CTX_set_select_certificate_cb(ctx, ssl_sock_switchctx_cbk);
SSL_CTX_set_tlsext_servername_callback(ctx, ssl_sock_switchctx_err_cbk);
#else
SSL_CTX_set_tlsext_servername_callback(ctx, ssl_sock_switchctx_cbk);
SSL_CTX_set_tlsext_servername_arg(ctx, bind_conf);
#endif
#endif
#if OPENSSL_VERSION_NUMBER >= 0x1000200fL #if OPENSSL_VERSION_NUMBER >= 0x1000200fL
conf_curves = (ssl_conf && ssl_conf->curves) ? ssl_conf->curves : bind_conf->ssl_conf.curves; conf_curves = (ssl_conf && ssl_conf->curves) ? ssl_conf->curves : bind_conf->ssl_conf.curves;
if (conf_curves) { if (conf_curves) {
@ -3738,6 +3737,19 @@ int ssl_sock_prepare_all_ctx(struct bind_conf *bind_conf)
/* Automatic memory computations need to know we use SSL there */ /* Automatic memory computations need to know we use SSL there */
global.ssl_used_frontend = 1; global.ssl_used_frontend = 1;
/* Make sure openssl opens /dev/urandom before the chroot */
if (!ssl_initialize_random()) {
Alert("OpenSSL random data generator initialization failed.\n");
err++;
}
/* Create initial_ctx used to start the ssl connection before do switchctx */
if (!bind_conf->initial_ctx) {
bind_conf->initial_ctx = ssl_sock_initial_ctx(bind_conf);
/* It should not be necessary to call this function, but it's
necessary first to check and move all initialisation related
to initial_ctx in ssl_sock_initial_ctx. */
err += ssl_sock_prepare_ctx(bind_conf, NULL, bind_conf->initial_ctx);
}
if (bind_conf->default_ctx) if (bind_conf->default_ctx)
err += ssl_sock_prepare_ctx(bind_conf, bind_conf->default_ssl_conf, bind_conf->default_ctx); err += ssl_sock_prepare_ctx(bind_conf, bind_conf->default_ssl_conf, bind_conf->default_ctx);
@ -3852,7 +3864,8 @@ void ssl_sock_free_all_ctx(struct bind_conf *bind_conf)
free(sni); free(sni);
node = back; node = back;
} }
SSL_CTX_free(bind_conf->initial_ctx);
bind_conf->initial_ctx = NULL;
bind_conf->default_ctx = NULL; bind_conf->default_ctx = NULL;
bind_conf->default_ssl_conf = NULL; bind_conf->default_ssl_conf = NULL;
} }
@ -4033,7 +4046,7 @@ static int ssl_sock_init(struct connection *conn)
retry_accept: retry_accept:
/* Alloc a new SSL session ctx */ /* Alloc a new SSL session ctx */
conn->xprt_ctx = SSL_new(objt_listener(conn->target)->bind_conf->default_ctx); conn->xprt_ctx = SSL_new(objt_listener(conn->target)->bind_conf->initial_ctx);
if (!conn->xprt_ctx) { if (!conn->xprt_ctx) {
if (may_retry--) { if (may_retry--) {
pool_gc2(); pool_gc2();