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

OpenSSL provider support #89167

Open
krwq opened this issue Jul 19, 2023 · 2 comments
Open

OpenSSL provider support #89167

krwq opened this issue Jul 19, 2023 · 2 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Security
Milestone

Comments

@krwq
Copy link
Member

krwq commented Jul 19, 2023

In #88656 we've added ENGINE support but ENGINEs are deprecated by OpenSSL. #55356 has approved APIs for provider but during testing (#55356 (comment)) we've found that APIs for provider will need to be adjusted since they require OSSL_LIB_CTX* to work correctly with i.e. tpm2 provider. To fix that we could create a container class which would manage lifetime of that.

Example design could look like:

public class OpenSslProvider : IDisposable
{
   public OpenSslProvider(string providerName);

   public RSA OpenRSAPrivateKey(string keyUri); // this could return alternative implementation of RSA which includes all implementation details - downside of that it will need some plumbing in X509Certificate2.CopyWithPrivateKey/X509Certificate2/SslStream etc
   // .. similar for other key types

  // other ideas to consider:
  public SafeEvpPKeyHandle OpenRSAPrivateKey(string keyUri); // this one has downside of having to attach library handle to SafeEvpPKeyHandle handle but all pieces already understand it
  public AssymetricAlgorithm OpenPrivateKey(string keyUri); // similar to above but it would be checking key type and creating appropriate instance
}

This needs a bit more experimentation. I've already done some experimentation trying to get originally designed APIs to work without changes but:

  • tpm2 provider requires that it's loaded in new library context - for me NULL did not work ever - it did load it but none of the keys could be used for signing and they ended up producing errors - this code be bug specific to version of OSSL I was using - I did not try with other versions
  • rather than EVP_PKEY_CTX_new overload taking library context must be used, i.e. EVP_PKEY_CTX_new_from_pkey - this is a change we would need to plumb everywhere

My experiments can be found here: https://github.com/krwq/runtime/blob/open_named_keys/src/libraries/System.Security.Cryptography/tests/osslplugins/testprovider.c

@krwq krwq added api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Security labels Jul 19, 2023
@krwq krwq added this to the 9.0.0 milestone Jul 19, 2023
@ghost
Copy link

ghost commented Jul 19, 2023

Tagging subscribers to this area: @dotnet/area-system-security, @bartonjs, @vcsjones
See info in area-owners.md if you want to be subscribed.

Issue Details

In #88656 we've added ENGINE support but ENGINEs are deprecated by OpenSSL. #55356 has approved APIs for provider but during testing (#55356 (comment)) we've found that APIs for provider will need to be adjusted since they require OSSL_LIB_CTX* to work correctly with i.e. tpm2 provider. To fix that we could create a container class which would manage lifetime of that.

Example design could look like:

public class OpenSslProvider : IDisposable
{
   public OpenSslProvider(string providerName);

   public RSA OpenRSAPrivateKey(string keyUri); // this could return alternative implementation of RSA which includes all implementation details - downside of that it will need some plumbing in X509Certificate2.CopyWithPrivateKey/X509Certificate2/SslStream etc
   // .. similar for other key types

  // other ideas to consider:
  public SafeEvpPKeyHandle OpenRSAPrivateKey(string keyUri); // this one has downside of having to attach library handle to SafeEvpPKeyHandle handle but all pieces already understand it
  public AssymetricAlgorithm OpenPrivateKey(string keyUri); // similar to above but it would be checking key type and creating appropriate instance
}

This needs a bit more experimentation. I've already done some experimentation trying to get originally designed APIs to work without changes but:

  • tpm2 provider requires that it's loaded in new library context - for me NULL did not work ever - it did load it but none of the keys could be used for signing and they ended up producing errors - this code be bug specific to version of OSSL I was using - I did not try with other versions
  • rather than EVP_PKEY_CTX_new overload taking library context must be used, i.e. EVP_PKEY_CTX_new_from_pkey - this is a change we would need to plumb everywhere

My experiments can be found here: https://github.com/krwq/runtime/blob/open_named_keys/src/libraries/System.Security.Cryptography/tests/osslplugins/testprovider.c

Author: krwq
Assignees: -
Labels:

api-suggestion, area-System.Security

Milestone: 9.0.0

@vcsjones
Copy link
Member

I'm not sure that a new API shape is needed; or that the suggestion here is the right approach. For all other crypto primitives in .NET, the object itself maintains all related handles to keep things working so that two things with two independent lifetimes need to be managed.

Using the example above:

OpenSslProvider provider = new("some_pkcs11_module");
SafeEvpPKeyHandle foo = provider.OpenRSAPrivateKey("my key uri");
provider.Dispose();
RSA rsa = new RSAOpenSsl(foo); // What happens here?

Trying to make that work sensibly, even just making sure we throw a managed exception instead of a SIGSEGV, I think would be difficult.

I think it probably makes sense to have the provider lifetime managed by the SafeEvpPKeyHandle. We have similar concepts already

  1. NCrypt has parent handles:

    protected SafeNCryptHandle(IntPtr handle, SafeHandle parentHandle)

    We use this to keep a CERT_CONTEXT alive when getting a key off of the certificate.

  2. macOS does not require you to manage a keychain lifetime separately from the key itself.

I view the way we could solve this with OSSL_LIB_CTX* similarly: A SafeEvpPKeyHandle could keep a related handle / parent handle so that developers do not have independent object lifetimes to manage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Security
Projects
None yet
Development

No branches or pull requests

2 participants