[PATCH 3 of 6] SSL: caching certificates
Aleksei Bavshin
a.bavshin at nginx.com
Thu Aug 29 23:28:49 UTC 2024
On 8/21/2024 3:04 PM, Sergey Kandaurov wrote:
> # HG changeset patch
> # User Sergey Kandaurov <pluknet at nginx.com>
> # Date 1721762857 0
> # Tue Jul 23 19:27:37 2024 +0000
> # Node ID 0d87e1495981ca541d8cdb947d94f20a686545a3
> # Parent 6fbe0bcb81696bba12d186e5c15323046bcac2d9
> SSL: caching certificates.
>
> Certificate chains are now loaded once.
>
> The certificate cache provides each chain as a unique stack of referenced
> counted elements. This shallow copy is required because OpenSSL's stacks
> aren't reference counted.
>
> Based on previous work by Mini Hawthorne.
>
> 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, NGX_SSL_CACHE_CERT, &err, cert, NULL);
> +
> + 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,10 @@ 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_connection_fetch(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 +582,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 +635,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
> @@ -202,6 +202,9 @@ typedef struct {
> #define NGX_SSL_BUFSIZE 16384
>
>
> +#define NGX_SSL_CACHE_CERT 0
> +
> +
> 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);
>
> 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
> @@ -39,6 +39,12 @@ typedef struct {
> 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 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,
> @@ -70,6 +76,10 @@ ngx_module_t ngx_openssl_cache_module =
>
> static ngx_ssl_cache_type_t ngx_ssl_cache_types[] = {
>
> + /* NGX_SSL_CACHE_CERT */
> + { ngx_ssl_cache_cert_create,
> + ngx_ssl_cache_cert_free,
> + ngx_ssl_cache_cert_ref },
> };
>
>
> @@ -191,6 +201,166 @@ ngx_ssl_cache_lookup(ngx_ssl_cache_t *ca
>
>
> static void *
> +ngx_ssl_cache_cert_create(ngx_str_t *id, char **err, void *data)
> +{
> + BIO *bio;
> + X509 *x509;
> + u_long n;
> + STACK_OF(X509) *chain;
> +
> + chain = sk_X509_new_null();
> + if (chain == NULL) {
> + *err = "sk_X509_new_null() failed";
> + return NULL;
> + }
> +
> + bio = ngx_ssl_cache_create_bio(id, err);
> + if (bio == NULL) {
> + sk_X509_pop_free(chain, X509_free);
> + 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);
> + sk_X509_pop_free(chain, X509_free);
> + return NULL;
> + }
> +
> + if (sk_X509_push(chain, x509) == 0) {
> + *err = "sk_X509_push() failed";
> + BIO_free(bio);
> + X509_free(x509);
> + sk_X509_pop_free(chain, X509_free);
> + return NULL;
> + }
> +
> + /* rest of the chain */
> +
> + for ( ;; ) {
> +
> + x509 = PEM_read_bio_X509(bio, NULL, NULL, NULL);
> + if (x509 == 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(chain, X509_free);
> + return NULL;
> + }
> +
> + if (sk_X509_push(chain, x509) == 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 chain;
> +}
> +
> +
> +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 *x509;
> + STACK_OF(X509) *chain;
> +
> + chain = sk_X509_dup(data);
> + if (chain == NULL) {
> + *err = "sk_X509_dup() failed";
> + return NULL;
> + }
> +
> + n = sk_X509_num(chain);
> +
> + for (i = 0; i < n; i++) {
> + x509 = sk_X509_value(chain, i);
> +
> +#if (OPENSSL_VERSION_NUMBER >= 0x10100000L)
> + X509_up_ref(x509);
> +#else
> + CRYPTO_add(&x509->references, 1, CRYPTO_LOCK_X509);
> +#endif
> + }
> +
> + return chain;
> +}
> +
> +
> +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;
> + }
> +
> + 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;
> +}
> +
> +
> +static void *
> ngx_openssl_cache_create_conf(ngx_cycle_t *cycle)
> {
> ngx_ssl_cache_t *cache;
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> https://mailman.nginx.org/mailman/listinfo/nginx-devel
The patch introduces a small difference in the behavior: we no longer
resolve the full name preemptively and modify the paths in place.
This results in:
* possible duplication if the same object is referenced both by
absolute and relative paths
* relative certificate paths in the name_rbtree and in the OCSP
stapling log
Below is the patch addressing that and a test update. Note that a few
lines dealing with NGX_SSL_CACHE_KEY should be applied to the patch 4.
Otherwise, the series looks good to me.
(There's one more difference in behavior; we started accepting "data:"
in the trusted CA and CRL configuration directives. I've been reviewing
with assumption that it is an intended consequence.)
diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c
index 0f97bc3af1..48155a2cdd 100644
--- a/src/event/ngx_event_openssl.c
+++ b/src/event/ngx_event_openssl.c
@@ -569,7 +569,7 @@ ngx_ssl_connection_certificate(ngx_connection_t *c,
ngx_pool_t *pool,
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(c, NGX_SSL_CACHE_CERT, &err,
cert,
NULL);
if (chain == NULL) {
@@ -611,7 +611,7 @@ ngx_ssl_connection_certificate(ngx_connection_t *c,
ngx_pool_t *pool,
#endif
- pkey = ngx_ssl_cache_connection_fetch(NGX_SSL_CACHE_KEY, &err, key,
+ pkey = ngx_ssl_cache_connection_fetch(c, NGX_SSL_CACHE_KEY, &err, key,
passwords);
if (pkey == NULL) {
diff --git a/src/event/ngx_event_openssl.h b/src/event/ngx_event_openssl.h
index 079d0003a1..e630767978 100644
--- a/src/event/ngx_event_openssl.h
+++ b/src/event/ngx_event_openssl.h
@@ -241,8 +241,8 @@ 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_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);
+void *ngx_ssl_cache_connection_fetch(ngx_connection_t *c, 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
index 2cbef83faa..7492b8399e 100644
--- a/src/event/ngx_event_openssl_cache.c
+++ b/src/event/ngx_event_openssl_cache.c
@@ -57,6 +57,9 @@ static void *ngx_ssl_cache_ca_create(ngx_str_t *id,
char **err, void *data);
static BIO *ngx_ssl_cache_create_bio(ngx_str_t *id, char **err);
+static ngx_int_t ngx_ssl_cache_full_name(ngx_pool_t *pool, ngx_uint_t
index,
+ ngx_str_t *id);
+
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,
@@ -120,6 +123,10 @@ ngx_ssl_cache_fetch(ngx_conf_t *cf, ngx_uint_t
index, char **err,
ngx_ssl_cache_type_t *type;
ngx_ssl_cache_node_t *cn;
+ if (ngx_ssl_cache_full_name(cf->pool, index, id) != NGX_OK) {
+ return NULL;
+ }
+
value = NULL;
hash = ngx_murmur_hash2(id->data, id->len);
@@ -168,9 +175,13 @@ ngx_ssl_cache_fetch(ngx_conf_t *cf, ngx_uint_t
index, char **err,
void *
-ngx_ssl_cache_connection_fetch(ngx_uint_t index, char **err,
- ngx_str_t *id, void *data)
+ngx_ssl_cache_connection_fetch(ngx_connection_t *c, ngx_uint_t index,
+ char **err, ngx_str_t *id, void *data)
{
+ if (ngx_ssl_cache_full_name(c->pool, index, id) != NGX_OK) {
+ return NULL;
+ }
+
return ngx_ssl_cache_types[index].create(id, err, data);
}
@@ -646,8 +657,6 @@ 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) {
@@ -660,35 +669,32 @@ ngx_ssl_cache_create_bio(ngx_str_t *id, char **err)
return bio;
}
- 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");
+ bio = BIO_new_file((char *) id->data, "r");
if (bio == NULL) {
*err = "BIO_new_file() failed";
}
- ngx_destroy_pool(temp_pool);
-
return bio;
}
+static ngx_int_t
+ngx_ssl_cache_full_name(ngx_pool_t *pool, ngx_uint_t index, ngx_str_t *id)
+{
+ if (ngx_strncmp(id->data, "data:", sizeof("data:") - 1) == 0) {
+ return NGX_OK;
+ }
+
+ if (index == NGX_SSL_CACHE_KEY
+ && ngx_strncmp(id->data, "engine:", sizeof("engine:") - 1) == 0)
+ {
+ return NGX_OK;
+ }
+
+ return ngx_get_full_name(pool, (ngx_str_t *)
&ngx_cycle->conf_prefix, id);
+}
+
+
static void *
ngx_openssl_cache_create_conf(ngx_cycle_t *cycle)
{
diff --git a/ssl_cache.t b/ssl_cache.t
index 96bd6a83ef..44935c3386 100644
--- a/ssl_cache.t
+++ b/ssl_cache.t
@@ -29,8 +29,10 @@ my $t = Test::Nginx->new();
plan(skip_all => "not yet") unless $t->has_version('1.27.2');
+my $d = $t->testdir();
+
$t->has(qw/http http_ssl socket_ssl/)->has_daemon('openssl')
- ->write_file_expand('nginx.conf', <<'EOF');
+ ->write_file_expand('nginx.conf', << "EOF");
%%TEST_GLOBALS%%
@@ -64,12 +66,22 @@ http {
ssl_client_certificate root.crt.fifo;
ssl_crl root.crl.fifo;
}
+
+ server {
+ listen 127.0.0.1:8445 ssl;
+ server_name localhost;
+
+ ssl_certificate $d/localhost.crt.fifo;
+ ssl_certificate_key $d/localhost.key.fifo;
+
+ ssl_verify_client on;
+ ssl_client_certificate $d/root.crt.fifo;
+ ssl_crl $d/root.crl.fifo;
+ }
}
EOF
-my $d = $t->testdir();
-
$t->write_file('openssl.conf', <<EOF);
[ req ]
default_bits = 2048
@@ -135,12 +147,13 @@ foreach my $name ('root.crt', 'root.crl',
'localhost.crt', 'localhost.key') {
$t->write_file('t', '');
-$t->plan(2)->run();
+$t->plan(3)->run();
###############################################################################
like(get(8443), qr/200 OK/, 'cached certificate');
like(get(8444, 'client'), qr/200 OK/, 'cached CA and CRL');
+like(get(8445, 'client'), qr/200 OK/, 'cached objects with absolute path');
###############################################################################
More information about the nginx-devel
mailing list