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

How to create Certificate Signing Request (CSR) when using dynamic engine for openssl #53345

Closed
Dvergatal opened this issue May 27, 2021 · 24 comments · Fixed by #53629
Closed
Labels
area-System.Security question Answer questions and provide assistance, not an issue with source code or documentation. untriaged New issue has not been triaged by the area owner

Comments

@Dvergatal
Copy link

Hi all,
i have configured OpenSSL library to use my pkcs11 dynamic engine. Now i am using Pkcs11Interop library to connect to the token and generate key pair. Key pair has been generated and i see it on the token via pkcs11-tool. The problem is now that i would like to generate CSR and i really don't know how to pass my private key object from the token to the the function of X509Request.

I was thinking of getting the key pointer from the pkcs11 token using ENGINE_load_private_key method which would return me SafeEvpPKeyHandle but i really don't know how to pass it to X509Request call.

Maybe there is a much better solution and someone could point me to it.

BR
Piotr

@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Security untriaged New issue has not been triaged by the area owner labels May 27, 2021
@ghost
Copy link

ghost commented May 27, 2021

Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq, @GrabYourPitchforks
See info in area-owners.md if you want to be subscribed.

Issue Details

Hi all,
i have configured OpenSSL library to use my pkcs11 dynamic engine. Now i am using Pkcs11Interop library to connect to the token and generate key pair. Key pair has been generated and i see it on the token via pkcs11-tool. The problem is now that i would like to generate CSR and i really don't know how to pass my private key object from the token to the the function of X509Request.

I was thinking of getting the key pointer from the pkcs11 token using ENGINE_load_private_key method which would return me SafeEvpPKeyHandle but i really don't know how to pass it to X509Request call.

Maybe there is a much better solution and someone could point me to it.

BR
Piotr

Author: Dvergatal
Assignees: -
Labels:

area-System.Security, untriaged

Milestone: -

@vcsjones
Copy link
Member

function of X509Request

Is this a class from another library?

return me SafeEvpPKeyHandle but i really don't know how to pass it

If you can get an EVP_PKEY handle to a key, you can construct an instance of the appropriate algorithm using RSAOpenSsl or ECDsaOpenSsl from the System.Security.Cryptography.OpenSsl package. Something like:

SafeEvpPKeyHandle handle; // Get your handle
RSA rsa = new RSAOpenSsl(handle);
// Pass your RSA object around as needed

If you need an X509Certificate2 object, you can use CopyWithPrivateKey so you have an instance backed by a key from PKCS11.

@vcsjones vcsjones added the question Answer questions and provide assistance, not an issue with source code or documentation. label May 27, 2021
@vcsjones
Copy link
Member

vcsjones commented May 27, 2021

If you can get a SafeEvpPKeyHandle, you can use CertificateRequest to generate a request. A contrived example would look something like this:

SafeEvpPKeyHandle handle; // Get your handle
RSA rsa = new RSAOpenSsl(handle);

CertificateRequest req = new CertificateRequest("CN=potato", rsa, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1);
byte[] requestDer = req.CreateSigningRequest();
string requestPem = new string(PemEncoding.Write("CERTIFICATE REQUEST", requestDer));
Console.WriteLine(requestPem);

@Dvergatal
Copy link
Author

Dvergatal commented May 27, 2021

Ok but i have a new problem. I am unable to use from System.Security.Cryptography.OpenSsl.dll. I see that i have this library in my sdk /usr/share/dotnet/shared/Microsoft.NETCore.App/5.0.6/ but i am unable to use it in my project. The project TargetFramework is Net5.0 so it should work but it is not.

I have even tried to use package from nuget and i see it is being downloaded to .nuget/packages/system.security.cryptography.openssl but it doesn' see symbols from it.

@vcsjones
Copy link
Member

I have even tried to use package from nuget and i see it is being downloaded to .nuget/packages/system.security.cryptography.openssl but it doesn' see symbols from it.

You should be able to use it as a package reference:

<PackageReference Include="System.Security.Cryptography.OpenSsl" Version="5.0.0" />

The RSAOpenSsl class is in the System.Security.Cryptography namespace. There is no System.Security.Cryptography.OpenSsl namespace.

If you are still running in to issues, can you create a sample project that reproduces the issue and share it on GitHub as a public repository?

@Dvergatal
Copy link
Author

Ok i have solved my problem. I was using version library 5.0.6 not 5.0.0 and also i was using namespace System.Security.Cryptography.OpenSsl instead of System.Security.Cryptography. My bad:P

Thx for help.

@Dvergatal
Copy link
Author

Dvergatal commented May 28, 2021

Btw. is there some mechanism ported to determine the mechanism of the SafeEvpPKeyHandle? Because in OpenSSL there is this EVP_PKEY_type field from which i can determine the correct key structure.

P.S. Btw. i've got an error on call of byte[] requestDer = req.CreateSigningRequest(); which looks like that:

