diff options
author | Stefan Bühler <stbuehler@web.de> | 2013-11-05 15:29:07 +0000 |
---|---|---|
committer | Stefan Bühler <stbuehler@web.de> | 2013-11-05 15:29:07 +0000 |
commit | 1af871fcef97574c71870309d572d6b1026ee605 (patch) | |
tree | 5b6312ab52f646e43583bf8a5d60f523bd2a9319 | |
parent | 3ce548c8d0759c201ba0bcecbc45661cfd22c792 (diff) | |
download | lighttpd-git-1af871fcef97574c71870309d572d6b1026ee605.tar.gz |
[ssl] fix SNI handling; only use key+cert+verify-client from SNI specific config (fixes #2525, CVE-2013-4508)
pull all ssl.ca-file values into all SSL_CTXs, but use only the local
ssl.ca-file for verify-client; correct SNI name is no requirement,
so enforcing verification for a subset of SNI names doesn't actually
protect those.
From: Stefan Bühler <stbuehler@web.de>
git-svn-id: svn://svn.lighttpd.net/lighttpd/branches/lighttpd-1.4.x@2913 152afb58-edef-0310-8abb-c4023f1b3aa9
-rw-r--r-- | NEWS | 1 | ||||
-rw-r--r-- | src/base.h | 6 | ||||
-rw-r--r-- | src/configfile.c | 12 | ||||
-rw-r--r-- | src/network.c | 219 | ||||
-rw-r--r-- | src/server.c | 3 |
5 files changed, 191 insertions, 50 deletions
@@ -6,6 +6,7 @@ NEWS - 1.4.34 * [mod_auth] explicitly link ssl for SHA1 (fixes #2517) * [mod_extforward] fix compilation without IPv6, (not) using undefined var (fixes #2515, thx mm) + * [ssl] fix SNI handling; only use key+cert from SNI specific config (fixes #2525, CVE-2013-4508) - 1.4.33 - 2013-09-27 * mod_fastcgi: fix mix up of "mode" => "authorizer" in other fastcgi configs (fixes #2465, thx peex) @@ -320,7 +320,11 @@ typedef struct { off_t *global_bytes_per_second_cnt_ptr; /* */ #ifdef USE_OPENSSL - SSL_CTX *ssl_ctx; + SSL_CTX *ssl_ctx; /* not patched */ + /* SNI per host: with COMP_SERVER_SOCKET, COMP_HTTP_SCHEME, COMP_HTTP_HOST */ + EVP_PKEY *ssl_pemfile_pkey; + X509 *ssl_pemfile_x509; + STACK_OF(X509_NAME) *ssl_ca_file_cert_names; #endif } specific_config; diff --git a/src/configfile.c b/src/configfile.c index 7408ed0a..18b36b37 100644 --- a/src/configfile.c +++ b/src/configfile.c @@ -339,9 +339,13 @@ int config_setup_connection(server *srv, connection *con) { PATCH(ssl_pemfile); #ifdef USE_OPENSSL - PATCH(ssl_ctx); + PATCH(ssl_pemfile_x509); + PATCH(ssl_pemfile_pkey); #endif PATCH(ssl_ca_file); +#ifdef USE_OPENSSL + PATCH(ssl_ca_file_cert_names); +#endif PATCH(ssl_cipher_list); PATCH(ssl_dh_file); PATCH(ssl_ec_curve); @@ -409,10 +413,14 @@ int config_patch_connection(server *srv, connection *con, comp_key_t comp) { } else if (buffer_is_equal_string(du->key, CONST_STR_LEN("ssl.pemfile"))) { PATCH(ssl_pemfile); #ifdef USE_OPENSSL - PATCH(ssl_ctx); + PATCH(ssl_pemfile_x509); + PATCH(ssl_pemfile_pkey); #endif } else if (buffer_is_equal_string(du->key, CONST_STR_LEN("ssl.ca-file"))) { PATCH(ssl_ca_file); +#ifdef USE_OPENSSL + PATCH(ssl_ca_file_cert_names); +#endif } else if (buffer_is_equal_string(du->key, CONST_STR_LEN("ssl.honor-cipher-order"))) { PATCH(ssl_honor_cipher_order); } else if (buffer_is_equal_string(du->key, CONST_STR_LEN("ssl.empty-fragments"))) { diff --git a/src/network.c b/src/network.c index cb0564f1..f6d890b6 100644 --- a/src/network.c +++ b/src/network.c @@ -112,20 +112,46 @@ static int network_ssl_servername_callback(SSL *ssl, int *al, server *srv) { config_patch_connection(srv, con, COMP_HTTP_SCHEME); config_patch_connection(srv, con, COMP_HTTP_HOST); - if (NULL == con->conf.ssl_ctx) { - /* ssl_ctx <=> pemfile was set <=> ssl_ctx got patched: so this should never happen */ + if (NULL == con->conf.ssl_pemfile_x509 || NULL == con->conf.ssl_pemfile_pkey) { + /* x509/pkey available <=> pemfile was set <=> pemfile got patched: so this should never happen, unless you nest $SERVER["socket"] */ log_error_write(srv, __FILE__, __LINE__, "ssb", "SSL:", - "null SSL_CTX for TLS server name", con->tlsext_server_name); + "no certificate/private key for TLS server name", con->tlsext_server_name); return SSL_TLSEXT_ERR_ALERT_FATAL; } - /* switch to new SSL_CTX in reaction to a client's server_name extension */ - if (con->conf.ssl_ctx != SSL_set_SSL_CTX(ssl, con->conf.ssl_ctx)) { - log_error_write(srv, __FILE__, __LINE__, "ssb", "SSL:", - "failed to set SSL_CTX for TLS server name", con->tlsext_server_name); + /* first set certificate! setting private key checks whether certificate matches it */ + if (!SSL_use_certificate(ssl, con->conf.ssl_pemfile_x509)) { + log_error_write(srv, __FILE__, __LINE__, "ssb:s", "SSL:", + "failed to set certificate for TLS server name", con->tlsext_server_name, + ERR_error_string(ERR_get_error(), NULL)); + return SSL_TLSEXT_ERR_ALERT_FATAL; + } + + if (!SSL_use_PrivateKey(ssl, con->conf.ssl_pemfile_pkey)) { + log_error_write(srv, __FILE__, __LINE__, "ssb:s", "SSL:", + "failed to set private key for TLS server name", con->tlsext_server_name, + ERR_error_string(ERR_get_error(), NULL)); return SSL_TLSEXT_ERR_ALERT_FATAL; } + if (con->conf.ssl_verifyclient) { + if (NULL == con->conf.ssl_ca_file_cert_names) { + log_error_write(srv, __FILE__, __LINE__, "ssb:s", "SSL:", + "can't verify client without ssl.ca-file for TLS server name", con->tlsext_server_name, + ERR_error_string(ERR_get_error(), NULL)); + return SSL_TLSEXT_ERR_ALERT_FATAL; + } + + SSL_set_client_CA_list(ssl, SSL_dup_CA_list(con->conf.ssl_ca_file_cert_names)); + /* forcing verification here is really not that useful - a client could just connect without SNI */ + SSL_set_verify( + ssl, + SSL_VERIFY_PEER | (con->conf.ssl_verifyclient_enforce ? SSL_VERIFY_FAIL_IF_NO_PEER_CERT : 0), + NULL + ); + SSL_set_verify_depth(ssl, con->conf.ssl_verifyclient_depth); + } + return SSL_TLSEXT_ERR_OK; } #endif @@ -491,9 +517,100 @@ typedef enum { NETWORK_BACKEND_SOLARIS_SENDFILEV } network_backend_t; +#ifdef USE_OPENSSL +static X509* x509_load_pem_file(server *srv, const char *file) { + BIO *in; + X509 *x = NULL; + + in = BIO_new(BIO_s_file()); + if (NULL == in) { + log_error_write(srv, __FILE__, __LINE__, "S", "SSL: BIO_new(BIO_s_file()) failed"); + goto error; + } + + if (BIO_read_filename(in,file) <= 0) { + log_error_write(srv, __FILE__, __LINE__, "SSS", "SSL: BIO_read_filename('", file,"') failed"); + goto error; + } + x = PEM_read_bio_X509(in, NULL, NULL, NULL); + + if (NULL == x) { + log_error_write(srv, __FILE__, __LINE__, "SSS", "SSL: couldn't read X509 certificate from '", file,"'"); + goto error; + } + + BIO_free(in); + return x; + +error: + if (NULL != x) X509_free(x); + if (NULL != in) BIO_free(in); + return NULL; +} + +static EVP_PKEY* evp_pkey_load_pem_file(server *srv, const char *file) { + BIO *in; + EVP_PKEY *x = NULL; + + in=BIO_new(BIO_s_file()); + if (NULL == in) { + log_error_write(srv, __FILE__, __LINE__, "s", "SSL: BIO_new(BIO_s_file()) failed"); + goto error; + } + + if (BIO_read_filename(in,file) <= 0) { + log_error_write(srv, __FILE__, __LINE__, "SSS", "SSL: BIO_read_filename('", file,"') failed"); + goto error; + } + x = PEM_read_bio_PrivateKey(in, NULL, NULL, NULL); + + if (NULL == x) { + log_error_write(srv, __FILE__, __LINE__, "SSS", "SSL: couldn't read private key from '", file,"'"); + goto error; + } + + BIO_free(in); + return x; + +error: + if (NULL != x) EVP_PKEY_free(x); + if (NULL != in) BIO_free(in); + return NULL; +} + +static int network_openssl_load_pemfile(server *srv, size_t ndx) { + specific_config *s = srv->config_storage[ndx]; + +#ifdef OPENSSL_NO_TLSEXT + { + data_config *dc = (data_config *)srv->config_context->data[i]; + if ((ndx > 0 && (COMP_SERVER_SOCKET != dc->comp || dc->cond != CONFIG_COND_EQ)) + || !s->ssl_enabled) { + log_error_write(srv, __FILE__, __LINE__, "ss", "SSL:", + "ssl.pemfile only works in SSL socket binding context as openssl version does not support TLS extensions"); + return -1; + } + } +#endif + + if (NULL == (s->ssl_pemfile_x509 = x509_load_pem_file(srv, s->ssl_pemfile->ptr))) return -1; + if (NULL == (s->ssl_pemfile_pkey = evp_pkey_load_pem_file(srv, s->ssl_pemfile->ptr))) return -1; + + if (!X509_check_private_key(s->ssl_pemfile_x509, s->ssl_pemfile_pkey)) { + log_error_write(srv, __FILE__, __LINE__, "sssb", "SSL:", + "Private key does not match the certificate public key, reason:", + ERR_error_string(ERR_get_error(), NULL), + s->ssl_pemfile); + return -1; + } + + return 0; +} +#endif + int network_init(server *srv) { buffer *b; - size_t i; + size_t i, j; network_backend_t backend; #if OPENSSL_VERSION_NUMBER >= 0x0090800fL @@ -580,18 +697,7 @@ int network_init(server *srv) { long ssloptions = SSL_OP_ALL | SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION | SSL_OP_NO_COMPRESSION; - if (buffer_is_empty(s->ssl_pemfile)) continue; - -#ifdef OPENSSL_NO_TLSEXT - { - data_config *dc = (data_config *)srv->config_context->data[i]; - if (COMP_HTTP_HOST == dc->comp) { - log_error_write(srv, __FILE__, __LINE__, "ss", "SSL:", - "can't use ssl.pemfile with $HTTP[\"host\"], openssl version does not support TLS extensions"); - return -1; - } - } -#endif + if (buffer_is_empty(s->ssl_pemfile) && buffer_is_empty(s->ssl_ca_file)) continue; if (srv->ssl_is_init == 0) { SSL_load_error_strings(); @@ -606,6 +712,29 @@ int network_init(server *srv) { } } + if (!buffer_is_empty(s->ssl_pemfile)) { +#ifdef OPENSSL_NO_TLSEXT + data_config *dc = (data_config *)srv->config_context->data[i]; + if (COMP_HTTP_HOST == dc->comp) { + log_error_write(srv, __FILE__, __LINE__, "ss", "SSL:", + "can't use ssl.pemfile with $HTTP[\"host\"], openssl version does not support TLS extensions"); + return -1; + } +#endif + if (network_openssl_load_pemfile(srv, i)) return -1; + } + + + if (!buffer_is_empty(s->ssl_ca_file)) { + s->ssl_ca_file_cert_names = SSL_load_client_CA_file(s->ssl_ca_file->ptr); + if (NULL == s->ssl_ca_file_cert_names) { + log_error_write(srv, __FILE__, __LINE__, "ssb", "SSL:", + ERR_error_string(ERR_get_error(), NULL), s->ssl_ca_file); + } + } + + if (buffer_is_empty(s->ssl_pemfile) || !s->ssl_enabled) continue; + if (NULL == (s->ssl_ctx = SSL_CTX_new(SSLv23_server_method()))) { log_error_write(srv, __FILE__, __LINE__, "ss", "SSL:", ERR_error_string(ERR_get_error(), NULL)); @@ -721,45 +850,42 @@ int network_init(server *srv) { #endif #endif - if (!buffer_is_empty(s->ssl_ca_file)) { - if (1 != SSL_CTX_load_verify_locations(s->ssl_ctx, s->ssl_ca_file->ptr, NULL)) { - log_error_write(srv, __FILE__, __LINE__, "ssb", "SSL:", - ERR_error_string(ERR_get_error(), NULL), s->ssl_ca_file); - return -1; - } - if (s->ssl_verifyclient) { - STACK_OF(X509_NAME) *certs = SSL_load_client_CA_file(s->ssl_ca_file->ptr); - if (!certs) { + /* load all ssl.ca-files specified in the config into each SSL_CTX to be prepared for SNI */ + for (j = 0; j < srv->config_context->used; j++) { + specific_config *s1 = srv->config_storage[j]; + + if (!buffer_is_empty(s1->ssl_ca_file)) { + if (1 != SSL_CTX_load_verify_locations(s->ssl_ctx, s1->ssl_ca_file->ptr, NULL)) { log_error_write(srv, __FILE__, __LINE__, "ssb", "SSL:", - ERR_error_string(ERR_get_error(), NULL), s->ssl_ca_file); - } - if (SSL_CTX_set_session_id_context(s->ssl_ctx, (void*) &srv, sizeof(srv)) != 1) { - log_error_write(srv, __FILE__, __LINE__, "ss", "SSL:", - ERR_error_string(ERR_get_error(), NULL)); + ERR_error_string(ERR_get_error(), NULL), s1->ssl_ca_file); return -1; } - SSL_CTX_set_client_CA_list(s->ssl_ctx, certs); - SSL_CTX_set_verify( - s->ssl_ctx, - SSL_VERIFY_PEER | (s->ssl_verifyclient_enforce ? SSL_VERIFY_FAIL_IF_NO_PEER_CERT : 0), - NULL + } + } + + if (s->ssl_verifyclient) { + if (NULL == s->ssl_ca_file_cert_names) { + log_error_write(srv, __FILE__, __LINE__, "s", + "SSL: You specified ssl.verifyclient.activate but no ca_file" ); - SSL_CTX_set_verify_depth(s->ssl_ctx, s->ssl_verifyclient_depth); + return -1; } - } else if (s->ssl_verifyclient) { - log_error_write( - srv, __FILE__, __LINE__, "s", - "SSL: You specified ssl.verifyclient.activate but no ca_file" + SSL_CTX_set_client_CA_list(s->ssl_ctx, SSL_dup_CA_list(s->ssl_ca_file_cert_names)); + SSL_CTX_set_verify( + s->ssl_ctx, + SSL_VERIFY_PEER | (s->ssl_verifyclient_enforce ? SSL_VERIFY_FAIL_IF_NO_PEER_CERT : 0), + NULL ); + SSL_CTX_set_verify_depth(s->ssl_ctx, s->ssl_verifyclient_depth); } - if (SSL_CTX_use_certificate_file(s->ssl_ctx, s->ssl_pemfile->ptr, SSL_FILETYPE_PEM) < 0) { + if (SSL_CTX_use_certificate(s->ssl_ctx, s->ssl_pemfile_x509) < 0) { log_error_write(srv, __FILE__, __LINE__, "ssb", "SSL:", ERR_error_string(ERR_get_error(), NULL), s->ssl_pemfile); return -1; } - if (SSL_CTX_use_PrivateKey_file (s->ssl_ctx, s->ssl_pemfile->ptr, SSL_FILETYPE_PEM) < 0) { + if (SSL_CTX_use_PrivateKey(s->ssl_ctx, s->ssl_pemfile_pkey) < 0) { log_error_write(srv, __FILE__, __LINE__, "ssb", "SSL:", ERR_error_string(ERR_get_error(), NULL), s->ssl_pemfile); return -1; @@ -856,7 +982,6 @@ int network_init(server *srv) { for (i = 1; i < srv->config_context->used; i++) { data_config *dc = (data_config *)srv->config_context->data[i]; specific_config *s = srv->config_storage[i]; - size_t j; /* not our stage */ if (COMP_SERVER_SOCKET != dc->comp) continue; diff --git a/src/server.c b/src/server.c index a7799287..1eb68b66 100644 --- a/src/server.c +++ b/src/server.c @@ -314,6 +314,9 @@ static void server_free(server *srv) { buffer_free(s->ssl_verifyclient_username); #ifdef USE_OPENSSL SSL_CTX_free(s->ssl_ctx); + EVP_PKEY_free(s->ssl_pemfile_pkey); + X509_free(s->ssl_pemfile_x509); + if (NULL != s->ssl_ca_file_cert_names) sk_X509_NAME_pop_free(s->ssl_ca_file_cert_names, X509_NAME_free); #endif free(s); } |