From f36ff3550a7271a618edb119f064dddd086cc380 Mon Sep 17 00:00:00 2001 From: Sergey Kandaurov Date: Mon, 9 Sep 2024 19:02:27 +0400 Subject: [PATCH] SSL: moved certificate storage out of exdata. Instead of cross-linking the objects using exdata, pointers to configured certificates are now stored in ngx_ssl_t, and OCSP staples are now accessed with rbtree in it. This allows sharing these objects between SSL contexts. Based on previous work by Mini Hawthorne. --- src/event/ngx_event_openssl.c | 80 ++++++++++---------------- src/event/ngx_event_openssl.h | 9 ++- src/event/ngx_event_openssl_stapling.c | 68 +++++++++++++++++----- 3 files changed, 90 insertions(+), 67 deletions(-) diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c index 8d1f5695c..3ed003062 100644 --- a/src/event/ngx_event_openssl.c +++ b/src/event/ngx_event_openssl.c @@ -131,10 +131,8 @@ int ngx_ssl_server_conf_index; int ngx_ssl_session_cache_index; int ngx_ssl_ticket_keys_index; int ngx_ssl_ocsp_index; -int ngx_ssl_certificate_index; -int ngx_ssl_next_certificate_index; +int ngx_ssl_index; int ngx_ssl_certificate_name_index; -int ngx_ssl_stapling_index; ngx_int_t @@ -258,21 +256,14 @@ ngx_ssl_init(ngx_log_t *log) return NGX_ERROR; } - ngx_ssl_certificate_index = SSL_CTX_get_ex_new_index(0, NULL, NULL, NULL, - NULL); - if (ngx_ssl_certificate_index == -1) { + ngx_ssl_index = SSL_CTX_get_ex_new_index(0, NULL, NULL, NULL, NULL); + + if (ngx_ssl_index == -1) { ngx_ssl_error(NGX_LOG_ALERT, log, 0, "SSL_CTX_get_ex_new_index() failed"); return NGX_ERROR; } - ngx_ssl_next_certificate_index = X509_get_ex_new_index(0, NULL, NULL, NULL, - NULL); - if (ngx_ssl_next_certificate_index == -1) { - ngx_ssl_error(NGX_LOG_ALERT, log, 0, "X509_get_ex_new_index() failed"); - return NGX_ERROR; - } - ngx_ssl_certificate_name_index = X509_get_ex_new_index(0, NULL, NULL, NULL, NULL); @@ -281,13 +272,6 @@ ngx_ssl_init(ngx_log_t *log) return NGX_ERROR; } - ngx_ssl_stapling_index = X509_get_ex_new_index(0, NULL, NULL, NULL, NULL); - - if (ngx_ssl_stapling_index == -1) { - ngx_ssl_error(NGX_LOG_ALERT, log, 0, "X509_get_ex_new_index() failed"); - return NGX_ERROR; - } - return NGX_OK; } @@ -308,12 +292,15 @@ ngx_ssl_create(ngx_ssl_t *ssl, ngx_uint_t protocols, void *data) return NGX_ERROR; } - if (SSL_CTX_set_ex_data(ssl->ctx, ngx_ssl_certificate_index, NULL) == 0) { + if (SSL_CTX_set_ex_data(ssl->ctx, ngx_ssl_index, ssl) == 0) { ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0, "SSL_CTX_set_ex_data() failed"); return NGX_ERROR; } + ngx_rbtree_init(&ssl->staple_rbtree, &ssl->staple_sentinel, + ngx_rbtree_insert_value); + ssl->buffer_size = NGX_SSL_BUFSIZE; /* client side options */ @@ -458,7 +445,7 @@ ngx_ssl_certificate(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *cert, ngx_str_t *key, ngx_array_t *passwords) { char *err; - X509 *x509; + X509 *x509, **elm; EVP_PKEY *pkey; STACK_OF(X509) *chain; @@ -490,29 +477,29 @@ ngx_ssl_certificate(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *cert, return NGX_ERROR; } - if (X509_set_ex_data(x509, ngx_ssl_next_certificate_index, - SSL_CTX_get_ex_data(ssl->ctx, ngx_ssl_certificate_index)) - == 0) - { - ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0, "X509_set_ex_data() failed"); + if (ssl->certs.elts == NULL) { + if (ngx_array_init(&ssl->certs, cf->pool, 1, sizeof(X509 *)) + != NGX_OK) + { + X509_free(x509); + sk_X509_pop_free(chain, X509_free); + return NGX_ERROR; + } + } + + elm = ngx_array_push(&ssl->certs); + if (elm == NULL) { X509_free(x509); sk_X509_pop_free(chain, X509_free); return NGX_ERROR; } - if (SSL_CTX_set_ex_data(ssl->ctx, ngx_ssl_certificate_index, x509) == 0) { - ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0, - "SSL_CTX_set_ex_data() failed"); - X509_free(x509); - sk_X509_pop_free(chain, X509_free); - return NGX_ERROR; - } + *elm = x509; /* * Note that x509 is not freed here, but will be instead freed in * ngx_ssl_cleanup_ctx(). This is because we need to preserve all - * certificates to be able to iterate all of them through exdata - * (ngx_ssl_certificate_index, ngx_ssl_next_certificate_index), + * certificates to be able to iterate all of them through ssl->certs, * while OpenSSL can free a certificate if it is replaced with another * certificate of the same type. */ @@ -3820,10 +3807,9 @@ ngx_ssl_session_id_context(ngx_ssl_t *ssl, ngx_str_t *sess_ctx, goto failed; } - for (cert = SSL_CTX_get_ex_data(ssl->ctx, ngx_ssl_certificate_index); - cert; - cert = X509_get_ex_data(cert, ngx_ssl_next_certificate_index)) - { + for (k = 0; k < ssl->certs.nelts; k++) { + cert = ((X509 **) ssl->certs.elts)[k]; + if (X509_digest(cert, EVP_sha1(), buf, &len) == 0) { ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0, "X509_digest() failed"); @@ -3837,9 +3823,7 @@ ngx_ssl_session_id_context(ngx_ssl_t *ssl, ngx_str_t *sess_ctx, } } - if (SSL_CTX_get_ex_data(ssl->ctx, ngx_ssl_certificate_index) == NULL - && certificates != NULL) - { + if (ssl->certs.nelts == 0 && certificates != NULL) { /* * If certificates are loaded dynamically, we use certificate * names as specified in the configuration (with variables). @@ -4851,14 +4835,12 @@ ngx_ssl_cleanup_ctx(void *data) { ngx_ssl_t *ssl = data; - X509 *cert, *next; + X509 *cert; + ngx_uint_t i; - cert = SSL_CTX_get_ex_data(ssl->ctx, ngx_ssl_certificate_index); - - while (cert) { - next = X509_get_ex_data(cert, ngx_ssl_next_certificate_index); + for (i = 0; i < ssl->certs.nelts; i++) { + cert = ((X509 **) ssl->certs.elts)[i]; X509_free(cert); - cert = next; } SSL_CTX_free(ssl->ctx); diff --git a/src/event/ngx_event_openssl.h b/src/event/ngx_event_openssl.h index ebb2c35bf..41566422a 100644 --- a/src/event/ngx_event_openssl.h +++ b/src/event/ngx_event_openssl.h @@ -90,6 +90,11 @@ struct ngx_ssl_s { SSL_CTX *ctx; ngx_log_t *log; size_t buffer_size; + + ngx_array_t certs; + + ngx_rbtree_t staple_rbtree; + ngx_rbtree_node_t staple_sentinel; }; @@ -330,10 +335,8 @@ extern int ngx_ssl_server_conf_index; extern int ngx_ssl_session_cache_index; extern int ngx_ssl_ticket_keys_index; extern int ngx_ssl_ocsp_index; -extern int ngx_ssl_certificate_index; -extern int ngx_ssl_next_certificate_index; +extern int ngx_ssl_index; extern int ngx_ssl_certificate_name_index; -extern int ngx_ssl_stapling_index; #endif /* _NGX_EVENT_OPENSSL_H_INCLUDED_ */ diff --git a/src/event/ngx_event_openssl_stapling.c b/src/event/ngx_event_openssl_stapling.c index e9bb8354e..a0a8031c7 100644 --- a/src/event/ngx_event_openssl_stapling.c +++ b/src/event/ngx_event_openssl_stapling.c @@ -15,6 +15,8 @@ typedef struct { + ngx_rbtree_node_t node; + ngx_str_t staple; ngx_msec_t timeout; @@ -154,6 +156,7 @@ static ngx_int_t ngx_ssl_stapling_responder(ngx_conf_t *cf, ngx_ssl_t *ssl, static int ngx_ssl_certificate_status_callback(ngx_ssl_conn_t *ssl_conn, void *data); +static ngx_ssl_stapling_t *ngx_ssl_stapling_lookup(ngx_ssl_t *ssl, X509 *cert); static void ngx_ssl_stapling_update(ngx_ssl_stapling_t *staple); static void ngx_ssl_stapling_ocsp_handler(ngx_ssl_ocsp_ctx_t *ctx); @@ -195,12 +198,12 @@ ngx_int_t ngx_ssl_stapling(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *file, ngx_str_t *responder, ngx_uint_t verify) { - X509 *cert; + X509 *cert; + ngx_uint_t k; + + for (k = 0; k < ssl->certs.nelts; k++) { + cert = ((X509 **) ssl->certs.elts)[k]; - for (cert = SSL_CTX_get_ex_data(ssl->ctx, ngx_ssl_certificate_index); - cert; - cert = X509_get_ex_data(cert, ngx_ssl_next_certificate_index)) - { if (ngx_ssl_stapling_certificate(cf, ssl, cert, file, responder, verify) != NGX_OK) { @@ -235,10 +238,9 @@ ngx_ssl_stapling_certificate(ngx_conf_t *cf, ngx_ssl_t *ssl, X509 *cert, cln->handler = ngx_ssl_stapling_cleanup; cln->data = staple; - if (X509_set_ex_data(cert, ngx_ssl_stapling_index, staple) == 0) { - ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0, "X509_set_ex_data() failed"); - return NGX_ERROR; - } + staple->node.key = (ngx_rbtree_key_t) cert; + + ngx_rbtree_insert(&ssl->staple_rbtree, &staple->node); #ifdef SSL_CTRL_SELECT_CURRENT_CERT /* OpenSSL 1.0.2+ */ @@ -545,14 +547,21 @@ ngx_int_t ngx_ssl_stapling_resolver(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_resolver_t *resolver, ngx_msec_t resolver_timeout) { - X509 *cert; + ngx_rbtree_t *tree; + ngx_rbtree_node_t *node; ngx_ssl_stapling_t *staple; - for (cert = SSL_CTX_get_ex_data(ssl->ctx, ngx_ssl_certificate_index); - cert; - cert = X509_get_ex_data(cert, ngx_ssl_next_certificate_index)) + tree = &ssl->staple_rbtree; + + if (tree->root == tree->sentinel) { + return NGX_OK; + } + + for (node = ngx_rbtree_min(tree->root, tree->sentinel); + node; + node = ngx_rbtree_next(tree, node)) { - staple = X509_get_ex_data(cert, ngx_ssl_stapling_index); + staple = ngx_rbtree_data(node, ngx_ssl_stapling_t, node); staple->resolver = resolver; staple->resolver_timeout = resolver_timeout; } @@ -567,6 +576,8 @@ ngx_ssl_certificate_status_callback(ngx_ssl_conn_t *ssl_conn, void *data) int rc; X509 *cert; u_char *p; + SSL_CTX *ssl_ctx; + ngx_ssl_t *ssl; ngx_connection_t *c; ngx_ssl_stapling_t *staple; @@ -583,7 +594,10 @@ ngx_ssl_certificate_status_callback(ngx_ssl_conn_t *ssl_conn, void *data) return rc; } - staple = X509_get_ex_data(cert, ngx_ssl_stapling_index); + ssl_ctx = SSL_get_SSL_CTX(ssl_conn); + ssl = SSL_CTX_get_ex_data(ssl_ctx, ngx_ssl_index); + + staple = ngx_ssl_stapling_lookup(ssl, cert); if (staple == NULL) { return rc; @@ -613,6 +627,30 @@ ngx_ssl_certificate_status_callback(ngx_ssl_conn_t *ssl_conn, void *data) } +static ngx_ssl_stapling_t * +ngx_ssl_stapling_lookup(ngx_ssl_t *ssl, X509 *cert) +{ + ngx_rbtree_key_t key; + ngx_rbtree_node_t *node, *sentinel; + + node = ssl->staple_rbtree.root; + sentinel = ssl->staple_rbtree.sentinel; + key = (ngx_rbtree_key_t) cert; + + while (node != sentinel) { + + if (key != node->key) { + node = (key < node->key) ? node->left : node->right; + continue; + } + + return ngx_rbtree_data(node, ngx_ssl_stapling_t, node); + } + + return NULL; +} + + static void ngx_ssl_stapling_update(ngx_ssl_stapling_t *staple) {