Replace ENGINE support with PROVIDER support on OpenSSL3+ - #13458
Replace ENGINE support with PROVIDER support on OpenSSL3+#13458JosiahWI wants to merge 7 commits into
Conversation
Fixes apache#13347 This patch changes the implementation of `SSLPrivateKeyHandler` to use `SSL_CTX_use_RSAPrivateKey_file` instead of `ENGINE_` APIs, since those APIs are deprecated in OpenSSL 3.x.
* Rename `load_xxx` to `use_xxx`
* Push `Dbg` message back to `SSLPrivateKeyHandler`
* Move `use_xxx` functions to SSLKeyUtils.{h,cc}
|
[approve ci autest 1] |
bneradt
left a comment
bneradt
left a comment
There was a problem hiding this comment.
[P2] Remove the remaining OpenSSL 3 ENGINE call
This PR says it resolves #13347, but SSLPostConfigInitialize() still calls ENGINE_load_dynamic(), one of the deprecated calls explicitly listed in that issue. With the current OpenSSL 3 compatibility setting, CMake still detects this API and ATS invokes it whenever proxy.config.ssl.engine.conf_file is configured. Please exclude this call under OPENSSL_IS_OPENSSL3 or replace the initialization path before resolving the issue.
| REQUIRE(provider.is_loaded()); | ||
|
|
||
| TempFile cert{ck.cert_pem}; | ||
| CHECK(load_key_via_load_certs(cert.get_path(), MockHardwareProvider::URI)); |
There was a problem hiding this comment.
[P1] Preserve provider URIs before filesystem path resolution
This test injects the raw URI directly into CertLoadData, bypassing the production configuration path. load_certs_and_cross_reference_names() passes every ssl_key_name through Layout::relative_to(params->serverKeyPathOnly, keyname), so the default key directory turns pkcs11:... into /.../pkcs11:.... OSSL_STORE_open() consequently receives a filesystem path instead of the provider scheme, making the new provider path unreachable from normal ssl_multicert.yaml configuration. Please preserve recognized store URIs before filesystem resolution and exercise the full normalization path in this test.
| } | ||
| } | ||
| bool result{false}; | ||
| if (keyPath && keyPath[0] != '\0') { |
There was a problem hiding this comment.
[P1] Keep SSL-secret plugin data authoritative
Trying keyPath before the secret_data returned by SSLSecret::getOrLoadSecret() reverses the documented TS_LIFECYCLE_SSL_SECRET_HOOK contract, where plugin data replaces disk loading. If the configured path contains a stale but parseable key, this helper succeeds and skips the plugin key; the later certificate check can then fail without ever trying the matching secret. This also defeats consistent updates made through TSSslSecretSet(). Please retain secret-data precedence for ordinary file names and use the path-first store lookup only for actual provider URIs.
| return false; | ||
| } | ||
|
|
||
| if (!SSL_CTX_check_private_key(ctx)) { |
There was a problem hiding this comment.
[P1] Do not require this comparison for opaque hardware keys
The previous e == nullptr guard was deliberate: hardware-backed ENGINE keys, including HSM/TPM implementations, may support signing without exposing enough key material for SSL_CTX_check_private_key() to compare them. Making the check unconditional rejects those keys in the retained pre-OpenSSL-3 ENGINE path and can likewise reject opaque provider keys. The mock provider does not cover this because it returns an ordinary exportable PEM key. Please preserve the hardware-key distinction or use a validation path that provider/ENGINE-backed opaque keys can support.
| #if HAVE_ENGINE_GET_DEFAULT_RSA && HAVE_ENGINE_LOAD_PRIVATE_KEY | ||
| // Before providers, a hardware key store was reached through an ENGINE. | ||
| // Absent a configured engine there is nothing to ask. | ||
| if (ENGINE *e = ENGINE_get_default_RSA(); e != nullptr) { |
There was a problem hiding this comment.
[P2] Release the functional ENGINE reference
ENGINE_get_default_RSA() returns an incremented functional reference that must be released with ENGINE_finish() before it is discarded. Neither the successful early return nor the failure/fallback path releases e, so repeated certificate loads and configuration reloads leak ENGINE references. Please manage this with an RAII deleter or explicitly finish the ENGINE on every path.
|
Opting to leave out provider support until there is a need. |
2 participants
Resolves #13347