Unhandled exception. Interop+Crypto+OpenSslCryptographicException: error:04075093:rsa routines:RSA_sign:value missing
   at System.Security.Cryptography.RSAOpenSsl.TrySignHash(ReadOnlySpan`1 hash, Span`1 destination, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding, Boolean allocateSignature, Int32& bytesWritten, Byte[]& signature)
   at System.Security.Cryptography.RSAOpenSsl.SignHash(Byte[] hash, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding)
   at System.Security.Cryptography.RSA.SignData(Byte[] data, Int32 offset, Int32 count, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding)
   at System.Security.Cryptography.RSA.SignData(Byte[] data, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding)
   at System.Security.Cryptography.X509Certificates.RSAPkcs1X509SignatureGenerator.SignData(Byte[] data, HashAlgorithmName hashAlgorithm)
   at System.Security.Cryptography.X509Certificates.Pkcs10CertificationRequestInfo.ToPkcs10Request(X509SignatureGenerator signatureGenerator, HashAlgorithmName hashAlgorithm)
   at System.Security.Cryptography.X509Certificates.CertificateRequest.CreateSigningRequest(X509SignatureGenerator signatureGenerator)
   at System.Security.Cryptography.X509Certificates.CertificateRequest.CreateSigningRequest()

The key has been successfully retrieved from the token and the RSAOpenSsl was made.

Any ideas? May it be, because the given key isn't private (but it should be, i'm calling ENGINE_load_private_key to retrieve it)?

@vcsjones
Copy link
Member

@Dvergatal can you share the code you had to produce this error, or did you use my example code verbatim?

@Dvergatal
Copy link
Author

Dvergatal commented May 28, 2021

Yeah i can give you the code of my DynamicEngine assembly. The usage is like that:

IEngine engine = EngineFactory.GetEngine(new EngineFactory.OpenSSLConfig
    {
        EngineId = "pkcs11",
        EnginePath = "/lib/engines-1.1/libpkcs11.so",
        ModulePath = "/lib/libtpm2_pkcs11.so"
    }
);
// ...
// Perform cryptographic operations
// ...
engine.Login("123456890");
SafeEvpPKeyHandle pKeyHandle = engine.GetPrivKey("key_label");
string requestPem = engine.GetCSR(pKeyHandle, "CN=potato", HashAlgorithmName.SHA256);
Console.WriteLine(requestPem);

But in order to work you need to have an initialized pkcs11 token with key pair generated. In my case i'm using physical tpm token with tpm2-pkcs11 module. Btw. i assume you know how to initialize token and generate all the data on it, but if you need help i can assist.

@Dvergatal
Copy link
Author

Dvergatal commented May 28, 2021

Ok. After some investigations i have found that this error is being called from dotnet code from file ./src/Native/Unix/System.Security.Cryptography.Native/pal_rsa.c at line 164 because key has no private key but that is not true. The keys on the token are private and public.

Is it possible that the dotnet openssl library cannot work with dynamic engines?

P.S. Oh i think i see the problem. In method HasNoPrivateKey you are retrieving all private parameters of the key and to what i know all these parameters are not being extractable from the token. The whole private key is not being extractable...
P.S.2 Another thing is that i have noticed in this method this lines:

    // The method has descibed itself as having the private key external to the structure.
    // That doesn't mean it's actually present, but we can't tell.
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wcast-qual"
    if (RSA_meth_get_flags((RSA_METHOD*)meth) & RSA_FLAG_EXT_PKEY)
#pragma clang diagnostic pop
    {
        return 0;
    }

My guess is that because i'm using external key, which is on the token, this method should return this 0 here am i right?

@vcsjones
Copy link
Member

My guess is that because i'm using external key, which is on the token, this method should return this 0 here am i right?

That should be the case. The function is a negative, HasNoPrivateKey, so returning 0 here would mean we would believe it has a private key.

Can you determine if RSA_meth_get_flags has the RSA_FLAG_EXT_PKEY flag for your key?

@Dvergatal
Copy link
Author

Dvergatal commented May 28, 2021

Yeah but does dotnet have this RSA_meth_get_flags equivalent?

Btw. i'm calling ENGINE_load_private_key like this:

SafeEvpPKeyHandle pkey = SafeNativeMethods.ENGINE_load_private_key(engine, keyId, IntPtr.Zero, IntPtr.Zero);

The 3rd parameter is passed as null. The UI_METHOD *ui_method is much work for porting and i couldn't find it in dotnet.

@Dvergatal
Copy link
Author

Ok this is a bug in the dotnet openssl implementation. This checking if key is privte shouldn't be in there. This should be handled by the openssl itself or the checking should be on the key itself. Not on the method of the key. I'm opening a bug and this is really a critical one,

@bartonjs
Copy link
Member

bartonjs commented Jun 1, 2021

This should be handled by the openssl itself

I agree, and OpenSSL 3.0 does check it. Unfortunately, OpenSSL 1.0 and 1.1 both segfault if you pass a public-only key into a private key operation, which is why we need the check. The check appears to be not quite right, but we'll need some help in figuring out what the right check should be.

@Dvergatal
Copy link
Author

Dvergatal commented Jun 1, 2021

