On 18 Jun 2025, at 22:39, minfrin (via GitHub) <g...@apache.org> wrote:
> minfrin opened a new pull request, #8: > URL: https://github.com/apache/serf/pull/8 > > - Add serf_ssl_cert_uri_set(), a callback to set the URL of a certificate > store. > > - Use the OSSL_STORE API from OpenSSL to read certificates and keys. Certs > and keys are read from a URL instead of a file path. The default URL scheme > is file:. > > - Keep fallback support for the existing > serf_ssl_client_cert_provider_set() callback, which reads exclusively from a > local PKCS12 file. > > - Support full intermediate certificate handling. Previously whatever was > in the PKCS12 file was blindly passed to the the server on the assumption the > administrator had pre-done the work constructing the certificate chain. Now > we make no assumption as to the size of the certificate store, if a Windows > personal certificate store of a MacOS keychain is used, we search for the > most appropriate leaf certificate that matches what is requested by the > server. > > - Update test cases to handle both URIs and PKCS12 files. > > Note: tests will fail on modern unix until reference to now-removed MD5 is > fixed. This test failure is unrelated. This is the same patch, backported to v1.3.9: diff -r -u serf-1.3.9-orig/buckets/ssl_buckets.c serf-1.3.9-patched/buckets/ssl_buckets.c --- serf-1.3.9-orig/buckets/ssl_buckets.c 2025-06-19 09:42:30.628157092 +0100 +++ serf-1.3.9-patched/buckets/ssl_buckets.c 2025-06-19 10:11:17.351810235 +0100 @@ -40,6 +40,16 @@ #include <openssl/pkcs12.h> #include <openssl/x509v3.h> +#if defined(SERF_HAVE_OSSL_STORE_OPEN_EX) +#include <openssl/store.h> +#include <openssl/evp.h> +#include <openssl/safestack.h> +#include <openssl/ui.h> +#ifndef sk_EVP_PKEY_new_null +DEFINE_STACK_OF(EVP_PKEY) +#endif +#endif + #ifndef APR_VERSION_AT_LEAST /* Introduced in APR 1.3.0 */ #define APR_VERSION_AT_LEAST(major,minor,patch) \ (((major) < APR_MAJOR_VERSION) \ @@ -106,6 +116,8 @@ * */ +static int ssl_x509_ex_data_idx = -1; + typedef struct bucket_list { serf_bucket_t *bucket; struct bucket_list *next; @@ -161,12 +173,20 @@ apr_pool_t *cert_pw_cache_pool; const char *cert_pw_success; + /* Cert uri callbacks */ + serf_ssl_need_cert_uri_t cert_uri_callback; + void *cert_uri_userdata; + apr_pool_t *cert_uri_cache_pool; + const char *cert_uri_success; + /* Server cert callbacks */ serf_ssl_need_server_cert_t server_cert_callback; serf_ssl_server_cert_chain_cb_t server_cert_chain_callback; void *server_cert_userdata; const char *cert_path; + const char *cert_uri; + const char *cert_pw; X509 *cached_cert; EVP_PKEY *cached_cert_pw; @@ -1209,10 +1229,50 @@ #define ERR_GET_FUNC(ec) (0) #endif +#if defined(SERF_HAVE_OSSL_STORE_OPEN_EX) + +static int ssl_pass_cb(UI *ui, UI_STRING *uis) +{ + serf_ssl_context_t *ctx = UI_get0_user_data(ui); + + const char *password; + apr_status_t status; + + if (ctx->cert_pw_success) { + status = APR_SUCCESS; + password = ctx->cert_pw_success; + ctx->cert_pw_success = NULL; + } + else if (ctx->cert_pw_callback) { + status = ctx->cert_pw_callback(ctx->cert_pw_userdata, + ctx->cert_uri, + &password); + } + else { + return 0; + } + + UI_set_result(ui, uis, password); + + ctx->cert_pw = apr_pstrdup(ctx->pool, password); + + return 1; +} + +#endif + static int ssl_need_client_cert(SSL *ssl, X509 **cert, EVP_PKEY **pkey) { serf_ssl_context_t *ctx = SSL_get_app_data(ssl); +#if defined(SERF_HAVE_OSSL_STORE_OPEN_EX) + STACK_OF(X509) *leaves; + STACK_OF(X509) *intermediates; + STACK_OF(EVP_PKEY) *keys; + X509_STORE *requests; + UI_METHOD *ui_method; +#endif apr_status_t status; + int retrying_success = 0; if (ctx->cached_cert) { *cert = ctx->cached_cert; @@ -1220,6 +1280,201 @@ return 1; } +#if defined(SERF_HAVE_OSSL_STORE_OPEN_EX) + + if (ssl_x509_ex_data_idx < 0) { + ssl_x509_ex_data_idx = X509_get_ex_new_index(0, NULL, NULL, NULL, NULL); + } + + /* until further notice */ + *cert = NULL; + *pkey = NULL; + + leaves = sk_X509_new_null(); + intermediates = sk_X509_new_null(); + keys = sk_EVP_PKEY_new_null(); + requests = X509_STORE_new(); + + ui_method = UI_create_method("passphrase"); + UI_method_set_reader(ui_method, ssl_pass_cb); + + while (ctx->cert_uri_callback) { + const char *cert_uri = NULL; + OSSL_STORE_CTX *store = NULL; + OSSL_STORE_INFO *info; + X509 *c; + STACK_OF(X509_NAME) *requested; + int type; + + retrying_success = 0; + + if (ctx->cert_uri_success) { + status = APR_SUCCESS; + cert_uri = ctx->cert_uri_success; + ctx->cert_uri_success = NULL; + retrying_success = 1; + } else { + status = ctx->cert_uri_callback(ctx->cert_uri_userdata, &cert_uri); + } + + if (status || !cert_uri) { + break; + } + + ctx->cert_uri = cert_uri; + + /* server side request some certs? this list may be empty */ + requested = SSL_get_client_CA_list(ssl); + + store = OSSL_STORE_open_ex(cert_uri, NULL, NULL, ui_method, ctx, NULL, NULL, NULL); + if (!store) { + break; + } + + /* walk the store, what are we working with */ + + while (!OSSL_STORE_eof(store)) { + info = OSSL_STORE_load(store); + + if (!info) { + break; + } + + type = OSSL_STORE_INFO_get_type(info); + if (type == OSSL_STORE_INFO_CERT) { + X509 *c = OSSL_STORE_INFO_get1_CERT(info); + + int n, i; + + int is_ca = X509_check_ca(c); + + /* split into leaves and intermediate certs */ + if (is_ca) { + sk_X509_push(intermediates, c); + } + else { + sk_X509_push(leaves, c); + } + + /* any cert with an issuer matching our requested CAs is also added to the requests list */ + n = sk_X509_NAME_num(requested); + for (i = 0; i < n; ++i) { + X509_NAME *name = sk_X509_NAME_value(requested, i); + if (X509_NAME_cmp(name, X509_get_issuer_name(c)) == 0) { + if (is_ca) { + X509_STORE_add_cert(requests, c); + } + else { + X509_set_ex_data(c, ssl_x509_ex_data_idx, (void *)1); + } + } + } + + } else if (type == OSSL_STORE_INFO_PKEY) { + EVP_PKEY *k = OSSL_STORE_INFO_get1_PKEY(info); + + sk_EVP_PKEY_push(keys, k); + } + + OSSL_STORE_INFO_free(info); + } + /* FIXME: openssl error checking goes here */ + + OSSL_STORE_close(store); + + /* walk the leaf certificates, choose the best one */ + + while ((c = sk_X509_pop(leaves))) { + + EVP_PKEY *k = NULL; + int i, n, found = 0; + + /* no key, skip */ + n = sk_EVP_PKEY_num(keys); + for (i = 0; i < n; ++i) { + k = sk_EVP_PKEY_value(keys, i); + if (X509_check_private_key(c, k)) { + found = 1; + break; + } + } + if (!found) { + continue; + } + + /* CAs requested? if so, skip non matches, if not, accept all */ + if (sk_X509_NAME_num(requested) && !X509_get_ex_data(c, ssl_x509_ex_data_idx)) { + STACK_OF(X509) *chain; + + chain = X509_build_chain(c, intermediates, requests, 0, NULL, NULL); + + if (!chain) { + continue; + } + + sk_X509_pop_free(chain, X509_free); + } + + /* no best candidate yet? we're in first place */ + if (!*cert) { + *cert = c; /* don't dup, we're returning this */ + *pkey = EVP_PKEY_dup(k); + continue; + } + + /* were we issued after the previous best? */ + if (ASN1_TIME_compare(X509_get0_notBefore(*cert), X509_get0_notBefore(c)) < 0) { + X509_free(*cert); + EVP_PKEY_free(*pkey); + *cert = c; /* don't dup, we're returning this */ + *pkey = EVP_PKEY_dup(k); + continue; + } + + X509_free(c); + } + + break; + } + + sk_X509_pop_free(leaves, X509_free); + sk_X509_pop_free(intermediates, X509_free); + sk_EVP_PKEY_pop_free(keys, EVP_PKEY_free); + X509_STORE_free(requests); + UI_destroy_method(ui_method); + + /* we settled on a cert and key, cache it for later */ + + if (*cert && *pkey) { + + ctx->cached_cert = *cert; + ctx->cached_cert_pw = *pkey; + if (!retrying_success && ctx->cert_cache_pool) { + const char *c; + + c = apr_pstrdup(ctx->cert_cache_pool, ctx->cert_uri); + + apr_pool_userdata_setn(c, "serf:ssl:cert", + apr_pool_cleanup_null, + ctx->cert_cache_pool); + } + + if (!retrying_success && ctx->cert_pw_cache_pool && ctx->cert_pw) { + const char *pw; + + pw = apr_pstrdup(ctx->cert_pw_cache_pool, + ctx->cert_pw); + + apr_pool_userdata_setn(pw, "serf:ssl:certpw", + apr_pool_cleanup_null, + ctx->cert_pw_cache_pool); + } + + return 1; + } + +#endif + while (ctx->cert_callback) { const char *cert_path; apr_file_t *cert_file; @@ -1227,7 +1482,7 @@ BIO_METHOD *biom; PKCS12 *p12; int i; - int retrying_success = 0; + retrying_success = 0; if (ctx->cert_file_success) { status = APR_SUCCESS; @@ -1374,6 +1629,20 @@ } } +void serf_ssl_cert_uri_set( + serf_ssl_context_t *context, + serf_ssl_need_cert_uri_t callback, + void *data, + void *cache_pool) +{ + context->cert_uri_callback = callback; + context->cert_uri_userdata = data; + context->cert_cache_pool = cache_pool; + if (context->cert_cache_pool) { + apr_pool_userdata_get((void**)&context->cert_uri_success, + "serf:ssl:certuri", cache_pool); + } +} void serf_ssl_server_cert_callback_set( serf_ssl_context_t *context, @@ -1419,6 +1688,7 @@ ssl_ctx->cert_callback = NULL; ssl_ctx->cert_pw_callback = NULL; + ssl_ctx->cert_uri_callback = NULL; ssl_ctx->server_cert_callback = NULL; ssl_ctx->server_cert_chain_callback = NULL; diff -r -u serf-1.3.9-orig/CMakeLists.txt serf-1.3.9-patched/CMakeLists.txt --- serf-1.3.9-orig/CMakeLists.txt 2025-06-19 09:42:30.635165569 +0100 +++ serf-1.3.9-patched/CMakeLists.txt 2025-06-19 10:11:01.216291612 +0100 @@ -218,6 +218,7 @@ CheckFunction("CRYPTO_set_locking_callback" "SERF_HAVE_SSL_LOCKING_CALLBACKS" ${OPENSSL_LIBRARIES} ${SERF_STANDARD_LIBRARIES}) CheckFunction("OpenSSL_version_num" "SERF_HAVE_OPENSSL_VERSION_NUM" ${OPENSSL_LIBRARIES} ${SERF_STANDARD_LIBRARIES}) CheckFunction("SSL_set_alpn_protos" "SERF_HAVE_OPENSSL_ALPN" ${OPENSSL_LIBRARIES} ${SERF_STANDARD_LIBRARIES}) +CheckFunction("OSSL_STORE_open_ex" "SERF_HAVE_OSSL_STORE_OPEN_EX" ${OPENSSL_LIBRARIES} ${SERF_STANDARD_LIBRARIES}) CheckFunctionMacro("OPENSSL_malloc_init" "SERF_HAVE_OPENSSL_MALLOC_INIT" "openssl/crypto.h" "${OPENSSL_INCLUDE_DIR}" ${OPENSSL_LIBRARIES} ${SERF_STANDARD_LIBRARIES}) CheckFunctionMacro("SSL_library_init" "SERF_HAVE_OPENSSL_SSL_LIBRARY_INIT" "openssl/ssl.h" Only in serf-1.3.9-patched: redhat-linux-build diff -r -u serf-1.3.9-orig/serf_bucket_types.h serf-1.3.9-patched/serf_bucket_types.h --- serf-1.3.9-orig/serf_bucket_types.h 2015-09-17 13:46:24.000000000 +0100 +++ serf-1.3.9-patched/serf_bucket_types.h 2025-06-19 09:57:44.832939940 +0100 @@ -515,6 +515,10 @@ const char *cert_path, const char **password); +typedef apr_status_t (*serf_ssl_need_cert_uri_t)( + void *data, + const char **cert_uri); + typedef apr_status_t (*serf_ssl_need_server_cert_t)( void *data, int failures, @@ -527,12 +531,29 @@ const serf_ssl_certificate_t * const * certs, apr_size_t certs_len); +/** + * Set a callback to provide a filesystem path to a PKCS12 file. + * + * This has been replaced by serf_ssl_cert_uri_set(). On Unix + * platforms the same path from serf_ssl_client_cert_provider_set() + * can be passed to serf_ssl_cert_uri_set(). On Windows the drive + * letter will be interpreted by serf_ssl_cert_uri_set() as a scheme, + * so the same path will not work, and will need to be escaped as + * a file URL instead. + */ void serf_ssl_client_cert_provider_set( serf_ssl_context_t *context, serf_ssl_need_client_cert_t callback, void *data, void *cache_pool); +/** + * Set a callback to provide the password corresponding to the URL of + * the client certificate store. + * + * If the serf_ssl_client_cert_provider_set callback is set, this + * password will also be used to decode the PKCS12 file. + */ void serf_ssl_client_cert_password_set( serf_ssl_context_t *context, serf_ssl_need_cert_password_t callback, @@ -549,6 +570,24 @@ void *data); /** + * Set a callback to provide the URL of the client certificate store. + * + * In the absence of a scheme the default scheme is file:, and the file + * can point to PKCS12, PEM or other supported certificates and keys. + * + * With the correct OpenSSL provider configured, URLs can be provided + * for pkcs11, tpm2, and other certificate stores. + * + * On Windows, file paths must be escaped as file: URLs to prevent the + * drive letter being intepreted as a scheme. + */ +void serf_ssl_cert_uri_set( + serf_ssl_context_t *context, + serf_ssl_need_cert_uri_t callback, + void *data, + void *cache_pool); + +/** * Set callbacks to override the default SSL server certificate validation * algorithm for the current certificate or the entire certificate chain. */ Regards, Graham --
