From 68a724cf7b2389ee32d9490c663b5267921f7869 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 4 Feb 2017 18:59:06 +0900 Subject: [PATCH] nghttpx: Select certificate by client's supported signature algo nghttpx supports multiple certificates using --subcert option. Previously, SNI hostname is used to select certificate. With this commit, signature algorithm presented by client is also taken into consideration. nghttpx now accepts certificates which share the same hostname (CN, SAN), but have different signature algorithm (e.g., ECDSA+SHA256, RSA+SHA256). Currently, this feature requires OpenSSL >= 1.0.2. BoringSSL, and LibreSSL do not work since they lack required APIs. --- src/shrpx-unittest.cc | 4 +- src/shrpx.cc | 11 ++- src/shrpx_connection_handler.cc | 23 ++++-- src/shrpx_connection_handler.h | 8 ++ src/shrpx_router.cc | 14 ++-- src/shrpx_router.h | 5 +- src/shrpx_ssl.cc | 127 ++++++++++++++++++++++---------- src/shrpx_ssl.h | 41 +++++++---- src/shrpx_ssl_test.cc | 29 ++++++-- src/shrpx_ssl_test.h | 2 +- 10 files changed, 181 insertions(+), 83 deletions(-) diff --git a/src/shrpx-unittest.cc b/src/shrpx-unittest.cc index c6a0940b..579c0326 100644 --- a/src/shrpx-unittest.cc +++ b/src/shrpx-unittest.cc @@ -72,8 +72,8 @@ int main(int argc, char *argv[]) { // add the tests to the suite if (!CU_add_test(pSuite, "ssl_create_lookup_tree", shrpx::test_shrpx_ssl_create_lookup_tree) || - !CU_add_test(pSuite, "ssl_cert_lookup_tree_add_cert_from_x509", - shrpx::test_shrpx_ssl_cert_lookup_tree_add_cert_from_x509) || + !CU_add_test(pSuite, "ssl_cert_lookup_tree_add_ssl_ctx", + shrpx::test_shrpx_ssl_cert_lookup_tree_add_ssl_ctx) || !CU_add_test(pSuite, "ssl_tls_hostname_match", shrpx::test_shrpx_ssl_tls_hostname_match) || !CU_add_test(pSuite, "http2_add_header", shrpx::test_http2_add_header) || diff --git a/src/shrpx.cc b/src/shrpx.cc index 951f1119..088eb23d 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -1943,9 +1943,14 @@ SSL/TLS: --subcert=:[[;]...] Specify additional certificate and private key file. nghttpx will choose certificates based on the hostname - indicated by client using TLS SNI extension. This - option can be used multiple times. To make OCSP - stapling work, must be absolute path. + indicated by client using TLS SNI extension. If nghttpx + is built with OpenSSL >= 1.0.2, signature algorithms + (e.g., ECDSA+SHA256, RSA+SHA256) presented by client are + also taken into consideration. This allows nghttpx to + send ECDSA certificate to modern clients, while sending + RSA based certificate to older clients. This option can + be used multiple times. To make OCSP stapling work, + must be absolute path. Additional parameter can be specified in . The available is "sct-dir=". diff --git a/src/shrpx_connection_handler.cc b/src/shrpx_connection_handler.cc index 307b3c9c..e9eef5a3 100644 --- a/src/shrpx_connection_handler.cc +++ b/src/shrpx_connection_handler.cc @@ -199,12 +199,13 @@ void ConnectionHandler::worker_replace_downstream( int ConnectionHandler::create_single_worker() { cert_tree_ = ssl::create_cert_lookup_tree(); - auto sv_ssl_ctx = ssl::setup_server_ssl_context(all_ssl_ctx_, cert_tree_.get() + auto sv_ssl_ctx = ssl::setup_server_ssl_context( + all_ssl_ctx_, indexed_ssl_ctx_, cert_tree_.get() #ifdef HAVE_NEVERBLEED - , - nb_.get() + , + nb_.get() #endif // HAVE_NEVERBLEED - ); + ); auto cl_ssl_ctx = ssl::setup_downstream_client_ssl_context( #ifdef HAVE_NEVERBLEED nb_.get() @@ -247,12 +248,13 @@ int ConnectionHandler::create_worker_thread(size_t num) { assert(workers_.size() == 0); cert_tree_ = ssl::create_cert_lookup_tree(); - auto sv_ssl_ctx = ssl::setup_server_ssl_context(all_ssl_ctx_, cert_tree_.get() + auto sv_ssl_ctx = ssl::setup_server_ssl_context( + all_ssl_ctx_, indexed_ssl_ctx_, cert_tree_.get() #ifdef HAVE_NEVERBLEED - , - nb_.get() + , + nb_.get() #endif // HAVE_NEVERBLEED - ); + ); auto cl_ssl_ctx = ssl::setup_downstream_client_ssl_context( #ifdef HAVE_NEVERBLEED nb_.get() @@ -841,4 +843,9 @@ SSL_CTX *ConnectionHandler::get_ssl_ctx(size_t idx) const { return all_ssl_ctx_[idx]; } +const std::vector & +ConnectionHandler::get_indexed_ssl_ctx(size_t idx) const { + return indexed_ssl_ctx_[idx]; +} + } // namespace shrpx diff --git a/src/shrpx_connection_handler.h b/src/shrpx_connection_handler.h index d4022401..dab41802 100644 --- a/src/shrpx_connection_handler.h +++ b/src/shrpx_connection_handler.h @@ -156,6 +156,8 @@ public: // array bound checking. SSL_CTX *get_ssl_ctx(size_t idx) const; + const std::vector &get_indexed_ssl_ctx(size_t idx) const; + #ifdef HAVE_NEVERBLEED void set_neverbleed(std::unique_ptr nb); neverbleed_t *get_neverbleed() const; @@ -175,6 +177,12 @@ public: private: // Stores all SSL_CTX objects. std::vector all_ssl_ctx_; + // Stores all SSL_CTX objects in a way that its index is stored in + // cert_tree. The SSL_CTXs stored in the same index share the same + // hostname, but could have different signature algorithm. The + // selection among them are performed by hostname presented by SNI, + // and signature algorithm presented by client. + std::vector> indexed_ssl_ctx_; OCSPUpdateContext ocsp_; std::mt19937 gen_; // ev_loop for each worker diff --git a/src/shrpx_router.cc b/src/shrpx_router.cc index 67516932..04f7fd74 100644 --- a/src/shrpx_router.cc +++ b/src/shrpx_router.cc @@ -69,7 +69,7 @@ void Router::add_node(RNode *node, const char *pattern, size_t patlen, add_next_node(node, std::move(new_node)); } -bool Router::add_route(const StringRef &pattern, size_t index) { +size_t Router::add_route(const StringRef &pattern, size_t index) { auto node = &root_; size_t i = 0; @@ -77,7 +77,7 @@ bool Router::add_route(const StringRef &pattern, size_t index) { auto next_node = find_next_node(node, pattern[i]); if (next_node == nullptr) { add_node(node, pattern.c_str() + i, pattern.size() - i, index); - return true; + return index; } node = next_node; @@ -93,11 +93,11 @@ bool Router::add_route(const StringRef &pattern, size_t index) { if (slen == node->len) { // Complete match if (node->index != -1) { - // Don't allow duplicate - return false; + // Return the existing index for duplicates. + return node->index; } node->index = index; - return true; + return index; } if (slen > node->len) { @@ -122,7 +122,7 @@ bool Router::add_route(const StringRef &pattern, size_t index) { if (slen == j) { node->index = index; - return true; + return index; } } @@ -131,7 +131,7 @@ bool Router::add_route(const StringRef &pattern, size_t index) { assert(pattern.size() > i); add_node(node, pattern.c_str() + i, pattern.size() - i, index); - return true; + return index; } } diff --git a/src/shrpx_router.h b/src/shrpx_router.h index a5a893d3..677af986 100644 --- a/src/shrpx_router.h +++ b/src/shrpx_router.h @@ -63,8 +63,9 @@ public: Router &operator=(Router &&) = default; Router &operator=(const Router &) = delete; - // Adds route |pattern| with its |index|. - bool add_route(const StringRef &pattern, size_t index); + // Adds route |pattern| with its |index|. If same pattern has + // already been added, the existing index is returned. + size_t add_route(const StringRef &pattern, size_t index); // Returns the matched index of pattern. -1 if there is no match. ssize_t match(const StringRef &host, const StringRef &path) const; // Returns the matched index of pattern |s|. -1 if there is no diff --git a/src/shrpx_ssl.cc b/src/shrpx_ssl.cc index 29cdf781..25e13221 100644 --- a/src/shrpx_ssl.cc +++ b/src/shrpx_ssl.cc @@ -183,9 +183,34 @@ int servername_callback(SSL *ssl, int *al, void *arg) { } auto conn_handler = worker->get_connection_handler(); - auto ssl_ctx = conn_handler->get_ssl_ctx(idx); - SSL_set_SSL_CTX(ssl, ssl_ctx); + const auto &ssl_ctx_list = conn_handler->get_indexed_ssl_ctx(idx); + assert(!ssl_ctx_list.empty()); + +#if !defined(OPENSSL_IS_BORINGSSL) && !defined(LIBRESSL_VERSION_NUMBER) && \ + OPENSSL_VERSION_NUMBER >= 0x10002000L + // boringssl removed SSL_get_sigalgs. + auto num_sigalg = + SSL_get_sigalgs(ssl, 0, nullptr, nullptr, nullptr, nullptr, nullptr); + + for (auto i = 0; i < num_sigalg; ++i) { + int sigalg; + SSL_get_sigalgs(ssl, i, nullptr, nullptr, &sigalg, nullptr, nullptr); + for (auto ssl_ctx : ssl_ctx_list) { + auto cert = SSL_CTX_get0_certificate(ssl_ctx); + // X509_get_signature_nid is available since OpenSSL 1.0.2. + auto cert_sigalg = X509_get_signature_nid(cert); + + if (sigalg == cert_sigalg) { + SSL_set_SSL_CTX(ssl, ssl_ctx); + return SSL_TLSEXT_ERR_OK; + } + } + } +#endif // !defined(OPENSSL_IS_BORINGSSL) && !defined(LIBRESSL_VERSION_NUMBER) && + // OPENSSL_VERSION_NUMBER >= 0x10002000L + + SSL_set_SSL_CTX(ssl, ssl_ctx_list[0]); return SSL_TLSEXT_ERR_OK; } @@ -1239,12 +1264,12 @@ int check_cert(SSL *ssl, const DownstreamAddr *addr, const Address *raddr) { CertLookupTree::CertLookupTree() {} -void CertLookupTree::add_cert(const StringRef &hostname, size_t idx) { +ssize_t CertLookupTree::add_cert(const StringRef &hostname, size_t idx) { std::array buf; // NI_MAXHOST includes terminal NULL byte if (hostname.empty() || hostname.size() + 1 > buf.size()) { - return; + return -1; } auto wildcard_it = std::find(std::begin(hostname), std::end(hostname), '*'); @@ -1260,8 +1285,8 @@ void CertLookupTree::add_cert(const StringRef &hostname, size_t idx) { WildcardPattern *wpat; - if (!rev_wildcard_router_.add_route(rev_suffix, - wildcard_patterns_.size())) { + if (wildcard_patterns_.size() != + rev_wildcard_router_.add_route(rev_suffix, wildcard_patterns_.size())) { auto wcidx = rev_wildcard_router_.match(rev_suffix); assert(wcidx != -1); @@ -1277,12 +1302,18 @@ void CertLookupTree::add_cert(const StringRef &hostname, size_t idx) { std::end(wildcard_prefix), std::begin(buf))}; + for (auto &p : wpat->rev_prefix) { + if (p.prefix == rev_prefix) { + return p.idx; + } + } + wpat->rev_prefix.emplace_back(rev_prefix, idx); - return; + return idx; } - router_.add_route(hostname, idx); + return router_.add_route(hostname, idx); } ssize_t CertLookupTree::lookup(const StringRef &hostname) { @@ -1357,10 +1388,24 @@ void CertLookupTree::dump() const { rev_wildcard_router_.dump(); } -int cert_lookup_tree_add_cert_from_x509(CertLookupTree *lt, size_t idx, - X509 *cert) { +int cert_lookup_tree_add_ssl_ctx( + CertLookupTree *lt, std::vector> &indexed_ssl_ctx, + SSL_CTX *ssl_ctx) { std::array buf; +#if !defined(LIBRESSL_VERSION_NUMBER) && OPENSSL_VERSION_NUMBER >= 0x10002000L + auto cert = SSL_CTX_get0_certificate(ssl_ctx); +#else // defined(LIBRESSL_VERSION_NUMBER) || OPENSSL_VERSION_NUMBER < + // 0x10002000L + auto tls_ctx_data = + static_cast(SSL_CTX_get_app_data(ssl_ctx)); + auto cert = load_certificate(tls_ctx_data->cert_file); + auto cert_deleter = defer(X509_free, cert); +#endif // defined(LIBRESSL_VERSION_NUMBER) || OPENSSL_VERSION_NUMBER < + // 0x10002000L + + auto idx = indexed_ssl_ctx.size(); + auto altnames = static_cast( X509_get_ext_d2i(cert, NID_subject_alt_name, nullptr, nullptr)); if (altnames) { @@ -1403,7 +1448,17 @@ int cert_lookup_tree_add_cert_from_x509(CertLookupTree *lt, size_t idx, auto end_buf = std::copy_n(name, len, std::begin(buf)); util::inp_strlower(std::begin(buf), end_buf); - lt->add_cert(StringRef{std::begin(buf), end_buf}, idx); + auto nidx = lt->add_cert(StringRef{std::begin(buf), end_buf}, idx); + if (nidx == -1) { + continue; + } + idx = nidx; + if (idx < indexed_ssl_ctx.size()) { + indexed_ssl_ctx[idx].push_back(ssl_ctx); + } else { + assert(idx == indexed_ssl_ctx.size()); + indexed_ssl_ctx.emplace_back(std::vector{ssl_ctx}); + } } // Don't bother CN if we have dNSName. @@ -1433,7 +1488,17 @@ int cert_lookup_tree_add_cert_from_x509(CertLookupTree *lt, size_t idx, util::inp_strlower(std::begin(buf), end_buf); - lt->add_cert(StringRef{std::begin(buf), end_buf}, idx); + auto nidx = lt->add_cert(StringRef{std::begin(buf), end_buf}, idx); + if (nidx == -1) { + return 0; + } + idx = nidx; + if (idx < indexed_ssl_ctx.size()) { + indexed_ssl_ctx[idx].push_back(ssl_ctx); + } else { + assert(idx == indexed_ssl_ctx.size()); + indexed_ssl_ctx.emplace_back(std::vector{ssl_ctx}); + } return 0; } @@ -1474,13 +1539,15 @@ X509 *load_certificate(const char *filename) { return cert; } -SSL_CTX *setup_server_ssl_context(std::vector &all_ssl_ctx, - CertLookupTree *cert_tree +SSL_CTX * +setup_server_ssl_context(std::vector &all_ssl_ctx, + std::vector> &indexed_ssl_ctx, + CertLookupTree *cert_tree #ifdef HAVE_NEVERBLEED - , - neverbleed_t *nb + , + neverbleed_t *nb #endif // HAVE_NEVERBLEED - ) { + ) { if (!upstream_tls_enabled()) { return nullptr; } @@ -1508,17 +1575,8 @@ SSL_CTX *setup_server_ssl_context(std::vector &all_ssl_ctx, return ssl_ctx; } -#if !defined(LIBRESSL_VERSION_NUMBER) && OPENSSL_VERSION_NUMBER >= 0x10002000L - auto cert = SSL_CTX_get0_certificate(ssl_ctx); -#else // defined(LIBRESSL_VERSION_NUMBER) || OPENSSL_VERSION_NUMBER < - // 0x10002000L - auto cert = load_certificate(tlsconf.cert_file.c_str()); - auto cert_deleter = defer(X509_free, cert); -#endif // defined(LIBRESSL_VERSION_NUMBER) || OPENSSL_VERSION_NUMBER < - // 0x10002000L - - if (ssl::cert_lookup_tree_add_cert_from_x509( - cert_tree, all_ssl_ctx.size() - 1, cert) == -1) { + if (ssl::cert_lookup_tree_add_ssl_ctx(cert_tree, indexed_ssl_ctx, ssl_ctx) == + -1) { LOG(FATAL) << "Failed to add default certificate."; DIE(); } @@ -1533,17 +1591,8 @@ SSL_CTX *setup_server_ssl_context(std::vector &all_ssl_ctx, ); all_ssl_ctx.push_back(ssl_ctx); -#if !defined(LIBRESSL_VERSION_NUMBER) && OPENSSL_VERSION_NUMBER >= 0x10002000L - auto cert = SSL_CTX_get0_certificate(ssl_ctx); -#else // defined(LIBRESSL_VERSION_NUMBER) || OPENSSL_VERSION_NUMBER < - // 0x10002000L - auto cert = load_certificate(c.cert_file.c_str()); - auto cert_deleter = defer(X509_free, cert); -#endif // defined(LIBRESSL_VERSION_NUMBER) || OPENSSL_VERSION_NUMBER < - // 0x10002000L - - if (ssl::cert_lookup_tree_add_cert_from_x509( - cert_tree, all_ssl_ctx.size() - 1, cert) == -1) { + if (ssl::cert_lookup_tree_add_ssl_ctx(cert_tree, indexed_ssl_ctx, + ssl_ctx) == -1) { LOG(FATAL) << "Failed to add sub certificate."; DIE(); } diff --git a/src/shrpx_ssl.h b/src/shrpx_ssl.h index 08f07a24..11865019 100644 --- a/src/shrpx_ssl.h +++ b/src/shrpx_ssl.h @@ -136,13 +136,20 @@ public: // |index| is returned. We support wildcard pattern. The left most // '*' is considered as wildcard character, and it must match at // least one character. If the same pattern has been already added, - // this function is noop. + // this function does not alter the tree, and returns the existing + // matching index. // // The caller should lower-case |hostname| since this function does // do that, and lookup function performs case-sensitive match. // // TODO Treat wildcard pattern described as RFC 6125. - void add_cert(const StringRef &hostname, size_t index); + // + // This function returns the index. It returns -1 if it fails + // (e.g., hostname is too long). If the returned index equals to + // |index|, then hostname is added to the tree with the value + // |index|. If it is not -1, and does not equal to |index|, same + // hostname has already been added to the tree. + ssize_t add_cert(const StringRef &hostname, size_t index); // Looks up index using the given |hostname|. The exact match takes // precedence over wildcard match. For wildcard match, longest @@ -166,12 +173,14 @@ private: std::vector wildcard_patterns_; }; -// Adds hostnames in |cert| to lookup tree |lt|. The subjectAltNames -// and commonName are considered as eligible hostname. If there is at -// least one dNSName in subjectAltNames, commonName is not considered. -// This function returns 0 if it succeeds, or -1. -int cert_lookup_tree_add_cert_from_x509(CertLookupTree *lt, size_t idx, - X509 *cert); +// Adds hostnames in certificate in |ssl_ctx| to lookup tree |lt|. +// The subjectAltNames and commonName are considered as eligible +// hostname. If there is at least one dNSName in subjectAltNames, +// commonName is not considered. |ssl_ctx| is also added to +// |indexed_ssl_ctx|. This function returns 0 if it succeeds, or -1. +int cert_lookup_tree_add_ssl_ctx( + CertLookupTree *lt, std::vector> &indexed_ssl_ctx, + SSL_CTX *ssl_ctx); // Returns true if |proto| is included in the // protocol list |protos|. @@ -194,14 +203,18 @@ int set_alpn_prefs(std::vector &out, // construct default SSL_CTX. If subcerts are available // (get_config()->subcerts), caller should provide CertLookupTree // object as |cert_tree| parameter, otherwise SNI does not work. All -// the created SSL_CTX is stored into |all_ssl_ctx|. -SSL_CTX *setup_server_ssl_context(std::vector &all_ssl_ctx, - CertLookupTree *cert_tree +// the created SSL_CTX is stored into |all_ssl_ctx|. They are also +// added to |indexed_ssl_ctx|. |cert_tree| uses its index to +// associate hostname to the SSL_CTX. +SSL_CTX * +setup_server_ssl_context(std::vector &all_ssl_ctx, + std::vector> &indexed_ssl_ctx, + CertLookupTree *cert_tree #ifdef HAVE_NEVERBLEED - , - neverbleed_t *nb + , + neverbleed_t *nb #endif // HAVE_NEVERBLEED - ); + ); // Setups client side SSL_CTX. SSL_CTX *setup_downstream_client_ssl_context( diff --git a/src/shrpx_ssl_test.cc b/src/shrpx_ssl_test.cc index 2e035f48..1de5db67 100644 --- a/src/shrpx_ssl_test.cc +++ b/src/shrpx_ssl_test.cc @@ -115,24 +115,39 @@ void test_shrpx_ssl_create_lookup_tree(void) { // -config=ca-config.json -profile=server test.example.com.csr | // cfssljson -bare test.example.com // -void test_shrpx_ssl_cert_lookup_tree_add_cert_from_x509(void) { +void test_shrpx_ssl_cert_lookup_tree_add_ssl_ctx(void) { int rv; constexpr char nghttp2_certfile[] = NGHTTP2_SRC_DIR "/test.nghttp2.org.pem"; - auto nghttp2_cert = ssl::load_certificate(nghttp2_certfile); - auto nghttp2_cert_deleter = defer(X509_free, nghttp2_cert); + auto nghttp2_ssl_ctx = SSL_CTX_new(SSLv23_server_method()); + auto nghttp2_ssl_ctx_del = defer(SSL_CTX_free, nghttp2_ssl_ctx); + auto nghttp2_tls_ctx_data = make_unique(); + nghttp2_tls_ctx_data->cert_file = nghttp2_certfile; + SSL_CTX_set_app_data(nghttp2_ssl_ctx, nghttp2_tls_ctx_data.get()); + rv = SSL_CTX_use_certificate_chain_file(nghttp2_ssl_ctx, nghttp2_certfile); + + CU_ASSERT(1 == rv); constexpr char examples_certfile[] = NGHTTP2_SRC_DIR "/test.example.com.pem"; - auto examples_cert = ssl::load_certificate(examples_certfile); - auto examples_cert_deleter = defer(X509_free, examples_cert); + auto examples_ssl_ctx = SSL_CTX_new(SSLv23_server_method()); + auto examples_ssl_ctx_del = defer(SSL_CTX_free, examples_ssl_ctx); + auto examples_tls_ctx_data = make_unique(); + examples_tls_ctx_data->cert_file = examples_certfile; + SSL_CTX_set_app_data(examples_ssl_ctx, examples_tls_ctx_data.get()); + rv = SSL_CTX_use_certificate_chain_file(examples_ssl_ctx, examples_certfile); + + CU_ASSERT(1 == rv); ssl::CertLookupTree tree; + std::vector> indexed_ssl_ctx; - rv = ssl::cert_lookup_tree_add_cert_from_x509(&tree, 0, nghttp2_cert); + rv = ssl::cert_lookup_tree_add_ssl_ctx(&tree, indexed_ssl_ctx, + nghttp2_ssl_ctx); CU_ASSERT(0 == rv); - rv = ssl::cert_lookup_tree_add_cert_from_x509(&tree, 1, examples_cert); + rv = ssl::cert_lookup_tree_add_ssl_ctx(&tree, indexed_ssl_ctx, + examples_ssl_ctx); CU_ASSERT(0 == rv); diff --git a/src/shrpx_ssl_test.h b/src/shrpx_ssl_test.h index 8128e8fd..b9caaee4 100644 --- a/src/shrpx_ssl_test.h +++ b/src/shrpx_ssl_test.h @@ -32,7 +32,7 @@ namespace shrpx { void test_shrpx_ssl_create_lookup_tree(void); -void test_shrpx_ssl_cert_lookup_tree_add_cert_from_x509(void); +void test_shrpx_ssl_cert_lookup_tree_add_ssl_ctx(void); void test_shrpx_ssl_tls_hostname_match(void); } // namespace shrpx