Closed apollo13 closed 2 years ago
The following code makes my VPN work again:
diff --git a/lib/pkcs11h-openssl.c b/lib/pkcs11h-openssl.c
index 78bb7fc..1ac1fd0 100644
--- a/lib/pkcs11h-openssl.c
+++ b/lib/pkcs11h-openssl.c
@@ -653,6 +653,10 @@ __pkcs11h_openssl_session_setRSA(
#if OPENSSL_VERSION_NUMBER < 0x10100001L
rsa->flags |= RSA_FLAG_SIGN_VER;
#endif
+ if (EVP_PKEY_set1_RSA (evp, rsa) != 1) {
+ _PKCS11H_LOG (PKCS11H_LOG_WARN, "PKCS#11: Cannot set RSA key");
+ goto cleanup;
+ }
#ifdef BROKEN_OPENSSL_ENGINE
if (!rsa->engine) {
@@ -848,6 +852,10 @@ __pkcs11h_openssl_session_setDSA(
DSA_set_method (dsa, __openssl_methods.dsa);
DSA_set_ex_data (dsa, __openssl_methods.dsa_index, openssl_session);
+ if (EVP_PKEY_set1_DSA (evp, dsa) != 1) {
+ _PKCS11H_LOG (PKCS11H_LOG_WARN, "PKCS#11: Cannot set DSA key");
+ goto cleanup;
+ }
ret = TRUE;
@@ -1046,6 +1054,10 @@ __pkcs11h_openssl_session_setECDSA(
EC_KEY_set_method (ec, __openssl_methods.eckey);
EC_KEY_set_ex_data (ec, __openssl_methods.eckey_index, openssl_session);
+ if (EVP_PKEY_set1_EC_KEY (evp, ec) != 1) {
+ _PKCS11H_LOG (PKCS11H_LOG_WARN, "PKCS#11: Cannot set ECDSA key");
+ goto cleanup;
+ }
ret = TRUE;
I do not know enough about openssl to be able to determine whether this is correct, so please feel free to update the code as needed and commit it :)
Great analysis, I truly appreciate this. I've not used openssl-3 so far. I need to look into the openssl code to see this pattern if getting the pkey from evp and then setting it again is the correct approach. Have you tested this with both openssl-1 and openssl-3?
openssl-3 introduces the provider concept, maybe openssl-3 support should be focused in leveraging that instead of the engine. But this will require a change in openvpn to acquire the objects from the certificate store.
No I have not tested against openssl-1 (at least not with these patches applied). I am currently reworking my openvpn setup to use management-external-key as a learning exercise so I can use that instead of the native pkcs11 support. As I said I know literally nothing about openssl, so while the above code works I have no idea if that is correct with regards to refcounting etc…
You are at the right path, openvpn should have removed internal crypto implementation long ago. But be prepared to bump into the same issue as openvpn is doing similar magic to externalize the private key operation.
I was prepared to run into this, but it worked fine :)
On Mon, Apr 4, 2022, at 19:37, Alon Bar-Lev wrote:
You are at the right path, openvpn should have removed internal crypto implementation long ago. But be prepared to bump into the same issue as openvpn is doing similar magic to externalize the private key operation. — Reply to this email directly, view it on GitHub https://github.com/OpenSC/pkcs11-helper/issues/52#issuecomment-1087830827, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAT5CYHMPPU5PVNWVMRLC3VDMSENANCNFSM5SP3V7LA. You are receiving this because you authored the thread.Message ID: @.***>
Hmmm... maybe the SSL_CTX_use_RSAPrivateKey make the difference.
https://github.com/OpenVPN/openvpn/blob/master/src/openvpn/ssl_openssl.c#L1340
Most likely, after all then it does not matter if the EVP has returned a cached copy or not. That said I think the way forward would be to start with a new branch that provides openssl compatibility via a custom provider. Even though it works for openVPN currently they are using deprecated APIs. They do have code bringing this up2speed for openssl3 though: https://github.com/OpenVPN/openvpn/pull/161
I have no idea who and what uses pkcs11-helper, but I think with regard to external keys quite a lot changed so I am not sure how feasible it is to support openssl 1.1 and openssl 3 at the same time.
Even though it works for openVPN currently they are using deprecated APIs.
Only OpenVPN 2.6 is compatible with both OpenSSL 3 and OpenSSL 1 there should be no deprecated OpenSSL APIs in use in that branch. At least that's what we are shooting for.
Mhm I just checked master and from what I can see https://github.com/OpenVPN/openvpn/blob/master/src/openvpn/ssl_openssl.c#L1460 is using deprecated API (https://www.openssl.org/docs/man3.0/man3/EVP_PKEY_assign_EC_KEY.html). Please do not get me wrong, I think it is perfectly fine to use deprecated APIs. But if only openvpn 2.6 is compatible with openssl 3 and 1.1, then openvpn 2.6 is also the first release to support openssl 3? Which somewhat begs the question why fedora compiles openvpn 2.5 against openssl 3 XD (I'll have to double check the actual versions tomorrow).
On Tue, Apr 5, 2022, at 16:01, Selva Nair wrote:
Even though it works for openVPN currently they are using deprecated APIs.
Only OpenVPN 2.6 is compatible with both OpenSSL 3 and OpenSSL 1 there should be no deprecated OpenSSL APIs in use in that branch. At least that's what we are shooting for.
— Reply to this email directly, view it on GitHub https://github.com/OpenSC/pkcs11-helper/issues/52#issuecomment-1088743289, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAT5C4HO3WRZCIKJOKSZW3VDRBVNANCNFSM5SP3V7LA. You are receiving this because you authored the thread.Message ID: @.***>
This is probably getting off-topic, can we move this to openvpn-devel list? If you could please report this there with compile logs showing deprecated warning, that would be great. We have been making changes so that 2.6 builds with OpenSSL 1.x and 3.x without any deprecation warnings. Would like to fix any that are still remaining. Officially OpenSSL 3.x support is only in 2.6 -- distributions may have their own patches applied.
Ok, I added the set1 key fix[1]. The openssl provides use the deprecated API, so they themselves do not know have a pure interface. Re-implementation of the entire provider makes no sense, I will find a time to discuss this with upstream. For now I am fine with using the same deprecated APIs to implement the engine.
merged
@selvanair Oh, I was just looking at the docs. I am off into vacation, but will see if I can break openvpn afterwards. Thanks for your support!
I am always getting
when connecting. Manually compiling everything with debug led me down a rabbithole to the function
__pkcs11h_openssl_session_setRSA
. This function callsEVP_PKEY_get1_RSA
and now this seems to be interesting (https://www.openssl.org/docs/man3.0/man3/EVP_PKEY_get1_RSA.html):So it seems the changes to https://github.com/OpenSC/pkcs11-helper/blob/28c1b389f3d3a4eb0d327c9aa699421430934858/lib/pkcs11h-openssl.c#L651-L652 are going back to nowhere. I verified that I could talk with the smartcard again by replacing
https://github.com/OpenSC/pkcs11-helper/blob/28c1b389f3d3a4eb0d327c9aa699421430934858/lib/pkcs11h-openssl.c#L671-L672
with
EVP_PKEY_assign_RSA
. I am not sure if this is correct but it seems to be that there is massive incompat between openssl 1.1 & 3.Related openssl ticket: https://github.com/openssl/openssl/issues/18028
Is my analysis correct?