Skip to content
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

Support key in PEM file (like TPM2 TSS KEY) #222

Closed
quality-leftovers opened this issue Mar 30, 2023 · 12 comments
Closed

Support key in PEM file (like TPM2 TSS KEY) #222

quality-leftovers opened this issue Mar 30, 2023 · 12 comments
Labels
enhancement New feature or request

Comments

@quality-leftovers
Copy link

Describe the feature
To integrate the provider with existing / COTS applications it would be nice to be able to provide the key in PEM format, likt it is possible in tpm2-tss.

Expected behavior
It should be possible to generate PEMs for keys and provide them to an application similar to "TPM2 TSS KEY"

Additional context
I've played around a bit with the pkcs11-provider and am trying to add the ability to load a key from a pem.
=> quality-leftovers@138a909

So far I've just taken the encoder / decoder bits from the tpm2-tss project and integrated them (https://github.com/tpm2-software/tpm2-tss) and now I'm wondering what the best way to load a key from a decoder would be.

Haven't done a lot with OpenSSL so far, so it's a bit difficult for me :)

@quality-leftovers quality-leftovers added the enhancement New feature or request label Mar 30, 2023
@simo5
Copy link
Member

simo5 commented Mar 30, 2023

What would the PEM file consist of?
A pkcs11 URI?

@simo5
Copy link
Member

simo5 commented Mar 30, 2023

I am not entirely sure about this, the code seem straightforward, but I question whether it really makes sense to provide this level of "obfuscation".
With TPM it may be needed because there is no standard URI scheme (afaik), so a PEM file may be a reasonable option.
But with pkcs11 we do have standard URIs and the OpenSSL 3 STORE API takes URIs just fine.

So what is the tradeoff here? Can you provide some pros/cons to help me evaluate if this is really desirable for the pkcs11 provider?

@quality-leftovers
Copy link
Author

quality-leftovers commented Mar 31, 2023

It is basically all about enabling "Plug & Play" of the pkcs11-provider with existing software.

Existing software that uses OpenSSL often does not support OpenSSL 1 key engines or OpenSSL 3 provider URIs.
These software packages usually load key "PEMs" using BIO_read_filename and PEM_read_bio_PrivateKey and will not accept key URIs.

For example there is an old issue in haproxy (haproxy/haproxy#71) where support to load an key from a pkcs11 OpenSSL key engine was requested, but it never did go anywhere (probably because haproxy relies on loading keys from a file named certificate-file-name.key-suffix and changing that would be a breaking changes in the haproxy configuration format (as far as I understand).

Pros

  • Adding key loading via PEM would open the door to use pkcs11 HSM with popular existing software solutions
  • Offering functionality to lots of people is more important then having it done the "correct" way

Cons

  • Additional code / complexity (less transparent)
  • It's not the clean / correct solution, it would be better if software supported loading keys via URIs from stores

Though maybe I've missed something and there are easier ways to get this done. Ideally I would've hoped it to be possible to write a decoder that returns an URI and OpenSSL would try look it up in all stores, but that does not seem to be possible.

@simo5
Copy link
Member

simo5 commented Mar 31, 2023

Ok I need to think about this, I am not convinced the #2 of the Pros is actually a pro.
I get why it can be tempting to trick applications, but this assumes these applications never look into the contents at all, which may be true, but still..

That said, I looked at your patches briefly, there seem to be quite some movement around the code auto-generation and it is probably too much as a single commit. Also not sure of why some things were moved around/renamed, but that is something we can handle in due time.

One main issue I see with this scheme is that the PEM file format would be completely non-standard. What OIDs wuld be used? Do we need to allocate some on our own (I can get OIDs from one of the Red Hat OID spaces if needed), the other is that I want to see what OpenSSL upstream thinks of this "hack".

@beldmit what do you think? ^

@quality-leftovers
Copy link
Author

Thanks. Take your time. We can talk about whether parts of the patch are of interest, if you decide that this would be something worth doing.

As of now the patch is only the encoder / decoder anyway and the decoder just errors out after parsing the input. So not a lot functionality it adds atm. I just wanted to do some coding to get a bit of an understanding (thankfully stumbled upon the tpm2-openssl decoder by Petr Gotthard).

@quality-leftovers
Copy link
Author

Hi. I was wondering whether you had time to consider this and got an update on the issue?

@simo5
Copy link
Member

simo5 commented Oct 12, 2023

I did consider this, but I am not yet certain it is a good idea, but I am not yet persuaded that it is a completely bad idea. Once I will make up my mind I will either implement the feature or close the issue for good.
At the moment it is low priority for me though, so I am not in a rush to deal with this.

@0pq76r
Copy link

0pq76r commented Dec 7, 2023

I'm also interested in this feature. Currently, there is an open ticket at nginx regarding the addition of support for openssl3 providers. However, it seems that there hasn't been much progress made on it so far (https://trac.nginx.org/nginx/ticket/2449).

What would it take to get this feature?

@simo5
Copy link
Member

simo5 commented Dec 7, 2023

It is not prioritized because I am not sure it is a good feature.
I understand it can work around some software limitations, but I do not know that it is is a good idea long term. Supporting the use of URIs is not hard Apache HTTPD does it, though in a weird way (supports exclusively pkcs11:// URIs).

@0pq76r
Copy link

0pq76r commented Dec 18, 2023

I prepared a patch for this feature where I tried to keep the changes to the code small. (0pq76r@47db75f)
The implementation of this feature would also solve the nuisance with the error message when running genpkey ( #303 ).

@simo5
Copy link
Member

simo5 commented Dec 18, 2023

@0pq76r this is outstanding work!
I am on vacation so I will be slow to respond, but please make a PR for this code.
I may ask you to hide it behind an option, so that we can control whether to allow using these PEM files or not, but I definitely do not want to lose this contribution!

@quality-leftovers
Copy link
Author

Nice, would be great if this feature makes it to main. Thanks for taking care of it.

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

No branches or pull requests

3 participants