Skip to content

SSL: support loading keys via OSSL_STORE. #436

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 26, 2025

Conversation

bavshin-f5
Copy link
Member

@bavshin-f5 bavshin-f5 commented Jan 9, 2025

A new "store:..." prefix for the "ssl_certificate_key" directive allows loading keys via the OSSL_STORE API.

The change is required to support hardware backed keys in OpenSSL 3.x using the new "provider(7ossl)" modules, such as "pkcs11-provider". While the engine API is present in 3.x, some operating systems (notably, RHEL10) have already disabled it in their builds of OpenSSL.

Related: https://trac.nginx.org/nginx/ticket/2449

Successfully tested with:

It is unclear if we want to handle property query to resolve ambiguities, e.g. -provider tpm2 -propquery '?provider=tpm2' -key file:$name.key.

@Maryna-f5 Maryna-f5 added this to the nginx-1.27.5 milestone Jan 15, 2025
@bavshin-f5 bavshin-f5 marked this pull request as ready for review January 16, 2025 00:56
@bavshin-f5
Copy link
Member Author

Changes:

  • rebased on top of the merged SSL cache patches
  • switched OSSL_STORE support detection to ERR_R_OSSL_STORE_LIB. The macro appeared in the same commit as OSSL_STORE, but unlike ERR_LIB_OSSL_STORE is not defined in any of the OpenSSL forks.
  • set default UI method to UI_null() during worker process initialization.

The last change may need more context than the commit message contains:

pkcs11-provider soft-resets its state on fork. It doesn't mean that the key handles obtained during configuration load become invalid, it just means that reopening the device and reloading the keys could be necessary. So the first attempt to access the EVP_PKEY from the worker process may result in a pin prompt.
Unfortunately, the operation is not initiated by us and we're not in control. More specifically, we cannot pass the password callback (or attach it to the provider with a missing equivalent of ENGINE_CTRL_SET_USER_INTERFACE). The pkcs11-provider upstream recommendation is just that: yes, providers API might be not optimal, use UI_set_default_method.

There are 3 provider-specific solutions:

  • pkcs11-module-cache-pins = cache, to save and reuse manually entered pin after forking
  • pkcs11-module-token-pin = file:... to re-read the pin from file
  • pin-source/pin-value components of the pkcs11 key URI

but nothing that works in general and ensures that the worker doesn't get blocked by a sudden unnoticed pin prompt.
I happen to have a ssl_password_file support patch on top of this PR, which did nothing to address this specific case, as the callback is not used and we're already wiped the passwords anyways. Therefore, a more radical solution was added which just prevents any prompts from appearing.


Tangentially related, but ssl_password_file is not really applicable here as is. This might be a softhsm2 thing, but the token has gone into invalid state on the first bad pin entry and refused any further communication attempts.

@bavshin-f5 bavshin-f5 requested a review from pluknet February 27, 2025 16:43
@bukka
Copy link

bukka commented Mar 15, 2025

I have been actually working quite heavily with nginx and pkcs11-provider in recent months and have got some fixes for the PIN issues (you can find more info in some of the PR's that I have open but I have got more things coming) . In my case it's even more complicated because I have pkcs11-proxy in between. But mostly it's more a problem that the key refresh uri (saved in provider object) is not always passed (that's where you need to specify pin-value or pin-source if the key is in different slot than the default). What is specified in pkcs11-module-token-pin is used as the default pin (currently it is the first slot in the list but added a PR to select it) if no pin is provided with a key. This is also important when client cert is used because it will use it for importing the public key (which is happening due to how provider works in OpenSSL). So basically you need to always set pkcs11-module-token-pin to the first slot PIN (or later selected ones once my PR is merged) and then specify PIN's for all key in the PKCS11 URI.

All of the above are just issue in pkcs11-provider and I didn't find any problem in nginx so far.

The loading keys in this way is not really necessary because the current way is to create PEM file containing the URI. That said it might be nicer to be able to specify it directly so it is a nice improvement.

