alonbl / gnupg-pkcs11-scd

PKCS#11 GnuPG SCD
http://gnupg-pkcs11.sourceforge.net/
Other
69 stars 17 forks source link

Issue with data longer than 256 bytes #5

Closed smunaut closed 7 years ago

smunaut commented 7 years ago

I'm not sure since when this happens, but in my current setup [ gpg (GnuPG) 2.1.15 ], the SETDATA call can have more than 256 bytes ( 257 in my case ) where there basically is zero prefix.

For instance :

SETDATA 0096431DB6EB6EF6D9E77AAEF08C3C3E37225B8A4B01CDA2519745F933E57BC87BC3B8765B1DD0E8E91B16B12CE73206355BE488F700AEB3E40C51AC588D97E140C7C3B6AE351B6ACF2CE32995A249E65D8FAFE16F967D55D3DCC6E106C76BF5F7B5AD8F0A633E004D91EFDE3C2C46FC49FED4C963A29267B4CFDA5C227E42F5EF724758CCBE1844357EF32130B8351CDD87697AC9B5797461BC371BABDA7E3935DC1E2E9CB543C5EF88D9037AA55DF280048929D2D34C1D2EA626AC3784275341AABC097C1B82021A1A6C9CBB15C57F971E5C91CD9E30741C2C3FF289E02BF51D20DD7393E56CCBBC62D8E9E0AA08A9C9C668CE971AEB785E89D893E501CB20B5

This is an issue because then the call to pkcs11h_certificate_decryptAny will fail because of an internal error ( Private key operation failed rv=33-'CKR_DATA_LEN_RANGE' ). Because for a 2048 bit RSA, the max data length is 256 bytes. But the first byte being '00' can be dropped (after all that string represent just a big number that must be lower than the modulus).

Here's the 'hacked up' patch I'm using to strip zero data :

fix-data.txt

It definitely use to work. I'm not sure if gpg changed, or if the pkcs11helper used to strip data, or if opensc used to do it ... but atm using:

this isn't working for me.

alonbl commented 7 years ago

Interesting!!! However... provided you example, you actually switch the sign of the big number... I wounder how the signature verification works... Does gpg always strip? Maybe we need to add 00 when decrypting? I will go thought gpg code (or libgcrypt) in order to try and understand...

alonbl commented 7 years ago

Hi, Can you please checkout the fixups branch? I've added similar solution, but worse, I had to modify the way I communicate the serial number of the card, please see the ChangeLog for instructions. I will appreciate if you can check it out and report any additional issue. Thanks!

smunaut commented 7 years ago

Hi,

Can you please checkout the fixups branch?

I've added similar solution, but worse, I had to modify the way I communicate the serial number of the card, please see the ChangeLog for instructions. I will appreciate if you can check it out and report any additional issue.

I'll try on Monday but there is something annoying :

Recreating the keys with :

gpg --card-edit
admin
generate

Yields a different Key ID because the key creating date is part of the data that's used for the Key ID. That means old files encrypted with the previous key ID won't be able to be decrypted ...

Cheers,

Sylvain

alonbl commented 7 years ago

Hi, I found that removing the secret key using --delete-secret-keys KEY and performing generate again without override keys attaches the private key into the existing key. Please backup your ~/.gnupg and check this out. If the instructions at the ChangeLog unclear I will try to improve it. Thanks!

alonbl commented 7 years ago

Please notice that I have a problem with gnupg-2.1.18 I have not actually tested which exactly it can be 17 or 16 as well, see [1], I am unsure how I can simply workaround this. The problem is that 2.1.15 smime is not working, this was fixed in 2.1.18... This scd protocol becomes stranger and stranger.

[1] https://lists.gnupg.org/pipermail/gnupg-devel/2017-February/032557.html

smunaut commented 7 years ago

Ok, I tried the 'fixups' branch and it works fine. Also indeed I started with just the public key in a fresh keychain and did the 'edit / admin / generate' and it learnt the new serial number. Previously I always said "yes" to "Replace existing keys" since it seemed to do nothing otherwise and that was my issue ...

alonbl commented 7 years ago

Thanks! Is it safe to merge? :)

smunaut commented 7 years ago

Well I'll be using it on my system at least :p

alonbl commented 7 years ago

Thanks! merged.