github-af / SmartPGP

SmartPGP is a JavaCard implementation of the OpenPGP card specifications
GNU General Public License v2.0
227 stars 47 forks source link

Algorithm Information for RSA lists Import-Format 1 or 3, but only 3 seems to work #39

Closed hko-s closed 2 years ago

hko-s commented 2 years ago

When importing RSA keys with Import-Format 1 (that is, without providing PQ, DP1 and DQ1), running in jcardsim-3.0.5, I'm getting the error status 0x6A80. I imagine this happens because priv.isInitialized() is false for a RSAPrivateCrtKey object https://github.com/ANSSI-FR/SmartPGP/blob/8e7ebfdcc4f9c55f1f89e0658cff5e01829def66/src/fr/anssi/smartpgp/PGPKey.java#L371 when these three parameters are unset.

It seems to me that SmartPGP should only list support for RSA keys with "Import-Format 3" in Algorithm Information?

af-anssi commented 2 years ago

There is no explicit list of supported algorithms in the OpenPGP card specifications. The applet supports the change of "algorithm attributes", but there is no restriction implemented (except the minimal size of RSA key) to authorize RSA "format 3" only, see: https://github.com/ANSSI-FR/SmartPGP/blob/8e7ebfdcc4f9c55f1f89e0658cff5e01829def66/src/fr/anssi/smartpgp/PGPKey.java#L150-L170

Actually, the import of RSA key according to "format 1" mainly depends on the underlying JavaCard implementation.

The applet is configured to use "format 3" (CRT with modulus) by default: https://github.com/ANSSI-FR/SmartPGP/blob/8e7ebfdcc4f9c55f1f89e0658cff5e01829def66/src/fr/anssi/smartpgp/Constants.java#L224

Which "client" software (e.g. gnupg v2.x) did you use to trigger a "format 1" import ?

hko-s commented 2 years ago

Thank you for the feedback!

I'm working on a new client library for the OpenPGP Card spec: https://gitlab.com/hkos/openpgp-card

In the process of importing a key, client software needs to determine and set the "algorithm attributes" (if the key is not already set to the desired algorithm/key size). I'm basing the exact "algorithm attributes" on the "algorithm information" list. So in the case of SmartPGP and RSA, this means I need to choose between the two "Import Format" variants.

My assumption is that "algorithm information" will list only configurations that the OpenPGP Card can actually process (at least for most card implementations).

With SmartPGP running on jcardsim, this is not the case: when configuring "algorithm attributes" to "RSA with Import Format 1" and sending the private key template and key data in that shape for key import, jcardsim returns status 0x6A80.

I assumed that all implementations of RSAPrivateCrtKey require the CRT fields (PQ, DP1, DQ1) to be set: https://github.com/ANSSI-FR/SmartPGP/blob/8e7ebfdcc4f9c55f1f89e0658cff5e01829def66/src/fr/anssi/smartpgp/PGPKey.java#L297 in order for priv.isInitialized() to be true https://github.com/ANSSI-FR/SmartPGP/blob/8e7ebfdcc4f9c55f1f89e0658cff5e01829def66/src/fr/anssi/smartpgp/PGPKey.java#L371 but haven't succeeded in finding documentation about this.

Either way, unless I'm missing something, I think it would be good for OpenPGP card applets to list only supported algorithm configurations in "algorithm information". If the supported import formats indeed depend on the JavaCard implementation, maybe it would be appropriate to introduce a user-configurable constant (something like "RSA_KEYS_NEED_CRT") to Constants.java? Based on this constant, the list of actually supported algorithm attributes (for a specific JavaCard implementation) could be returned to clients.

af-anssi commented 2 years ago

Thank you for the feedback!

I'm working on a new client library for the OpenPGP Card spec: https://gitlab.com/hkos/openpgp-card

A Rust tool, that's very cool :)

In the process of importing a key, client software needs to determine and set the "algorithm attributes" (if the key is not already set to the desired algorithm/key size). I'm basing the exact "algorithm attributes" on the "algorithm information" list. So in the case of SmartPGP and RSA, this means I need to choose between the two "Import Format" variants.

My assumption is that "algorithm information" will list only configurations that the OpenPGP Card can actually process (at least for most card implementations)

With SmartPGP running on jcardsim, this is not the case: when configuring "algorithm attributes" to "RSA with Import Format 1" and sending the private key template and key data in that shape for key import, jcardsim returns status 0x6A80.

I assumed that all implementations of RSAPrivateCrtKey require the CRT fields (PQ, DP1, DQ1) to be set:

https://github.com/ANSSI-FR/SmartPGP/blob/8e7ebfdcc4f9c55f1f89e0658cff5e01829def66/src/fr/anssi/smartpgp/PGPKey.java#L297

in order for priv.isInitialized() to be true

https://github.com/ANSSI-FR/SmartPGP/blob/8e7ebfdcc4f9c55f1f89e0658cff5e01829def66/src/fr/anssi/smartpgp/PGPKey.java#L371

but haven't succeeded in finding documentation about this.

Either way, unless I'm missing something, I think it would be good for OpenPGP card applets to list only supported algorithm configurations in "algorithm information".

Yes, you are right.

If the supported import formats indeed depend on the JavaCard implementation, maybe it would be appropriate to introduce a user-configurable constant (something like "RSA_KEYS_NEED_CRT") to Constants.java? Based on this constant, the list of actually supported algorithm attributes (for a specific JavaCard implementation) could be returned to clients.

Yes, it seems a good idea. I will try to introduce this variable (and obviously set it to falseby default) in a future release.

Thank you for reporting this problem.

hko-s commented 2 years ago

If the supported import formats indeed depend on the JavaCard implementation, maybe it would be appropriate to introduce a user-configurable constant (something like "RSA_KEYS_NEED_CRT") to Constants.java? Based on this constant, the list of actually supported algorithm attributes (for a specific JavaCard implementation) could be returned to clients.

Yes, it seems a good idea. I will try to introduce this variable (and obviously set it to falseby default) in a future release.

Great! Thank you.

af-anssi commented 2 years ago

Could you test with any v1.20 version ?

hko-s commented 2 years ago

Wow, that was very fast!

I just tested with the current git master and can confirm that the "algorithm information" list now looks as I think it should (only the "Import Format 3" RSA algorithm variants are listed by default).

hko-s commented 2 years ago

This makes me think of one more (orthogonal, but related) possible improvement: RSA3k and RSA4k could be omitted from the algorithm information list based on the value of the constant INTERNAL_BUFFER_MAX_LENGTH

af-anssi commented 2 years ago

This would be a very good improvment, could you open an dedicated issue ?

hko-s commented 2 years ago

This would be a very good improvment, could you open an dedicated issue ?

I was just starting to write a new issue, but then it occurred to me that SmartPGP always supports RSA3k and RSA4k (with key generation on the card). So these algorithms should always be listed in the "algorithm information" list.

Importing larger RSA keys may fail, but the OpenPGP card spec doesn't offer a mechanism for the card to signal this specific limitation to the client.

Therefore I retract this suggestion.