The UI method change might be also nicer way to fail but not a big deal either. The UI method actually immediately fails anyway if running in background so it might be just a slightly nicer error. Currently one can actually specify the password if running nginx in foreground but that's not really useful in real use.

@Maryna-f5 Maryna-f5 modified the milestones: nginx-1.27.5, nginx-1.27.6 Apr 7, 2025
@pluknet
Copy link
Contributor

pluknet commented May 19, 2025

For the record.

Using pkcs11-provider won't obviously work in OpenSSL 1.1.1, although the OSSL_STORE interface is supported there
(OSSL_STORE_R_UNREGISTERED_SCHEME is returned). This may not be considered a problem though, because 1) we cannot control in general which OpenSSL versions a certain provider implementation supports, 2) the change itself isn't about providers.

@bavshin-f5
Copy link
Member Author

Using pkcs11-provider won't obviously work in OpenSSL 1.1.1, although the OSSL_STORE interface is supported there (OSSL_STORE_R_UNREGISTERED_SCHEME is returned). This may not be considered a problem though, because 1) we cannot control in general which OpenSSL versions a certain provider implementation supports, 2) the change itself isn't about providers.

I considered making the change about providers and hiding everything under OpenSSL >= 3.0.0 check, but decided against that. Nothing in the code is specific to providers, and it is technically possible to register a scheme handler in 1.1.1 via OSSL_STORE_LOADER interface. Even if nobody actually does that.

The store:... scheme is chosen for a similar reason: while there are requests to implement this specifically as provider:... (#452, #453), the implementation is not specific to providers, and a single provider can register multiple schemes. Case in point, tpm2-openssl registers 3 URI schemes.

Although I still consider uri:... prefix a reasonable alternative and haven't decided between two. Your input is welcome.

@pluknet
Copy link
Contributor

pluknet commented May 20, 2025

Changes:
...
but nothing that works in general and ensures that the worker doesn't get blocked by a sudden unnoticed pin prompt. I happen to have a ssl_password_file support patch on top of this PR, which did nothing to address this specific case, as the callback is not used and we're already wiped the passwords anyways. Therefore, a more radical solution was added which just prevents any prompts from appearing.

I don't see how default UI_null() prevents blocking on prompt.
I tested provider keys (using your test) and don't see any difference.
Without pkcs11-module-token-pin and with the default set toUI_null() it still blocks on the pkcs11-provider prompt:
Enter PIN for PKCS#11 Token (...)

Interestingly enough, passing UI_get_default_method() changes this prompt in the master process to:
Enter pass phrase for PKCS#11 Token
And UI_UTIL_wrap_read_pem_callback() - to:
Enter PEM pass phrase

Also, in the above case, when using OSSL_STORE_open() with a NULL method, regardless of UI_set_default_method() set, loading provider keys with manually typed passphrase leaves the error queue.
Here it pop ups in unrelated connection.

[alert] 59396#3959716: *1 ignoring stale global SSL error (SSL: error:07880106:common libcrypto routines::passed invalid argument:No password method specified) while SSL handshaking to upstream ...

So, this needs to be addressed in some way.

Tangentially related, but ssl_password_file is not really applicable here as is. This might be a softhsm2 thing, but the token has gone into invalid state on the first bad pin entry and refused any further communication attempts.

This will break loading password protected private keys from disk via "store:" without any diagnostics.
Passing NULL to ui_method here makes OSSL_STORE_load() to always return NULL.
Adding some non-empty *err shows that the reason is the same:

nginx: [emerg] cannot load certificate key "store:file:/path/to/key": loading "store:" key failed (SSL: error:1608010C:STORE routines::unsupported error:07880106:common libcrypto routines::passed invalid argument:No password method specified error:1C80009F:Provider routines::unable to get passphrase)

Apparently, ui_method should be set.
This is not clearly articulated in the OpenSSL documentation.

Setting ui_method to UI_get_default_method() makes OpenSSL prompt to pop up, as expected.
And setting to UI_UTIL_wrap_read_pem_callback() makes loading from disk similar to NGX_SSL_CACHE_PATH.

To sum up:

  • UI_get_default_method() doesn't seem to make any difference
  • UI_UTIL_wrap_read_pem_callback needs at least to load regular keys from disk

Patch on top (default to UI_null reversion omitted for brevity):

diff --git a/src/event/ngx_event_openssl_cache.c b/src/event/ngx_event_openssl_cache.c
index d0b81e469..74131ce79 100644
--- a/src/event/ngx_event_openssl_cache.c
+++ b/src/event/ngx_event_openssl_cache.c
@@ -727,6 +727,21 @@ ngx_ssl_cache_pkey_create(ngx_ssl_cache_key_t *id, char **err, void *data)
 #endif
     }
 
+    cb_data.encrypted = 0;
+
+    if (*passwords) {
+        cb_data.pwd = (*passwords)->elts;
+        tries = (*passwords)->nelts;
+        pwd = &cb_data;
+        cb = ngx_ssl_cache_pkey_password_callback;
+
+    } else {
+        cb_data.pwd = NULL;
+        tries = 1;
+        pwd = NULL;
+        cb = NULL;
+    }
+
     if (id->type == NGX_SSL_CACHE_STORE) {
 
 #ifdef ERR_R_OSSL_STORE_LIB
@@ -737,7 +752,9 @@ ngx_ssl_cache_pkey_create(ngx_ssl_cache_key_t *id, char **err, void *data)
 
         uri = id->data + sizeof("store:") - 1;
 
-        store = OSSL_STORE_open((char *) uri, NULL, NULL, NULL, NULL);
+        store = OSSL_STORE_open((char *) uri,
+                                UI_UTIL_wrap_read_pem_callback(cb, 0), pwd,
+                                NULL, NULL);
 
         if (store == NULL) {
             *err = "OSSL_STORE_open() failed";
@@ -762,7 +779,12 @@ ngx_ssl_cache_pkey_create(ngx_ssl_cache_key_t *id, char **err, void *data)
 
         OSSL_STORE_close(store);
 
-        return pkey;
+        if (pkey == NULL) {
+            *err = "OSSL_STORE_load() failed";
+            return NULL;
+        }
+
+        goto done;
 
 #else
 
@@ -777,21 +799,6 @@ ngx_ssl_cache_pkey_create(ngx_ssl_cache_key_t *id, char **err, void *data)
         return NULL;
     }
 
-    cb_data.encrypted = 0;
-
-    if (*passwords) {
-        cb_data.pwd = (*passwords)->elts;
-        tries = (*passwords)->nelts;
-        pwd = &cb_data;
-        cb = ngx_ssl_cache_pkey_password_callback;
-
-    } else {
-        cb_data.pwd = NULL;
-        tries = 1;
-        pwd = NULL;
-        cb = NULL;
-    }
-
     for ( ;; ) {
 
         pkey = PEM_read_bio_PrivateKey(bio, NULL, cb, pwd);
@@ -811,12 +818,15 @@ ngx_ssl_cache_pkey_create(ngx_ssl_cache_key_t *id, char **err, void *data)
         return NULL;
     }
 
+
+    BIO_free(bio);
+
+done:
+
     if (cb_data.encrypted) {
         *passwords = NGX_SSL_CACHE_DISABLED;
     }
 
-    BIO_free(bio);
-
     return pkey;
 }
 

Copy link
Contributor

@pluknet pluknet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Posted my review, please see above.
[Unfortunately, deficient github interface force me to write something here in order to press the button.]

@bavshin-f5 bavshin-f5 force-pushed the ssl-provider-keys branch from 6701e23 to fec4dbb Compare May 21, 2025 05:08
A new "store:..." prefix for the "ssl_certificate_key" directive allows
loading keys via the OSSL_STORE API.

The change is required to support hardware backed keys in OpenSSL 3.x using
the new "provider(7ossl)" modules, such as "pkcs11-provider".  While the
engine API is present in 3.x, some operating systems (notably, RHEL10)
have already disabled it in their builds of OpenSSL.

