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

Always use ECDSA with off card hashing #28

Closed
wants to merge 1 commit into from

Conversation

swissbit-csteuer
Copy link
Contributor

@swissbit-csteuer swissbit-csteuer commented Nov 17, 2022

With this PR raw ECDSA without hashing is performed instead of ECDSA with SHA1 hashing.
The hashing has now to be done on the host side (See OpenSC/OpenSC#2642).

The benefit of this is that all ECDSA mechanisms of the pkcs#11 interface can be used with the ISOApplet.

@philipWendland
Copy link
Owner

Hi,
as I wrote in the OpenSC PR, we should make sure that Java Cards with v2.2.2 still work.
IsoApplet will get a legacy branch for those cards - main-javacard-v2.2.2.

When introducing a Version 1 of the IsoApplet (and a new API/protocol), I'd like to add the features that a started to work on some time ago but got distracted from.
I hence cherry-picked your commit to the isoapplet-v1 branch. This will be the new "main" branch when we are done.

We still need a solution to differentiate between the IsoApplet versions at the OpenSC side, so that both the v2.2.2 and 3.04 cards work. I am going to look at this when I find time (might be as late as next weekend, I am maintaining this project in my spare time.). If you have any insights or code to share, feel free to let me know.

@swissbit-csteuer
Copy link
Contributor Author

Hi and thanks for your response.

I agree: we should make sure that old cards still work.
I'm going to implement the version check/switch in OpenSC and add it to the PR, so that OpenSC continues to work with pre 1.x versions of the IsoApplet.

Which features do you plan to add to the isoapplet-v1 branch? Maybe I can help with some of those.

@philipWendland
Copy link
Owner

philipWendland commented Nov 23, 2022

I agree: we should make sure that old cards still work.
I'm going to implement the version check/switch in OpenSC and add it to the PR, so that OpenSC continues to work with pre 1.x versions of the IsoApplet.

Thank you!

Which features do you plan to add to the isoapplet-v1 branch? Maybe I can help with some of those.

Mainly:

  • TLS v1.3 compatibility (RSA PSS padding) - done for IsoApplet, I don't remember how far the state of the OpenSC driver was.
  • RSA 4096 bit (done I think)
  • ECC off-card hashes using your contributions 👍
  • A more standard-compliant way to get the features of the applet with GET DATA instead of returning them as a response to applet selection - done)
  • Initially I thought it would be nice to store private keys in the file system using a special file type, as well as PINs, and to have a separate SO-PIN to differentiate between card administration and use. Also, to configure the access rights of files and keys and PINs via OpenSC's PKCS15 profile file for the IsoApplet. But I think we should postpone this and rather get the other features done.
  • I thought about requiring extended APDU support. The command chaining code is so hard to maintain, and gets even worse when using 4096 bit RSA keys. With this requirement, we can remove it. There are different opinions whether the command chaining is actually standard-compliant when used at the "application side" to transmit more than 255 bytes. I am now unsure, given that some readers do not support extended APDUs. I am not sure whether all JavaCards with version >= 3.0.4 support this. Your opinion is welcome - would your use case be affected by this requirement?

Also see here.

@swissbit-csteuer
Copy link
Contributor Author

swissbit-csteuer commented Nov 25, 2022

TLS v1.3 compatibility (RSA PSS padding) - done for IsoApplet, I don't remember how far the state of the OpenSC driver was.

That's nice! I actually stumbled upon this problem a few days ago 😅
I don't know about the state of OpenSC in that regard as well, but if I find the time, I will test it next week.

Initially I thought it would be nice to store private keys in the file system using a special file type, as well as PINs, and to have a separate SO-PIN to differentiate between card administration and use. Also, to configure the access rights of files and keys and PINs via OpenSC's PKCS15 profile file for the IsoApplet. But I think we should postpone this and rather get the other features done.

Sounds like a nice feature. However, I would also prefer to postpone this so that we get the changes for ECC off-card hashing, RSA-4096 and RSA PSS padding support faster into OpenSC mainline.

I thought about requiring extended APDU support. The command chaining code is so hard to maintain, and gets even worse when using 4096 bit RSA keys. With this requirement, we can remove it. There are different opinions whether the command chaining is actually standard-compliant when used at the "application side" to transmit more than 255 bytes. I am now unsure, given that some readers do not support extended APDUs. I am not sure whether all JavaCards with version >= 3.0.4 support this. Your opinion is welcome - would your use case be affected by this requirement?

Requiring extended APDUs should not be a problem.

I have updated the OpenSC PR so that it should now work with the v0 and v1 version of the IsoApplet.
For now I have only tested the v1 branch with the simulator (I will try it with real hardware next week) and found some problems in the applet code for which I created PR #29.

@philipWendland
Copy link
Owner

That's nice! I actually stumbled upon this problem a few days ago sweat_smile
I don't know about the state of OpenSC in that regard as well, but if I find the time, I will test it next week.

We should use this branch as basis, I rebased the old commits upon the current OpenSC master. There is a commit about announcing the PSS feature of IsoApplet to OpenSC, but it might not be tested.

I have updated the OpenSC PR so that it should now work with the v0 and v1 version of the IsoApplet.
For now I have only tested the v1 branch with the simulator (I will try it with real hardware next week) and found some problems in the applet code for which I created PR #29.

Thanks, I merged your changes into the IsoApplet-v1 branch.

@swissbit-csteuer
Copy link
Contributor Author

We should use this branch as basis, I rebased the old commits upon the current OpenSC master. There is a commit about announcing the PSS feature of IsoApplet to OpenSC, but it might not be tested.

I have rebased the OpenSC PR onto this branch with some changes.

Last week I started testing with real hardware and found some more issues for which I created another PR (#30).
Tomorrow I will test RSA-PSS signature creation and RSA decryption.

@swissbit-csteuer
Copy link
Contributor Author

With the changes to the v1 IsoApplet in PR #30 and the latest changes to OpenSC in PR OpenSC/OpenSC#2642 everything is working now:

  • EC
    • EC key creation
    • ECDSA with SHA1, SHA224, SHA256, SHA384 and SHA512
    • EC key import
    • Tested key creation, sign and verify with all supported named curves
  • RSA
    • RSA 2048 and RSA 4096 key creation
    • RSA PKCS1 with SHA1, SHA224, SHA256, SHA384 and SHA512
    • RSA PSS with SHA1, SHA224, SHA256, SHA384 and SHA512
    • RSA decryption
    • RSA key import
  • Random number generation

I also tested EC and RSA key generation as well as signature creation with SHA1 (EC and RSA) with the v0 IsoApplet and the OpenSC version from the PR and everything seems to work fine.

RSA 4096 could only be tested with the simulator since I do not have hardware with support for that at the moment.

@philipWendland
Copy link
Owner

The changes of this PR are already in the IsoApplet-v1 branch. I am closing this PR, our discussion can be continued in PR #30.

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

Successfully merging this pull request may close these issues.

None yet

3 participants