diff --git a/src/ngx_http_ffi_client.h b/src/ngx_http_ffi_client.h index dc4e256..5030c09 100644 --- a/src/ngx_http_ffi_client.h +++ b/src/ngx_http_ffi_client.h @@ -199,12 +199,19 @@ typedef struct ngx_http_ffi_client_keepalive_item_s #if (NGX_SSL) -/* one cached client SSL_CTX per distinct trust config (trusted CA path) */ +/* + * One cached client SSL_CTX per distinct trust config. The key is the trusted + * CA path *and* whether verification is on: a verify-off context carries no + * trust store at all, so reusing one for a verify-on request would fail every + * chain it was asked to check. The CA is only part of the key when verify is + * on; with it off every caller shares the one storeless context. + */ typedef struct ngx_http_ffi_client_ssl_ctx_s ngx_http_ffi_client_ssl_ctx_t; struct ngx_http_ffi_client_ssl_ctx_s { ngx_http_ffi_client_ssl_ctx_t *next; ngx_str_t ca; + unsigned verify:1; ngx_ssl_t *ssl; }; #endif diff --git a/src/ngx_http_ffi_client_request.c b/src/ngx_http_ffi_client_request.c index 0ee73ab..4bb0292 100644 --- a/src/ngx_http_ffi_client_request.c +++ b/src/ngx_http_ffi_client_request.c @@ -60,7 +60,10 @@ static ngx_int_t ngx_http_ffi_client_handle_read_wait( ngx_http_ffi_client_op_t *op); #if (NGX_SSL) static ngx_ssl_t *ngx_http_ffi_client_ssl_ctx( - ngx_http_ffi_client_main_conf_t *fmcf, ngx_str_t *ca, ngx_log_t *log); + ngx_http_ffi_client_main_conf_t *fmcf, ngx_str_t *ca, ngx_uint_t verify, + ngx_log_t *log); +static ngx_int_t ngx_http_ffi_client_ssl_default_store(X509_STORE *store, + ngx_log_t *log); static ngx_int_t ngx_http_ffi_client_ssl_init_connection( ngx_http_ffi_client_op_t *op); static ngx_uint_t ngx_http_ffi_client_name_is_ip(ngx_str_t *name); @@ -1537,23 +1540,116 @@ ngx_http_ffi_client_handle_read_wait(ngx_http_ffi_client_op_t *op) #if (NGX_SSL) +/* + * Load the platform trust store into `store`. + * + * This is what SSL_CTX_set_default_verify_paths() would do, except that it + * registers both locations as X509_FILETYPE_PEM rather than + * X509_FILETYPE_DEFAULT. The filetype is not cosmetic: with DEFAULT, OpenSSL + * enumerates the whole hashed CA directory (a getdents64 sweep of every entry + * in /etc/ssl/certs) on *each* handshake, instead of opening the single + * .N file the lookup actually needs. On a fresh-connection-per-request + * workload that directory walk cost ~11% of the worker's CPU. + * + * The SSL_CERT_FILE / SSL_CERT_DIR environment overrides are honoured the same + * way OpenSSL honours them, so behaviour is unchanged apart from the cost. + * + * Returns NGX_OK if either location loaded; a trust store with neither is + * unusable and would fail every verify. + */ +static ngx_int_t +ngx_http_ffi_client_ssl_default_store(X509_STORE *store, ngx_log_t *log) +{ + const char *file, *dir; + ngx_uint_t loaded; + X509_LOOKUP *lookup; + + loaded = 0; + + file = getenv(X509_get_default_cert_file_env()); + if (file == NULL) { + file = X509_get_default_cert_file(); + } + + dir = getenv(X509_get_default_cert_dir_env()); + if (dir == NULL) { + dir = X509_get_default_cert_dir(); + } + + lookup = X509_STORE_add_lookup(store, X509_LOOKUP_file()); + if (lookup == NULL) { + ngx_ssl_error(NGX_LOG_ALERT, log, 0, + "X509_STORE_add_lookup(X509_LOOKUP_file) failed"); + return NGX_ERROR; + } + + if (X509_LOOKUP_load_file(lookup, file, X509_FILETYPE_PEM) == 1) { + loaded = 1; + + } else { + /* not fatal on its own: the hashed directory may still cover it */ + ERR_clear_error(); + } + + lookup = X509_STORE_add_lookup(store, X509_LOOKUP_hash_dir()); + if (lookup == NULL) { + ngx_ssl_error(NGX_LOG_ALERT, log, 0, + "X509_STORE_add_lookup(X509_LOOKUP_hash_dir) failed"); + return NGX_ERROR; + } + + if (X509_LOOKUP_add_dir(lookup, dir, X509_FILETYPE_PEM) == 1) { + loaded = 1; + + } else { + ERR_clear_error(); + } + + if (!loaded) { + ngx_ssl_error(NGX_LOG_ALERT, log, 0, + "failed to load the default trust store from \"%s\"" + " or \"%s\"", file, dir); + return NGX_ERROR; + } + + return NGX_OK; +} + + /* * Lazily build one client SSL_CTX per distinct trust config, cached on * main_conf. `ca` is an optional trusted-CA file path (empty = system store - * only). Returns NULL on failure without caching a half-built context. + * only). `verify` says whether this context will ever be asked to check a + * chain; when it is off no trust store is loaded at all, because OpenSSL + * verifies the chain during every handshake regardless of the verify mode and + * ngx_http_ffi_client_ssl_handshake_handler() only reads the result when the + * caller asked for it, so with verify off `ca` is folded away and every such + * caller shares one context. Returns NULL on failure without caching a + * half-built context. */ static ngx_ssl_t * ngx_http_ffi_client_ssl_ctx(ngx_http_ffi_client_main_conf_t *fmcf, - ngx_str_t *ca, ngx_log_t *log) + ngx_str_t *ca, ngx_uint_t verify, ngx_log_t *log) { u_char *path; + ngx_str_t none = ngx_null_string; ngx_ssl_t *ssl; ngx_pool_cleanup_t *cln; ngx_http_ffi_client_ssl_ctx_t *node; + /* Nothing below reads `ca` unless `verify` is set, so a verify-off context + * is the same whatever CA the caller named. Fold it here, ahead of both the + * lookup and the cache node, so the two always agree: normalising only one + * of them would miss every time and leak a context per request. This must + * move if `ca` ever gains meaning with verify off. */ + if (!verify) { + ca = &none; + } + for (node = fmcf->ssl_ctxs; node != NULL; node = node->next) { /* guard memcmp: NULL pointers with len 0 are UB */ - if (node->ca.len == ca->len + if (node->verify == (verify ? 1u : 0u) + && node->ca.len == ca->len && (ca->len == 0 || ngx_memcmp(node->ca.data, ca->data, ca->len) == 0)) { @@ -1575,32 +1671,39 @@ ngx_http_ffi_client_ssl_ctx(ngx_http_ffi_client_main_conf_t *fmcf, /* verify mode stays NONE so the handshake completes and we inspect the * result ourselves. An unusable trust store is a hard init failure: don't * cache a context that would fail every verify-on request. */ - if (SSL_CTX_set_default_verify_paths(ssl->ctx) == 0) { - ngx_ssl_error(NGX_LOG_ALERT, log, 0, - "SSL_CTX_set_default_verify_paths() failed"); - ngx_ssl_cleanup_ctx(ssl); - return NULL; - } - - if (ca->len) { - path = ngx_pnalloc(ngx_cycle->pool, ca->len + 1); - if (path == NULL) { + if (verify) { + if (ngx_http_ffi_client_ssl_default_store( + SSL_CTX_get_cert_store(ssl->ctx), log) != NGX_OK) + { ngx_ssl_cleanup_ctx(ssl); return NULL; } - ngx_memcpy(path, ca->data, ca->len); - path[ca->len] = '\0'; + if (ca->len) { + path = ngx_pnalloc(ngx_cycle->pool, ca->len + 1); + if (path == NULL) { + ngx_ssl_cleanup_ctx(ssl); + return NULL; + } - if (SSL_CTX_load_verify_locations(ssl->ctx, (char *) path, NULL) == 0) { - ngx_ssl_error(NGX_LOG_ALERT, log, 0, - "SSL_CTX_load_verify_locations(\"%s\") failed", path); - ngx_ssl_cleanup_ctx(ssl); - return NULL; + ngx_memcpy(path, ca->data, ca->len); + path[ca->len] = '\0'; + + /* load_verify_locations registers PEM, not DEFAULT, so it does not + * pay the per-handshake directory walk described above */ + if (SSL_CTX_load_verify_locations(ssl->ctx, (char *) path, NULL) + == 0) + { + ngx_ssl_error(NGX_LOG_ALERT, log, 0, + "SSL_CTX_load_verify_locations(\"%s\") failed", + path); + ngx_ssl_cleanup_ctx(ssl); + return NULL; + } } - } - SSL_CTX_set_verify_depth(ssl->ctx, 100); + SSL_CTX_set_verify_depth(ssl->ctx, 100); + } /* build the cache node before registering cleanup so an alloc failure * frees the context instead of orphaning it until worker exit */ @@ -1611,6 +1714,7 @@ ngx_http_ffi_client_ssl_ctx(ngx_http_ffi_client_main_conf_t *fmcf, } node->ssl = ssl; + node->verify = verify ? 1 : 0; node->ca.len = ca->len; if (ca->len) { node->ca.data = ngx_pnalloc(ngx_cycle->pool, ca->len); @@ -1740,7 +1844,7 @@ ngx_http_ffi_client_ssl_init_connection(ngx_http_ffi_client_op_t *op) fmcf = ngx_http_get_module_main_conf(op->r, ngx_http_ffi_client_module); ssl = ngx_http_ffi_client_ssl_ctx(fmcf, &op->ssl_trusted_certificate, - c->log); + op->ssl_verify, c->log); if (ssl == NULL) { ngx_http_ffi_client_finalize(op, NGX_ERROR, "failed to init TLS"); return NGX_ERROR;