Please consider allowing custom elliptic curves in Fedora. These are required by the PACE protocol used by AusweisApp2. Here are the most relevant comments from bug #2228325 in which the issue has been discussed in detail. +++ This bug was initially created as a clone of Bug #2228325 +++ --- Additional comment from Julian Sikorski on 2023-11-23 17:07:40 UTC --- I can now confirm that reverting https://src.fedoraproject.org/rpms/openssl/c/2b0eda88de8c3f7b84c603882aad160c019ae3a4?branch=f39 is sufficient to get AusweisApp working. For a private rebuild commenting out the patch entirely is probably quicker though. --- Additional comment from Clemens Lang on 2023-11-23 17:14:39 UTC --- Can you try the attached patch? It avoids using explicitly specified curves and instead just uses the group name, which should be OK with Fedora's copy of OpenSSL. You may also have to enable brainpool support depending on which curves the app uses: $> cat /etc/crypto-policies/policies/modules/BRAINPOOL.pmod group = BRAINPOOL*+ $> update-crypto-policies --set DEFAULT:BRAINPOOL I can't test this myself, unfortunately. I know that the patch builds, though. --- Additional comment from Julian Sikorski on 2023-11-23 18:09:06 UTC --- Thanks for the patch, much appreciated! I applied it on top of my 2.0.1 fork as 1.26.7 does not build currently. Unfortunately, now I am getting a different error: no protocol error but both PIN and CAN are recognised as incorrect: support 2023.11.23 19:02:06.574 781134 I ...ionWorker::establishPaceChannel(card/base/CardConnectionWorker.cpp:254) : Starting PACE for PACE_CAN card 2023.11.23 19:02:08.043 781134 C ...nt::transmitGAMutualAuthentication(card/base/pace/KeyAgreement.cpp:230) : Error on GA(Mutual Authentication): "Es ist kein Fehler aufgetreten. | VERIFICATION_FAILED" card 2023.11.23 19:02:08.043 781134 C ...Apdu::GAResponseApdu(card/base/apdu/GeneralAuthenticateResponse.cpp:22) : General authentication data seems invalid because of StatusCode: VERIFICATION_FAILED support 2023.11.23 19:02:08.043 781134 I ...ionWorker::establishPaceChannel(card/base/CardConnectionWorker.cpp:304) : Finished PACE for PACE_CAN with result INVALID_CAN support 2023.11.23 19:02:08.143 781134 I Reader::updateRetryCounter(card/base/Reader.cpp:186) : retrieved retry counter: 1 , was: 1 , PIN deactivated: false , PIN initial: false default 2023.11.23 19:02:08.147 781128 W (/qml/Governikus/Global/TintableAnimation.qml:24) : qrc:/qml/Governikus/Global/TintableAnimation.qml:24:2: QML AnimatedImage: Error Reading Animated Image File qrc:///images/pin_person.webp default 2023.11.23 19:02:08.152 781128 W (/qml/Governikus/Global/TintableAnimation.qml:24) : qrc:/qml/Governikus/Global/TintableAnimation.qml:24:2: QML AnimatedImage: Error Reading Animated Image File qrc:///images/can.webp default 2023.11.23 19:02:08.152 781128 W (/qml/Governikus/Global/GFlickableColumnLayout.qml:24) : qrc:/qml/Governikus/Global/GFlickableColumnLayout.qml:24:2: QML ColumnLayout: Layout polish loop detected for QQuickColumnLayout(0x55edac533250, parent=0x55edac52ff20, geometry=0,0 888x497). Aborting after two iterations. Regarding enabling brainpool: I did not have to do that with explicit curves enabled. Is this expected? I created /etc/crypto-policies/policies/modules/BRAINPOOL.pmod file and ran update-crypto-policies but it did not help. --- Additional comment from Julian Sikorski on 2023-11-24 12:08:14 UTC --- I managed to get debugger set up but I am not really much wiser. If there is a particular variable you would like me to watch/compare please feel free to let me know. As a completely wild guess: EcUtil::create call arguments are considerably different with the patch. Doesn't the code in lines 119 onwards need to account for these changes? Alternatively, would you be open to submitting your patch as a draft PR? Maybe upstream developers can provide a hint why the PIN and CAN are not recognised as correct with the patch. --- Additional comment from Clemens Lang on 2023-11-24 15:56:29 UTC --- Maybe the terminal needs the public key that's being sent to be encoded using explicit parameters, and the patch changes what's being sent to the terminal? In that case, there would probably have to be a change in EcdhKeyAgreement::performKeyExchange() to make sure that the EVP_PKEY is being converted into explicit representation before sending it to the terminal. That's just a random guess, though. It could just as well be a completely different problem and the patch that I wrote solves the original issue we were looking at. Since it seems authentication is attempted that means that the key generation was successful. I would need to look into the cyrptographic protocol spoken between the host and the terminal to be able to tell. --- Additional comment from Julian Sikorski on 2023-11-24 17:39:10 UTC --- In my case the terminal is AusweisApp running on a smartphone. I will see if I can find any usable documentation. There is the SDK documentation here: https://www.ausweisapp.bund.de/sdk/intro.html Additionally, there is the technical specification from the BSI: https://www.bsi.bund.de/SharedDocs/Downloads/DE/BSI/Publikationen/TechnischeRichtlinien/TR03124/TR-03124-1.pdf?__blob=publicationFile&v=2 --- Additional comment from Julian Sikorski on 2023-11-25 08:10:40 UTC --- Disclaimer: I am way out of my depth so below needs to be taken with a huge grain of salt. It is also quite likely that I am conflating loosely related things. While looking for the specification, I found the following: https://www.bsi.bund.de/DE/Themen/Unternehmen-und-Organisationen/Standards-und-Zertifizierung/Technische-Richtlinien/TR-nach-Thema-sortiert/tr03110/TR-03110_node.html Part 1 refers to PACE being described in https://www.icao.int/publications/pages/publication.aspx?docnum=9303 In part 12 we have the following (section 4.1.6.3): Those issuing States or organizations implementing ECDSA for signature generation or verification SHALL use [X9.62] or [ISO/IEC 15946]. The elliptic curve domain parameters used to generate the ECDSA key pair MUST be described explicitly in the parameters of the public key, i.e. parameters MUST be of type ECParameters (no named curves, no implicit parameters) and MUST include the optional co-factor. ECPoints MUST be in uncompressed format. There is a similar issue at upstream openssl: https://github.com/openssl/openssl/issues/20119 --- Additional comment from Clemens Lang on 2023-11-27 12:18:23 UTC --- Can you check whether `terminalEphemeralPublicKeyBytes` in `EcdhKeyAgreement::performKeyExchange` in `src/card/base/pace/ec/EcdhKeyAgreement.cpp` changes in length with and without my patch? The log you report says that validation of the MAC that is computed with the shared secret that was established using ECDH fails. I guess this means that the key agreement didn't actually pass, and the terminal is rejecting the transaction. I tried reproducing this (not on Fedora), but I don't have a card that would work with this flow, so I can't get this particular part of the code to run. A patch like this should just dump the encoded public key, it would be helpful to see what this prints with and without my patch: diff --git a/src/card/base/pace/ec/EcdhKeyAgreement.cpp b/src/card/base/pace/ec/EcdhKeyAgreement.cpp index 065cf388..efb816a8 100644 --- a/src/card/base/pace/ec/EcdhKeyAgreement.cpp +++ b/src/card/base/pace/ec/EcdhKeyAgreement.cpp @@ -108,6 +108,8 @@ KeyAgreement::CardResult EcdhKeyAgreement::performKeyExchange() #if OPENSSL_VERSION_NUMBER >= 0x30000000L const QByteArray terminalEphemeralPublicKeyBytes = EcUtil::getEncodedPublicKey(terminalEphemeralKey); + qCDebug(secure) << "Sending encoded public key: " << terminalEphemeralPublicKeyBytes.toHex(); + const auto& privKeyPtr = EcUtil::getPrivateKey(terminalEphemeralKey); const BIGNUM* terminalEphemeralPrivateKey = privKeyPtr.data(); #else Chances are the change to use named curves will cause the encoded public key to also use a named curve, and we'd need to do the equivalent of setting OSSL_PKEY_PARAM_EC_ENCODING to "explicit" (see https://www.openssl.org/docs/manmaster/man7/EVP_PKEY-EC.html) when exporting the public key. --- Additional comment from Julian Sikorski on 2023-11-27 13:04:43 UTC --- I checked and I cannot see any obvious difference with your patch and without. The key value changes every time, but what is dumped into the console is the same length regardless of your patch. Without it: 04618cfb924db258fff07b852de0bd4da22bdff4d2b0783aa62ad98fd3acead7600f6534ef963faf4ea45f2c0f394f6318ce1c21eb1ef32bb98a04e0adf0aeeea3 043f05ce5f4070c4f58c007c7dc27eeecdbbd15098e3f3bcd8872fc6a5eca33cd49dbfb3e5dc93ad1cbb333a084e486234c66edeb859c29d67fd9b3924246e988f 0462bd6764200fd3670fab2ee24a58062a35da498c182d70d886e741b76acfe8b436e5087c2c4b34369ff4651d7193860f5fd4b39d9e919b4cf5d627559e8bbe87 04213d1be1a33b637668a22baaae819e4564354a38c956c119eeb4d2c85e5cd40732f13ed4c9677df2375db14828c7bcda7a45cbde35ad449d4d740f19a94e46a5 With the patch: 045bd4f19b1c2733cabf26c9ddec7f43d5e8bd6bf9f8c95c140d683975b75975111e97dacd411b009018367f01032843d8db839b286c09a2405fe72e5071c8c5f3 047c33f9018a8b494888c153d7c15662f47828e69902f428b207283c503a5146b77cba9c8704596e3ce1a6a69540b3948cac47edd3cdcda94e8df64eb01480b635 I had to go for qCCritical(card), I was not able to get the output to show otherwise - probably due to Q_LOGGING_CATEGORY(secure, "secure", QtFatalMsg) in AusweisApp-2.0.1/src/global/LogCategories.cpp. --- Additional comment from Julian Sikorski on 2023-11-27 13:24:18 UTC --- (In reply to Julian Sikorski from comment #31) > I had to go for qCCritical(card), I was not able to get the output to show > otherwise - probably due to Q_LOGGING_CATEGORY(secure, "secure", QtFatalMsg) > in AusweisApp-2.0.1/src/global/LogCategories.cpp. Nevermind, this was my error. qcDebug(secure) works perfectly fine. It actually dumps quite a lot more into the console, but the majority is confidential. --- Additional comment from Clemens Lang on 2023-11-27 16:01:43 UTC --- Yeah, those look like public keys encoded in the uncompressed format (note that the output always starts with 04), so that doesn't seem to be the issue here. That seems to suggest that my patch works as expected, but there's a different issue at a later point that causes this to fail. I tried running the test suite, but that fails to compile against Qt6 in many places. Maybe this is the point to approach upstream and ask? --- Additional comment from Julian Sikorski on 2023-11-27 16:09:37 UTC --- It might be worth a shot, but we should keep our expectations low. Using the support form got me nowhere. A draft PR might go further. There is also support e-mail, but there is a chance it goes to the same people the support form does. --- Additional comment from Julian Sikorski on 2023-12-13 12:25:35 UTC --- @cllang, would you be willing to open a draft PR with upstream? I can try sending a liaising email with you in CC as well. Please let me know your preference. --- Additional comment from Clemens Lang on 2023-12-13 16:20:31 UTC --- I'm sorry, this is currently not a priority for me due to other more urgent topics. Feel free to remind me next year. --- Additional comment from Julian Sikorski on 2023-12-13 17:38:57 UTC --- Fair enough, you have already done more than I ever could have. --- Additional comment from Julian Sikorski on 2024-01-05 18:38:20 UTC --- The issue with the workaround is that it needs to be set up on a different device, meaning we cannot change the default setting in the Fedora package to make sure it works out of the box. It is of course better than not having a workaround at all. --- Additional comment from Julian Sikorski on 2024-01-06 07:45:10 UTC --- I managed to discuss the issue with one of the upstream developers and got the following feedback: your patch cannot work because you drop the custom generator of the curve. The used curve in that function has a changed generator. [1] It is the implementation of PACE [2] in chapter 3.2.1 if you want to know why it is necessary. Maybe you could try the "deprecated" function for openssl 1.1.1 in [3] as it is a much simpler openssl API and uses simple EC_KEY_set_group. But I think it will be a problem for you, too. [1] https://github.com/Governikus/AusweisApp/blob/5f812fba540d6c4d21739fcda7de8630cc2400f6/src/card/base/pace/ec/EcdhGenericMapping.cpp#L131 [2] https://www.bsi.bund.de/SharedDocs/Downloads/EN/BSI/Publications/TechGuidelines/TR03110/BSI_TR-03110_Part-2-V2_2.pdf?__blob=publicationFile&v=1 [3] https://github.com/Governikus/AusweisApp/blob/5f812fba540d6c4d21739fcda7de8630cc2400f6/src/card/base/pace/ec/EcUtil.cpp#L222 I have then attempted the approach of using the deprecated functions and, with some further help, managed to get it working [4]. I do not understand the code enough to tell whether it is working because the explicit curves are not disabled in the deprecated functions, or whether the deprecated functions do not need explicit curves at all. In any case, it would be great if the approach could be replicated using the current functions so that AusweisApp keeps working for Fedora users once OpenSSL (or Governikus) decide to drop the deprecated functions entirely. @cllang, I hope this sheds some more light on the situation so that we can try for a better patch once you have the capacity and would be willing to work on this again. Thanks for all the help so far. [4] https://src.fedoraproject.org/rpms/AusweisApp2/c/77260f49503a10da97f530bc6102dba1901a26a0?branch=rawhide --- Additional comment from Clemens Lang on 2024-01-11 12:24:34 UTC --- (In reply to Julian Sikorski from comment #48) > your patch cannot work because you drop the custom generator of the curve. > The used curve in that function has a changed generator. [1] It is the > implementation of PACE [2] in chapter 3.2.1 if you want to know why it is > necessary. OK, that explains why my patch doesn't work. Unfortunately section 3.2.1 in [2] does not actually explain why they believe they need a custom generator. In any case, a custom generator essentially means this is a custom curve, and we don't want to support custom curves in OpenSSL in Fedora due to the risk associated with them (see for example CVE-2022-0778 in https://www.openssl.org/news/secadv/20220315.txt). If using custom curves with EC_KEY_set_group() works, that's probably something we should consider a bug. I'll talk to some people to see whether we want to disable that or keep it working. --- Additional comment from André Klitzing on 2024-01-17 14:07:04 UTC --- > Unfortunately section 3.2.1 in [2] does not actually explain why they > believe they need a custom generator. In any case, a custom generator > essentially means this is a custom curve, and we don't want to support > custom curves in OpenSSL in Fedora due to the risk associated with them (see > for example CVE-2022-0778 in > https://www.openssl.org/news/secadv/20220315.txt). Well, we used that openssl API to implement that case. If there is a better way I will review patches for this and merge them. I don't see an easy way to use another openssl API that purpose. Maybe there should be a mechanism for those exceptions. Only fedora patches openssl like this. FYI: It looks https://github.com/frankmorgner/openpace uses EC_KEY_set_group for that, too. Best regards André --- Additional comment from Clemens Lang on 2024-01-17 15:50:02 UTC --- (In reply to André Klitzing from comment #52) > Well, we used that openssl API to implement that case. If there is a better > way I will review patches for this and merge them. I don't see an easy way > to use another openssl API that purpose. > Maybe there should be a mechanism for those exceptions. Only fedora patches > openssl like this. I know. Maybe we should also reconsider applying this patch on Fedora and only ship it in ELN/CentOS Stream/RHEL. Ideally, nobody would use custom elliptic curves. There are typically good ways to solve problems with the existing well-known curves. In this particular case, the authors of the specification decided that they want to use a custom elliptic curve, so we essentially have three options: - allow changing the generator, - add an API to selectively allow changing the generator for use-cases that need it, or - not supporting this use case. I think the first option is the simplest here. I also really don't understand BSI's fondness of edge cases of elliptic curve cryptography. Why do they need to use a brainpool curve with custom generators instead of just relying on secp256r1, or if they are afraid the US government might have backdoored that, x25519? They also didn't explain their rationale, either, so it's anybody's guess why they did it. The NIST and x25519 curves have seen a lot more testing, and we are a lot more confident that they don't contain timing side channels, and every use of a non-default parameter for these operations puts us further into less tested territory. In any case, there's no todo for you here. From your point of view as the maintainer of AusweisApp2, you did everything correctly. --- Additional comment from André Klitzing on 2024-01-17 16:24:17 UTC --- > I also really don't understand BSI's fondness of edge cases of elliptic > curve cryptography. Why do they need to use a brainpool curve with custom > generators instead of just relying on secp256r1, or if they are afraid the > US government might have backdoored that, x25519? They also didn't explain > their rationale, either, so it's anybody's guess why they did it. The NIST > and x25519 curves have seen a lot more testing, and we are a lot more > confident that they don't contain timing side channels, and every use of a > non-default parameter for these operations puts us further into less tested > territory. Just for your information. It starts with a named curved without any custom generator. Then it will derive a new key from previous curve to do a handshake with the eID card. That would be the same with secp256r1 or any other curve. I could ask them if you really need to know why they need brainpool. But do not expect a quick answer. ;-) Ok, let me know if I can help you. --- Additional comment from Julian Sikorski on 2024-01-17 17:49:59 UTC --- Just to be clear, I believe the question is not why do BSI need a brainpool curve, but rather why do BSI need a brainpool curve with a custom generator. Brainpool itself used to be a problem in Fedora as well, but this has been resolved in openssl-3.0.8-2 and above. Putting my tinfoil hat on, maybe they are not afraid that US government put a backdoor in but they put a backdoor in themselves instead. Why should only US government be the only one with the backdoors? /jk.
We discussed it in the team and agreed not to implement this feature. Sorry.