Migration to OpenSSL 3.0 API

[xhanulik]

Follows on from: #2337, related: #2308

This PR rewrites OpenSSL API deprecated in version 3.0. Where possible, it is replaced with EVP interface:

• key generation, encryption, decryption
• i2d_* and d2i_* functions

but there is some code, which can not be simply rewritten in that way, this includes:

• getting parameters of RSA and EC keys - in OpenSSL 3.0 this is to be done with EVP_PKEY_get_*_param()
• setting parameters of RSA and EC keys - building EVP_PKEY via OSSL_PARAM and EVP_PKEY_fromdata(), keys should be immutable once they are created, therefore for setting new parameters, it is needed to create new key with use of EVP_PKEY_todata() and OSSL_PARAM_merge()
• loading legacy provider for EVP_des_ecb() in cardos-tool.c and RIPEMD160 in pkcs11-tool.c
• public key encoding to bio in pkcs15-westcos. - OSSL_ENCODER used
• rewritting of PEM_read_bio_RSAPrivateKey() - with OSSL_DECODER

In these situations, the alternate code is wrapped in #if for given version of OpenSSL.

What is not resolved yet:

• gost card engines calls in openssl.c
• use of SHA internal structure in card-iasecc.c

Also, added container and test for build with OpenSSL 3.0. For the above mentioned deprecated calls it is building without -Werror.

Checklist

• PKCS#11 module is tested
• Windows minidriver is tested
• macOS tokend is tested

[beldmit]

(GOST expert hat on)

I'd suggest not to rely on EC_KEY layer dealing with the GOST stuff. I'd suggest to make a buffer using a technique described in RFC 5933, and then load the GOST key to a EVP_PKEY object. In this case you will be able to use normal EVP layer which is the same for OpenSSL 1.0+ versions.

I can ask my colleagues in Russia to test the code, if necessary, though can't promise the fast response.

[dengert]

@xhanulik when you get around to looking at card-piv.c, please look at the #2053 version that already added support for 1.1.1 and 3.0.0 in its secure messaging code. Thanks.

[Jakuje]

@frankmorgner @dengert Do you have some comments to this PR or can I merge it during the next week? Or do you need more time for review? AFAIK this should not conflict with #2053.

[dengert]

Is this intended to be in the next release?

Do we have a way to test card-westcos.c, cwa-dnie.c and cwa14890.c? (I don't)

There are still questions about gost.

@xhanulik Can you say some more about what options where used to build OpenSSL-3.0.0?
Was it built from github? and from a 3.0.0 tag?
Did you try the no-deprecated
Did you use the no-gost

When building OpenSC, rather then using -Werror have you tried -DOPENSSL_USE_DEPRECATED and --disable-strict which will still catch errors, and print waring messages such as use of deprecated but allow the build to continue.
Have you tried the use of -DOPENSSL_API_COMPAT=0x10101000L

I am OK with it committing to master, as that is the only way this will get tested. Developers can use no-gost' and use the above configure options to get OpenSC to compile But before the next release we would need to fix the gost` (Or make it optional by default) and the use of internal SHA1 structure. and any other bugs.

[popovec]

I tried to reproduce the error caught by test-oseid-openssl-3. RSA key 2048, signature mechanism SHA512-RSA-PKCS-PSS. It seems that the signature has not been generated correctly:

using pkcs11 interface to sign message, mechanism SHA512-RSA-PKCS-PSS
Using slot with index 0 (0x0)
Using signature algorithm SHA512-RSA-PKCS-PSS
PSS parameters: hashAlg=SHA512, mgf=MGF1-SHA512, salt_len=64 B
8002766C367F0000:error:02000086:rsa routines:RSA_verify_PKCS1_PSS_mgf1:last octet invalid:crypto/rsa/rsa_pss.c:94:
8002766C367F0000:error:1C880004:Provider routines:rsa_verify:RSA lib:providers/implementations/signature/rsa_sig.c:816:
Signature Verification Failure
openssl signature test fail
sinature verification (pkcs11-tool)
Using slot with index 0 (0x0)
Using signature algorithm SHA512-RSA-PKCS-PSS
PSS parameters: hashAlg=SHA512, mgf=MGF1-SHA512, salt_len=64 B
pkcs11-tool --verify failed

This error can come from OsEID simulation - I can't guarantee the complete error-free algorithms used in OsEID.

For now, I am unable to reproduce this error. Test environment: lxc debian bullseye, openssl from https://github.com/openssl/openssl, openssl-3.0 branch, commit 015e3f59434651c454c94888d0c6d57c2203cd42 , opensc from https://github.com/xhanulik/OpenSC/tree/openssl-3 , commit 87fd906

[xhanulik]

@dengert OpenSSL was from package (rel 4.el9), so mentioned options for OpenSSL were tested in container (as in 383c8f2, OpenSSL built from github, tag openssl-3.0.0). With no-deprecated I just discovered some more deprecated typedefs and macros, which I will remove/rewrite.

As of building OpenSC, omitting -Werror would keep warning messages about deprecated functions. What would be the point of using -DOPENSSL_USE_DEPRECATED and --disable-strict?
-DOPENSSL_API_COMPAT=0x10101000L led to some macro redefinitions, which should be fixed in b24d647.

[frankmorgner]

Thanks for taking the time for updating to the new API. The changes look good. I fear we're punished with such messy code at least until the en of 2023. In the long run, we should try to extract individual OpenSSL code to some more generic crypto layer...

DNIe and CWA are the two cards that have the most individual changes. @germanblanco @rickyepoderi, would you mind testing the changes?

[Jakuje]

In the long run, we should try to extract individual OpenSSL code to some more generic crypto layer...

This is certainly worth the effort in case 1) We will be able to create a better API 2) We would need to support different crypto backends. In other cases, creating abstraction layer just on different OpenSSL versions would only cause more work maintaining IMHO.

