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

Reduce ciphersuites to RAW mechanism (support for PSS/PKCS15 padding in software by macOS) #40

Merged
merged 2 commits into from
Jun 16, 2023

Conversation

frankmorgner
Copy link
Owner

No description provided.

@frankmorgner
Copy link
Owner Author

Untested at the monment

@kirichkov
Copy link

I'm trying to compile this branch so I can test this, but my xcode (on Intel) is failing with:

ld: symbol(s) not found for architecture arm64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

I did change the signing certificates to a locally generated one and changed the signing to "Sign to run locally." Any guidance on how to fix this will be appreciated!

@frankmorgner

This comment has been minimized.

@kirichkov

This comment has been minimized.

@frankmorgner
Copy link
Owner Author

Sorry, there was an error in the PR. Please use the updated binaries https://github.com/OpenSC/Nightly/tree/2021-04-16_70c16f12 , see README.md for reconstructing the dmg

@kirichkov
Copy link

kirichkov commented Apr 16, 2021

I am getting the same behavior from Safari as before (logs from both intel and arm), however, Now I see also the smartcard pairing notification, which I don't think I saw until now. Not sure how the latter is relevant.

P.S. Chrome is also displaying the certificate selection, but not asking me for a PIN - same as Safari. Mail decryption works.

@ElMostafaIdrassi
Copy link
Contributor

Here are my 2 cents on what's going on here.
CryptoTokenKit has a funny way of doing things when performing crypto operations (e.g. signing) :

First, CryptoTokenKit calls TKTokenSession::signData(), which, in the case of OpenSCToken, internally calls sc_pkcs15_compute_signature :

  • If this call succeeds, then CryptoTokenKit does not show any PIN prompt as the card has successfully signed the data, and it uses the returned data.
  • If this call fails with TKErrorCodeAuthenticationNeeded, then CryptoTokenKit calls beginAuthForOperation(), which is responsible for showing the PIN prompt.
  • If this calls fails with any other error code, then CryptoTokenKit halts the signature because of the failure.

Now, taking this into consideration in addition to the debug file you've attached , you're not getting the PIN prompt when signing because sc_pkcs15_compute_signature fails with -1303 (Buffer too small), due to this check https://github.com/OpenSC/OpenSC/blob/master/src/libopensc/pkcs15-sec.c#L615-L616.

Can you add more logs into the signData() method in order to know which algorithm is being used, the size of the data to be signed and the size of the constructed output ? Maybe Safari is trying to sign an arbitrary length data using something like kSecKeyAlgorithmRSASignatureMessagePSSSHA256 ?

@frankmorgner
Copy link
Owner Author

@ElMostafaIdrassi , thanks for your input; this sounds reasonable. I've added some debug messages and rebuilt the binaries. @kirichkov , you can find the dmg here:

https://github.com/OpenSC/Nightly/tree/2021-04-16_70c16f12

@kirichkov
Copy link

kirichkov commented Apr 27, 2021

Unfortunately it's still not working. Here's debug gist

Now also the signing in Mail.app is broken. I get the following error message:

An error occurred while trying to sign this message with a certificate from “EMAIL. Verify that your certificate for this address is correct, and that its private key is in your keychain.

P.S. I've updated the gist to include (I think) Mail.app's log as well.

@frankmorgner
Copy link
Owner Author

@kirichkov I just added some more debugging, could you run your testing again? Yes, it will result in an error, but I hope that the debug log will help to confirm @ElMostafaIdrassi 's observation.

The new binaries are, again, available here:

https://github.com/OpenSC/Nightly/tree/2021-04-16_70c16f12

@kirichkov
Copy link

Here's the gist

@ElMostafaIdrassi
Copy link
Contributor

ElMostafaIdrassi commented Apr 27, 2021

The input data is 11664 bytes, which exceeds 1024 bytes by a lot, and which causes the aforementioned check failure. AFAIK, since IDPrime supports signing external hashes, a simple solution will be to calculate the digest in soft in OpenSCToken before calling sc_pkcs15_compute_signature(). However, I don't know whether you need to also pad in soft or the card does it for you. Also, this modification might not work with other cards which only support doing everything in the card or hashing the data internally.

@frankmorgner
Copy link
Owner Author

well, we could restrict the input to hashed data only, for the moment (by removing kSecKeyAlgorithm*Message* from the list of supported algorithms).

But I wonder why this problem was hidden for so long. @Jakuje , we do have a test case which signs more than 1 KB of input that should have triggered https://github.com/OpenSC/OpenSC/blob/master/src/libopensc/pkcs15-sec.c#L615-L616, don't we?

@Jakuje
Copy link
Contributor

