philipWendland / IsoApplet

A Java Card PKI Applet aiming to be ISO 7816 compliant
GNU General Public License v3.0
165 stars 72 forks source link

Always use ECDSA with off card hashing #28

Closed swissbit-csteuer closed 1 year ago

swissbit-csteuer commented 1 year ago

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 https://github.com/OpenSC/OpenSC/pull/2642).

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

philipWendland commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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:

Also see here.

swissbit-csteuer commented 1 year ago

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 :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.

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 commented 1 year ago

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 https://github.com/philipWendland/IsoApplet/pull/29.

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

swissbit-csteuer commented 1 year ago

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 commented 1 year ago

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

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 commented 1 year ago

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.