I tried to reproduce the error caught by test-oseid-openssl-3. RSA key 2048, signature mechanism SHA512-RSA-PKCS-PSS. It seems that the signature has not been generated correctly:

Thanks for checking. Looks like the failure is gone, am I right?

[rickyepoderi]

Hi! The current status of this PR makes DNIe core-dump. At least with my current openssl 1.1.1l the changes in this commit are needed. But I also did a quick test with openssl 3.0.0 and it core-dumps even with my changes (something related with the RSA private key, it's like it's not created OK). But I don't know if I have to check 3.0.0 more at this moment, I think the PR is just moving from old API to the new API. If you need more tests I'll try to do them but be calm with me, I have very little time nowadays.

[rickyepoderi]

OK, it was faster than expected. Now it works woth openssl 1.1.1l and 3.0.0. With 3.0.0 there are deprecated warnings but not in the DNIe area so I did not touch that part. There were several minor issues in dnie so please contact people that tests the affected cards because doing a mistake is easy.

Please @xhanulik incorporate this commit on top of this PR.

Thanks!

[xhanulik]

@rickyepoderi, thank you for your time for testing and fixing the issues!

With 3.0.0 there are deprecated warnings but not in the DNIe area so I did not touch that part.

These warnings are connected with gost and SHA internal structures, as mentioned above.

[Jakuje]

@dengert did you get back to this comment? Can you help us to figure out the purpose of this function and if we still need it for something?

[dengert]

#878 appears to be first time LibresSSL was requested.
Last time I built it was on June 9, 2021 from "git@github.com:libressl-portable/portable.git" and tag v3.3.3. OpenSC at that time compiled. First time was in 2016 with version 2.5.0 after #878.

Today, with OpenSC master as of 12/13/2020 c96ec18 using libressl v3.3.3 OpenSC fails to compile with problems with:
"FIPS_mode" in pkcs11/openssl.c,
"EVP_sha3_*", "EVP_MD" in pkxs11-tool.c
"EVP_PKEY_25519" in tests/p11test/p11test_case_ routines.

I have not tried it with #2438 as the effort did not appear to be worth it. Maybe after #2438 is merged I could try again and with newer version of LibreSSL.

https://openports.se/security/opensc says OpenSC is ported to OpenBSD.
https://openports.se/security/openssl says OpenSSL is ported too. But not clear if ported OpenSC uses OpenSSL or LibreSSL.

[mouse07410]