Jakuje commented Apr 28, 2021

This problem is hidden in most of the common tokens, because they usually support RAW-RSA and application will do the PKCS/PSS padding in software (and we did not have RSA-PSS support until few years back either). But as far as I know, the IDPrime is one of the first drivers which does not support RAW-RSA (for obvious security reasons). For testing, I think I never tried larger inputs for RSA mechanisms, but it is indeed a good idea to test.

@frankmorgner
Copy link
Owner Author

I've disabled the problematic algorithms for now. we need to solve this in core opensc.

the updated installer can, again, be found here:
https://github.com/OpenSC/Nightly/tree/2021-04-16_70c16f12

@kirichkov
Copy link

kirichkov commented Apr 28, 2021

I'm not sure whether I should provide a log, given that the issue is in core OpenSC, but I did anyways: The log

Also Mail signing is still broken in the last version. I believe this is to be expected due to the previous change.

@frankmorgner
Copy link
Owner Author

I've implemented a quick fix in OpenSC core. would you mind testing it again?

https://github.com/OpenSC/Nightly/tree/2021-04-28_85f4bde1

@kirichkov
Copy link

There you go

@frankmorgner
Copy link
Owner Author

It seems that this build didn't get the update about opensctoken. I'll trigger it to rebuild. sorry for the inconvenience.

@frankmorgner
Copy link
Owner Author

The new build is available

here https://github.com/OpenSC/Nightly/tree/2021-04-29_212bbfc9

it also contains a fix for the same problem within PKCS#11

@kirichkov
Copy link

Unfortunately situation is the same - no TLS, no mail signing - here's the gist

It seems that this build didn't get the update about opensctoken. I'll trigger it to rebuild. sorry for the inconvenience.

No worries! I'll test as much as it's needed :)

@frankmorgner
Copy link
Owner Author

Finally, there's some new error, which actually comes from the card. When sending 00 22 41 B6 ..., the card responds with
6D 00. @Jakuje could you have a look, please?

@kirichkov
Copy link

kirichkov commented Apr 29, 2021

Just to clarify for @Jakuje that I tried the TLS identification on the "working" site - i.e. the one with AES 128 GCM encryption, not the AES 256 GCM one we're debugging in OpenSC/OpenSC#2309

P.S. Just to be sure I tested with the Firefox PKCS11 module - TLS authentication works in Firefox, so it's not something in OpenSC main.

@Jakuje
Copy link
Contributor

Jakuje commented Apr 29, 2021

Finally, there's some new error, which actually comes from the card. When sending 00 22 41 B6 ..., the card responds with
6D 00. @Jakuje could you have a look, please?

The 0x6D00 is instruction code invalid, which is suspicious ... first couple of attempts are with alg. 0x45 which is RSA-PSS-SHA256 and then it is retried with 0x02, which should be normal RSA-PKCS (but it should be RSA-PKCS with SHA2 from the log saying kSecKeyAlgorithmRSASignatureMessagePKCS1v15SHA256, but that is a bug in OpenSC/OpenSC#2309). Could this be some key that has even more restricted mechanisms or that it can not be used for signatures?

@kirichkov
Copy link

Finally, there's some new error, which actually comes from the card. When sending 00 22 41 B6 ..., the card responds with
6D 00. @Jakuje could you have a look, please?

The 0x6D00 is instruction code invalid, which is suspicious ... first couple of attempts are with alg. 0x45 which is RSA-PSS-SHA256 and then it is retried with 0x02, which should be normal RSA-PKCS (but it should be RSA-PKCS with SHA2 from the log saying kSecKeyAlgorithmRSASignatureMessagePKCS1v15SHA256, but that is a bug in OpenSC/OpenSC#2309). Could this be some key that has even more restricted mechanisms or that it can not be used for signatures?

If you mean public/private key on the card - no. There's two pairs and two certificates - the first certificate is no longer valid, but the second is. I checked and both private keys have "Decryption" and "Signing" capabilities.

MessageDigest means that input is of variable size and needs to be hashed with OpenSC
SignatureDigest means input with size of hash function output and doesn't need to be hashed by OpenSC
Obviously, the translation between OpenSC and CTK was not correct. Since the applications are choosing mechanisms with specific padding rather than raw operations, this was leading to an error. Removing the bad algorithms seems to be enough to use macOS' builtin mechanism for emulating the padding schemes if needed.
@frankmorgner
Copy link
Owner Author

I changed this PR to reduce the suite of algorithms down to just the raw mechanisms. It turns out, that macOS does the rest in software in the background. Tested on Big Sur in Safari and Chrome (i.e. including PSS).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants