OpenSSL provider support

[krwq]

In #88656 we've added ENGINE support but we've left out providers pending more investigation on issues discovered during implementation. This API was already approved last year but asking for re-approval given time which passed.
Original issue: #55356

Background

ENGINE and providers are plug-in systems for OpenSSL which allow to make custom implementation of some crypto algorithms. That enables scenarios such as using TPM keys.

ENGINE is an old (now deprecated) plug-in system which was the only FIPS approved plugin system until recently. Providers is new plug-in system allowing a bit more flexibility to developers (but also harder to implement).

Proposal

namespace System.Security.Cryptography
{
    public partial class SafeEvpPKeyHandle
    {
        // existing OpenSSL ENGINE APIs (only plugin model for OSSL 1.1, deprectated (but present) in 3.0+)
        // public static SafeEvpPKeyHandle OpenPrivateKeyFromEngine(string engineName, string keyId) => throw null;
        // public static SafeEvpPKeyHandle OpenPublicKeyFromEngine(string engineName, string keyId) => throw null;

        // OpenSSL Providers (new plugin model for OSSL 3.0+)
        public static SafeEvpPKeyHandle OpenKeyFromProvider(string providerName, string keyUri) => throw null;
    }
}

Usage Example

byte[] data = ...;

// For TPM settings refer to provider documentation you're using, i.e. https://github.com/tpm2-software/tpm2-openssl/tree/master
// specific values are just example
using (SafeEvpPKeyHandle priKeyHandle = SafeEvpPKeyHandle.OpenKeyFromProvider("tpm2", "handle:0x81000007"))
using (ECDsa ecdsaPri = new ECDsaOpenSsl(priKeyHandle))
{
    byte[] signature = ecdsaPri.SignData(data, HashAlgorithmName.SHA256);
    // do stuff with signature
    // note: tpm2 does not allow Verify operations, public key needs to be exported and re-imported into new instance
}

Security considerations

Those APIs are not meant to be used with untrusted inputs. Allowing untrusted inputs may potentially allow attacker to use TPM or other private keys in an unintended way. Some providers i.e. "default" allow reading files from arbitrary paths (in order to read the key) which makes it further dangerous to use with untrusted inputs. Additionally providerName may load libraries into the process which adds additional risk (depending on OSSL_PROVIDER_set_default_search_path setting this may or may not be a big issue).

(considerations also apply to already added ENGINE APIs)

[ghost]

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]

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:
   

   runtime/src/libraries/System.Security.Cryptography/src/Microsoft/Win32/SafeHandles/NCryptSafeHandles.cs

   Line 75 in d051a84

   protected SafeNCryptHandle(IntPtr handle, SafeHandle parentHandle)

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

   runtime/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePal.Windows.PrivateKey.cs

   Line 260 in d051a84

   return new SafeNCryptKeyHandle(privateKeyPtr, certificateContext);

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.

[krwq]

Note: I've updated proposal to look as it was before.

[krwq]

per offline discussion with @bartonjs it's already approved in #55356 so it doesn't need to be re-approved. PR should point to the old issue which approved it

[terrajobst]

(marking as api-approved for clarity)