Set public exponent and modulus on key import (Fixes READKEY)

[jborg]

Make sure the public exponent and module are initialized on key import.

This is required to be able to extract public keys from imported keys. Previously this only worked for generated keys. Public key extraction is used by gpg-agent when performing ssh authentication.

[klali]

Good catch!

I'd like to change the setting of modulus into a function in the PGPKey class like the other parts of the key. To conform with the standard we need to change the algorithm to 3 (rsa-crt with modulus) in the attributes part of the key as well.

/klas

[jborg]

Agreed, I can provide an updated patch in a few days. But since I now finally managed to import my pgp keys to my only neo I would like to test it for a few days before wiping it again.

[jborg]

I just pushed a new commit containing your proposed changes. I also took the liberty to enable the "Import support" flag since key import now seems to work well with both keyParser.py and with a patched gnupg.

I have a gnupg patch which adds RSA_CRT and RSA_CRT_N support. I plan to submit my patch upstream as soon as I have some time to clean it up.

Btw: I'm ok with licensing my changes as GPLv2+

[jas4711]

Many thanks for this work, and especially the gnupg patches which I hope will get applied quickly! The patch seems fine to me, I'll let Klas merge it.

[klali]

Very good!

I'm a bit unsure about setting the import supported flag as that will lead to old versions of gnupg thinking it can import keys. I'm vary about extra support calls to Yubico here, though more opinions on that topic is certainly welcome..

/klas

[jborg]

With the import flag set unpatched versions of gnupg will detect detect an unsupported key format (RSA_CRT_N) and refuse to import the key. I think it says something like "Error: Not supported" or similar (no crash).

What's the alternative? Wait 5+ years until everyone has upgraded their gnupg?

[klali]

Good point.

For me the error is: gpg: sending command `SCD WRITEKEY' to agent failed: ec=6.7 gpg: storing key onto card failed: general error Key generation failed: general error

and it manages to write the fingerprints for two of the subkeys to the card, leaving it in an undetermined state.

/klas

[jborg]

What's weird. Is that a completely unmodified gnupg? (and which version)

As far as I know app-openpgp.c:2406 should detect an incompatible key format and abort with "GPG_ERR_NOT_SUPPORTED" long before any fingerprint is written:

http://git.gnupg.org/cgi-bin/gitweb.cgi?p=gnupg.git;a=blob;f=scd/app-openpgp.c;h=4af4e933639e167285c66839c0d9f2cef069660f;hb=refs/heads/master#l2406

[klali]

Yeah.. that error is with gpg-1.4 through gpg-agent, gpg-2 gives:

gpg: storing key onto card failed: Not supported Key generation failed: Not supported

though still leaving half-generated keys. What seems to be happening is that it for some reason generates keys on-card then attempts to overwrite them with imported keys.

[jborg]

That's interesting. I've never seen any messages about "key generation" when importing keys. Could you please outline the steps performed to trigger this?

I usually import keys like this:

gpg --edit foo@example.com   (Key with RSA2048 subkeys)
toggle
key 1
keytocard

[klali]

ah.. and I was doing:

gpg --card-edit
admin
generate

for the keytocard case I don't mind the failure case, I don't like the generate failure case though.

[jborg]

Ok, so key import (keytocard) actually works/aborts as expected with gpg2 when "key import" flags is enabled. But for some reason key generation (generate) has stopped working when the "key import" flags is set (even though key generation shouldn't be controlled by that flag).

I didn't expect that, but I'll try to reproduce it later.

[jborg]

I think github lost my comment so I'll try again:

I've figured out the generate failure you're seeing.

When running "generate" on a card with import support gnupg will ask one additional question:

Make off-card backup of encryption key? (Y/n)

If you answer Yes to this question gnupg will only generate the authentication and signature subkey on card. The encryption subkey will be generated off-card and then later imported to the card. And this import will fail unless gnupg is patched.

If you answer No all subkeys will be generated on card the same way as if the card didn't support key import at all. So this will work regardless if gnupg is patched or not.

So to summarize: The only way to backup the encryption key is to use a patched version of gnupg.

[jas4711]

So here is a proposal for moving forward:

1) We apply the first patch which seemed trivially okay (right Klas?) 2) You work with Werner to get the GnuPG fixes included into a released GnuPG version 3) Then we apply your second patch to set the import flag -- and a hint in README that this requires GnuPG X.Y or later, AND that earlier versions will fail with errors (preferrably including those error messages in the README as well)

What do you think? My thinking is that your second patch is of no use to anyone that doesn't have a fixed GnuPG, so it should wait until that has materialized. Have you published these GnuPG patches?

Thanks again!

[jborg]

I'm okay with your proposal, my main reason for pushing for my other changes was to make it easier for any gnupg people wanting to test my gnupg-patch with an actual yubikey neo. But they can always pull from my repo directly.

The one thing missing from my first patch is that the key format is not updated to RSA_CRT_N but that's really a non-issue since since key import is disabled anyway.

I haven't published my gnupg patch yet but I'll try to do that as soon as I get some free time. But I'm glad to send it to anyone wanting to test it.

[jas4711]

Great! I think most of what's in your second patch should go in now as well, except for the s/D0/F0/ part. Can you prepare an updated patch that does everything that we could merge? Thanks again.

[jborg]

Sure, I'll prepare a new patch tonight.

Btw, I've sent my patch to gnupg-devel: http://lists.gnupg.org/pipermail/gnupg-devel/2013-August/027874.html

[jborg]

I've now moved everything but the import flag change into the first commit so this can me merged in stages.

[jas4711]

Splendid. I have merged the first part. Once it is in a GnuPG release, we'll merge the second part. Btw, I looked at your GnuPG patch, and I think it looks fine. Let's see what Werner says...