int RSA_test_flags(const RSA *r, int flags) 

and this is the docs for the method https://www.openssl.org/docs/man1.1.0/man3/RSA_test_flags.html
RSA_test_flags() tests to see whether the flags passed in the flags parameter are currently set in the RSA object. Multiple flags can be tested in one go. All flags that are currently set are returned, or zero if none of the flags are set.

Because as you can see in bugs section of RSA_flags:
The behaviour of RSA_flags() is a mis-feature that is left as-is for now to avoid creating compatibility problems. RSA functionality, such as the encryption functions, are controlled by the flags value in the RSA key itself, not by the flags value in the RSA_METHOD attached to the RSA key (which is what this function returns). If the flags element of an RSA key is changed, the changes will be honoured by RSA functionality but will not be reflected in the return value of the RSA_flags() function - in effect RSA_flags() behaves more like an RSA_default_flags() function (which does not currently exist).

This line:

if (RSA_meth_get_flags((RSA_METHOD*)meth) & RSA_FLAG_EXT_PKEY)

should be changed to:

if (RSA_test_flags(rsa,  RSA_FLAG_EXT_PKEY))

@vcsjones
Copy link
Member

vcsjones commented Jun 1, 2021

That looks promising. However RSA_test_flags is new in OpenSSL 1.1.0, it would need to be shimmed for OpenSSL 1.0.2 support.

Since the RSA object is not opaque in 1.0.2, I think it would be a matter of checking the flags field.

@Dvergatal
Copy link
Author

That looks promising. However RSA_test_flags is new in OpenSSL 1.1.0, it would need to be shimmed for OpenSSL 1.0.2 support.

Since the RSA object is not opaque in 1.0.2, I think it would be a matter of checking the flags field.

Yes. In libp11 there was also the same bug but with setting this field. And it was fixed as you say.

@vcsjones
Copy link
Member

vcsjones commented Jun 1, 2021

@bartonjs what do you think? I can prepare a PR for this change if it seems reasonable to you.

@Dvergatal
Copy link
Author

Dvergatal commented Jun 1, 2021

Btw. for a quick checking i have added in the code of libp11 a simple change:

        if (key->isPrivate) {
                RSA_set_method(rsa, PKCS11_get_rsa_method());
                RSA_meth_set_flags(RSA_get_method(rsa), RSA_FLAG_EXT_PKEY);
#if OPENSSL_VERSION_NUMBER >= 0x10100005L && !defined(LIBRESSL_VERSION_NUMBER)
                RSA_set_flags(rsa, RSA_FLAG_EXT_PKEY);
#else
                rsa->flags |= RSA_FLAG_EXT_PKEY;
#endif
                printf("flaga ustawiona\n");
        }

The line RSA_meth_set_flags(RSA_get_method(rsa), RSA_FLAG_EXT_PKEY); was added and CSR started to be generated.
The error Unhandled exception. Interop+Crypto+OpenSslCryptographicException: error:04075093:rsa routines:RSA_sign:value missing is gone.

@bartonjs
Copy link
Member

bartonjs commented Jun 1, 2021

I guess I'm surprised by how anything would be working with this already. If RSA_FLAG_EXT_PKEY isn't set, then things like RSA_sign are going to try using the CRT parameter fields, which I expect would be null... The flag being set means it'll call the callback instead, so it seems like it should already be set at the engine level. (Unless the point is just that we checked the engine and we should have checked the key).

I'd be concerned that something is already relying on the "we checked the engine" behavior, so I'd or-in the new acceptance criteria, not just change it. But maybe I'm being overly cautious.

(

if (RSA_test_flags(rsa,  RSA_FLAG_EXT_PKEY) || RSA_meth_get_flags((RSA_METHOD*)meth) & RSA_FLAG_EXT_PKEY)
{
   return 0;
}

)

@Dvergatal
Copy link
Author

Dvergatal commented Jun 1, 2021

Have you been verifying, if it works with external key on HSM? In my case i'm using pkcs11 engine together with tpm2-pkcs11 module and from what i have been talking with guys from openssl this is how it should be implemented. The checking should be on the object itself not the method because the method is global per engine. This is a relict left when no one thought about changing engines during the work of crypto materials.

@Dvergatal
Copy link
Author

Hi, is it possible for a quick fix ASAP?

@vcsjones
Copy link
Member

vcsjones commented Jun 2, 2021

is it possible for a quick fix ASAP?

Is your expectation that this is fixed for .NET 5? I would not think any changes for this would come until .NET 6, unless you are comfortable using pre-releases for urgency.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jun 2, 2021
@Dvergatal
Copy link
Author

Yeah i can switch to .NET 6 pre-release, because this is a critical bug in our company assembly to go further with the work. For now i am working with workaround in the pkcs11 engine but i do not want to make rubbish in our OS firmware.

@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jun 2, 2021
@dotnet dotnet locked as resolved and limited conversation to collaborators Jul 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Security question Answer questions and provide assistance, not an issue with source code or documentation. untriaged New issue has not been triaged by the area owner
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants