[PATCH 1 of 6] SSL: moved certificate storage out of SSL_CTX exdata
Sergey Kandaurov
pluknet at nginx.com
Wed Aug 21 13:32:21 UTC 2024
On Tue, Jul 23, 2024 at 07:30:24PM +0000, Mini Hawthorne wrote:
> # HG changeset patch
> # User Mini Hawthorne <mini at f5.com>
> # Date 1721762810 0
> # Tue Jul 23 19:26:50 2024 +0000
> # Node ID 59ac183dfee8e9641563e043eb19480d91dd7cc0
> # Parent d1b8568f3042f6019a2302dda4afbadd051fe54b
> SSL: moved certificate storage out of SSL_CTX exdata.
>
> Certain SSL objects (certificates, certificate names, and OCSP staples) are now
> accessed with ngx_array_t and ngx_rbtree_t instead of cross-linking the objects
> using SSL_CTX exdata. This allows sharing these objects between SSL contexts.
>
> diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c
> --- a/src/event/ngx_event_openssl.c
> +++ b/src/event/ngx_event_openssl.c
> @@ -131,10 +131,7 @@ 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_certificate_name_index;
> -int ngx_ssl_stapling_index;
> +int ngx_ssl_index;
>
>
> ngx_int_t
> @@ -258,34 +255,11 @@ 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_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);
> -
> - if (ngx_ssl_certificate_name_index == -1) {
> - ngx_ssl_error(NGX_LOG_ALERT, log, 0, "X509_get_ex_new_index() failed");
> - 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;
> + 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;
bad ident
> }
>
> return NGX_OK;
> @@ -308,12 +282,18 @@ ngx_ssl_create(ngx_ssl_t *ssl, ngx_uint_
> 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->name_rbtree, &ssl->name_sentinel,
> + ngx_rbtree_insert_value);
This part converts stored certificate names from certificate ex_data
to per-SSL context's rbtree, but it is a useless change. Although
using rbtree may ensure to keep the correct reference to cert->data
for the given SSL context, yet certificate name is a static string,
so keeping references either per SSL context or in the certificate
itself doesn't alter behaviour because referenced strings have the
same content, but expands ngx_ssl_t unnecessarily (and the relevant
codebase). Here I restored ngx_ssl_certificate_name_index.
The relevant diff is below (to simplify review).
# HG changeset patch
# User Sergey Kandaurov <pluknet at nginx.com>
# Date 1724174931 -14400
# Tue Aug 20 21:28:51 2024 +0400
# Node ID 3b62f8c36298439b7acb668885d8f8d5a98592b6
# Parent bb017f29c004079bdc321fcd43bb9831e87ea245
imported patch index_name
diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c
--- a/src/event/ngx_event_openssl.c
+++ b/src/event/ngx_event_openssl.c
@@ -131,6 +131,7 @@ 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_name_index;
int ngx_ssl_index;
@@ -256,12 +257,21 @@ ngx_ssl_init(ngx_log_t *log)
}
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_certificate_name_index = X509_get_ex_new_index(0, NULL, NULL, NULL,
+ NULL);
+
+ if (ngx_ssl_certificate_name_index == -1) {
+ ngx_ssl_error(NGX_LOG_ALERT, log, 0, "X509_get_ex_new_index() failed");
+ return NGX_ERROR;
+ }
+
return NGX_OK;
}
@@ -462,6 +472,15 @@ ngx_ssl_certificate(ngx_conf_t *cf, ngx_
return NGX_ERROR;
}
+ if (X509_set_ex_data(x509, ngx_ssl_certificate_name_index, cert->data)
+ == 0)
+ {
+ ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0, "X509_set_ex_data() failed");
+ X509_free(x509);
+ sk_X509_pop_free(chain, X509_free);
+ return NGX_ERROR;
+ }
+
name = ngx_pcalloc(cf->pool, sizeof(ngx_ssl_name_t));
if (name == NULL) {
X509_free(x509);
diff --git a/src/event/ngx_event_openssl.h b/src/event/ngx_event_openssl.h
--- a/src/event/ngx_event_openssl.h
+++ b/src/event/ngx_event_openssl.h
@@ -345,6 +345,7 @@ extern int ngx_ssl_session_cache_index;
extern int ngx_ssl_ticket_keys_index;
extern int ngx_ssl_ocsp_index;
extern int ngx_ssl_index;
+extern int ngx_ssl_certificate_name_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
--- a/src/event/ngx_event_openssl_stapling.c
+++ b/src/event/ngx_event_openssl_stapling.c
@@ -261,6 +261,8 @@ ngx_ssl_stapling_certificate(ngx_conf_t
staple->timeout = 60000;
staple->verify = verify;
staple->cert = cert;
+ staple->name = X509_get_ex_data(staple->cert,
+ ngx_ssl_certificate_name_index);
name = ngx_ssl_stapling_lookup_name(&ssl->name_rbtree, cert);
if (name == NULL) {
# HG changeset patch
# User Sergey Kandaurov <pluknet at nginx.com>
# Date 1724174965 -14400
# Tue Aug 20 21:29:25 2024 +0400
# Node ID 30307c9e9625b8dc230ce0ef6701a1f47a9760a5
# Parent 3b62f8c36298439b7acb668885d8f8d5a98592b6
imported patch name_rbtree
diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c
--- a/src/event/ngx_event_openssl.c
+++ b/src/event/ngx_event_openssl.c
@@ -298,9 +298,6 @@ ngx_ssl_create(ngx_ssl_t *ssl, ngx_uint_
return NGX_ERROR;
}
- ngx_rbtree_init(&ssl->name_rbtree, &ssl->name_sentinel,
- ngx_rbtree_insert_value);
-
ngx_rbtree_init(&ssl->staple_rbtree, &ssl->staple_sentinel,
ngx_rbtree_insert_value);
@@ -451,7 +448,6 @@ ngx_ssl_certificate(ngx_conf_t *cf, ngx_
X509 *x509, **elm;
EVP_PKEY *pkey;
STACK_OF(X509) *chain;
- ngx_ssl_name_t *name;
x509 = ngx_ssl_load_certificate(cf->pool, &err, cert, &chain);
if (x509 == NULL) {
@@ -481,13 +477,6 @@ ngx_ssl_certificate(ngx_conf_t *cf, ngx_
return NGX_ERROR;
}
- name = ngx_pcalloc(cf->pool, sizeof(ngx_ssl_name_t));
- if (name == NULL) {
- X509_free(x509);
- sk_X509_pop_free(chain, X509_free);
- return NGX_ERROR;
- }
-
if (ssl->certs.elts == NULL) {
if (ngx_array_init(&ssl->certs, cf->pool, 16, sizeof(X509 *))
!= NGX_OK)
@@ -505,12 +494,6 @@ ngx_ssl_certificate(ngx_conf_t *cf, ngx_
return NGX_ERROR;
}
- name->node.key = (ngx_rbtree_key_t) x509;
- name->name.len = cert->len;
- name->name.data = cert->data;
-
- ngx_rbtree_insert(&ssl->name_rbtree, &name->node);
-
/*
* 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
diff --git a/src/event/ngx_event_openssl.h b/src/event/ngx_event_openssl.h
--- a/src/event/ngx_event_openssl.h
+++ b/src/event/ngx_event_openssl.h
@@ -86,12 +86,6 @@
typedef struct ngx_ssl_ocsp_s ngx_ssl_ocsp_t;
-typedef struct {
- ngx_rbtree_node_t node;
- ngx_str_t name;
-} ngx_ssl_name_t;
-
-
struct ngx_ssl_s {
SSL_CTX *ctx;
ngx_log_t *log;
@@ -99,9 +93,6 @@ struct ngx_ssl_s {
ngx_array_t certs;
- ngx_rbtree_t name_rbtree;
- ngx_rbtree_node_t name_sentinel;
-
ngx_rbtree_t staple_rbtree;
ngx_rbtree_node_t staple_sentinel;
};
diff --git a/src/event/ngx_event_openssl_stapling.c b/src/event/ngx_event_openssl_stapling.c
--- a/src/event/ngx_event_openssl_stapling.c
+++ b/src/event/ngx_event_openssl_stapling.c
@@ -153,8 +153,6 @@ static ngx_int_t ngx_ssl_stapling_issuer
ngx_ssl_stapling_t *staple);
static ngx_int_t ngx_ssl_stapling_responder(ngx_conf_t *cf, ngx_ssl_t *ssl,
ngx_ssl_stapling_t *staple, ngx_str_t *responder);
-static ngx_ssl_name_t *ngx_ssl_stapling_lookup_name(ngx_rbtree_t *rbtree,
- X509 *cert);
static int ngx_ssl_certificate_status_callback(ngx_ssl_conn_t *ssl_conn,
void *data);
@@ -225,7 +223,6 @@ ngx_ssl_stapling_certificate(ngx_conf_t
ngx_str_t *file, ngx_str_t *responder, ngx_uint_t verify)
{
ngx_int_t rc;
- ngx_ssl_name_t *name;
ngx_pool_cleanup_t *cln;
ngx_ssl_stapling_t *staple;
@@ -264,13 +261,6 @@ ngx_ssl_stapling_certificate(ngx_conf_t
staple->name = X509_get_ex_data(staple->cert,
ngx_ssl_certificate_name_index);
- name = ngx_ssl_stapling_lookup_name(&ssl->name_rbtree, cert);
- if (name == NULL) {
- return NGX_ERROR;
- }
-
- staple->name = name->name.data;
-
if (file->len) {
/* use OCSP response from the file */
@@ -553,33 +543,6 @@ ngx_ssl_stapling_responder(ngx_conf_t *c
}
-static ngx_ssl_name_t *
-ngx_ssl_stapling_lookup_name(ngx_rbtree_t *rbtree, X509 *cert)
-{
- ngx_ssl_name_t *n;
- ngx_rbtree_key_t key;
- ngx_rbtree_node_t *node, *sentinel;
-
- node = rbtree->root;
- sentinel = rbtree->sentinel;
- key = (ngx_rbtree_key_t) cert;
-
- while (node != sentinel) {
-
- n = ngx_rbtree_data(node, ngx_ssl_name_t, node);
-
- if (key != node->key) {
- node = (key < node->key) ? node->left : node->right;
- continue;
- }
-
- return n;
- }
-
- return NULL;
-}
-
-
ngx_int_t
ngx_ssl_stapling_resolver(ngx_conf_t *cf, ngx_ssl_t *ssl,
ngx_resolver_t *resolver, ngx_msec_t resolver_timeout)
> +
> + ngx_rbtree_init(&ssl->staple_rbtree, &ssl->staple_sentinel,
> + ngx_rbtree_insert_value);
> +
> ssl->buffer_size = NGX_SSL_BUFSIZE;
>
> /* client side options */
> @@ -458,9 +438,10 @@ ngx_ssl_certificate(ngx_conf_t *cf, ngx_
> ngx_str_t *key, ngx_array_t *passwords)
> {
> char *err;
> - X509 *x509;
> + X509 *x509, **elm;
> EVP_PKEY *pkey;
> STACK_OF(X509) *chain;
> + ngx_ssl_name_t *name;
>
> x509 = ngx_ssl_load_certificate(cf->pool, &err, cert, &chain);
> if (x509 == NULL) {
> @@ -481,42 +462,46 @@ ngx_ssl_certificate(ngx_conf_t *cf, ngx_
> return NGX_ERROR;
> }
>
> - if (X509_set_ex_data(x509, ngx_ssl_certificate_name_index, cert->data)
> - == 0)
> - {
> - ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0, "X509_set_ex_data() failed");
> + name = ngx_pcalloc(cf->pool, sizeof(ngx_ssl_name_t));
> + if (name == NULL) {
> X509_free(x509);
> sk_X509_pop_free(chain, X509_free);
> 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, 16, 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;
> - }
> + name->node.key = (ngx_rbtree_key_t) x509;
> + name->name.len = cert->len;
> + name->name.data = cert->data;
> +
> + ngx_rbtree_insert(&ssl->name_rbtree, &name->node);
>
> /*
> * 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.
> */
>
> + *elm = x509;
> +
> #ifdef SSL_CTX_set0_chain
>
> if (SSL_CTX_set0_chain(ssl->ctx, chain) == 0) {
> @@ -3820,10 +3805,9 @@ ngx_ssl_session_id_context(ngx_ssl_t *ss
> 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 +3821,7 @@ ngx_ssl_session_id_context(ngx_ssl_t *ss
> }
> }
>
> - 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 +4833,12 @@ ngx_ssl_cleanup_ctx(void *data)
> {
> ngx_ssl_t *ssl = data;
>
> - X509 *cert, *next;
> -
> - 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);
> - X509_free(cert);
> - cert = next;
> + X509 *cert;
> + ngx_uint_t i;
> +
> + for (i = 0; i < ssl->certs.nelts; i++) {
> + cert = ((X509 **) ssl->certs.elts)[i];
> + X509_free(cert);
bad ident
> }
>
> SSL_CTX_free(ssl->ctx);
> diff --git a/src/event/ngx_event_openssl.h b/src/event/ngx_event_openssl.h
> --- a/src/event/ngx_event_openssl.h
> +++ b/src/event/ngx_event_openssl.h
> @@ -86,10 +86,24 @@
> typedef struct ngx_ssl_ocsp_s ngx_ssl_ocsp_t;
>
>
> +typedef struct {
> + ngx_rbtree_node_t node;
> + ngx_str_t name;
> +} ngx_ssl_name_t;
> +
> +
> struct ngx_ssl_s {
> SSL_CTX *ctx;
> ngx_log_t *log;
> size_t buffer_size;
> +
> + ngx_array_t certs;
> +
> + ngx_rbtree_t name_rbtree;
> + ngx_rbtree_node_t name_sentinel;
> +
> + ngx_rbtree_t staple_rbtree;
> + ngx_rbtree_node_t staple_sentinel;
> };
>
>
> @@ -330,10 +344,7 @@ 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_certificate_name_index;
> -extern int ngx_ssl_stapling_index;
> +extern int ngx_ssl_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
> --- 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;
>
> @@ -151,9 +153,13 @@ static ngx_int_t ngx_ssl_stapling_issuer
> ngx_ssl_stapling_t *staple);
> static ngx_int_t ngx_ssl_stapling_responder(ngx_conf_t *cf, ngx_ssl_t *ssl,
> ngx_ssl_stapling_t *staple, ngx_str_t *responder);
> +static ngx_ssl_name_t *ngx_ssl_stapling_lookup_name(ngx_rbtree_t *rbtree,
> + X509 *cert);
>
> static int ngx_ssl_certificate_status_callback(ngx_ssl_conn_t *ssl_conn,
> void *data);
> +static ngx_ssl_stapling_t *ngx_ssl_stapling_lookup_staple(ngx_rbtree_t *rbtree,
> + 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 +201,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;
> -
> - 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))
> - {
> + X509 *cert;
> + ngx_uint_t k;
> +
> + for (k = 0; k < ssl->certs.nelts; k++) {
> + cert = ((X509 **) ssl->certs.elts)[k];
> +
> if (ngx_ssl_stapling_certificate(cf, ssl, cert, file, responder, verify)
> != NGX_OK)
> {
> @@ -219,6 +225,7 @@ ngx_ssl_stapling_certificate(ngx_conf_t
> ngx_str_t *file, ngx_str_t *responder, ngx_uint_t verify)
> {
> ngx_int_t rc;
> + ngx_ssl_name_t *name;
> ngx_pool_cleanup_t *cln;
> ngx_ssl_stapling_t *staple;
>
> @@ -235,10 +242,8 @@ ngx_ssl_stapling_certificate(ngx_conf_t
> 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+ */
> @@ -256,8 +261,13 @@ ngx_ssl_stapling_certificate(ngx_conf_t
> staple->timeout = 60000;
> staple->verify = verify;
> staple->cert = cert;
> - staple->name = X509_get_ex_data(staple->cert,
> - ngx_ssl_certificate_name_index);
> +
> + name = ngx_ssl_stapling_lookup_name(&ssl->name_rbtree, cert);
> + if (name == NULL) {
> + return NGX_ERROR;
> + }
> +
> + staple->name = name->name.data;
>
> if (file->len) {
> /* use OCSP response from the file */
> @@ -541,18 +551,52 @@ ngx_ssl_stapling_responder(ngx_conf_t *c
> }
>
>
> +static ngx_ssl_name_t *
> +ngx_ssl_stapling_lookup_name(ngx_rbtree_t *rbtree, X509 *cert)
> +{
> + ngx_ssl_name_t *n;
> + ngx_rbtree_key_t key;
> + ngx_rbtree_node_t *node, *sentinel;
> +
> + node = rbtree->root;
> + sentinel = rbtree->sentinel;
> + key = (ngx_rbtree_key_t) cert;
> +
> + while (node != sentinel) {
> +
> + n = ngx_rbtree_data(node, ngx_ssl_name_t, node);
> +
> + if (key != node->key) {
> + node = (key < node->key) ? node->left : node->right;
> + continue;
> + }
> +
> + return n;
> + }
> +
> + return NULL;
> +}
> +
> +
> 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_ssl_stapling_t *) node;
> staple->resolver = resolver;
> staple->resolver_timeout = resolver_timeout;
> }
> @@ -567,6 +611,8 @@ ngx_ssl_certificate_status_callback(ngx_
> 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 +629,19 @@ ngx_ssl_certificate_status_callback(ngx_
> return rc;
> }
>
> - staple = X509_get_ex_data(cert, ngx_ssl_stapling_index);
> + ssl_ctx = SSL_get_SSL_CTX(ssl_conn);
> +
> + if (ssl_ctx == NULL) {
> + return rc;
> + }
> +
> + ssl = SSL_CTX_get_ex_data(ssl_ctx, ngx_ssl_index);
> +
> + if (ssl == NULL) {
> + return rc;
> + }
the added checks should never fail
> +
> + staple = ngx_ssl_stapling_lookup_staple(&ssl->staple_rbtree, cert);
>
> if (staple == NULL) {
> return rc;
> @@ -613,6 +671,33 @@ ngx_ssl_certificate_status_callback(ngx_
> }
>
>
> +static ngx_ssl_stapling_t *
> +ngx_ssl_stapling_lookup_staple(ngx_rbtree_t *rbtree, X509 *cert)
> +{
> + ngx_rbtree_key_t key;
> + ngx_rbtree_node_t *node, *sentinel;
> + ngx_ssl_stapling_t *n;
> +
> + node = rbtree->root;
> + sentinel = rbtree->sentinel;
> + key = (ngx_rbtree_key_t) cert;
> +
> + while (node != sentinel) {
> +
> + n = ngx_rbtree_data(node, ngx_ssl_stapling_t, node);
> +
> + if (key != node->key) {
> + node = (key < node->key) ? node->left : node->right;
> + continue;
> + }
> +
> + return n;
> + }
> +
> + return NULL;
> +}
> +
> +
> static void
> ngx_ssl_stapling_update(ngx_ssl_stapling_t *staple)
> {
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> https://mailman.nginx.org/mailman/listinfo/nginx-devel
On Mon, Aug 12, 2024 at 01:52:31PM -0700, Aleksei Bavshin wrote:
> On 7/23/2024 12:30 PM, Mini Hawthorne wrote:
> > # HG changeset patch
> > # User Mini Hawthorne <mini at f5.com>
> > # Date 1721762842 0
> > # Tue Jul 23 19:27:22 2024 +0000
> > # Node ID 8eee61e223bb7cb7475e50b866fd6b9a83fa5fa0
> > # Parent 59ac183dfee8e9641563e043eb19480d91dd7cc0
> > SSL: object caching.
> >
> > Added ngx_openssl_cache_module, which indexes a type-aware object cache.
> > It maps an id to a unique instance, and provides references to it, which
> > are dropped when the cycle's pool is destroyed. Also, for those objects
> > that can be cached, valid references may be pulled from cycle->old_cycle.
Although they may, they don't.
I tend to drop the last sentence for clarity.
> >
> > The cache will be used in subsequent patches.
> >
> > diff --git a/auto/modules b/auto/modules
> > --- a/auto/modules
> > +++ b/auto/modules
> > @@ -1307,10 +1307,11 @@ fi
> > if [ $USE_OPENSSL = YES ]; then
> > ngx_module_type=CORE
> > - ngx_module_name=ngx_openssl_module
> > + ngx_module_name="ngx_openssl_module ngx_openssl_cache_module"
> > ngx_module_incs=
> > ngx_module_deps=src/event/ngx_event_openssl.h
> > ngx_module_srcs="src/event/ngx_event_openssl.c
> > + src/event/ngx_event_openssl_cache.c
> > src/event/ngx_event_openssl_stapling.c"
> > ngx_module_libs=
> > ngx_module_link=YES
> > diff --git a/src/event/ngx_event_openssl.h b/src/event/ngx_event_openssl.h
> > --- a/src/event/ngx_event_openssl.h
> > +++ b/src/event/ngx_event_openssl.h
> > @@ -83,7 +83,8 @@
> > #endif
> > -typedef struct ngx_ssl_ocsp_s ngx_ssl_ocsp_t;
> > +typedef struct ngx_ssl_ocsp_s ngx_ssl_ocsp_t;
> > +typedef struct ngx_ssl_cache_type_s ngx_ssl_cache_type_t;
Although a lot of type exposure was fixed during internal review,
this unnecessarily exposes a new type, which can be made local to
ngx_event_openssl_cache.c after SSL object cache API adjustment.
See below.
> > typedef struct {
> > @@ -233,6 +234,9 @@ ngx_int_t ngx_ssl_ocsp_get_status(ngx_co
> > void ngx_ssl_ocsp_cleanup(ngx_connection_t *c);
> > ngx_int_t ngx_ssl_ocsp_cache_init(ngx_shm_zone_t *shm_zone, void *data);
> > +void *ngx_ssl_cache_fetch(ngx_cycle_t *cycle, ngx_pool_t *pool,
> > + ngx_ssl_cache_type_t *type, char **err, ngx_str_t *id, void *data);
> > +
> > ngx_array_t *ngx_ssl_read_password_file(ngx_conf_t *cf, ngx_str_t *file);
> > ngx_array_t *ngx_ssl_preserve_passwords(ngx_conf_t *cf,
> > ngx_array_t *passwords);
> > diff --git a/src/event/ngx_event_openssl_cache.c b/src/event/ngx_event_openssl_cache.c
> > new file mode 100644
> > --- /dev/null
> > +++ b/src/event/ngx_event_openssl_cache.c
> > @@ -0,0 +1,277 @@
> > +
> > +/*
> > + * Copyright (C) Nginx, Inc.
> > + */
> > +
> > +
> > +#include <ngx_config.h>
> > +#include <ngx_core.h>
> > +#include <ngx_event.h>
> > +
> > +
> > +typedef struct {
> > + ngx_rbtree_node_t node;
> > + ngx_str_t id;
> > +
> > + ngx_ssl_cache_type_t *type;
> > + void *value;
> > +} ngx_ssl_cache_node_t;
> > +
> > +
> > +typedef void *(*ngx_ssl_cache_create_pt)(ngx_str_t *id, char **err, void *data);
> > +typedef void (*ngx_ssl_cache_free_pt)(void *data);
> > +typedef void *(*ngx_ssl_cache_ref_pt)(char **err, void *data);
> > +
> > +
> > +struct ngx_ssl_cache_type_s {
> > + const char *name;
This field is unused.
> > +
> > + ngx_ssl_cache_create_pt create;
> > + ngx_ssl_cache_free_pt free;
> > + ngx_ssl_cache_ref_pt ref;
> > +};
> > +
> > +
> > +typedef struct {
> > + ngx_rbtree_t rbtree;
> > + ngx_rbtree_node_t sentinel;
> > +} ngx_ssl_cache_t;
> > +
> > +
> > +static void *ngx_openssl_cache_create_conf(ngx_cycle_t *cycle);
> > +static void ngx_ssl_cache_cleanup(void *data);
> > +static void ngx_ssl_cache_node_insert(ngx_rbtree_node_t *temp,
> > + ngx_rbtree_node_t *node, ngx_rbtree_node_t *sentinel);
I'd rather move this block used to configure module (and functions)
to the end, same as we ought to do in other modules.
> > +
> > +static ngx_ssl_cache_node_t *ngx_ssl_cache_lookup(ngx_ssl_cache_t *cache,
> > + ngx_ssl_cache_type_t *type, ngx_str_t *id, uint32_t hash);
> > +
> > +
> > +static ngx_core_module_t ngx_openssl_cache_module_ctx = {
> > + ngx_string("openssl_cache"),
> > + ngx_openssl_cache_create_conf,
> > + NULL
> > +};
> > +
> > +
> > +ngx_module_t ngx_openssl_cache_module = {
> > + NGX_MODULE_V1,
> > + &ngx_openssl_cache_module_ctx, /* module context */
> > + NULL, /* module directives */
> > + NGX_CORE_MODULE, /* module type */
> > + NULL, /* init master */
> > + NULL, /* init module */
> > + NULL, /* init process */
> > + NULL, /* init thread */
> > + NULL, /* exit thread */
> > + NULL, /* exit process */
> > + NULL, /* exit master */
> > + NGX_MODULE_V1_PADDING
> > +};
> > +
> > +
> > +static void *
> > +ngx_openssl_cache_create_conf(ngx_cycle_t *cycle)
> > +{
> > + ngx_ssl_cache_t *cache;
> > + ngx_pool_cleanup_t *cln;
> > +
> > + cache = ngx_pcalloc(cycle->pool, sizeof(ngx_ssl_cache_t));
> > + if (cache == NULL) {
> > + return NULL;
> > + }
> > +
> > + cln = ngx_pool_cleanup_add(cycle->pool, 0);
> > + if (cln == NULL) {
> > + return NULL;
> > + }
> > +
> > + cln->handler = ngx_ssl_cache_cleanup;
> > + cln->data = cache;
> > +
> > + ngx_rbtree_init(&cache->rbtree, &cache->sentinel,
> > + ngx_ssl_cache_node_insert);
> > +
> > + return cache;
> > +}
> > +
> > +
> > +static void
> > +ngx_ssl_cache_cleanup(void *data)
> > +{
> > + ngx_ssl_cache_t *cache = data;
> > +
> > + ngx_rbtree_t *tree;
> > + ngx_rbtree_node_t *node;
> > + ngx_ssl_cache_node_t *cn;
> > +
> > + tree = &cache->rbtree;
> > +
> > + if (tree->root == tree->sentinel) {
> > + return;
> > + }
> > +
> > + for (node = ngx_rbtree_min(tree->root, tree->sentinel);
> > + node;
> > + node = ngx_rbtree_next(tree, node))
> > + {
> > + cn = ngx_rbtree_data(node, ngx_ssl_cache_node_t, node);
> > +
> > + if (cn->type != NULL && cn->value != NULL) {
> > + cn->type->free(cn->value);
> > + }
> > + }
> > +}
> > +
> > +
> > +static void
> > +ngx_ssl_cache_node_insert(ngx_rbtree_node_t *temp,
> > + ngx_rbtree_node_t *node, ngx_rbtree_node_t *sentinel)
> > +{
> > + ngx_rbtree_node_t **p;
> > + ngx_ssl_cache_node_t *n, *t;
> > +
> > + for ( ;; ) {
> > +
> > + n = ngx_rbtree_data(node, ngx_ssl_cache_node_t, node);
> > + t = ngx_rbtree_data(temp, ngx_ssl_cache_node_t, node);
> > +
> > + if (node->key != temp->key) {
> > +
> > + p = (node->key < temp->key) ? &temp->left : &temp->right;
> > +
> > + } else if (n->type != t->type) {
> > +
> > + p = (n->type < t->type) ? &temp->left : &temp->right;
> > +
> > + } else {
> > +
> > + p = (ngx_memn2cmp(n->id.data, t->id.data, n->id.len, t->id.len)
> > + < 0) ? &temp->left : &temp->right;
> > + }
> > +
> > + if (*p == sentinel) {
> > + break;
> > + }
> > +
> > + temp = *p;
> > + }
> > +
> > + *p = node;
> > + node->parent = temp;
> > + node->left = sentinel;
> > + node->right = sentinel;
> > + ngx_rbt_red(node);
> > +}
> > +
> > +
> > +void *
> > +ngx_ssl_cache_fetch(ngx_cycle_t *cycle, ngx_pool_t *pool,
> > + ngx_ssl_cache_type_t *type, char **err, ngx_str_t *id, void *data)
> > +{
> > + void *value;
> > + uint32_t hash;
> > + ngx_ssl_cache_t *cache;
> > + ngx_ssl_cache_node_t *cn;
> > +
> > + value = NULL;
> > +
> > + hash = ngx_murmur_hash2(id->data, id->len);
> > +
> > + cache = (ngx_ssl_cache_t *) ngx_get_conf(cycle->conf_ctx,
> > + ngx_openssl_cache_module);
> > +
> > + if (ngx_process == NGX_PROCESS_WORKER
> > + || ngx_process == NGX_PROCESS_SINGLE)
> > + {
> > + return type->create(id, err, data);
> > + }
>
> This check kept bothering me, so I figured we can try to improve it.
>
> The logic for configuration time and per-connection fetches is quite
> different and we're always aware of the context we're working in. It would
> be cleaner to have separate fetch implementations, like that:
>
> void *
> ngx_ssl_cache_fetch(ngx_conf_t *cf, ngx_ssl_cache_type_t *type, char
> **err,
> ngx_str_t *id, void *data)
> {
> ...
> }
>
>
> void *
> ngx_ssl_cache_connection_fetch(ngx_ssl_cache_type_t *type, char **err,
> ngx_str_t *id, void *data)
> {
> return type->create(id, err, data);
> }
>
> Full diff is attached, but it needs to be properly split between patches
> 2-6.
>
> As a bonus, the change makes it easier to isolate any future logic for
> dynamic certificate caching. It was noticeable on our review of early
> prototypes how combining both code paths in the `ngx_ssl_cache_fetch` code
> made the function unnecessarily complicated.
I concur, applied.
This also enables SSL object caching to actually work.
The reason is that ngx_ssl_cache_fetch() is called during
configuration parsing, which means that ngx_process initial
value NGX_PROCESS_SINGLE is not yet updated.
While here, rewritten fetch API to eliminate ngx_ssl_cache_type_t
exposure. Previously it was used to index a type-specific object
cache, now it is replaced with a pure index.
The relevant change below, it reverts parts of this 2nd patch.
(Applying it makes it meaningful to qfold the next change as well,
due to non-sensical ngx_ssl_cache_types[] as is.)
I will send the updates patch series separately
after commenting the rest.
# HG changeset patch
# User Sergey Kandaurov <pluknet at nginx.com>
# Date 1724175777 -14400
# Tue Aug 20 21:42:57 2024 +0400
# Node ID c0fb7ecae98b28b3220a70a62d0cdf08dd240b74
# Parent c1b4471b3ecb4407a41b123f85c34f73db3d32ad
imported patch api_cln
diff --git a/src/event/ngx_event_openssl.h b/src/event/ngx_event_openssl.h
--- a/src/event/ngx_event_openssl.h
+++ b/src/event/ngx_event_openssl.h
@@ -83,8 +83,7 @@
#endif
-typedef struct ngx_ssl_ocsp_s ngx_ssl_ocsp_t;
-typedef struct ngx_ssl_cache_type_s ngx_ssl_cache_type_t;
+typedef struct ngx_ssl_ocsp_s ngx_ssl_ocsp_t;
struct ngx_ssl_s {
@@ -225,10 +224,10 @@ ngx_int_t ngx_ssl_ocsp_get_status(ngx_co
void ngx_ssl_ocsp_cleanup(ngx_connection_t *c);
ngx_int_t ngx_ssl_ocsp_cache_init(ngx_shm_zone_t *shm_zone, void *data);
-void *ngx_ssl_cache_fetch(ngx_conf_t *cf, ngx_ssl_cache_type_t *type,
- char **err, ngx_str_t *id, void *data);
-void *ngx_ssl_cache_connection_fetch(ngx_ssl_cache_type_t *type,
- char **err, ngx_str_t *id, void *data);
+void *ngx_ssl_cache_fetch(ngx_conf_t *cf, ngx_uint_t index, char **err,
+ ngx_str_t *id, void *data);
+void *ngx_ssl_cache_connection_fetch(ngx_uint_t index, char **err,
+ ngx_str_t *id, void *data);
ngx_array_t *ngx_ssl_read_password_file(ngx_conf_t *cf, ngx_str_t *file);
ngx_array_t *ngx_ssl_preserve_passwords(ngx_conf_t *cf,
diff --git a/src/event/ngx_event_openssl_cache.c b/src/event/ngx_event_openssl_cache.c
--- a/src/event/ngx_event_openssl_cache.c
+++ b/src/event/ngx_event_openssl_cache.c
@@ -9,6 +9,18 @@
#include <ngx_event.h>
+typedef void *(*ngx_ssl_cache_create_pt)(ngx_str_t *id, char **err, void *data);
+typedef void (*ngx_ssl_cache_free_pt)(void *data);
+typedef void *(*ngx_ssl_cache_ref_pt)(char **err, void *data);
+
+
+typedef struct {
+ ngx_ssl_cache_create_pt create;
+ ngx_ssl_cache_free_pt free;
+ ngx_ssl_cache_ref_pt ref;
+} ngx_ssl_cache_type_t;
+
+
typedef struct {
ngx_rbtree_node_t node;
ngx_str_t id;
@@ -18,18 +30,6 @@ typedef struct {
} ngx_ssl_cache_node_t;
-typedef void *(*ngx_ssl_cache_create_pt)(ngx_str_t *id, char **err, void *data);
-typedef void (*ngx_ssl_cache_free_pt)(void *data);
-typedef void *(*ngx_ssl_cache_ref_pt)(char **err, void *data);
-
-
-struct ngx_ssl_cache_type_s {
- ngx_ssl_cache_create_pt create;
- ngx_ssl_cache_free_pt free;
- ngx_ssl_cache_ref_pt ref;
-};
-
-
typedef struct {
ngx_rbtree_t rbtree;
ngx_rbtree_node_t sentinel;
@@ -68,13 +68,19 @@ ngx_module_t ngx_openssl_cache_module =
};
+static ngx_ssl_cache_type_t ngx_ssl_cache_types[] = {
+
+};
+
+
void *
-ngx_ssl_cache_fetch(ngx_conf_t *cf, ngx_ssl_cache_type_t *type, char **err,
+ngx_ssl_cache_fetch(ngx_conf_t *cf, ngx_uint_t index, char **err,
ngx_str_t *id, void *data)
{
void *value;
uint32_t hash;
ngx_ssl_cache_t *cache;
+ ngx_ssl_cache_type_t *type;
ngx_ssl_cache_node_t *cn;
value = NULL;
@@ -84,6 +90,8 @@ ngx_ssl_cache_fetch(ngx_conf_t *cf, ngx_
cache = (ngx_ssl_cache_t *) ngx_get_conf(cf->cycle->conf_ctx,
ngx_openssl_cache_module);
+ type = &ngx_ssl_cache_types[index];
+
cn = ngx_ssl_cache_lookup(cache, type, id, hash);
if (cn == NULL) {
@@ -123,10 +131,10 @@ ngx_ssl_cache_fetch(ngx_conf_t *cf, ngx_
void *
-ngx_ssl_cache_connection_fetch(ngx_ssl_cache_type_t *type, char **err,
+ngx_ssl_cache_connection_fetch(ngx_uint_t index, char **err,
ngx_str_t *id, void *data)
{
- return type->create(id, err, data);
+ return ngx_ssl_cache_types[index].create(id, err, data);
}
>
> > +
> > + cn = ngx_ssl_cache_lookup(cache, type, id, hash);
> > +
> > + if (cn == NULL) {
> > + cn = ngx_palloc(pool, sizeof(ngx_ssl_cache_node_t) + id->len + 1);
> > + if (cn == NULL) {
> > + return NULL;
> > + }
> > +
> > + cn->node.key = hash;
> > + cn->id.data = (u_char *)(cn + 1);
> > + cn->id.len = id->len;
> > + cn->type = type;
> > + cn->value = NULL;
> > +
> > + ngx_cpystrn(cn->id.data, id->data, id->len + 1);
> > +
> > + ngx_rbtree_insert(&cache->rbtree, &cn->node);
> > + }
> > +
> > + /* try to use a reference from the cache */
> > +
> > + if (cn->value != NULL) {
> > + value = type->ref(err, cn->value);
> > + }
> > +
> > + if (value == NULL) {
> > + value = type->create(id, err, data);
> > + }
> > +
> > + if (value != NULL && cn->value == NULL) {
> > + /* we have a value and the node needs one; try to reference it */
> > + cn->value = type->ref(err, value);
> > + }
> > +
> > + return value;
> > +}
> > +
> > +
> > +static ngx_ssl_cache_node_t *
> > +ngx_ssl_cache_lookup(ngx_ssl_cache_t *cache, ngx_ssl_cache_type_t *type,
> > + ngx_str_t *id, uint32_t hash)
> > +{
> > + ngx_int_t rc;
> > + ngx_rbtree_node_t *node, *sentinel;
> > + ngx_ssl_cache_node_t *cn;
> > +
> > + node = cache->rbtree.root;
> > + sentinel = cache->rbtree.sentinel;
> > +
> > + while (node != sentinel) {
> > +
> > + if (hash < node->key) {
> > + node = node->left;
> > + continue;
> > + }
> > +
> > + if (hash > node->key) {
> > + node = node->right;
> > + continue;
> > + }
> > +
> > + /* hash == node->key */
> > +
> > + cn = (ngx_ssl_cache_node_t *) node;
> > +
> > + if ((ngx_uint_t) type < (ngx_uint_t) cn->type) {
> > + node = node->left;
> > + continue;
> > + }
> > +
> > + if ((ngx_uint_t) type > (ngx_uint_t) cn->type) {
> > + node = node->right;
> > + continue;
> > + }
> > +
> > + /* type == cn->type */
> > +
> > + rc = ngx_memn2cmp(id->data, cn->id.data, id->len, cn->id.len);
> > +
> > + if (rc == 0) {
> > + return cn;
> > + }
> > +
> > + node = (rc < 0) ? node->left : node->right;
> > + }
> > +
> > + return NULL;
> > +}
> > _______________________________________________
> > nginx-devel mailing list
> > nginx-devel at nginx.org
> > https://mailman.nginx.org/mailman/listinfo/nginx-devel
On Tue, Jul 23, 2024 at 07:30:26PM +0000, Mini Hawthorne wrote:
> # HG changeset patch
> # User Mini Hawthorne <mini at f5.com>
> # Date 1721762857 0
> # Tue Jul 23 19:27:37 2024 +0000
> # Node ID 42e86c051200bf00d9ae6e38d6c87a916391b642
> # Parent 8eee61e223bb7cb7475e50b866fd6b9a83fa5fa0
> SSL: caching certificates.
>
> Added ngx_ssl_cache_cert, which loads certificate chains once via BIO's
> created by ngx_ssl_cache_create_bio() which will be used by the following
> patches as well.
>
> The certificate cache provides each chain as a unique stack of shared
> references. This shallow copy is required because OpenSSL's stacks aren't
> reference counted; instead they contain a unique array of referenced entries.
> Also note that callers must pop the first certificate off of the stack due to
> awkwardness in OpenSSL certificate API.
>
> diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c
> --- a/src/event/ngx_event_openssl.c
> +++ b/src/event/ngx_event_openssl.c
> @@ -18,8 +18,6 @@ typedef struct {
> } ngx_openssl_conf_t;
>
>
> -static X509 *ngx_ssl_load_certificate(ngx_pool_t *pool, char **err,
> - ngx_str_t *cert, STACK_OF(X509) **chain);
> static EVP_PKEY *ngx_ssl_load_certificate_key(ngx_pool_t *pool, char **err,
> ngx_str_t *key, ngx_array_t *passwords);
> static int ngx_ssl_password_callback(char *buf, int size, int rwflag,
> @@ -443,8 +441,9 @@ ngx_ssl_certificate(ngx_conf_t *cf, ngx_
> STACK_OF(X509) *chain;
> ngx_ssl_name_t *name;
>
> - x509 = ngx_ssl_load_certificate(cf->pool, &err, cert, &chain);
> - if (x509 == NULL) {
> + chain = ngx_ssl_cache_fetch(cf->cycle, cf->pool, &ngx_ssl_cache_cert,
> + &err, cert, NULL);
This is logically not an allocating function, so its check(s)
should be separated with an empty line, also to be in line wrt
a nearby style consistency for SSL_CTX_get_cert_store() in p6.
> + if (chain == NULL) {
> if (err != NULL) {
> ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
> "cannot load certificate \"%s\": %s",
> @@ -454,6 +453,8 @@ ngx_ssl_certificate(ngx_conf_t *cf, ngx_
> return NGX_ERROR;
> }
>
> + x509 = sk_X509_shift(chain);
> +
> if (SSL_CTX_use_certificate(ssl->ctx, x509) == 0) {
> ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
> "SSL_CTX_use_certificate(\"%s\") failed", cert->data);
> @@ -568,8 +569,9 @@ ngx_ssl_connection_certificate(ngx_conne
> EVP_PKEY *pkey;
> STACK_OF(X509) *chain;
>
> - x509 = ngx_ssl_load_certificate(pool, &err, cert, &chain);
> - if (x509 == NULL) {
> + chain = ngx_ssl_cache_fetch((ngx_cycle_t *) ngx_cycle, c->pool,
> + &ngx_ssl_cache_cert, &err, cert, NULL);
> + if (chain == NULL) {
> if (err != NULL) {
> ngx_ssl_error(NGX_LOG_ERR, c->log, 0,
> "cannot load certificate \"%s\": %s",
> @@ -579,6 +581,8 @@ ngx_ssl_connection_certificate(ngx_conne
> return NGX_ERROR;
> }
>
> + x509 = sk_X509_shift(chain);
> +
> if (SSL_use_certificate(c->ssl->connection, x509) == 0) {
> ngx_ssl_error(NGX_LOG_ERR, c->log, 0,
> "SSL_use_certificate(\"%s\") failed", cert->data);
> @@ -630,96 +634,6 @@ ngx_ssl_connection_certificate(ngx_conne
> }
>
>
> -static X509 *
> -ngx_ssl_load_certificate(ngx_pool_t *pool, char **err, ngx_str_t *cert,
> - STACK_OF(X509) **chain)
> -{
> - BIO *bio;
> - X509 *x509, *temp;
> - u_long n;
> -
> - if (ngx_strncmp(cert->data, "data:", sizeof("data:") - 1) == 0) {
> -
> - bio = BIO_new_mem_buf(cert->data + sizeof("data:") - 1,
> - cert->len - (sizeof("data:") - 1));
> - if (bio == NULL) {
> - *err = "BIO_new_mem_buf() failed";
> - return NULL;
> - }
> -
> - } else {
> -
> - if (ngx_get_full_name(pool, (ngx_str_t *) &ngx_cycle->conf_prefix, cert)
> - != NGX_OK)
> - {
> - *err = NULL;
> - return NULL;
> - }
> -
> - bio = BIO_new_file((char *) cert->data, "r");
> - if (bio == NULL) {
> - *err = "BIO_new_file() failed";
> - return NULL;
> - }
> - }
> -
> - /* certificate itself */
> -
> - x509 = PEM_read_bio_X509_AUX(bio, NULL, NULL, NULL);
> - if (x509 == NULL) {
> - *err = "PEM_read_bio_X509_AUX() failed";
> - BIO_free(bio);
> - return NULL;
> - }
> -
> - /* rest of the chain */
> -
> - *chain = sk_X509_new_null();
> - if (*chain == NULL) {
> - *err = "sk_X509_new_null() failed";
> - BIO_free(bio);
> - X509_free(x509);
> - return NULL;
> - }
> -
> - for ( ;; ) {
> -
> - temp = PEM_read_bio_X509(bio, NULL, NULL, NULL);
> - if (temp == NULL) {
> - n = ERR_peek_last_error();
> -
> - if (ERR_GET_LIB(n) == ERR_LIB_PEM
> - && ERR_GET_REASON(n) == PEM_R_NO_START_LINE)
> - {
> - /* end of file */
> - ERR_clear_error();
> - break;
> - }
> -
> - /* some real error */
> -
> - *err = "PEM_read_bio_X509() failed";
> - BIO_free(bio);
> - X509_free(x509);
> - sk_X509_pop_free(*chain, X509_free);
> - return NULL;
> - }
> -
> - if (sk_X509_push(*chain, temp) == 0) {
> - *err = "sk_X509_push() failed";
> - BIO_free(bio);
> - X509_free(x509);
> - sk_X509_pop_free(*chain, X509_free);
> - return NULL;
> - }
> - }
> -
> - BIO_free(bio);
> -
> - return x509;
> -}
> -
> -
> static EVP_PKEY *
> ngx_ssl_load_certificate_key(ngx_pool_t *pool, char **err,
> ngx_str_t *key, ngx_array_t *passwords)
> diff --git a/src/event/ngx_event_openssl.h b/src/event/ngx_event_openssl.h
> --- a/src/event/ngx_event_openssl.h
> +++ b/src/event/ngx_event_openssl.h
> @@ -351,4 +351,7 @@ extern int ngx_ssl_ocsp_index;
> extern int ngx_ssl_index;
>
>
> +extern ngx_ssl_cache_type_t ngx_ssl_cache_cert;
Throughout the series, this is replaced with a corresponding macro.
The change on top of the series would be (posted for review clarity):
diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c
--- a/src/event/ngx_event_openssl.c
+++ b/src/event/ngx_event_openssl.c
@@ -445,7 +445,7 @@ ngx_ssl_certificate(ngx_conf_t *cf, ngx_
EVP_PKEY *pkey;
STACK_OF(X509) *chain;
- chain = ngx_ssl_cache_fetch(cf, &ngx_ssl_cache_cert, &err, cert, NULL);
+ chain = ngx_ssl_cache_fetch(cf, NGX_SSL_CACHE_CERT, &err, cert, NULL);
if (chain == NULL) {
if (err != NULL) {
ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
@@ -535,7 +535,7 @@ ngx_ssl_certificate(ngx_conf_t *cf, ngx_
}
#endif
- pkey = ngx_ssl_cache_fetch(cf, &ngx_ssl_cache_key, &err, key, passwords);
+ pkey = ngx_ssl_cache_fetch(cf, NGX_SSL_CACHE_KEY, &err, key, passwords);
if (pkey == NULL) {
if (err != NULL) {
ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
@@ -568,7 +568,7 @@ ngx_ssl_connection_certificate(ngx_conne
EVP_PKEY *pkey;
STACK_OF(X509) *chain;
- chain = ngx_ssl_cache_connection_fetch(&ngx_ssl_cache_cert, &err, cert,
+ chain = ngx_ssl_cache_connection_fetch(NGX_SSL_CACHE_CERT, &err, cert,
NULL);
if (chain == NULL) {
if (err != NULL) {
@@ -609,7 +609,7 @@ ngx_ssl_connection_certificate(ngx_conne
#endif
- pkey = ngx_ssl_cache_connection_fetch(&ngx_ssl_cache_key, &err, key,
+ pkey = ngx_ssl_cache_connection_fetch(NGX_SSL_CACHE_KEY, &err, key,
passwords);
if (pkey == NULL) {
if (err != NULL) {
@@ -686,7 +686,7 @@ ngx_ssl_client_certificate(ngx_conf_t *c
return NGX_ERROR;
}
- chain = ngx_ssl_cache_fetch(cf, &ngx_ssl_cache_ca, &err, cert, NULL);
+ chain = ngx_ssl_cache_fetch(cf, NGX_SSL_CACHE_CA, &err, cert, NULL);
if (chain == NULL) {
if (err != NULL) {
ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
@@ -799,7 +799,7 @@ ngx_ssl_trusted_certificate(ngx_conf_t *
return NGX_ERROR;
}
- chain = ngx_ssl_cache_fetch(cf, &ngx_ssl_cache_ca, &err, cert, NULL);
+ chain = ngx_ssl_cache_fetch(cf, NGX_SSL_CACHE_CA, &err, cert, NULL);
if (chain == NULL) {
if (err != NULL) {
ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
@@ -866,7 +866,7 @@ ngx_ssl_crl(ngx_conf_t *cf, ngx_ssl_t *s
return NGX_ERROR;
}
- chain = ngx_ssl_cache_fetch(cf, &ngx_ssl_cache_crl, &err, crl, NULL);
+ chain = ngx_ssl_cache_fetch(cf, NGX_SSL_CACHE_CRL, &err, crl, NULL);
if (chain == NULL) {
if (err != NULL) {
ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
diff --git a/src/event/ngx_event_openssl.h b/src/event/ngx_event_openssl.h
--- a/src/event/ngx_event_openssl.h
+++ b/src/event/ngx_event_openssl.h
@@ -194,6 +193,12 @@ typedef struct {
#define NGX_SSL_BUFSIZE 16384
+#define NGX_SSL_CACHE_CERT 0
+#define NGX_SSL_CACHE_CRL 1
+#define NGX_SSL_CACHE_KEY 2
+#define NGX_SSL_CACHE_CA 3
+
+
ngx_int_t ngx_ssl_init(ngx_log_t *log);
ngx_int_t ngx_ssl_create(ngx_ssl_t *ssl, ngx_uint_t protocols, void *data);
@@ -345,10 +350,4 @@ extern int ngx_ssl_index;
extern int ngx_ssl_certificate_name_index;
-extern ngx_ssl_cache_type_t ngx_ssl_cache_cert;
-extern ngx_ssl_cache_type_t ngx_ssl_cache_crl;
-extern ngx_ssl_cache_type_t ngx_ssl_cache_key;
-extern ngx_ssl_cache_type_t ngx_ssl_cache_ca;
-
-
#endif /* _NGX_EVENT_OPENSSL_H_INCLUDED_ */
diff --git a/src/event/ngx_event_openssl_cache.c b/src/event/ngx_event_openssl_cache.c
--- a/src/event/ngx_event_openssl_cache.c
+++ b/src/event/ngx_event_openssl_cache.c
@@ -86,41 +86,38 @@ ngx_module_t ngx_openssl_cache_module =
};
-ngx_ssl_cache_type_t ngx_ssl_cache_cert = {
- ngx_ssl_cache_cert_create,
- ngx_ssl_cache_cert_free,
- ngx_ssl_cache_cert_ref,
-};
+static ngx_ssl_cache_type_t ngx_ssl_cache_types[] = {
-
-ngx_ssl_cache_type_t ngx_ssl_cache_crl = {
- ngx_ssl_cache_crl_create,
- ngx_ssl_cache_crl_free,
- ngx_ssl_cache_crl_ref,
-};
+ /* NGX_SSL_CACHE_CERT */
+ { ngx_ssl_cache_cert_create,
+ ngx_ssl_cache_cert_free,
+ ngx_ssl_cache_cert_ref },
+ /* NGX_SSL_CACHE_CRT */
+ { ngx_ssl_cache_crl_create,
+ ngx_ssl_cache_crl_free,
+ ngx_ssl_cache_crl_ref },
-ngx_ssl_cache_type_t ngx_ssl_cache_key = {
- ngx_ssl_cache_key_create,
- ngx_ssl_cache_key_free,
- ngx_ssl_cache_key_ref,
-};
+ /* NGX_SSL_CACHE_KEY */
+ { ngx_ssl_cache_key_create,
+ ngx_ssl_cache_key_free,
+ ngx_ssl_cache_key_ref },
-
-ngx_ssl_cache_type_t ngx_ssl_cache_ca = {
- ngx_ssl_cache_ca_create,
- ngx_ssl_cache_cert_free,
- ngx_ssl_cache_cert_ref,
+ /* NGX_SSL_CACHE_CA */
+ { ngx_ssl_cache_ca_create,
+ ngx_ssl_cache_cert_free,
+ ngx_ssl_cache_cert_ref }
};
void *
-ngx_ssl_cache_fetch(ngx_conf_t *cf, ngx_ssl_cache_type_t *type, char **err,
+ngx_ssl_cache_fetch(ngx_conf_t *cf, ngx_uint_t index, char **err,
ngx_str_t *id, void *data)
{
void *value;
uint32_t hash;
ngx_ssl_cache_t *cache;
+ ngx_ssl_cache_type_t *type;
ngx_ssl_cache_node_t *cn;
value = NULL;
@@ -130,6 +127,8 @@ ngx_ssl_cache_fetch(ngx_conf_t *cf, ngx_
cache = (ngx_ssl_cache_t *) ngx_get_conf(cf->cycle->conf_ctx,
ngx_openssl_cache_module);
+ type = &ngx_ssl_cache_types[index];
+
cn = ngx_ssl_cache_lookup(cache, type, id, hash);
if (cn == NULL) {
@@ -169,10 +168,10 @@ ngx_ssl_cache_fetch(ngx_conf_t *cf, ngx_
void *
-ngx_ssl_cache_connection_fetch(ngx_ssl_cache_type_t *type, char **err,
+ngx_ssl_cache_connection_fetch(ngx_uint_t index, char **err,
ngx_str_t *id, void *data)
{
- return type->create(id, err, data);
+ return ngx_ssl_cache_types[index].create(id, err, data);
}
> +
> +
> #endif /* _NGX_EVENT_OPENSSL_H_INCLUDED_ */
> diff --git a/src/event/ngx_event_openssl_cache.c b/src/event/ngx_event_openssl_cache.c
> --- a/src/event/ngx_event_openssl_cache.c
> +++ b/src/event/ngx_event_openssl_cache.c
> @@ -46,6 +46,12 @@ static void ngx_ssl_cache_node_insert(ng
> static ngx_ssl_cache_node_t *ngx_ssl_cache_lookup(ngx_ssl_cache_t *cache,
> ngx_ssl_cache_type_t *type, ngx_str_t *id, uint32_t hash);
>
> +static void *ngx_ssl_cache_cert_create(ngx_str_t *id, char **err, void *data);
> +static void ngx_ssl_cache_cert_free(void *data);
> +static void *ngx_ssl_cache_cert_ref(char **err, void *data);
> +
> +static BIO *ngx_ssl_cache_create_bio(ngx_str_t *id, char **err);
> +
>
> static ngx_core_module_t ngx_openssl_cache_module_ctx = {
> ngx_string("openssl_cache"),
> @@ -54,6 +60,15 @@ static ngx_core_module_t ngx_openssl_ca
> };
>
>
> +ngx_ssl_cache_type_t ngx_ssl_cache_cert = {
> + "certificate chain",
> +
> + ngx_ssl_cache_cert_create,
> + ngx_ssl_cache_cert_free,
> + ngx_ssl_cache_cert_ref,
> +};
> +
> +
This is ought to be placed after module variables (but see above).
> ngx_module_t ngx_openssl_cache_module = {
> NGX_MODULE_V1,
> &ngx_openssl_cache_module_ctx, /* module context */
> @@ -275,3 +290,165 @@ ngx_ssl_cache_lookup(ngx_ssl_cache_t *ca
>
> return NULL;
> }
> +
> +
> +static void *
> +ngx_ssl_cache_cert_create(ngx_str_t *id, char **err, void *data)
> +{
> + BIO *bio;
> + X509 *x;
> + u_long n;
> + STACK_OF(X509) *sk;
Style:
thoughout nginx codebase, variables of such OpenSSL types are used
to have a different consistent naming, such as "x509" and "chain".
Applied to my series, to be posted separately.
> +
> + /* start with an empty certificate chain */
While here, rearranged comments.
> + sk = sk_X509_new_null();
> + if (sk == NULL) {
> + *err = "sk_X509_new_null() failed";
> + return NULL;
> + }
> +
> + /* figure out where to load from */
> + bio = ngx_ssl_cache_create_bio(id, err);
> + if (bio == NULL) {
> + sk_X509_pop_free(sk, X509_free);
> + return NULL;
> + }
> +
> + /* certificate itself */
> + x = PEM_read_bio_X509_AUX(bio, NULL, NULL, NULL);
> + if (x == NULL) {
> + *err = "PEM_read_bio_X509_AUX() failed";
> + BIO_free(bio);
> + sk_X509_pop_free(sk, X509_free);
> + return NULL;
> + }
> +
> + if (sk_X509_push(sk, x) <= 0) {
> + *err = "sk_X509_push() failed";
> + BIO_free(bio);
> + X509_free(x);
> + sk_X509_pop_free(sk, X509_free);
> + return NULL;
> + }
> +
> + /* rest of the chain */
> + for ( ;; ) {
> +
> + x = PEM_read_bio_X509(bio, NULL, NULL, NULL);
> + if (x == NULL) {
> + n = ERR_peek_last_error();
> +
> + if (ERR_GET_LIB(n) == ERR_LIB_PEM
> + && ERR_GET_REASON(n) == PEM_R_NO_START_LINE)
> + {
> + /* end of file */
> + ERR_clear_error();
> + break;
> + }
> +
> + /* some real error */
> + *err = "PEM_read_bio_X509() failed";
> + BIO_free(bio);
> + sk_X509_pop_free(sk, X509_free);
> + return NULL;
> + }
> +
> + if (sk_X509_push(sk, x) <= 0) {
X509 object leak on error path
> + /* memory allocation failed */
> + *err = "sk_X509_push() failed";
> + BIO_free(bio);
> + sk_X509_pop_free(sk, X509_free);
> + return NULL;
> + }
> + }
> +
> + BIO_free(bio);
> + return sk;
> +}
> +
> +
> +static void
> +ngx_ssl_cache_cert_free(void *data)
> +{
> + sk_X509_pop_free(data, X509_free);
> +}
> +
> +
> +static void *
> +ngx_ssl_cache_cert_ref(char **err, void *data)
> +{
> + int n, i;
> + X509 *x;
> + STACK_OF(X509) *sk;
> +
> + /* stacks aren't reference-counted, so shallow copy into a new stack */
> + sk = sk_X509_dup(data);
> + if (sk == NULL) {
> + *err = "sk_X509_dup() failed";
> + return NULL;
> + }
> +
> + /* bump the certificates' reference counts */
> + n = sk_X509_num(sk);
> +
> + for (i = 0; i < n; i++) {
> + x = sk_X509_value(sk, i);
> +
> +#if (OPENSSL_VERSION_NUMBER >= 0x10100000L)
> + X509_up_ref(x);
> +#else
> + CRYPTO_add(&x->references, 1, CRYPTO_LOCK_X509);
> +#endif
> + }
> +
> + return sk;
> +}
> +
> +
> +static BIO *
> +ngx_ssl_cache_create_bio(ngx_str_t *id, char **err)
> +{
> + BIO *bio;
> + ngx_str_t path;
> + ngx_pool_t *temp_pool;
> +
> + if (ngx_strncmp(id->data, "data:", sizeof("data:") - 1) == 0) {
> + bio = BIO_new_mem_buf(id->data + sizeof("data:") - 1,
> + id->len - (sizeof("data:") - 1));
> +
> + if (bio == NULL) {
> + *err = "BIO_new_mem_buf() failed";
> + }
> +
> + return bio;
> + }
> +
> + /* generate a translated path */
> + temp_pool = ngx_create_pool(NGX_MAX_PATH, ngx_cycle->log);
> + if (temp_pool == NULL) {
> + *err = NULL;
> + return NULL;
> + }
> +
> + ngx_memcpy(&path, id, sizeof(ngx_str_t));
> +
> + if (ngx_get_full_name(temp_pool,
> + (ngx_str_t *) &ngx_cycle->conf_prefix,
> + &path)
> + != NGX_OK)
> + {
> + *err = NULL;
> + ngx_destroy_pool(temp_pool);
> + return NULL;
> + }
> +
> + bio = BIO_new_file((char *) path.data, "r");
> +
> + if (bio == NULL) {
> + *err = "BIO_new_file() failed";
> + }
> +
> + ngx_destroy_pool(temp_pool);
> +
> + return bio;
> +}
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> https://mailman.nginx.org/mailman/listinfo/nginx-devel
On Tue, Jul 23, 2024 at 07:30:27PM +0000, Mini Hawthorne wrote:
> # HG changeset patch
> # User Mini Hawthorne <mini at f5.com>
> # Date 1721762914 0
> # Tue Jul 23 19:28:34 2024 +0000
> # Node ID 867e05f555e6f593589a0278c865e7dcffe597f4
> # Parent 42e86c051200bf00d9ae6e38d6c87a916391b642
> SSL: caching certificate revocation lists.
>
> Added ngx_ssl_cache_crl which is similar to certificate caching.
> It basically calls X509_CRL versions of APIs instead of X509 versions.
>
> diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c
> --- a/src/event/ngx_event_openssl.c
> +++ b/src/event/ngx_event_openssl.c
> @@ -886,17 +886,16 @@ ngx_ssl_trusted_certificate(ngx_conf_t *
> ngx_int_t
> ngx_ssl_crl(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *crl)
> {
> - X509_STORE *store;
> - X509_LOOKUP *lookup;
> + int n, i;
> + char *err;
> + X509_CRL *xc;
> + X509_STORE *store;
> + STACK_OF(X509_CRL) *xcsk;
>
> if (crl->len == 0) {
> return NGX_OK;
> }
>
> - if (ngx_conf_full_name(cf->cycle, crl, 1) != NGX_OK) {
> - return NGX_ERROR;
> - }
> -
> store = SSL_CTX_get_cert_store(ssl->ctx);
>
> if (store == NULL) {
> @@ -905,20 +904,44 @@ ngx_ssl_crl(ngx_conf_t *cf, ngx_ssl_t *s
> return NGX_ERROR;
> }
>
> - lookup = X509_STORE_add_lookup(store, X509_LOOKUP_file());
> -
> - if (lookup == NULL) {
> - ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
> - "X509_STORE_add_lookup() failed");
> + xcsk = ngx_ssl_cache_fetch(cf->cycle, cf->pool, &ngx_ssl_cache_crl,
> + &err, crl, NULL);
> + if (xcsk == NULL) {
> + if (err != NULL) {
> + ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
> + "failed to load \"%s\": %s", crl->data, err);
> + }
> +
> return NGX_ERROR;
> }
>
> - if (X509_LOOKUP_load_file(lookup, (char *) crl->data, X509_FILETYPE_PEM)
> - == 0)
> - {
> - ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
> - "X509_LOOKUP_load_file(\"%s\") failed", crl->data);
> - return NGX_ERROR;
> + n = sk_X509_CRL_num(xcsk);
> +
> + for (i = 0; i < n; i++) {
> + xc = sk_X509_CRL_value(xcsk, i);
> +
> + if (X509_STORE_add_crl(store, xc) != 1) {
> +
> +#if !(OPENSSL_VERSION_NUMBER >= 0x1010009fL \
> + || LIBRESSL_VERSION_NUMBER >= 0x3050000fL)
> + u_long error;
> +
> + /* not reported in OpenSSL 1.1.0i+ */
> +
> + error = ERR_peek_last_error();
> +
> + if (ERR_GET_LIB(error) == ERR_LIB_X509
> + && ERR_GET_REASON(error) == X509_R_CERT_ALREADY_IN_HASH_TABLE)
> + {
> + ERR_clear_error();
> + continue;
> + }
> +#endif
> +
> + ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
> + "X509_STORE_add_crl() failed");
> + return NGX_ERROR;
> + }
> }
This now leaks X509_CRL chain.
It is not managed neither by OpenSSL nor referenced with
our own SSL object cache, so should be freed explicitly.
Note that X509_STORE_add_crl() adds its own refcount.
A simple sk_X509_CRL_pop_free() should do the thing.
>
> X509_STORE_set_flags(store,
> diff --git a/src/event/ngx_event_openssl.h b/src/event/ngx_event_openssl.h
> --- a/src/event/ngx_event_openssl.h
> +++ b/src/event/ngx_event_openssl.h
> @@ -352,6 +352,7 @@ extern int ngx_ssl_index;
>
>
> extern ngx_ssl_cache_type_t ngx_ssl_cache_cert;
> +extern ngx_ssl_cache_type_t ngx_ssl_cache_crl;
>
>
> #endif /* _NGX_EVENT_OPENSSL_H_INCLUDED_ */
> diff --git a/src/event/ngx_event_openssl_cache.c b/src/event/ngx_event_openssl_cache.c
> --- a/src/event/ngx_event_openssl_cache.c
> +++ b/src/event/ngx_event_openssl_cache.c
> @@ -50,6 +50,10 @@ static void *ngx_ssl_cache_cert_create(n
> static void ngx_ssl_cache_cert_free(void *data);
> static void *ngx_ssl_cache_cert_ref(char **err, void *data);
>
> +static void *ngx_ssl_cache_crl_create(ngx_str_t *id, char **err, void *data);
> +static void ngx_ssl_cache_crl_free(void *data);
> +static void *ngx_ssl_cache_crl_ref(char **err, void *data);
> +
> static BIO *ngx_ssl_cache_create_bio(ngx_str_t *id, char **err);
>
>
> @@ -69,6 +73,15 @@ ngx_ssl_cache_type_t ngx_ssl_cache_cert
> };
>
>
> +ngx_ssl_cache_type_t ngx_ssl_cache_crl = {
> + "certificate revocation list",
> +
> + ngx_ssl_cache_crl_create,
> + ngx_ssl_cache_crl_free,
> + ngx_ssl_cache_crl_ref,
> +};
> +
> +
> ngx_module_t ngx_openssl_cache_module = {
> NGX_MODULE_V1,
> &ngx_openssl_cache_module_ctx, /* module context */
> @@ -405,6 +418,96 @@ ngx_ssl_cache_cert_ref(char **err, void
> }
>
>
> +static void *
> +ngx_ssl_cache_crl_create(ngx_str_t *id, char **err, void *data)
> +{
> + BIO *bio;
> + u_long n;
> + X509_CRL *xc;
> + STACK_OF(X509_CRL) *sk;
> +
> + /* start with an empty revocation list */
> + sk = sk_X509_CRL_new_null();
> + if (sk == NULL) {
> + *err = "sk_X509_CRL_new_null() failed";
> + return NULL;
> + }
> +
> + /* figure out where to load from */
> + bio = ngx_ssl_cache_create_bio(id, err);
> + if (bio == NULL) {
> + sk_X509_CRL_pop_free(sk, X509_CRL_free);
> + return NULL;
> + }
> +
> + /* read all of the revocations */
> + while ((xc = PEM_read_bio_X509_CRL(bio, NULL, NULL, NULL)) != NULL) {
> + if (sk_X509_CRL_push(sk, xc) <= 0) {
> + *err = "sk_X509_CRL_push() failed";
> + BIO_free(bio);
> + sk_X509_CRL_pop_free(sk, X509_CRL_free);
> + return NULL;
> + }
> + }
> +
> + BIO_free(bio);
> +
> + n = ERR_peek_last_error();
> + if (sk_X509_CRL_num(sk) == 0
> + || ERR_GET_LIB(n) != ERR_LIB_PEM
> + || ERR_GET_REASON(n) != PEM_R_NO_START_LINE)
> + {
> + /* the failure wasn't "no more revocations to load" */
> + *err = "PEM_read_bio_X509_CRL() failed";
> + sk_X509_CRL_pop_free(sk, X509_CRL_free);
> + return NULL;
> + }
> +
Same as above, plus making this function look more similar
to ngx_ssl_cache_cert_create(), for maintenance purpose.
> + /* success leaves errors on the error stack */
> + ERR_clear_error();
> +
> + return sk;
> +}
> +
> +
> +static void
> +ngx_ssl_cache_crl_free(void *data)
> +{
> + sk_X509_CRL_pop_free(data, X509_CRL_free);
> +}
> +
> +
> +static void *
> +ngx_ssl_cache_crl_ref(char **err, void *data)
> +{
> + int n, i;
> + X509_CRL *xc;
> + STACK_OF(X509_CRL) *sk;
> +
> + /* stacks aren't reference-counted, so shallow copy into a new stack */
> + sk = sk_X509_CRL_dup(data);
> + if (sk == NULL) {
> + *err = "sk_X509_CRL_dup() failed";
> + return NULL;
> + }
> +
> + /* bump the revocations' reference counts */
> + n = sk_X509_CRL_num(sk);
> +
> + for (i = 0; i < n; i++) {
> + xc = sk_X509_CRL_value(sk, i);
> +
> +#if (OPENSSL_VERSION_NUMBER >= 0x10100000L)
> + X509_CRL_up_ref(xc);
> +#else
> + CRYPTO_add(&xc->references, 1, CRYPTO_LOCK_X509_CRL);
> +#endif
> + }
> +
> + return sk;
> +}
> +
> +
> static BIO *
> ngx_ssl_cache_create_bio(ngx_str_t *id, char **err)
> {
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> https://mailman.nginx.org/mailman/listinfo/nginx-devel
On Tue, Jul 23, 2024 at 07:30:28PM +0000, Mini Hawthorne wrote:
> # HG changeset patch
> # User Mini Hawthorne <mini at f5.com>
> # Date 1721762945 0
> # Tue Jul 23 19:29:05 2024 +0000
> # Node ID 298a9eaa59d2a16f85b6aa3584eb5f8298e6c9bc
> # Parent 867e05f555e6f593589a0278c865e7dcffe597f4
> SSL: caching private keys.
>
> Added ngx_ssl_cache_key which caches private keys. Special support is included
> for "engine:..." keys.
>
> EVP_KEY objects are a reference-counted container for key material, so shallow
> copies and OpenSSL stack management aren't needed as with certificates.
>
> diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c
> --- a/src/event/ngx_event_openssl.c
> +++ b/src/event/ngx_event_openssl.c
> @@ -18,10 +18,6 @@ typedef struct {
> } ngx_openssl_conf_t;
>
>
> -static EVP_PKEY *ngx_ssl_load_certificate_key(ngx_pool_t *pool, char **err,
> - ngx_str_t *key, ngx_array_t *passwords);
> -static int ngx_ssl_password_callback(char *buf, int size, int rwflag,
> - void *userdata);
> static int ngx_ssl_verify_callback(int ok, X509_STORE_CTX *x509_store);
> static void ngx_ssl_info_callback(const ngx_ssl_conn_t *ssl_conn, int where,
> int ret);
> @@ -536,7 +532,8 @@ ngx_ssl_certificate(ngx_conf_t *cf, ngx_
> }
> #endif
>
> - pkey = ngx_ssl_load_certificate_key(cf->pool, &err, key, passwords);
> + pkey = ngx_ssl_cache_fetch(cf->cycle, cf->pool, &ngx_ssl_cache_key, &err,
> + key, passwords);
> if (pkey == NULL) {
> if (err != NULL) {
> ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
> @@ -610,10 +607,11 @@ ngx_ssl_connection_certificate(ngx_conne
>
> #endif
>
> - pkey = ngx_ssl_load_certificate_key(pool, &err, key, passwords);
> + pkey = ngx_ssl_cache_fetch((ngx_cycle_t *) ngx_cycle, c->pool,
> + &ngx_ssl_cache_key, &err, key, passwords);
> if (pkey == NULL) {
> if (err != NULL) {
> - ngx_ssl_error(NGX_LOG_ERR, c->log, 0,
> + ngx_ssl_error(NGX_LOG_EMERG, c->log, 0,
wrong log level change, seemingly a result of copy'n'paste
> "cannot load certificate key \"%s\": %s",
> key->data, err);
> }
> @@ -634,151 +632,6 @@ ngx_ssl_connection_certificate(ngx_conne
> }
>
>
> -static EVP_PKEY *
> -ngx_ssl_load_certificate_key(ngx_pool_t *pool, char **err,
> - ngx_str_t *key, ngx_array_t *passwords)
> -{
> - BIO *bio;
> - EVP_PKEY *pkey;
> - ngx_str_t *pwd;
> - ngx_uint_t tries;
> - pem_password_cb *cb;
> -
> - if (ngx_strncmp(key->data, "engine:", sizeof("engine:") - 1) == 0) {
> -
> -#ifndef OPENSSL_NO_ENGINE
> -
> - u_char *p, *last;
> - ENGINE *engine;
> -
> - p = key->data + sizeof("engine:") - 1;
> - last = (u_char *) ngx_strchr(p, ':');
> -
> - if (last == NULL) {
> - *err = "invalid syntax";
> - return NULL;
> - }
> -
> - *last = '\0';
> -
> - engine = ENGINE_by_id((char *) p);
> -
> - *last++ = ':';
> -
> - if (engine == NULL) {
> - *err = "ENGINE_by_id() failed";
> - return NULL;
> - }
> -
> - pkey = ENGINE_load_private_key(engine, (char *) last, 0, 0);
> -
> - if (pkey == NULL) {
> - *err = "ENGINE_load_private_key() failed";
> - ENGINE_free(engine);
> - return NULL;
> - }
> -
> - ENGINE_free(engine);
> -
> - return pkey;
> -
> -#else
> -
> - *err = "loading \"engine:...\" certificate keys is not supported";
> - return NULL;
> -
> -#endif
> - }
> -
> - if (ngx_strncmp(key->data, "data:", sizeof("data:") - 1) == 0) {
> -
> - bio = BIO_new_mem_buf(key->data + sizeof("data:") - 1,
> - key->len - (sizeof("data:") - 1));
> - if (bio == NULL) {
> - *err = "BIO_new_mem_buf() failed";
> - return NULL;
> - }
> -
> - } else {
> -
> - if (ngx_get_full_name(pool, (ngx_str_t *) &ngx_cycle->conf_prefix, key)
> - != NGX_OK)
> - {
> - *err = NULL;
> - return NULL;
> - }
> -
> - bio = BIO_new_file((char *) key->data, "r");
> - if (bio == NULL) {
> - *err = "BIO_new_file() failed";
> - return NULL;
> - }
> - }
> -
> - if (passwords) {
> - tries = passwords->nelts;
> - pwd = passwords->elts;
> - cb = ngx_ssl_password_callback;
> -
> - } else {
> - tries = 1;
> - pwd = NULL;
> - cb = NULL;
> - }
> -
> - for ( ;; ) {
> -
> - pkey = PEM_read_bio_PrivateKey(bio, NULL, cb, pwd);
> - if (pkey != NULL) {
> - break;
> - }
> -
> - if (tries-- > 1) {
> - ERR_clear_error();
> - (void) BIO_reset(bio);
> - pwd++;
> - continue;
> - }
> -
> - *err = "PEM_read_bio_PrivateKey() failed";
> - BIO_free(bio);
> - return NULL;
> - }
> -
> - BIO_free(bio);
> -
> - return pkey;
> -}
> -
> -
> -static int
> -ngx_ssl_password_callback(char *buf, int size, int rwflag, void *userdata)
> -{
> - ngx_str_t *pwd = userdata;
> -
> - if (rwflag) {
> - ngx_log_error(NGX_LOG_ALERT, ngx_cycle->log, 0,
> - "ngx_ssl_password_callback() is called for encryption");
> - return 0;
> - }
> -
> - if (pwd == NULL) {
> - return 0;
> - }
> -
> - if (pwd->len > (size_t) size) {
> - ngx_log_error(NGX_LOG_ERR, ngx_cycle->log, 0,
> - "password is truncated to %d bytes", size);
> - } else {
> - size = pwd->len;
> - }
> -
> - ngx_memcpy(buf, pwd->data, size);
> -
> - return size;
> -}
> -
> -
> ngx_int_t
> ngx_ssl_ciphers(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *ciphers,
> ngx_uint_t prefer_server_ciphers)
> diff --git a/src/event/ngx_event_openssl.h b/src/event/ngx_event_openssl.h
> --- a/src/event/ngx_event_openssl.h
> +++ b/src/event/ngx_event_openssl.h
> @@ -353,6 +353,7 @@ extern int ngx_ssl_index;
>
> extern ngx_ssl_cache_type_t ngx_ssl_cache_cert;
> extern ngx_ssl_cache_type_t ngx_ssl_cache_crl;
> +extern ngx_ssl_cache_type_t ngx_ssl_cache_key;
>
>
> #endif /* _NGX_EVENT_OPENSSL_H_INCLUDED_ */
> diff --git a/src/event/ngx_event_openssl_cache.c b/src/event/ngx_event_openssl_cache.c
> --- a/src/event/ngx_event_openssl_cache.c
> +++ b/src/event/ngx_event_openssl_cache.c
> @@ -54,6 +54,12 @@ static void *ngx_ssl_cache_crl_create(ng
> static void ngx_ssl_cache_crl_free(void *data);
> static void *ngx_ssl_cache_crl_ref(char **err, void *data);
>
> +static void *ngx_ssl_cache_key_create(ngx_str_t *id, char **err, void *data);
> +static int ngx_ssl_cache_key_password_callback(char *buf, int size, int rwflag,
> + void *userdata);
> +static void ngx_ssl_cache_key_free(void *data);
> +static void *ngx_ssl_cache_key_ref(char **err, void *data);
> +
> static BIO *ngx_ssl_cache_create_bio(ngx_str_t *id, char **err);
>
>
> @@ -82,6 +88,15 @@ ngx_ssl_cache_type_t ngx_ssl_cache_crl
> };
>
>
> +ngx_ssl_cache_type_t ngx_ssl_cache_key = {
> + "private key",
> +
> + ngx_ssl_cache_key_create,
> + ngx_ssl_cache_key_free,
> + ngx_ssl_cache_key_ref,
> +};
> +
> +
> ngx_module_t ngx_openssl_cache_module = {
> NGX_MODULE_V1,
> &ngx_openssl_cache_module_ctx, /* module context */
> @@ -508,6 +523,154 @@ ngx_ssl_cache_crl_ref(char **err, void *
> }
>
>
> +static void *
> +ngx_ssl_cache_key_create(ngx_str_t *id, char **err, void *data)
> +{
> + ngx_array_t *passwords = data;
> +
> + BIO *bio;
> + EVP_PKEY *pkey;
> + ngx_str_t *pwd;
> + ngx_uint_t tries;
> + pem_password_cb *cb;
> +
> + if (ngx_strncmp(id->data, "engine:", sizeof("engine:") - 1) == 0) {
> +
> +#ifndef OPENSSL_NO_ENGINE
> +
> + u_char *p, *last;
> + ENGINE *engine;
> +
> + p = id->data + sizeof("engine:") - 1;
> + last = (u_char *) ngx_strchr(p, ':');
> +
> + if (last == NULL) {
> + *err = "invalid syntax";
> + return NULL;
> + }
> +
> + *last = '\0';
> +
> + engine = ENGINE_by_id((char *) p);
> +
> + *last++ = ':';
> +
> + if (engine == NULL) {
> + *err = "ENGINE_by_id() failed";
> + return NULL;
> + }
> +
> + pkey = ENGINE_load_private_key(engine, (char *) last, 0, 0);
> +
> + if (pkey == NULL) {
> + *err = "ENGINE_load_private_key() failed";
> + ENGINE_free(engine);
> + return NULL;
> + }
> +
> + ENGINE_free(engine);
> +
> + return pkey;
> +#else
> + *err = "loading \"engine:...\" certificate keys is not supported";
> + return NULL;
> +#endif
> + }
> +
same as above, plus restoring some existing style from
ngx_ssl_load_certificate_key() where it was copied from
> + /* figure out where to load from */
> + bio = ngx_ssl_cache_create_bio(id, err);
> + if (bio == NULL) {
> + return NULL;
> + }
> +
> + if (passwords) {
> + tries = passwords->nelts;
> + pwd = passwords->elts;
> + cb = ngx_ssl_cache_key_password_callback;
> +
> + } else {
> + tries = 1;
> + pwd = NULL;
> + cb = NULL;
> + }
> +
> + for ( ;; ) {
> +
> + pkey = PEM_read_bio_PrivateKey(bio, NULL, cb, pwd);
> + if (pkey != NULL) {
> + break;
> + }
> +
> + if (tries-- > 1) {
> + ERR_clear_error();
> + (void) BIO_reset(bio);
> + pwd++;
> + continue;
> + }
> +
> + *err = "PEM_read_bio_PrivateKey() failed";
> + BIO_free(bio);
> + return NULL;
> + }
> +
> + BIO_free(bio);
> +
> + return pkey;
> +}
> +
> +
> +static int
> +ngx_ssl_cache_key_password_callback(char *buf, int size, int rwflag,
> + void *userdata)
> +{
> + ngx_str_t *pwd = userdata;
> +
> + if (rwflag) {
> + ngx_log_error(NGX_LOG_ALERT, ngx_cycle->log, 0,
> + "ngx_ssl_cache_key_password_callback() is called "
> + "for encryption");
> + return 0;
> + }
> +
> + if (pwd == NULL) {
> + return 0;
> + }
> +
> + if (pwd->len > (size_t) size) {
> + ngx_log_error(NGX_LOG_ERR, ngx_cycle->log, 0,
> + "password is truncated to %d bytes", size);
> + } else {
> + size = pwd->len;
> + }
> +
> + ngx_memcpy(buf, pwd->data, size);
> +
> + return size;
> +}
> +
> +
> +static void
> +ngx_ssl_cache_key_free(void *data)
> +{
> + EVP_PKEY_free(data);
> +}
> +
> +
> +static void *
> +ngx_ssl_cache_key_ref(char **err, void *data)
> +{
> + EVP_PKEY *pkey = data;
> +
> +#if (OPENSSL_VERSION_NUMBER >= 0x10100000L)
> + EVP_PKEY_up_ref(pkey);
> +#else
> + CRYPTO_add(&pkey->references, 1, CRYPTO_LOCK_EVP_PKEY);
> +#endif
> +
> + return data;
> +}
> +
> +
> static BIO *
> ngx_ssl_cache_create_bio(ngx_str_t *id, char **err)
> {
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> https://mailman.nginx.org/mailman/listinfo/nginx-devel
On Tue, Jul 23, 2024 at 07:30:29PM +0000, Mini Hawthorne wrote:
> # HG changeset patch
> # User Mini Hawthorne <mini at f5.com>
> # Date 1721762968 0
> # Tue Jul 23 19:29:28 2024 +0000
> # Node ID c4a90845888cfa20a4f622eb97954dfbd54af5c6
> # Parent 298a9eaa59d2a16f85b6aa3584eb5f8298e6c9bc
> SSL: caching CA certificates.
>
> This can potentially provide a large amount of savings,
> because CA certificates can be quite large.
>
> diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c
> --- a/src/event/ngx_event_openssl.c
> +++ b/src/event/ngx_event_openssl.c
> @@ -18,6 +18,8 @@ typedef struct {
> } ngx_openssl_conf_t;
>
>
> +static int ngx_ssl_x509_name_cmp(const X509_NAME *const *a,
> + const X509_NAME *const *b);
> static int ngx_ssl_verify_callback(int ok, X509_STORE_CTX *x509_store);
> static void ngx_ssl_info_callback(const ngx_ssl_conn_t *ssl_conn, int where,
> int ret);
> @@ -651,10 +653,23 @@ ngx_ssl_ciphers(ngx_conf_t *cf, ngx_ssl_
> }
>
>
> +static int
> +ngx_ssl_x509_name_cmp(const X509_NAME *const *a, const X509_NAME *const *b)
> +{
> + return (X509_NAME_cmp(*a, *b));
> +}
> +
> +
For no apparent reason, comparing to my patch posted internally
to fix sending CA list with duplicate entities, here the location
of ngx_ssl_x509_name_cmp() was moved before its caller.
This contradicts nginx style and was reverted in my series
(to be posted separately).
> ngx_int_t
> ngx_ssl_client_certificate(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *cert,
> ngx_int_t depth)
> {
> + int n, i;
> + char *err;
> + X509 *x;
> + X509_NAME *xn;
> + X509_STORE *xs;
> + STACK_OF(X509) *xsk;
> STACK_OF(X509_NAME) *list;
>
> SSL_CTX_set_verify(ssl->ctx, SSL_VERIFY_PEER, ngx_ssl_verify_callback);
> @@ -665,33 +680,91 @@ ngx_ssl_client_certificate(ngx_conf_t *c
> return NGX_OK;
> }
>
> - if (ngx_conf_full_name(cf->cycle, cert, 1) != NGX_OK) {
> + list = sk_X509_NAME_new(ngx_ssl_x509_name_cmp);
> + if (list == NULL) {
> return NGX_ERROR;
> }
>
> - if (SSL_CTX_load_verify_locations(ssl->ctx, (char *) cert->data, NULL)
> - == 0)
> - {
> - ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
> - "SSL_CTX_load_verify_locations(\"%s\") failed",
> - cert->data);
> + xs = SSL_CTX_get_cert_store(ssl->ctx);
Although there is no evidence SSL_CTX_get_cert_store() may ever fail,
because X509_STORE is allocated as part of SSL_CTX, yet we have
existing checks elsewhere, so they should be repeated for consistency.
Removing them can be discussed separately, after landing this series.
> + xsk = ngx_ssl_cache_fetch(cf->cycle, cf->pool, &ngx_ssl_cache_ca, &err,
> + cert, NULL);
> + if (xsk == NULL) {
> + if (err != NULL) {
> + ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
> + "failed to load \"%s\": %s", cert->data, err);
> + }
> +
> + sk_X509_NAME_pop_free(list, X509_NAME_free);
> return NGX_ERROR;
> }
>
> - /*
> - * SSL_CTX_load_verify_locations() may leave errors in the error queue
> - * while returning success
> - */
> -
> - ERR_clear_error();
> -
> - list = SSL_load_client_CA_file((char *) cert->data);
> -
> - if (list == NULL) {
> - ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
> - "SSL_load_client_CA_file(\"%s\") failed", cert->data);
> - return NGX_ERROR;
> - }
> + n = sk_X509_num(xsk);
> +
> + for (i = 0; i < n; i++) {
> + x = sk_X509_value(xsk, i);
> +
> + xn = X509_get_subject_name(x);
> + if (xn == NULL) {
> + ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
> + "X509_get_subject_name() failed");
> + sk_X509_NAME_pop_free(list, X509_NAME_free);
> + sk_X509_pop_free(xsk, X509_free);
> + return NGX_ERROR;
> + }
> +
> + xn = X509_NAME_dup(xn);
> + if (xn == NULL) {
> + ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0, "X509_NAME_dup() failed");
> + sk_X509_NAME_pop_free(list, X509_NAME_free);
> + sk_X509_pop_free(xsk, X509_free);
> + return NGX_ERROR;
> + }
> +
> +#ifdef OPENSSL_IS_BORINGSSL
> + if (sk_X509_NAME_find(list, NULL, xn) > 0) {
> +#else
> + if (sk_X509_NAME_find(list, xn) >= 0) {
> +#endif
> + X509_NAME_free(xn);
> + continue;
> + }
> +
> + if (!sk_X509_NAME_push(list, xn)) {
> + ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
> + "sk_X509_NAME_push() failed");
> + sk_X509_NAME_pop_free(list, X509_NAME_free);
> + sk_X509_pop_free(xsk, X509_free);
> + X509_NAME_free(xn);
> + return NGX_ERROR;
> + }
Reshuffled a bit, moving constructing CA list after X509_STORE
is prepared, to make the code sequence look more natural, which
is also in line with previously used SSL_load_client_CA_file()
this code is used to re-implement.
> +
> + if (X509_STORE_add_cert(xs, x) != 1) {
> +
> +#if !(OPENSSL_VERSION_NUMBER >= 0x1010009fL \
> + || LIBRESSL_VERSION_NUMBER >= 0x3050000fL)
> + u_long error;
> +
> + /* not reported in OpenSSL 1.1.0i+ */
> +
> + error = ERR_peek_last_error();
> +
> + if (ERR_GET_LIB(error) == ERR_LIB_X509
> + && ERR_GET_REASON(error) == X509_R_CERT_ALREADY_IN_HASH_TABLE)
> + {
> + ERR_clear_error();
> + continue;
> + }
> +#endif
> +
> + ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
> + "X509_STORE_add_cert() failed");
> + sk_X509_NAME_pop_free(list, X509_NAME_free);
> + sk_X509_pop_free(xsk, X509_free);
> + return NGX_ERROR;
> + }
> + }
> +
> + sk_X509_pop_free(xsk, X509_free);
>
> SSL_CTX_set_client_CA_list(ssl->ctx, list);
>
> @@ -703,6 +776,12 @@ ngx_int_t
> ngx_ssl_trusted_certificate(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *cert,
> ngx_int_t depth)
> {
> + int i, n;
> + char *err;
> + X509 *x;
> + X509_STORE *xs;
> + STACK_OF(X509) *xsk;
> +
> SSL_CTX_set_verify(ssl->ctx, SSL_CTX_get_verify_mode(ssl->ctx),
> ngx_ssl_verify_callback);
>
> @@ -712,25 +791,49 @@ ngx_ssl_trusted_certificate(ngx_conf_t *
> return NGX_OK;
> }
>
> - if (ngx_conf_full_name(cf->cycle, cert, 1) != NGX_OK) {
> + xs = SSL_CTX_get_cert_store(ssl->ctx);
> + xsk = ngx_ssl_cache_fetch(cf->cycle, cf->pool, &ngx_ssl_cache_ca, &err,
> + cert, NULL);
> + if (xsk == NULL) {
> + if (err != NULL) {
> + ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
> + "failed to load \"%s\": %s", cert->data, err);
> + }
> +
> return NGX_ERROR;
> }
>
> - if (SSL_CTX_load_verify_locations(ssl->ctx, (char *) cert->data, NULL)
> - == 0)
> - {
> - ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
> - "SSL_CTX_load_verify_locations(\"%s\") failed",
> - cert->data);
> - return NGX_ERROR;
> - }
> -
> - /*
> - * SSL_CTX_load_verify_locations() may leave errors in the error queue
> - * while returning success
> - */
> -
> - ERR_clear_error();
> + n = sk_X509_num(xsk);
> +
> + for (i = 0; i < n; i++) {
> + x = sk_X509_value(xsk, i);
> +
> + if (X509_STORE_add_cert(xs, x) != 1) {
> +
> +#if !(OPENSSL_VERSION_NUMBER >= 0x1010009fL \
> + || LIBRESSL_VERSION_NUMBER >= 0x3050000fL)
> + u_long error;
> +
> + /* not reported in OpenSSL 1.1.0i+ */
> +
> + error = ERR_peek_last_error();
> +
> + if (ERR_GET_LIB(error) == ERR_LIB_X509
> + && ERR_GET_REASON(error) == X509_R_CERT_ALREADY_IN_HASH_TABLE)
> + {
> + ERR_clear_error();
> + continue;
> + }
> +#endif
> +
> + ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
> + "X509_STORE_add_cert() failed");
> + sk_X509_pop_free(xsk, X509_free);
> + return NGX_ERROR;
> + }
> + }
> +
> + sk_X509_pop_free(xsk, X509_free);
>
> return NGX_OK;
> }
> diff --git a/src/event/ngx_event_openssl.h b/src/event/ngx_event_openssl.h
> --- a/src/event/ngx_event_openssl.h
> +++ b/src/event/ngx_event_openssl.h
> @@ -351,6 +351,7 @@ extern int ngx_ssl_ocsp_index;
> extern int ngx_ssl_index;
>
>
> +extern ngx_ssl_cache_type_t ngx_ssl_cache_ca;
> extern ngx_ssl_cache_type_t ngx_ssl_cache_cert;
> extern ngx_ssl_cache_type_t ngx_ssl_cache_crl;
> extern ngx_ssl_cache_type_t ngx_ssl_cache_key;
> diff --git a/src/event/ngx_event_openssl_cache.c b/src/event/ngx_event_openssl_cache.c
> --- a/src/event/ngx_event_openssl_cache.c
> +++ b/src/event/ngx_event_openssl_cache.c
> @@ -46,6 +46,7 @@ static void ngx_ssl_cache_node_insert(ng
> static ngx_ssl_cache_node_t *ngx_ssl_cache_lookup(ngx_ssl_cache_t *cache,
> ngx_ssl_cache_type_t *type, ngx_str_t *id, uint32_t hash);
>
> +static void *ngx_ssl_cache_ca_create(ngx_str_t *id, char **err, void *data);
It's probably better move this declaration to a distinct block,
to make it similar to other SSL object type functions.
> static void *ngx_ssl_cache_cert_create(ngx_str_t *id, char **err, void *data);
> static void ngx_ssl_cache_cert_free(void *data);
> static void *ngx_ssl_cache_cert_ref(char **err, void *data);
> @@ -70,6 +71,15 @@ static ngx_core_module_t ngx_openssl_ca
> };
>
>
> +ngx_ssl_cache_type_t ngx_ssl_cache_ca = {
> + "certificate CA list",
> +
> + ngx_ssl_cache_ca_create,
> + ngx_ssl_cache_cert_free,
> + ngx_ssl_cache_cert_ref,
> +};
> +
> +
> ngx_ssl_cache_type_t ngx_ssl_cache_cert = {
> "certificate chain",
>
> @@ -321,6 +331,58 @@ ngx_ssl_cache_lookup(ngx_ssl_cache_t *ca
>
>
> static void *
> +ngx_ssl_cache_ca_create(ngx_str_t *id, char **err, void *data)
> +{
> + BIO *bio;
> + X509 *x;
> + u_long n;
> + STACK_OF(X509) *sk;
> +
> + /* start with an empty certificate chain */
> + sk = sk_X509_new_null();
> + if (sk == NULL) {
> + *err = "sk_X509_new_null() failed";
> + return NULL;
> + }
> +
> + /* figure out where to load from */
> + bio = ngx_ssl_cache_create_bio(id, err);
> + if (bio == NULL) {
> + sk_X509_pop_free(sk, X509_free);
> + return NULL;
> + }
> +
> + /* read all of the certificates */
> + while ((x = PEM_read_bio_X509_AUX(bio, NULL, NULL, NULL)) != NULL) {
> + if (sk_X509_push(sk, x) <= 0) {
> + *err = "sk_X509_push() failed";
> + BIO_free(bio);
> + sk_X509_pop_free(sk, X509_free);
> + return NULL;
> + }
> + }
> +
> + BIO_free(bio);
> +
> + n = ERR_peek_last_error();
> + if (sk_X509_num(sk) == 0
> + || ERR_GET_LIB(n) != ERR_LIB_PEM
> + || ERR_GET_REASON(n) != PEM_R_NO_START_LINE)
> + {
> + /* the failure wasn't "no more certificates to load" */
> + *err = "PEM_read_bio_X509() failed";
> + sk_X509_pop_free(sk, X509_free);
> + return NULL;
> + }
See above, largely follows ngx_ssl_cache_crl_create() rewrite.
> +
> + /* success leaves errors on the error stack */
> + ERR_clear_error();
> +
> + return sk;
> +}
> +
> +
> +static void *
> ngx_ssl_cache_cert_create(ngx_str_t *id, char **err, void *data)
> {
> BIO *bio;
More information about the nginx-devel
mailing list