How crucial is it to support LibReSSL? I'd personage prefer not to bother.
BoringSSL is an obvious no-go.

[dengert]

I personally don't use LibreSSL and don't know if anyone uses OpenSC with LibreSSL out side of OpenBSD. So it very low on my list.

Someone is porting OpenSC to OpenBSD and patching it.
I see there http://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/security/opensc/patches/ shows a patch for sc-ossl-compat.h. But don't see how to download the CVS version of the package to see what they have really modified.

[frankmorgner]

Thanks for highlighting the limited scope of BoringSSL; I agree that we don't need to be compatible with it.

Also, LibreSSL is not an high priority target, but it seems that we're breaking compatibility with OpenSSL 1.0.2. OpenSSL 1.0.2 is not supported since one year now and systems building a newer version of OpenSC will surely upgrade OpenSSL as well. If this is what we agree on, then we should adjust the OpenSSL version in configure.ac.

[mouse07410]

IMHO, it is Ok to break compatibility with OpenSSL-1.0.2.

LibreSSL - simply don't care.

[dengert]

Installed openbsd 7.0 as Virtualbox guest. Attach card reader with card to the VM then:

#pkg_add opensc
#rcctl enable pcscd 
#rcctl start pcscd
$openssl --version
LibreSSL 3.4.1
$opensc-tool version
OpenSC 0.21.0
$pkcs11-tool --test --login
(ran as expected)

using ldd it looks like they have built OpenSC using libreSSL. /usr/include/openssl/ has #define LIBRESSL_VERSION_NUMBER 0x3040100fL and #define OPENSSL_VERSION_NUMBER 0x20000000L

Hopefully no one is using openssl < 1.0.2.
https://www.openbsd.org/papers/bsdcan14-libressl/ says it was forked from OpenSSL-1.0.1g in 2014.

https://openports.pl/path/security/openssl/1.1 says: "openssl-1.1.1m ... This package is not intended for general-purpose use in OpenBSD - it is present for test/comparison purposes, and occasionally to provide support for applications which cannot be made compatible with LibreSSL (mostly due to use of removed APIs); in the latter case care must be taken - it will conflict if library dependencies use LibreSSL libraries."

So they are taking the OpenSC source and using the fact that we are testing for the above two defines and building OpenSC using the OpenBSDB LibreSSL. So we should be OK testing and setting minimal OpenSSL number to 1.0.2 or 1.1

[Jakuje]

Also, LibreSSL is not an high priority target, but it seems that we're breaking compatibility with OpenSSL 1.0.2. OpenSSL 1.0.2 is not supported since one year now and systems building a newer version of OpenSC will surely upgrade OpenSSL as well. If this is what we agree on, then we should adjust the OpenSSL version in configure.ac.

@xhanulik can you update the minimal version in the configura.ac I think it is reasonable to not support 1.0.2 as it is no longer supported by openssl upstream.

[frankmorgner]

thank you

[Jakuje]

All green. Thank you Veronika for the contribution and all the others for your inputs!

[dengert]

Compiling on Ubuntu against 3.0.0 I get a lot of warnings about redefines from this this section: https://github.com/OpenSC/OpenSC/blob/master/src/libopensc/sc-ossl-compat.h#L50-L72
lines 68, 65, 53, 59, 62

There are 45 lines like: "../../../src/src/libopensc/sc-ossl-compat.h:68: note: this is the location of the previous definition"

All OpenSSL includes need to be done before sc-ossl-compat.h is included. internal.h also includes sc-ossl-compat.h

This appears to fix these:
misplaced-includes.txt

Only other warnings are about ENGINE_* routines are depricated.

[Jakuje]

Thank you for checking. The proposed changes look good to me. Please, add them as a new PR. Weirdly enough, I do not see these with OpenSSL 3.0 in RHEL9 or CentOS9 Stream where we have OpenSSL 3.0 already package nor in the ubuntu in the CI.. But these changes should not harm.

[dengert]

Looks like may be my fault.
I had built an early 3.0 on Jun 29 2021 which put the libs in my /opt/ossl-3.0.0/lib but the official 3.0.0 I built on Sept 18 2021 has a pkgconfig directory under the the libdir which has have libdir=${exec_prefix}/lib64 which is my /opt/ossl-3.0.0/lib64 but I was still linking OpenSC against the /opt/ossl-3.0.0/lib

