OpenSC
OpenSC copied to clipboard
sc-ossl-compat.h cleanup for OpenSSL and LibreSSL
Remove unused code and misplaced defines from sc-ossl-compat.h to support OpenSSL 1.1.1 and 3.0.0+ and LibreSSL 3.4.2+
The "inline" routines are no longer needed and have been removed, as new functions replaced them.
Many of the changes to previous version of sc-ossl-compat.h
added many #ifndef ... #define ... #endif
but made the assumption that the the header files where already included. But this depended on the order of include files in each source file. This had lead to confusion. This dependency has been removed.
The proper way to add #ifndef ... #define... #endif
to sc-ossl-compat.h
would be to include the needed header files in the source files before sc-ossl-compat.h
is included, or to add the include to sc-ossl-compat.h
.
Several other source files were modified to include additional header files or use newer names for functions or macros which are defined in OpenSSL and LibreSSL.
This replaces #2492 which did not include LibreSSL.
Date: Tue Feb 1 20:58:08 2022 -0600 On branch sc-ossl-compat-cleanup Changes to be committed: modified: sc-ossl-compat.h modified: ../pkcs11/framework-pkcs15.c modified: ../pkcs11/openssl.c modified: ../pkcs15init/pkcs15-westcos.c modified: ../tools/piv-tool.c modified: ../tools/pkcs15-init.c
A spreed sheet of comments and actions: sc-ossl-compat.pdf You will note that many of the routines used in all three are deprecated in 3.0.0, so these also need to be addressed in future changes.
Tested using PIV card with OpenSSL-1.1.1m, OpenSSL-3.0.1 and LibreSSL-3.4.2
Checklist
- [X] PKCS#11 module is tested
Both OSX and AppVeyor builds still seem to fail because of some missing API.
Anyone want to look at Appveyer configuration?
These look like conflicting configuration for cygwin: --disable-openssl
, /DOPENSSL_SECURE_MALLOC_SIZE=65536
and /DENABLE_OPENSSL
Half of the Appveyer builds work, half fail.
for example: Environment: APPVEYOR_BUILD_WORKER_IMAGE=Visual Studio 2015, VCVARSALL=%VS140COMNTOOLS%/../../VC/vcvarsall.bat, DO_PUSH_ARTIFACT=yes; Configuration: Release; Platform: x86
https://ci.appveyor.com/project/LudovicRousseau/opensc/builds/42429370/job/6vwlqiljmgfjy63h
bash -c "exec 0</dev/null && ./configure --with-cygwin-native --disable-openssl --disable-readline --disable-zlib || cat config.log"
cl /O1 /GS /W3 /WX /D_CRT_SECURE_NO_DEPRECATE /D_CRT_NONSTDC_NO_WARNINGS /MT /nologo /DHAVE_CONFIG_H /I....\win32 /I....\src /IC:\openpace-Win32\src /IC:\OpenSSL-v111-Win32\include /DOPENSSL_SECURE_MALLOC_SIZE=65536 /IC:\zlib-Win32 "/IC:\Program Files (x86)\Windows Kits\10\Cryptographic Provider Development Kit\Include" "/IC:\Program Files (x86)\Microsoft CNG Development Kit\Include" "/IC:\Program Files (x86)\WiX Toolset v3.11\SDK\VS2015\inc" /DWINVER=0x0601 /D_WIN32_WINNT=0x0601 /DWIN32_LEAN_AND_MEAN /DENABLE_OPENPACE /DENABLE_OPENSSL /DENABLE_ZLIB /DENABLE_MINIDRIVER /DENABLE_SM /DOPENSC_FEATURES=""pcsc openssl zlib"" /Zi /c sc.c ctx.c log.c errors.c asn1.c base64.c sec.c card.c iso7816.c dir.c ef-atr.c ef-gdo.c padding.c apdu.c simpletlv.c gp.c pkcs15.c pkcs15-cert.c pkcs15-data.c pkcs15-pin.c pkcs15-prkey.c pkcs15-pubkey.c pkcs15-skey.c pkcs15-sec.c pkcs15-algo.c pkcs15-cache.c pkcs15-syn.c pkcs15-emulator-filter.c muscle.c muscle-filesystem.c ctbcs.c reader-ctapi.c reader-pcsc.c reader-openct.c
ctx.c(848): error C2220: warning treated as error - no 'object' file generated ctx.c(848): warning C4013: 'CRYPTO_secure_malloc_initialized' undefined; assuming extern returning int ctx.c(849): warning C4013: 'CRYPTO_secure_malloc_init' undefined; assuming extern returning int log.c errors.c
and ctx.c:847: #if defined(ENABLE_OPENSSL) && defined(OPENSSL_SECURE_MALLOC_SIZE)
Looks like OSX builds has same problem, with ctx.c https://github.com/OpenSC/OpenSC/runs/5039586360?check_suite_focus=true
Before this PR, it looks like #if defined(ENABLE_OPENSSL) && defined(OPENSSL_SECURE_MALLOC_SIZE) worked, because of bad configuration, but then sc-ossl-compat.h defined the macro/function so no error.
support for secure memory was added with https://github.com/openssl/openssl/commit/74924dcb3802640d7e2ae2e80ca6515d0a53de7a , i.e. it was added in 1.1.0 and not present in 1.0.2
support for secure memory was added with openssl/openssl@74924dc , i.e. it was added in 1.1.0 and not present in 1.0.2
So what is your point?
As a general note this PR was not trying to address deprecated functions in 3.0.0, but rather it was trying to get ride of parts of sc-ossl-compat.h that are for versions < 1.1.1 and work with LibreSSL 3.4.2 and above as well as OpenSSL 1.1.1 and OpenSSL 3.0.0. More changes will be needed as we address building without deprecated 3.0.0 functions.
Note at all the checks are clean, accept for one that is not related to the PR: https://ci.appveyor.com/project/LudovicRousseau/opensc/builds/42440190/job/mbuaf50nxxc79q2c
While writing the last commit, https://github.com/OpenSC/OpenSC/pull/2506/commits/b0b7cc30e8a59d267a6dd6be945fc6096b096108 showed that LibreSSL may have started as a fork of 1.0.2 but LibreSSL 3.4.2 these days is more like OpenSSL 1.1.1.
man -M /opt/ossl-libressl3.4.2/share/man ERR_load_CRYPTO_strings
man -M /opt/ossl-libressl3.4.2/share/man ERR_load_crypto_strings
man -M /opt/ossl-libressl3.4.2/share/man OPENSSL_init_crypto
"These functions are deprecated. It is never useful for an application program to call either of them
explicitly".
man -M /opt/ossl-1.1.1m/share/man OPENSSL_init_crypto
man -M /opt/ossl-3.0.1/share/man OPENSSL_init_crypto
"As of version 1.1.0 OpenSSL will automatically allocate all resources that it needs so no explicit
initialisation is required."
LibreSSL and OpenSSL all says OPENSSL_INIT_LOAD_CRYPTO_STRINGS
and OPENSSL_init_crypto
are deprecated, and are not needed. OpenSSL leaves there use open for special situations
but OpenSC does not appears to have any special situations.
OpenSC calls ERR_load_crypto_strings in 9 source files and calls OPENSSL_init_crypto in 4 tools.
So https://github.com/OpenSC/OpenSC/pull/2506/commits/b0b7cc30e8a59d267a6dd6be945fc6096b096108 removes all usages of ERR_load_CRYPTO_strings and friends, including OPENSSL_config is also not needed and deprecated in OpenSSL 1.1.0
Both LibreSSL and OpenSSL have been providing macros and dummy functions for some time. But if OPENSSL_NO_DEPRECATED_1_1_0 or OPENSSL_NO_DEPRECATED_3_0 are set these macros or dummy functions are not defined.
We may be close to building OpenSC with OpenSSL-3.0.x with no-deprecated
. Building this PR and using make -k only card-iasecc.c
has many errors. It is using SHA_CTX, SHA_LONG, SHA256_CTX and members of the structures in iasecc_qsign_data_sha1
and iasecc_qsign_data_sha256
and does not compile and thus libopensc is not built. But all other files compile.
I suspect card-iasecc.c
does the last part of the hash on the card, but it not clear how to do that in OpenSSL 3.0.
Anyone want to take a look at card-iasecc.c
?
Thank you for polishing this PR and taking care to remove the old cruft and moving the rest of the code. Added some minor comments.
We may be close to building OpenSC with OpenSSL-3.0.x with
no-deprecated
. Building this PR and using make -k onlycard-iasecc.c
has many errors. It is using SHA_CTX, SHA_LONG, SHA256_CTX and members of the structures iniasecc_qsign_data_sha1
andiasecc_qsign_data_sha256
and does not compile and thus libopensc is not built. But all other files compile.
If that is so, can you remove the --disable-strict
from the openssl 3.0 pipeline in CI (as well as the added CFLAGS):
https://github.com/OpenSC/OpenSC/blob/b71ca16b28c4e1a8d35a326f65bed29a8dec2183/.github/build.sh#L52
I suspect
card-iasecc.c
does the last part of the hash on the card, but it not clear how to do that in OpenSSL 3.0.Anyone want to take a look at
card-iasecc.c
?
Yes. We discussed this in https://github.com/OpenSC/OpenSC/pull/2337#issuecomment-840697055 last year. I think it is not possible to do that right now with 3.0, but I did not ask OpenSSL developers yet. I did not look into that recently, but if you (or @vjardin or @viktorTarasov) have the overview of the use case, it would be great to summarize it in an issue. We might hear there is already a way to do that, they might propose some new API or come up with some other solution.
I have been looking at card-iasec.c too. It might be possible to update it. Will look closer in next few days.
On Sun, Feb 6, 2022, 9:35 AM Jakub Jelen @.***> wrote:
Thank you for polishing this PR and taking care to remove the old cruft and moving the rest of the code. Added some minor comments.
We may be close to building OpenSC with OpenSSL-3.0.x with no-deprecated. Building this PR and using make -k only card-iasecc.c has many errors. It is using SHA_CTX, SHA_LONG, SHA256_CTX and members of the structures in iasecc_qsign_data_sha1 and iasecc_qsign_data_sha256 and does not compile and thus libopensc is not built. But all other files compile.
If that is so, can you remove the --disable-strict from the openssl 3.0 pipeline in CI (as well as the added CFLAGS):
https://github.com/OpenSC/OpenSC/blob/b71ca16b28c4e1a8d35a326f65bed29a8dec2183/.github/build.sh#L52
I suspect card-iasecc.c does the last part of the hash on the card, but it not clear how to do that in OpenSSL 3.0.
Anyone want to take a look at card-iasecc.c?
Yes. We discussed this in #2337 (comment) https://github.com/OpenSC/OpenSC/pull/2337#issuecomment-840697055 last year. I think it is not possible to do that right now with 3.0, but I did not ask OpenSSL developers yet. I did not look into that recently, but if you (or @vjardin https://github.com/vjardin or @viktorTarasov https://github.com/viktorTarasov) have the overview of the use case, it would be great to summarize it in an issue. We might hear there is already a way to do that, they might propose some new API or come up with some other solution.
— Reply to this email directly, view it on GitHub https://github.com/OpenSC/OpenSC/pull/2506#issuecomment-1030856489, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGTIMPUESFGTKHTI2IIY5TUZ2ILFANCNFSM5NMQCYNQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you authored the thread.Message ID: @.***>
The last set of commits includes https://github.com/OpenSC/OpenSC/pull/2506/commits/1f3df786738431fa00fab23a6a830f32eb3e24e0
I Have not tested it, but it compiles on LibreSSL, OpenSSL 1.1.1m and 3.0.1 and 3.0.1-no-deprecated
@Jakuje you commented on talking to OpenSSL developers. As noted in the commit message, there are arguments for a discussion with OpenSSL developers to not deprecated the SHA_CTX and SHA256_CTX structures that we use. (There may be other hash structures that other may also want to use.)
- 3.0.0 provides
EVP_MD_CTX_get0_md_data
to get pointer to these structures. - What good is the pointer if you don't have the definition of the structure?
- ISO/IEC 7816-8 has a valid reason to access the structure in "5.5 HASH operation" "The HASH operation initiates the computation of a hash-code by performing: either the complete computation inside the card or a partial computation inside the card (e.g. last round of hashing)."
- Access is required as the SHA "h" integers from the the next to last round is send to the card to start the last round.
- The structures define the basic SHA data and are not expected to change.
The change may be as simple as moving two #ifdefs: OpenSSL-3-sha.diff.txt
Thank you for looking into that. The changes to iasec look reasonable, but still it looks pretty fragile depending on internal structure. I assume the more correct way would be using some OSSL_PARAMS to get the the content of the internal strucutre, but I did not see any OSSL_PARAM that would provide this info.
I added one comment (hope it arrived correctly) to the use of #ifs, which would be better in the sc-ossl-compat.h.
Could this be handled with a compat define in sc-ossl-compat.h to avoid these ifs? Yes. But this is the only place in the code that needs to get at the SHA_CTX structures.
Can we get this code tested before trying to move the code to sc-ossl-compat.h? I do not have a iasecc card. @viktorTarasov @vjardin @xhanulik can you test this code?
In response to:
Thank you for looking into that. The changes to iasec look reasonable, but still it looks pretty fragile depending on internal structure. I assume the more correct way would be using some OSSL_PARAMS to get the the content of the internal structure, but I did not see any OSSL_PARAM that would provide this info.
I submitted an issue to OpenSSL on the deprecation of the SHA_CTX in 3.0.0 See: https://github.com/openssl/openssl/issues/17688
@viktorTarasov do you have any comments on this PR?
Based on the response in https://github.com/openssl/openssl/issues/17688 it is not likely OpenSSL will provide access to internal hash algorithm data.
So we have some choices:
-
Drop support for
card-iasecc.c
functionsiasecc_qsign_data_sha1
andiasecc_qsign_data_sha256
if not supported in the OpenSSL/LibreSSL. i.e. OpenSSL 3.0 and above when no-deprecated is supported. -
Include an internal SHA256 function used only for cards doing this last round of hash on card.
It is not clear if these cards are still in use and these days SHA1 should not be used for signatures so if we want to continue to uses these cards, we would only need to address SHA256.
If we do (2.) there are a number of open source routines or examples. These have license issues. Anyone want to pick one?
So in any case, I will rework the PR to do (1.). If we decide to do (2.) or OpenSSL provides a way to use their internal hash algorithm data. that can be added.
Any one have IAS-ECC smart card? Developer or user? This PR needs testing with OpenSSL 3.0, as well as OpenSSL-1.1.1 and Libressl.
Based on OpenSSL's responses to https://github.com/openssl/openssl/issues/17688 it is likely the card when used with OpenSSL 3.0 will not do signatures.
The last commit https://github.com/OpenSC/OpenSC/pull/2506/commits/541bfe7ec5c38949870553cd05c0de58d3b5b98f implements the Option (1.) in https://github.com/OpenSC/OpenSC/pull/2506#issuecomment-1039381365 which may not be acceptable to many.
Option (2.) is still possible, by using an existing implementation of SHA256, or OpenSC could write one.
So far no developer or user with interest in the card-isaecc.c
driver has responded to my 3 comments in the last ten days for help in testing.
The card drivers implements i "IAS ECC Identification Authentication Signature European Citizen Card", "Technical Specifications Revision: 1.0.1", "21/03/2008" which requires partial hash on card. Also see: https://github.com/OpenSC/OpenSC/wiki/IAS-ECC
Does the lack of interest mean there are no active cards that require this driver and it "qualified signatures"? I can find what appears to be newer specifications for sale. Being in the U.S.A. I have no personal stack in this driver.
But for now I am intending to clean up this PR with the "qualified signatures" returning not supported if linked with OpenSSL 3.0.
P.S. I have written a SHA256 implementation from FISP-180-4 that could be used for this card is really needed. (Option (2. as listed above.) With some testing it gives the same results as the OpenSSL SHA256 version.
Force-pushed this PR to remove a commit and its revert, squashed another and changed name of another.
I believe all this review issues have been addressed, accept for the need for SHA_CTX and SHA256_CTX. These are only needed in the card-iasecc.c
driver to support partial hash in software and last round of hash on the card. So these these changes where not moved to sc-ossl-compat.h
.
card-iasecc.c
when complied with OpenSSL > 3.0 will not sign data due to OpenSSL changes to not expose SHA_CTX and SHA256_CTX. This driver may not be used with any current IAS ECC cards. If it is any additional changes can be in an other PR.
Thanks. Feedback and testing are needed.
I've tested with IAS/ECC and SM (despite https://github.com/OpenSC/OpenSC/pull/2522 it looks good)
Can you resolve the conflicts and rebase on current master? Is there something to address in this issue or should we merge it for the next 0.23.0 release?
Can you resolve the conflicts and rebase on current master?
Rebased against master of today. I also added https://github.com/OpenSC/OpenSC/pull/2506/commits/d4c4a98c89c88171372fe24aae39e0234259d9d9 to address use of engine for GOST #2586
OpenSC builds using OpenSSL-1.1.1q, OpenSSL 3.0.5 and LibreSSL-3.4.1.
OpenSC builds on XUbuntu-22.04 using Ubuntu provided OpenSSL 3.0.2 15 Mar 2022.
Is there something to address in this issue or should we merge it for the next 0.23.0 release?
This is a lot here, as it gets rid of support for OpenSSL < 1.1.1. So when do you want to drop that too?
nevermind, I just saw that check which disabled gost on openssl >= 3.0
Sorry, one more thing, now when the OpenSSL 3.0 build works without the warnings, we should remove the workaround in
https://github.com/OpenSC/OpenSC/blob/58d8099cb65d02589b4c54fbb5c820b0f4183aa5/.github/build.sh#L52
and build again with -Werror
as on any other platform. Then I think we are good to merge this for the 0.23.0 release.
This is a lot here, as it gets rid of support for OpenSSL < 1.1.1. So when do you want to drop that too?
I think dropping support for OpenSSL <1.1.1 can wait and will be problematic because of libressl which does not support much of the newer APIs as far as I know. We can create a new issue for this to keep track of that.
If there will be no more comments, I will merge it in few days.
While developing the sc-compact-cleanup I was surprised that Libressl has been keeping up with newer versions of OpenSSL. I was using LibreSSL 3.4.1 in February from October 14th, 2021. The latest stable release is 3.5.3 from May 22 See https://www.libressl.org/releases.html
The only warning where a number of these: /usr/bin/ld: ../../src/libopensc/.libs/libopensc.so: warning: EVP_EncryptFinal is often misused, please use EVP_EncryptFinal_ex and EVP_CIPHER_CTX_cleanup