[PATCH 0/4] SSL: Add support for loading X.509 certificates from openssl engine

[Maxim Dounin]

Hello!

On Thu, Jul 13, 2023 at 10:38:02AM +0300, Vesa Jääskeläinen via nginx-devel wrote:

> Hi Maxim,
> 
> On 13.7.2023 3.48, Maxim Dounin wrote:
> > Hello!
> >
> > On Wed, Jul 12, 2023 at 05:07:03PM +0300, Vesa Jääskeläinen via nginx-devel wrote:
> >
> >> (I hope this goes properly out as I had major issues with hg email so
> >> combined hg export + git send-email)
> >>
> >> It is convenient to keep X.509 certificates related to key pairs stored in
> >> openssl engine within the engine.
> >>
> >> Implementation uses 'LOAD_CERT_CTRL' extension to fetch certificate from
> >> the engine. This extension is not supported by all engines and in those
> >> cases it should report with an error.
> >>
> >> Configuration is similar to what it is for 'ssl_certificate_key'.
> >>
> >> First certificate must match with ssl_certificate_key's key pair rest of
> >> the certificiates are added to the certificate chain.
> >>
> >> Example configuration with libp11's pkcs11 engine:
> >>
> >>    ssl_certificate      "engine:pkcs11:pkcs11:token=mytoken;object=mykey
> >>                          engine:pkcs11:pkcs11:token=mytoken;object=int-ca";
> >>    ssl_certificate_key  "engine:pkcs11:pkcs11:token=mytoken;object=mykey?pin-value=mypin";
> >>
> >> Tested the loading with two pkcs11 implementations SoftHSMv2 and with
> >> OP-TEE's PKCS11 Trusted Application running on Embedded Linux device.
> >>
> >> First three commits is the main beef and in order to make it more flexible
> >> added also last commit allowing intermediate certificates loaded from file
> >> system.
> >>
> >> Separator of space is used as there was already existing use of array for
> >> ssl_certificate configuration.
> > Just in case, a similar proposal was previously discussed here:
> >
> > https://mailman.nginx.org/pipermail/nginx-devel/2020-April/013130.html
> > https://mailman.nginx.org/pipermail/nginx-devel/2020-May/013142.html
> >
> > Notably, the review is here:
> >
> > https://mailman.nginx.org/pipermail/nginx-devel/2020-May/013152.html
> >
> > I'm additionally sceptical about this given that engine interface
> > is deprecated by OpenSSL.
> 
> Thanks for the links.
> 
> The provider interface in OpenSSL 3 is yet to be matured for PKCS11 usage.
> 
> libp11 so far seems to have stayed with "legacy" engine and just adapted 
> for OpenSSL 3.
> 
> Then there is new project that is still in development phase but looks 
> promising:
> 
> https://github.com/latchset/pkcs11-provider
> 
> But before that provider implementation matures and is available in 
> distributions it takes one or more major LTS releases.
> 
> While waiting for that it would be nice to have the support.
> 
> What is interesting that the CMD extension is already being integrated 
> to other software like curl, wpa-supplicant/hostapd, stunnel, ppp, 
> M2Crypto -- so I would say the feature is there to stay for a while. 
> Even not documented as official openssl "extension" it is there and is 
> being used.
> 
> Now openssl also has "store" API which can be extended with different 
> schemes too but in example libp11 does not today support that.
> 
> https://www.openssl.org/docs/man1.1.1/man7/ossl_store.html
> https://www.openssl.org/docs/man1.1.1/man3/OSSL_STORE_LOADER.html
> 
> Now the Nginx is already using the "legacy" interface so if it is enable 
> why not then support the certificate loading when it is available.

As pointed out in the review referenced above, the certificate 
loading in question is a limited interface specific to pkcs11 
engine only.  That is, it's not even the legacy interface, it's a 
non-portable extension to a legacy interface.

[...]

> With PEM files this is easy as one can craft them freely.

And that's the recommended approach.  Exporting appropriate 
certificates to a PEM file and using it in nginx configuration 
should be trivial enough.

>From security point of view there are no benefits in using engines 
to load certificates.

[...]

> Why I believe it is a good idea to support engine/or in future the 
> provider interface is that to keep the certificates in one place next to 
> the keys. With this if the key pair gets re-created or new certificates 
> to be updated if you have the labels there you just need to trigger the 
> refresh. Then there is no extra data to extract or such as then those 
> could be out-of-sync and people try to figure out problems with that.

Note that with the approach you suggest, re-creating certificates 
might anyway require editing nginx configuration, since 
certificate chain for the new certificate might be different and 
might require different number of certificates in the chain.

> If I am not mistaken the previous discussion comments has been addressed 
> in this change set.

As far as I see, you've tried to address the issue with 
certificate chains.  I can't say I like the approach you've taken 
though, as it basically introduced yet another array inside the 
ssl_certificate directive parameter.

> I am happy to test out the provider with pkcs11-provider too but I would 
> recommend that we also support today's methods so people can start using 
> the feature.

Thank you for your work, though I tend to think that introducing 
loading certificates from the pkcs11 engine is not something we 
are willing to do now, especially considering questionable 
approach to configuring things and the fact that engines are 
already deprecated.

Rather, I would consider introducing loading private keys and 
certificates via providers once this will be available.

-- 
Maxim Dounin
http://mdounin.ru/