I will have to look at how to handle this. So hold off on the fix above it may not be needed.

[dengert]

https://github.com/OpenSC/OpenSC/blob/master/src/libopensc/sc-ossl-compat.h#L50-L72 allows us to use older definitions of 5 macros or defines that have been renamed over the years (or are still used by libressl). sc-ossl-compat.h first checks if OpenSSL has done a define. But this only works as expected if all the OpenSSL includes are included before sc-ossl-compat.h. OpenSSL includes after sc-ossl-compat.h may give a warning message for duplicate defines.

The above is complicated by the use of OpenSSL OPENSSL_API_COMPAT= to define the version compatibility or if OpenSSL is configured with "no-deprecated".

For example in OpenSSL-3.0.1, https://github.com/openssl/openssl/blob/master/include/openssl/evp.h#L869-L872
openssl/evp.h has:

# ifndef OPENSSL_NO_DEPRECATED_1_1_0
#  define EVP_CIPHER_CTX_init(c)      EVP_CIPHER_CTX_reset(c)
#  define EVP_CIPHER_CTX_cleanup(c)   EVP_CIPHER_CTX_reset(c)
# endif

https://github.com/openssl/openssl/blob/master/include/openssl/err.h.in#L456-L460
handles ERR_load_crypto_strings and ERR_free_strings but not the same way sc-ossl-compat.h does.

So what do we do?

1. Do nothing. as suggested in Migration to OpenSSL 3.0 API #2438 (comment)
2. Change all the uses of the old names to the new names in the code and remove the definitions in sc-ossl-compat.c and reverse the definitions for older versions of OpenSSL and Libressl.
3. Reorder the includes in all the files so all OpenSSL includes are done before sc-ossl-compat.h
4. Something else.

Doing 1. may lead to unexpected errors as our version may not work as expected as noted in the ERR_load_crypto_strings case

Doing 2. may cause problems with libressl, but that will be their problem and not ours.

Doing 3. means we always use the definition provided by OpenSSL, and only use ours if OpenSSL did not provide a version.

In #2438 (comment) I addressed only 2 routines that had warning messages, There are 6 other files that need the order of include fixed: card-authentic.c, card-npa.c, card-westcos.c, cryptoflex-tool.c, gids-tool.c and pkcs15-init.c. There may be others these are the ones that produced warnings.

[mouse07410]

Doing (3) makes the most sense to me.

[frankmorgner]

No. It makes no sense at all, because includes should almost never depend on any order. If sc-ossl-compat.h needs any openssl headers to be included first, it those includes need to be in sc-ossl-compat.h before anything meaningful is done.

[dengert]

OK, I won't do anything.

[frankmorgner]

To my understanding, sc-ossl-compat.h allows using the 1.1.0 API of OpenSSL throughout the whole codebase of OpenSC. This PR touched all places, which were not easily mappable to the 3.0 API.

At the end of 2023 when 1.1.1 is not supported anymore, someone will eventually have to rewrite all code so that OpenSC uses the 3.0 API and only that API.

Above, I've suggested a dedicated crypto-layer to hide this version complexity, because it is starting to get messy... but that, too, is a lot of work...

[dengert]

78c66a5 part of this PR sets m4_define([openssl_minimum_version], [1.1.1]) de176f5 "Fix macros for compatibility" with 1.1.0" and changes sc-ossl-compat.h If we really are setting the minimum to 1.1.1 we could drop most if not all of sc-ossl-compat.h accept for the liberssl.

Then leave up to the porting of OpenSC to the libressl people.

Openssl 1.1.1 used EVP_CIPHER_CTX_reset, we could change EVP_CIPHER_CTX_cleanup(x) to EVP_CIPHER_CTX_reset(x) and
EVP_CIPHER_CTX_cleanup(c) to EVP_CIPHER_CTX_reset(c) everywhere in the code base.

change in code base ERR_load_crypto_strings to OPENSSL_init_crypto(OPENSSL_INIT_LOAD_CRYPTO_STRINGS, NULL)

