summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStefan Bühler <stbuehler@web.de>2013-11-05 15:29:07 +0000
committerStefan Bühler <stbuehler@web.de>2013-11-05 15:29:07 +0000
commit1af871fcef97574c71870309d572d6b1026ee605 (patch)
tree5b6312ab52f646e43583bf8a5d60f523bd2a9319
parent3ce548c8d0759c201ba0bcecbc45661cfd22c792 (diff)
downloadlighttpd-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--NEWS1
-rw-r--r--src/base.h6
-rw-r--r--src/configfile.c12
-rw-r--r--src/network.c219
-rw-r--r--src/server.c3
5 files changed, 191 insertions, 50 deletions
diff --git a/NEWS b/NEWS
index 2b93378f..6aee80da 100644
--- a/NEWS
+++ b/NEWS
@@ -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)
diff --git a/src/base.h b/src/base.h
index 5d79a33c..6a8df14f 100644
--- a/src/base.h
+++ b/src/base.h
@@ -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);
}