From a144d828cb70f788e2f8b06e820af95ae2e28f75 Mon Sep 17 00:00:00 2001 From: Sergey Kandaurov Date: Thu, 29 May 2025 17:49:48 +0400 Subject: [PATCH] SSL: fixed "key values mismatch" with object cache inheritance. In rare cases, it was possible to get into this error state on reload with improperly updated file timestamps for certificate and key pairs. The fix is to retry on X509_R_KEY_VALUES_MISMATCH, similar to 5d5d9adcc. Additionally, loading SSL certificate is updated to avoid certificates discarded on retry to appear in ssl->certs and in extra chain. --- src/event/ngx_event_openssl.c | 58 ++++++++++++++++++++++++----- src/event/ngx_event_openssl_cache.c | 17 ++++++++- 2 files changed, 63 insertions(+), 12 deletions(-) diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c index 375d58be6..1a29f0e75 100644 --- a/src/event/ngx_event_openssl.c +++ b/src/event/ngx_event_openssl.c @@ -458,10 +458,18 @@ ngx_ssl_certificate(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *cert, { char *err; X509 *x509, **elm; + u_long n; EVP_PKEY *pkey; + ngx_uint_t mask; STACK_OF(X509) *chain; - chain = ngx_ssl_cache_fetch(cf, NGX_SSL_CACHE_CERT, &err, cert, NULL); + mask = 0; + elm = NULL; + +retry: + + chain = ngx_ssl_cache_fetch(cf, NGX_SSL_CACHE_CERT | mask, + &err, cert, NULL); if (chain == NULL) { if (err != NULL) { ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0, @@ -501,11 +509,16 @@ ngx_ssl_certificate(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *cert, } } - elm = ngx_array_push(&ssl->certs); if (elm == NULL) { - 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; + } + + } else { + X509_free(*elm); } *elm = x509; @@ -528,11 +541,21 @@ ngx_ssl_certificate(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *cert, } #else - { - int n; /* SSL_CTX_set0_chain() is only available in OpenSSL 1.0.2+ */ +#ifdef SSL_CTRL_CLEAR_EXTRA_CHAIN_CERTS + /* OpenSSL 1.0.1+ */ + SSL_CTX_clear_extra_chain_certs(ssl->ctx); +#else + + if (ssl->ctx->extra_certs) { + sk_X509_pop_free(ssl->ctx->extra_certs, X509_free); + ssl->ctx->extra_certs = NULL; + } + +#endif + n = sk_X509_num(chain); while (n--) { @@ -548,10 +571,11 @@ ngx_ssl_certificate(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *cert, } sk_X509_free(chain); - } + #endif - pkey = ngx_ssl_cache_fetch(cf, NGX_SSL_CACHE_PKEY, &err, key, passwords); + pkey = ngx_ssl_cache_fetch(cf, NGX_SSL_CACHE_PKEY | mask, + &err, key, passwords); if (pkey == NULL) { if (err != NULL) { ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0, @@ -563,9 +587,23 @@ ngx_ssl_certificate(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *cert, } if (SSL_CTX_use_PrivateKey(ssl->ctx, pkey) == 0) { + EVP_PKEY_free(pkey); + + /* there can be mismatched pairs on uneven cache update */ + + n = ERR_peek_last_error(); + + if (ERR_GET_LIB(n) == ERR_LIB_X509 + && ERR_GET_REASON(n) == X509_R_KEY_VALUES_MISMATCH + && mask == 0) + { + ERR_clear_error(); + mask = NGX_SSL_CACHE_INVALIDATE; + goto retry; + } + ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0, "SSL_CTX_use_PrivateKey(\"%s\") failed", key->data); - EVP_PKEY_free(pkey); return NGX_ERROR; } diff --git a/src/event/ngx_event_openssl_cache.c b/src/event/ngx_event_openssl_cache.c index 42f5e1c9f..61fceed7b 100644 --- a/src/event/ngx_event_openssl_cache.c +++ b/src/event/ngx_event_openssl_cache.c @@ -193,6 +193,7 @@ ngx_ssl_cache_fetch(ngx_conf_t *cf, ngx_uint_t index, char **err, time_t mtime; uint32_t hash; ngx_int_t rc; + ngx_uint_t invalidate; ngx_file_uniq_t uniq; ngx_file_info_t fi; ngx_ssl_cache_t *cache, *old_cache; @@ -202,10 +203,17 @@ ngx_ssl_cache_fetch(ngx_conf_t *cf, ngx_uint_t index, char **err, *err = NULL; + invalidate = index & NGX_SSL_CACHE_INVALIDATE; + index &= ~NGX_SSL_CACHE_INVALIDATE; + if (ngx_ssl_cache_init_key(cf->pool, index, path, &id) != NGX_OK) { return NULL; } + if (id.type == NGX_SSL_CACHE_DATA) { + invalidate = 0; + } + cache = (ngx_ssl_cache_t *) ngx_get_conf(cf->cycle->conf_ctx, ngx_openssl_cache_module); @@ -215,7 +223,12 @@ ngx_ssl_cache_fetch(ngx_conf_t *cf, ngx_uint_t index, char **err, cn = ngx_ssl_cache_lookup(cache, type, &id, hash); if (cn != NULL) { - return type->ref(err, cn->value); + if (!invalidate) { + return type->ref(err, cn->value); + } + + type->free(cn->value); + ngx_rbtree_delete(&cache->rbtree, &cn->node); } value = NULL; @@ -236,7 +249,7 @@ ngx_ssl_cache_fetch(ngx_conf_t *cf, ngx_uint_t index, char **err, old_cache = ngx_ssl_cache_get_old_conf(cf->cycle); - if (old_cache && old_cache->inheritable) { + if (old_cache && old_cache->inheritable && !invalidate) { cn = ngx_ssl_cache_lookup(old_cache, type, &id, hash); if (cn != NULL) {