Drop the mapping of SSL_load_error_strings as it is only used in sc-ossl-compat.h

ERR_free_strings is a nop so could be removed

ERR_load_crypto_strings was deprecated in 1.1 and could removed.

Thus
https://github.com/OpenSC/OpenSC/blob/master/src/libopensc/sc-ossl-compat.h#L50-L72
could also be removed.

[dengert]

PR #2492 was submitted to address some of the remaining issues with sc-ossl-compat.h many of the older OpenSC only supporting OpenSSL 1.1.1 and 3.0 and above and dropping of support for any LibreSSL, most of sc-ossl-compat.h can be removed.

What remains is a few differenced between 1.1.1 and 3.0.x. that were introduced by #2438 and these lines:

/* As defined in openssl/include/openssl/evp.h */
# ifndef EVP_PK_RSA
#  define EVP_PK_RSA      0x0001
#  define EVP_PK_DSA      0x0002
#  define EVP_PK_DH       0x0004
#  define EVP_PK_EC       0x0008
#  define EVP_PKT_SIGN    0x0010
#  define EVP_PKT_ENC     0x0020
#  define EVP_PKT_EXCH    0x0040
#  define EVP_PKS_RSA     0x0100
#  define EVP_PKS_DSA     0x0200
#  define EVP_PKS_EC      0x0400
# endif

remain and only be needed by the ./tests/p11test/* files. It might be possible to remove these lines too if openssl/include/openssl/evp.h was included in the tests rather then in the sc-ossl-compat.h

As @frankmorgner pointed out "No. It makes no sense at all, because includes should almost never depend on any order."
When an include file does a #ifndef foo it is testing if a previous include file, source file or -Doption) has set foo.

Looking at the ./tests/p11test/* they appear to use in some cases libopensc/internal.h, libopensc/sc-ossl-compat.h, p11test_case_common.h but libopensc/internal.h also includes libopensc/sc-ossl-compat.h and I may have missed some.

I would like to ask @xhanulik and @Jakuje to look at these tests and #2492

#2492 has one failure in a test and it is not clear why the simplest RSA-PKCS would fail and only for test-oseid.sh

[popovec]

We already had a problem with OpenSSL 3.0 and oseid #2438 (comment) but with a different mechanism - SHA512-RSA-PKCS-PSS. I can't guarantee that oseid will calculate RSA absolutely safely - there can be a bug in every program. It is strange that this manifests itself when using openssl 3.0. I did not notice a similar error when testing with an older version of openssl. I have tried to reproduce this error so far without success. The OsEID code (for RSA) has not changed since September 2019.

The error in test https://github.com/OpenSC/OpenSC/runs/4873291258?check_suite_focus=true can be caused in oseid, opensc or even openssl pkeyutl.It can even be caused by a bitflip when executing a program.

I will add another debug output to OsEID tests, so that in case of another error it is easier to analyze where the error occurred.

[dengert]

I also tried running that segment of the code on my Ubuntu with a real card. Worked OK against OpenSSL 1.1.1m, 3.0.0 and 3.0.1

Are any of these tests run in parallel? test-oseid.sh is used for both test-oseid and test-oseid-openssl-3
The same "tmp" is used in all. Would a tmp dir based on PID of the script help?
Any problems with the oseid driver being used from two scripts at same time?

The data to be signed, is only 3 bytes, usually the data to be padded is a hash or some multiple of 8. Some problem with the OSEID code?

It looks like from the output of the hd command the data is the encrypted data not the decrypted data. So this could be a decryption problem. An extra hd to list the input would show if it is the same or not.

[popovec]

I found what's wrong. The RSA calculation in the OsEID project assumes that prime1 > prime2. Even if prime1 < prime2, it still depends on the sign/decrypt input whether the result is correct or incorrect.

openssl from debian (1.1.1k-1+deb11u1):
I generated 1000 RSA keys, in each of the keys it is true that prime1> prime2.

openssl commit ca048994ae1431965a068b17e1f17afa2345e1f5 (HEAD -> openssl-3.0, origin/openssl-3.0)
I generated 1000 RSA keys, in 496 of these keys is prime1 < prime2...

I will prepare a fix for OsEID, which will eliminate this problem.