Related: https://trac.nginx.org/nginx/ticket/2449
Certain providers may attempt to reload the key on the first use after a
fork.  Such attempt would require re-prompting the pin, and this time we
are not able to pass the password callback.

While it is addressable with configuration for a specific provider, it would
be prudent to ensure that no such prompts could block worker processes by
setting the default UI method.

UI_null() first appeared in 1.1.1 along with the OSSL_STORE, so it is safe
to assume the same set of guards.
@bavshin-f5 bavshin-f5 force-pushed the ssl-provider-keys branch from 5ddb503 to 9dffdaa Compare May 22, 2025 20:47
@bavshin-f5
Copy link
Member Author

For the record: the following error and related issues with UI_set_default_method were caused by linking nginx with static non-system OpenSSL.

[alert] 59396#3959716: *1 ignoring stale global SSL error (SSL: error:07880106:common libcrypto routines::passed invalid argument:No password method specified) while SSL handshaking to upstream ...

As pkcs11-provider links to the system libcrypto, we end up with two copies of libcrypto in the process address space. The default method is successfully set for the static copy within the NGINX, but p11prov_session_prompt_for_pin invokes method from the dynamically loaded copy of the code, which has it's own set of static data in an unknown state.

After an internal discussion, we agreed that we are not going to support this configuration. This is the least of the problems possible with intermixed use of code from two different builds (and versions) of libcrypto.
All the options to avoid the pkcs11-provider PIN prompts referenced in #436 (comment) were confirmed to work though.


There's a known issue with pkcs11-provider, a pin passed through ssl_password_file and a static ssl_certificate_key (specified without variables), where the key handle is invalidated on fork and no pin source is available to reopen the pkcs11 device. It's nothing we can fix in the nginx, but there's a workaround: pkcs11-module-cache-pins = cache in the provider configuration.

@bavshin-f5 bavshin-f5 merged commit 3d5889a into nginx:master May 26, 2025
1 check passed
@bavshin-f5 bavshin-f5 deleted the ssl-provider-keys branch May 26, 2025 13:56
@josuerocha
Copy link

Hello @bavshin-f5,

As of NGINX versions prior to the change in this PR, is it possible to configure OpenSSL providers, via an OpenSSL configuration file, in such a way that the provider's key and certificate can be used by NGINX to establish TLS connections?

From what I understand, this pull request enables selecting specific keys. However, is it also possible for the provider to expose a default key that NGINX could use without explicitly selecting it?

@bukka
Copy link

bukka commented Jun 20, 2025

As of NGINX versions prior to the change in this PR, is it possible to configure OpenSSL providers, via an OpenSSL configuration file, in such a way that the provider's key and certificate can be used by NGINX to establish TLS connections?

This depends on the provider. In case of pkcs11-provider, it was already possible to use keys through the special PEM key (and possibly cert as well) that has specific ASN.1 structure. The pkcs11-provider has it's own decoded for such format so such key can already be loaded. I developed a small tool that sets everythin up for testing. The key format part is as follows: https://github.com/bukka/nginx-pkcs11-provider/blob/main/nginx_pkcs11_provider/generate_keys.py#L98-L106 .

@josuerocha
Copy link

@bukka
Thank you for the information.

What would the NGINX configuration file look like I'm this case? Is it possible to omit the ssl_key directive whilst having SSL activated?

@bukka
Copy link

bukka commented Jun 20, 2025

It's the same as normal nginx configuration. There is still ssl_certificate_key but the file contains the PEM that I mentioned before. The only notable part is that for some extra envs might need to set for workers because some providers might need to reset things (e.g. pkcs11-provider) so things like SoftHSMv2 config need to be available for the workers.

As I said this is provider specific because the provider loads the key so it can have it's own decoding logic like in the case of pkcs11-provider. This is not just nginx thing - you can do the same in curl or whatever other OpenSSL users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Support for OpenSSL 3.0 Provider API in ssl_certificate_key : provider:name:id
5 participants