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

OpenPGP card ECC SSH keys problem #1906

Closed
ghost opened this issue Jan 17, 2020 · 53 comments · Fixed by #1911
Closed

OpenPGP card ECC SSH keys problem #1906

ghost opened this issue Jan 17, 2020 · 53 comments · Fixed by #1911

Comments

@ghost
Copy link

ghost commented Jan 17, 2020

Problem Description

I am trying to set up my Nitrokey Storage 2 to connect to remote hosts with ECC. I am using NIST P-384 curve. I have no problems at listing all the keys:

pkcs15-tool --list-keys
Using reader with a card: Nitrokey Nitrokey Storage (0000000000000) 00 00
Private EC Key [Signature key]
	Object Flags   : [0x03], private, modifiable
	Usage          : [0x20C], sign, signRecover, nonRepudiation
	Access Flags   : [0x1D], sensitive, alwaysSensitive, neverExtract, local
	FieldLength    : 0
	Key ref        : 0 (0x00)
	Native         : yes
	Auth ID        : 01
	ID             : 01
	MD:guid        : 017a9f15-2c68-ad5e-12d1-00407af06829

Private EC Key [Encryption key]
	Object Flags   : [0x03], private, modifiable
	Usage          : [0x22], decrypt, unwrap
	Access Flags   : [0x1D], sensitive, alwaysSensitive, neverExtract, local
	FieldLength    : 0
	Key ref        : 1 (0x01)
	Native         : yes
	Auth ID        : 02
	ID             : 02
	MD:guid        : 250550b6-fb76-8e76-294e-a44fc8d0e445

Private EC Key [Authentication key]
	Object Flags   : [0x03], private, modifiable
	Usage          : [0x222], decrypt, unwrap, nonRepudiation
	Access Flags   : [0x1D], sensitive, alwaysSensitive, neverExtract, local
	FieldLength    : 0
	Key ref        : 2 (0x02)
	Native         : yes
	Auth ID        : 02
	ID             : 03
	MD:guid        : 3281a6a6-167a-76eb-33c7-c000af63b6e3

But then, there is a problem on exporting the third (auth) key in SSH form:

pkcs15-tool --read-ssh-key 3
Using reader with a card: Nitrokey Nitrokey Storage (0000000000000) 00 00
Unsupported curve

NIST P-384 should be supported by both this token and OpenSSH. Of course, this problem persists for two other keyslots.

ssh-keygen returns similar problems:

ssh-keygen -D /usr/lib/opensc-pkcs11.so 
C_GetAttributeValue failed: 18
failed to fetch key
C_GetAttributeValue failed: 18
failed to fetch key
Enter PIN for 'OpenPGP card (User PIN)': 
C_GetAttributeValue failed: 18
failed to fetch key
C_GetAttributeValue failed: 18
failed to fetch key
C_GetAttributeValue failed: 18
failed to fetch key
Enter PIN for 'OpenPGP card (User PIN (sig))': 
C_GetAttributeValue failed: 18
failed to fetch key
cannot read public key from pkcs11

Of course, I have no problems just reading any of those keys:

pkcs15-tool --read-public-key 3
Using reader with a card: Nitrokey Nitrokey Storage (0000000000000) 00 00
-----BEGIN PUBLIC KEY-----
MHEwCwYHKoZIzj0CAQUAA2IABE+dm3v2tYPaS2jfosey7Kc+6MiUmYu40vdRU84c
CurIla87/3hIkaCvvn+UhU1SChsggtgyrZP1V/9A6k/CrdwBOh8g3B90FhWvOuDF
pEJJkUQYs/mqY4k8ry9FhWsBlg==
-----END PUBLIC KEY-----

I also have another Nitrokey with RSA keys from my work, it looks like it works perfectly with ssh and opensc_pkcs11 module.

Steps to reproduce

  1. Install OpenSC 0.20 and OpenSSH 8.1
  2. Set up an OpenPGP card with NIST-P384 keys
  3. Try to generate SSH key
@frankmorgner
Copy link
Member

Did you check https://support.nitrokey.com/?

@alex-nitrokey?

@ghost
Copy link
Author

ghost commented Jan 17, 2020

Tried to do some gooling first. There was another issue here back in 2016 which was resolved with updates to both opensc and openssh to support ECC.
#803
Also they've mentioned some problems with NISTp521 compatibility with Nitrokeys, so I chose NISTp384...

@dengert
Copy link
Member

dengert commented Jan 18, 2020

The pubkey above does not have the curve OID. Could be bug in pkcs15-tool --read-ssh-key 3 with ECC. Or a bug in the Nitro code. The namedCurve may not be accessible to the driver, so is not added to the ECC pubkey. An OpenSC debug should show what is returned.

I don't have a Nitro card, but a PIV card with ECC:

pkcs15-tool --read-ssh-key 03
Using reader with a card: SCM Microsystems Inc. SCR 355 [CCID Interface] 00 00
ecdsa-sha2-nistp384 AAAAE2VjZHNhLXNoYTItbmlzdHAzODQAAAAIbmlzdHAzODQAAABhBHERARoxPsnHKuI4ajUcrfxhGMnZOLNH1CZhpFLn7bO0twhiEC6xn8tpSVlwaQyJJLpVqxehdHWVxW4WLEhcgQR16hTvJ5VvoRWv3ccJw/svpj05z2V9nT3n6h17rYkWiA== KEY MAN pubkey

Try something like this to see the public key with parameters:

pkcs15-tool --read-public-key 03 | openssl pkey -pubin -text
Using reader with a card: SCM Microsystems Inc. SCR 355 [CCID Interface] 00 00
-----BEGIN PUBLIC KEY-----
MHYwEAYHKoZIzj0CAQYFK4EEACIDYgAEcREBGjE+yccq4jhqNRyt/GEYydk4s0fU
JmGkUufts7S3CGIQLrGfy2lJWXBpDIkkulWrF6F0dZXFbhYsSFyBBHXqFO8nlW+h
Fa/dxwnD+y+mPTnPZX2dPefqHXutiRaI
-----END PUBLIC KEY-----
Public-Key: (384 bit)
pub:
    04:71:11:01:1a:31:3e:c9:c7:2a:e2:38:6a:35:1c:
    ad:fc:61:18:c9:d9:38:b3:47:d4:26:61:a4:52:e7:
    ed:b3:b4:b7:08:62:10:2e:b1:9f:cb:69:49:59:70:
    69:0c:89:24:ba:55:ab:17:a1:74:75:95:c5:6e:16:
    2c:48:5c:81:04:75:ea:14:ef:27:95:6f:a1:15:af:
    dd:c7:09:c3:fb:2f:a6:3d:39:cf:65:7d:9d:3d:e7:
    ea:1d:7b:ad:89:16:88
ASN1 OID: secp384r1
NIST CURVE: P-384

@ghost
Copy link
Author

ghost commented Jan 18, 2020

@dengert You seem to be right

pkcs15-tool --read-public-key 03 | openssl pkey -pubin -text
Using reader with a card: Nitrokey Nitrokey Storage (0000000000000) 00 00
unable to load Public Key
140500963497216:error:100DC08E:elliptic curve routines:eckey_type2param:decode error:crypto/ec/ec_ameth.c:124:
140500963497216:error:100D7010:elliptic curve routines:eckey_pub_decode:EC lib:crypto/ec/ec_ameth.c:151:
140500963497216:error:0B09407D:x509 certificate routines:x509_pubkey_decode:public key decode error:crypto/x509/x_pubkey.c:125:
140500963497216:error:0906700D:PEM routines:PEM_ASN1_read_bio:ASN1 lib:crypto/pem/pem_oth.c:33:

It can't get EC parameters. Should I report it to Nitrokey's repo issues?

@dengert
Copy link
Member

dengert commented Jan 18, 2020

@alex-nitrokey Do you have any comments on this?

From what I can tell from https://shop.nitrokey.com/shop/product/nitrokey-storage-2-56
It says:

  • Elliptic curves: NIST P-256, P-384, P-521 (secp256r1/prime256v1, secp384r1/prime384v1, secp521r1/prime521v1), brainpoolP256r1, brainpoolP384r1, brainpoolP512r1
  • OpenPGP Card 3.3
  • It also does not say list the ATR of any of its devices.

https://www.nitrokey.com/documentation/frequently-asked-questions-faq#which-gnupg,-opensc-and-libccid-versions-are-required says: "OpenSC 0.18 or above should work."

Problem could be in card-openpgp.c It has a list of supported ATRs, which map to 4 SC_CARD_TYPE_OPENPGP_* card types. It can also check for the OpenPGP AID to determine the device supports OpenPGP. It may not set the card->type correctly in this case.

@unb9rn an OpenSC debug log would help answer many of these questions to see why the curve is not being set. It should be set in cardf-openpgp.c pgp_parse_algo_attr_blob

@ghost
Copy link
Author

ghost commented Jan 18, 2020

@dengert I'm not sure if there is verbosity levels higher than 9, hope these two logs may help.
card-openpgp seem to return states correctly, but I can't see where it calls pgp_parse_algo_attr_blob function in those logs. Maybe I'm doing something wrong.
list-keys.log
read-ssh-key.log

@dengert
Copy link
Member

dengert commented Jan 19, 2020

card-openpgp.c does not have a lot of debugging. So it is not easy to see what it doing. Looking at read-ssh-key.log at lines 1319-1333

4F 10 D2 76 00 01 24 01 03 03 00 05 00 00 69 81 O..v..$.......i.
00 00 5F 52 0A 00 31 F5 73 C0 01 60 05 90 00 7F .._R..1.s..`....
66 08 02 02 08 00 02 02 08 00 73 81 B7 C0 0A 7F f.........s.....
00 08 00 08 00 08 00 00 01 C1 06 13 2B 81 04 00 ............+...
23 C2 06 12 2B 81 04 00 23 C3 06 13 2B 81 04 00 #...+...#...+...
23 C4 07 01 40 40 40 03 00 03 C5 3C 30 B6 FB 94 #...@@@....<0...
8F 73 D6 10 01 A5 3E 4C FE E0 3F DE FE E4 FA CC .s....>L..?.....
2D 83 51 36 DE C0 D1 4E 50 97 38 98 7F 55 F8 76 -.Q6...NP.8..U.v
FF AC DD 36 4B 21 81 3A F2 BE 66 00 84 52 14 94 ...6K!.:..f..R..
83 DF 61 8D 71 10 D9 DA C6 3C 00 00 00 00 00 00 ..a.q....<......
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 CD 0C 5E 22 AD 18 5E 22 AD 18 ........^"..^"..
5E 22 AD 18 90 00                               ^"....

It reads in all the DOs, then later uses read_binary to parse this looking for path=006e007300c2, 006e007300c3 and 006e007300c1 which are "4.4.3.7 Algorithm Attributes" in V3.0

C2 06 12 2B 81 04 00 23

C3 06 13 2B 81 04 00 23

C1 06 13 2B 81 04 00 23

0x12 = 18 ECDH, 0x13 = 19 ECDSA for PSO:CDS and INT-AUT

These all say the keys are: ansix9p521r1, OID = {1.3.132.0.35} = '2B81040023'

But the real problem is the meaning of a "pubkey" RSA has no parameters, EC has a parameter, the OID of the algorithm. For some uses the parameter is required and this is the case for EC.

pkcs15-tool in read_ssh_key and read_public_key will call:
sc_pkcs15_find_pubkey_by_id, if not found, sc_pkcs15_find_cert_by_id, sc_pkcs15_read_certificate to get SPKI from certificate.

In either case, the pubkey->u.ec.params.id should have the OID of the curve.
But it looks like the pubkey returned by the card-openpgp.c does not add the u.ec.params.id.

Adding to the pkcs15-tool --verbose would show more debugging info.

@ghost
Copy link
Author

ghost commented Jan 19, 2020

Looks like adding --verbose somehow conflicts with debug info. After adding this key there is almost no debug info (tried both opensc.conf and overriding via OPENSC_DEBUG=9). All info that remains:


P:71984; T:0x139634675271744 10:48:09.191 [pkcs15-tool] ctx.c:851:sc_context_create: ===================================
P:71984; T:0x139634675271744 10:48:09.191 [pkcs15-tool] ctx.c:852:sc_context_create: opensc version: 0.20.0
P:71984; T:0x139634675271744 10:48:09.191 [pkcs15-tool] reader-pcsc.c:858:pcsc_init: PC/SC options: connect_exclusive=0 disconnect_action=0 transaction_end_action=0 reconnect_action=0 enable_pinpad=1 enable_pace=1
P:71984; T:0x139634675271744 10:48:09.192 [pkcs15-tool] reader-pcsc.c:1347:pcsc_detect_readers: called
P:71984; T:0x139634675271744 10:48:09.192 [pkcs15-tool] reader-pcsc.c:1360:pcsc_detect_readers: Probing PC/SC readers
P:71984; T:0x139634675271744 10:48:09.192 [pkcs15-tool] reader-pcsc.c:1411:pcsc_detect_readers: Establish PC/SC context
P:71984; T:0x139634675271744 10:48:09.722 [pkcs15-tool] reader-pcsc.c:1296:pcsc_add_reader: Adding new PC/SC reader 'Nitrokey Nitrokey Storage (0000000000000) 00 00'
P:71984; T:0x139634675271744 10:48:09.722 [pkcs15-tool] reader-pcsc.c:333:refresh_attributes: Nitrokey Nitrokey Storage (0000000000000) 00 00 check
P:71984; T:0x139634675271744 10:48:09.722 [pkcs15-tool] reader-pcsc.c:381:refresh_attributes: current  state: 0x00000022
P:71984; T:0x139634675271744 10:48:09.722 [pkcs15-tool] reader-pcsc.c:382:refresh_attributes: previous state: 0x00000000
P:71984; T:0x139634675271744 10:48:09.722 [pkcs15-tool] reader-pcsc.c:435:refresh_attributes: card present, changed
P:71984; T:0x139634675271744 10:48:09.723 [pkcs15-tool] reader-pcsc.c:1500:pcsc_detect_readers: Nitrokey Nitrokey Storage (0000000000000) 00 00:SCardConnect(SHARED): 0x00000000
P:71984; T:0x139634675271744 10:48:09.723 [pkcs15-tool] reader-pcsc.c:1114:detect_reader_features: called
P:71984; T:0x139634675271744 10:48:09.723 [pkcs15-tool] reader-pcsc.c:1116:detect_reader_features: Requesting reader features ... 
P:71984; T:0x139634675271744 10:48:09.723 [pkcs15-tool] reader-pcsc.c:1137:detect_reader_features: Reader feature 12 found
P:71984; T:0x139634675271744 10:48:09.723 [pkcs15-tool] reader-pcsc.c:1054:part10_detect_max_data: get dwMaxAPDUDataSize property returned 65536
P:71984; T:0x139634675271744 10:48:09.723 [pkcs15-tool] reader-pcsc.c:1246:detect_reader_features: Reader supports transceiving 65536 bytes of data
P:71984; T:0x139634675271744 10:48:09.723 [pkcs15-tool] reader-pcsc.c:1093:part10_get_vendor_product: id_vendor=20a0 id_product=4109
P:71984; T:0x139634675271744 10:48:09.723 [pkcs15-tool] reader-pcsc.c:1515:pcsc_detect_readers: returning with: 0 (Success)

Removing --verbose key spews debug info as before...

@dengert
Copy link
Member

dengert commented Jan 20, 2020

Does you card have certificates for the 3 keys? (See second fix below)

pkcs15-tool, will use either the public key, returned by the card, or if not found it will try and use the certificate and obtain the public key SPKI from the certificate.

EC public keys have a parameter, usually the curve OID. The SPKI from a certificate will always have one. OpenPGP specs for 3.0 and above refer to RFC 6637 Section 9. Encoding of Public and Private Keys says: "Algorithm-Specific Fields for ECDSA keys: a variable-length field containing a curve OID"

The OpenSC card-openpgp.c does not add the curve OID to a public key as requested by pkcs15-tool so pkcs15-tool says "Unsupported curve".

The Algorithm attributes DOs, C1, C2 and C3 for the 3 key contain the content of the OID so the curve OID can be derived. Card-openpgp.c will parse when needed but when returning a public key to a caller it does not add these to the response.

There are at least 2 ways to fix this:

  • Add code to card-openpgp.c to use the Algorithm attributes to add the EC curve OID to EC public keys like other card drivers do so pkcs15-tool and pkcs11 and other tools will work.
    PROs: Best way to fix it. All the OpenSC routines are already there, only changes to card-openpgp.c are needed.
    CONs: Complicated code. I don't have any OpenPGP devices that support EC. Some one else needs to do this.

  • Modify pkcs15-tool that when it reads a public EC key and it does not have the curve OID try to read the certificate.
    PROs: Easy to fix. And would address this issue, to get the SSH formatted entry.
    CONs: Only works if certificates are present. Only fixes pkcs15-tool.c in a few cases.

@ghost
Copy link
Author

ghost commented Jan 20, 2020

Unfortunately, the only objects on my card are 3 GPG keys (one key and two subkeys)
I hope someone would be able to fix it the right way, as I am convinced, GPG is much more popuar than X509 with non-enterprise OpenPGP card users.

@popovec
Copy link
Member

popovec commented Jan 20, 2020

pkcs15-tool need OID from pubkey->u.ec.params.id and field length (even this can be deduced from OID) in pubkey->u.ec.params.field_length.

Probably the easiest way is ad small part of code to card-openpgp.c:

else if ((r = pgp_get_blob(card, blob, 0x0086, &pubkey_blob)) >= 0
		&& (r = pgp_read_blob(card, pubkey_blob)) >= 0) {

		memset(&pubkey, 0, sizeof(pubkey));

		pubkey.algorithm = SC_ALGORITHM_EC;
		pubkey.u.ec.ecpointQ.value = pubkey_blob->data;
		pubkey.u.ec.ecpointQ.len = pubkey_blob->len;
	

pubkey.u.ec.ecpointQ.len can be used so select correct OID and field lenght from structure:

static struct supported_ec_curves {
			size_t len;
			struct sc_object_id curve_oid;
			size_t size;
		} ec_curves[] = {
			{32, {{1, 2, 840, 10045, 3, 1, 7, -1}},256},
			{48, {{1, 3, 132, 0, 34, -1}},         384},
			{65, {{1, 3, 132, 0, 35, -1}},         521},
		        {0 {{-1}}, 0},
		};

The data from proposed structure is then used to set pubkey->u.ec.params.id and pubkey->u.ec.params.field_length. I do not have OpenPGP device, can only guess what is in pubkey_blob->len and if this corresponds to values in proposed structure ..

Of course, this is not a clean solution, but it will work.

@szszszsz
Copy link

cc @Nitrokey

@szszszsz
Copy link

Hi! I am not familiarized with the OpenSC internals, but I can support testing for this case. If you have any dev branch with the patch just let me know.

@dengert
Copy link
Member

dengert commented Jan 20, 2020

You can not really deduce the curve name from the pubkey.nu.ec.ecpointQ.len. For example, current card-openpgp.c ec_curves and ec_curves_gnuk have asn1 and brainpool curves with the same lengths. In future may also have more curves with similar lengths. The DOs C1, C2, and C3 i.e. the Algorithm attributes which are present on the card in question have enough of the OID to create a proper OID to be added to ecpointQ to create a sc_pkcs15_pubkey_t.

card-openpgp.c uses pubkey in a number of places to point at different structures:
sc_pkcs15_pubkey_t pubkey; or sc_cardctl_openpgp_keygen_info_t pubkey;
The sc_pkcs15_pubkey_t pubkey; should be change to sc_pkcs15_pubkey_t p15pubkey; to keep things straight.

The sc_pkcs15_pubkey_t pubkey; has pointers when using params but the code does not use sc_pkcs15_erase_pubkey(struct sc_pkcs15_pubkey *); thus may have memory leaks, especially if
the code just does LOG_FUNC_RETURN without cleaning up the pubkey.

sc_pkcs15_fix_ec_parameters(card->ctx, &pubkey.u.ec.params); could be used as it has a builtin list of valid algorithms supported by OpenSC to accept an OID and find the name and lengths. It us used by all the other EC drivers so why not card-openpgp.c too.

A developer really needs a V3.0 card with ECC support to make and debug these changes.

@dengert
Copy link
Member

dengert commented Jan 21, 2020

I may have a fix. It using the OpenPGP "Algorithm Attributes" to add the curve OID ,then uses sc_pkcs15_fix_ec_parameters to look up the name curve so sc_encode_pubkey will pass it to pkcs15-tool.
Its untested. Will have it done tomorrow.

dengert added a commit to dengert/OpenSC that referenced this issue Jan 21, 2020
…SC#1906

The EC Parameters are the way the EC curve is presented to the outside world,
and in most cases is present in a matching certificate in the SPKI.

card-openpgp.c is modified to add the EC named_curve to the PKCS15 public key.
OpenPGP specs only provide this via the "Algorithm Attributes" for the 3 keys
via tags C1, C2 and C3 These contain the OID for the EC curve, which
is then mapped by sc_pkcs15_fix_ec_parameters to a EC Curve name.

This is the first attempt to fix this problem without having a token that
supports OpenGPG V3.3, so it will need additional testing and debugging by
developers who have such a token.

On branch openpgp-ec-pub-curve
 Changes to be committed:
	modified:   card-openpgp.c
@alex-nitrokey
Copy link
Contributor

I can jump in on Thursday to help/debug this issue as I added most of the ECC related stuff in card-openpgp.c. I will see if I can improve the code regarding the sc_pkcs15_fix_ec_paramters in the future.
I am sorry for my unresponsiveness so far.

@dengert
Copy link
Member

dengert commented Jan 21, 2020

@unb9rn You did not say how you created the EC keys on the token. And you also said: " I am using NIST P-384 curve." But the The DOs C1, C2, and C3 i.e. the Algorithm attributes in the debug output says these have have the OID for ansix9p521r1. Have you tried both? Did you reinitialize the card?

@dengert
Copy link
Member

dengert commented Jan 21, 2020

@szszszsz @unb9rn @alex-nitrokey have a look at #1911 which may fix the problem.

Other questions:

  • OpenPGP specs refer to RFC 6637, which only defines 3 curves. But card-openpgp.c also lists brain pool curves.
  • How are blobs 00C1, 00C2, and 00C3 created. The OpenSC code does not appear to do this. I assume GnuPG is used to create the keys and uses some curve. SOme card vendors may also provide code to do this too.

@ghost
Copy link
Author

ghost commented Jan 21, 2020

@dengert oh, I'm sorry. Yes, the card was initialized with GPG (the keys were generated on my machine, not on the card itsef. then i moved them to the card). At that point I've found another bug either in GPG or in Nitrokey's firmware, which allowed me to push P-384 keys, but not P-521 to the card, but I decided to re-check and it turned out that P-521 keys could still be generated on the card itself. So, answering your question-yes, I've re-initialized the card with GPG, generating the keys directly on the card.

Regarding Brainpool curves, I've tried them too, they seem to work, but it looks like SSH is incompatible with Brainpool curves, so i decided to use the NIST one.

@dengert
Copy link
Member

dengert commented Jan 21, 2020

Thanks. Your experience brings up an issue. It appears that the curve to be used, is specified in the "Algorithm Attribute" and is the curve used by the card. From the debug output the curve OID is for ansiX9p521r1 and the pubkey read from the card is for this size curve. The pub key read from the card (DO A400 lines 1622-1630 in the debug log) does not have an OID parameter. So the curve is not stored with the pubkey . So the card does not look at the pubkey DO, but does not say the curve OID could be stored in the pubkey for use by the caller.

It is not clear who sets "Algorithm Attribute" for each key. A card could change the "Algorithm Attribute" as part of a generate or import a key operation or the calling application sets the "Algorithm Attribute" and then does a generate key or import. (I am not an OpenPGP expert, does anyone know how this is done?)

OpenPGP-smart-card-application-3.3.1 says:

"From this recommendation the following curves should be used for an OpenPGP smart card
and will be available in common smart card products:
The prime field curves
● ansix9p256r1
● ansix9p384r1
● ansix9p521r1
from [ANSI X9.62], compliant with [FIPS-186-3] and
● brainpoolP256r1
● brainpoolP384r1
● brainpoolP512r1
from [RFC5639]

At least one of this curves shall be supported by an OpenPGP smart card if EC is available."

Note that ansix9p384r1 and brainpoolP384r1 have the same pubkey size. So the curve must be available to the card as well as the application.

@ghost
Copy link
Author

ghost commented Jan 22, 2020

I've patched 0.20 with card-openpgp.c and tried re-building. It compiled, but the problem still persists... @szszszsz Can you re-check and confirm please?

@dengert
Copy link
Member

dengert commented Jan 22, 2020

Did you get a opensc debug log?
Does it have a line with sc_pkcs15_fix_ec_parameters?

If not, can you run GDB on it.

You may have to compile with debugging. One way to do this is to set CFLAGS and LDFLAGS before running configure:

CFLAGS=-g
LDFLAGS=-g
export CFLAGS LDFLAGS

then rebuild.

The to debug you can the run something like this: gdb --args /opt/ossl-1.1/bin/pkcs15-tool --read-ssh-key 3
at the (gdb) prompt then do:

(gdb) break pgp_get_pubkey_pem
No symbol table is loaded.  Use the "file" command.
Make breakpoint pending on future shared library load? (y or [n]) y
Breakpoint 1 (pgp_get_pubkey_pem) pending.
(gdb) run

It should stop at the breakpoint, and you can get a backtrace, and step through the code next and look at structures and variables print set other breakpoints like break card-openpgp.c:1716 which is the big if statement. Use continue to start running again until the next breakpoint.

I am interested if it got to this if statement,, then do next. If the if worked, then do print key_info and next then print p15pubkey next which should run the sc_pkcs15_fix_ec_parameters then print p15pubkey

If the if at 1716 failed you will be at 1735, and you will have to step through it, by doing
run (and say yes to start from beginning.) the continue until you get to card-openpgp.c:1716
then set these: break pgp_get_blob and pgp_parse_algo_attr_blob then continue
You can then use continue to step through each call to pgp_get_blob to see which one failed. If it gets to pgp_parse_algo_attr_blob then step through it to see if it fails and why.

These instructions may be a little off, as I don't have a card that will go through code. But the "if" at 1721 is probably the problem.

@ghost
Copy link
Author

ghost commented Jan 22, 2020

Nope, there is no such function in logs. Will rebuild and install gdb tonight. Thanks!
log-readkey-ssh.log

@ghost
Copy link
Author

ghost commented Jan 22, 2020

I think it doesn't enter pgp_get_blob after :1721 at all. Or maybe I'm doing something wrong

gdb.log

@dengert
Copy link
Member

dengert commented Jan 22, 2020

I think we are close. (Your line numbers and mine may not match. the gdb list command can be used to show a number of source lines which helpful.)

1735 in card-openpgp.c
(gdb) print key_info
$1 = {key_id = 3 '\003', algorithm = 19 '\023', u = {rsa = {modulus = 0x0, modulus_len = 18446744073709551615, exponent = 0x300000001 <error: Cannot access memory at address 0x300000001>, exponent_len = 132, keyformat = 35 '#'}, ec = {
ecpoint = 0x0, ecpoint_len = 18446744073709551615, oid = {value = {1, 3, 132, 0, 35, -1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}, oid_len = 32 ' ', key_length = 521}}}

The key_info looks good, id=3, algorithm =0x13 and oid = {1, 3, 132, 0, 35} and key_length=521 match what is expected from the card. (ecpoint and ecpoint_len are not filled in yet.)

(gdb) next
1736 in card-openpgp.c
(gdb) print p15pubkey
$2 = {algorithm = 2, alg_id = 0x0, u = {rsa = {modulus = {data = 0x0, len = 0}, exponent = {data = 0x0, len = 0}}, dsa = {pub = {data = 0x0, len = 0}, p = {data = 0x0, len = 0}, q = {data = 0x0, len = 0}, g = {data = 0x0, len = 0}},
ec = {params = {named_curve = 0x0, id = {value = {0 <repeats 16 times>}}, der = {value = 0x5555555a81b0 "+\201\004", len = 5}, type = 0, field_length = 0}, ecpointQ = {
value = 0x5555555a8120 "\004\001\n\243\235\273\266\262\371\231,m\266\bϜ\220\201", len = 133}
}, gostr3410 = {params = {key = {value = {0 <repeats 16 times>}}, hash = {value = {0, 0, 1431994800, 21845, 5, 0, 0, 0, 0, 0,
1431994656, 21845, 133, 0, 0, 0}}, cipher = {value = {0 <repeats 16 times>}}}, xy = {data = 0x0, len = 0}}}}

The above looks good too. algorithm=SC_ALGORITHM_EC, (named_curve not filled in yet), the der value looks good, gdb only showed the first 3 bytes is asci with escaped octal
"+\201\004" == 0x2b, 0x81, 0x04 with length 5.

But in my line 1723 "&key_info.u.ec.oid) > 0) {" has an error.
Can you replace ">" with "==" so line 1724 sc_pkcs15_fix_ec_parameters will be executed to update the named_curve = 0x0 to string of "secp521r1" (some curves have multiple names).

I am assuming that you are expecting the 521 EC key.

I also need to do some cleanup, as p15pubkey may have some memory leaks.

@ghost
Copy link
Author

ghost commented Jan 23, 2020

Hm, I've changed the condition on 1723 to &key_info.u.ec.oid) == 0 and still named_curve is 0x0.

gdb-condition.log
gdb-fix-ec-params.log

@dengert
Copy link
Member

dengert commented Jan 23, 2020

The "gdb-fix-ec-params.log" does not show the output of sc_pkcs15_fix_ec_parameters. It only shows the backtrace that it is at the start of the routine. sc_pkcs15_fix_ec_parameters should udate the named_curve in the p15pubkey.

Note that GDB stops before executing the line at the breakpoint so you can examine input.
When you set the breakpoint to a function it is at the start of the function. In gdb type help running to see a list of commands that can be useful: next, step, finish, until. finish runs to the end of a function.

I also notice that when you run gdb, it can not find the source code.
1517 pkcs15-pubkey.c: No such file or directory.
This makes it harder to debug as it does not list the next line to be executed.
When I run gcb (Ubuntu-18.04) it finds the source (I do use VPATH to build in different directory. so source shows up like at ../../../src/src/libopensc/card-openpgp.c:1668

But gdb has a way to add the path to source directories. Google for: gnu gdb set source path.

(My build environment has been setup over years of programming and I am not sure what trick I used to get gdb to always find the source to work. )

I also have export CPPFLAGS=-DDEBUG Not sure if this makes a difference.

@ghost
Copy link
Author

ghost commented Jan 23, 2020

Sorry, the log wasn't full. I've added the code to help you understand line-by-line, placed a breakpoint on sc_pkcs15_fix_ec_parameters, tried running it with some steps where it passes the conditional and enters loop and then ran it to finish. It returned -1408 and didn't update named_curve... Maybe I'm not helpful enough =)
gdb.log

dengert added a commit to dengert/OpenSC that referenced this issue Jan 23, 2020
…SC#1906

The EC Parameters are the way the EC curve is presented to the outside world,
and in most cases is present in a matching certificate in the SPKI.

card-openpgp.c is modified to add the EC named_curve to the PKCS15 public key.
OpenPGP specs only provide this via the "Algorithm Attributes" for the 3 keys
via tags C1, C2 and C3 These contain the OID for the EC curve, which
is then mapped by sc_pkcs15_fix_ec_parameters to a EC Curve name.

This is the first attempt to fix this problem without having a token that
supports OpenGPG V3.3, so it will need additional testing and debugging by
developers who have such a token.

On branch openpgp-ec-pub-curve
 Changes to be committed:
	modified:   card-openpgp.c
@dengert
Copy link
Member

dengert commented Jan 23, 2020

I just pushed a minor fix which you have plus some debugging that would end up in opensc debug log. I also rebased on current master from today. There are a few changes to card-openpgp.c in current master that could make a difference.

Yes you are very helpful. What makes OpenSC is difficult, is not every developer has all the cards. So the only way to debug is like what we are doing now.

Can you run again with what you have or with the new patch based on the current master.
After the call to pgp_parse_algo_attr_blob can you
print key_info. It is not clear if the oid der value and length are correct.
Then after the sc_asn1_encode_object_id(&p15pubkey.u.ec.params.der.value,
1722 &p15pubkey.u.ec.params.der.len,
1723 &key_info.u.ec.oid

print p15pubkey.u.ec.params

The GDB x command can also be helpful with binary data.

Something like x/16xb p15pubkey.u.ec.params.der.value would show 16 bytes if binary.

the -1408 is SC_ERROR_NOT_SUPPORTED.
you can also get an opensc debug log while running GDB. That would show what it did not like.

@ghost
Copy link
Author

ghost commented Jan 23, 2020

opensc-new.log
Hope that helps. This is built from your repo.
gdb-new.log

@dengert
Copy link
Member

dengert commented Jan 23, 2020

I think I have it. Used the wrong encoding routine.

@dengert
Copy link
Member

dengert commented Jan 24, 2020

@unb9rn , can you try #1911 again with the latest update 118a22e that uses the correct sc_encode_oid Thanks.

@ghost
Copy link
Author

ghost commented Jan 25, 2020

It seems like it finally updates named_curve, yet the final result is still "unsupported curve"...
gdb-named_curve.log

@dengert
Copy link
Member

dengert commented Jan 25, 2020

Loos like the card-openpgp.c is working.
Line 1811 "Unsupported curve"is from pkcs15-tool.c:1014 I don't see why it is failing. Can you run again and also step pkcs15-tool.c at 1009 the loop before fprintf(stderr, "Unsupported curve\n");
Examine n during this. it should set n=521 on the 3rd time. Look at the ec_curves[]

Maybe set if (!n) to if (n != 0)`

Also try without gdb: pkcs15-tool --read-public-key 3 | openssl pkey -pubin -text

@szszszsz
Copy link

Sorry for being late for the party.
@dengert We (Nitrokey) can send you a Nitrokey Pro device, containing OpenPGP v3.3 smart card, to support the development for this ticket. I am writing here as well in case the email would not reach you.

@ghost
Copy link
Author

ghost commented Jan 25, 2020

Seems like it doesn't pass curve_name further...
n doesn't seem to change at all, made a few runs with printing n every iteration, it remains 0 all the time.
gdb-named_curve.log

@dengert
Copy link
Member

dengert commented Jan 26, 2020

I agree.

I think I see the problem I should have spotted this early.
Other card drivers call sc_pkcs15_encode_pubkey_as_spki.
In card-openpgp.c Can you change line 1735 from
r = sc_pkcs15_encode_pubkey(card->ctx, &p15pubkey, &data, &len);
to
r = sc_pkcs15_encode_pubkey-as-spki(card->ctx, &p15pubkey, &data, &len);

If that works, you can also try removing line 1724, because sc_pkcs15_encode_pubkey-as-spki
calls sc_pkcs15_fix_ec_parameters for EC pubkeys so we don't need to do it in card-openpgp.c

The term pubkey is used loosely. RSA does not have any parameters, most if not all other public keys have parameters. The SPKI in a certificate include the DER of a public key with parameters.

If that does not solve it, a few things to try.
print p15pubkey before and after sc_pkcs15_fix_ec_parameters
after line 1735 r = sc_pkcs15_encode_pubkey(card->ctx, &p15pubkey, &data, &len);
print p15pubkey
print len
x/??????xb data where ?????? is the decimal value of len. This will dump the ASN1 version of the pub key with the curve OID .

The before if (len > buf_len)
print len
print buf_len 8192 I think which is ok.

@dengert
Copy link
Member

dengert commented Jan 26, 2020

Sorry, change to:
r = sc_pkcs15_encode_pubkey_as_spki(card->ctx, &p15pubkey, &data, &len);

@ghost
Copy link
Author

ghost commented Jan 26, 2020

And we have a lift-off! =)
Replaced call on 1735 and it worked.
But it seems it breaks compatibility with RSA keys. With my other RSA Nitrokey it complains about "Public key enumeration failed: Required ASN.1 object not found"
It hapens with 1724 line both commented out and intact.

@dengert
Copy link
Member

dengert commented Jan 26, 2020

@unb9rn, I pushed another commit. If this works for you I will squash into one commit.
This will also need to be rebased on #1914 and should wait till after it is committed.

@ghost
Copy link
Author

ghost commented Jan 27, 2020

Almost everything is good. Both RSA and ECC are functioning, yet something is wrong- after returning SSH RSA key pkcs15-tool crashes with

free(): double free detected in tcache 2
Aborted (core dumped)

@dengert
Copy link
Member

dengert commented Jan 27, 2020

I have been able to test the card-openpgp.c with RSA keys with a Yubico 4 and to test the pkcs15-tool.c with EC using a PIV card. But can not test both pkcs15-tool and card-openpgp.c with the same card.

Thanks to Nitrokey, I should have a Nitrokey Pro in 2 to 3 weeks. so I can reproduce the problem.

If you are willing to continue debugging and testing possible fixes:

  • you run under gdb so when it crashes you can do a backtrace to see what is being freed twice.

  • try deleting the call to sc_pkcs15_erase_pubkey(&p15pubkey);

  • run using valgrind which should shho where the double free memory was allocate. It is real slow. Something like::

valgrind  -v --log-file=/tmp/valgrind.log.txt \
   --leak-check=full --leak-resolution=high \
   --show-reachable=yes \
   --num-callers=40 --track-origins=yes \
   gdb --args /opt/ossl-1.1/bin/pkcs15-tool --read-ssh-key 3

You can leave out the gdb --args if you want.

@ghost
Copy link
Author

ghost commented Jan 27, 2020

Haven't tried disabling function call yet, but here you go, valgrind logs with gdb bt
gdb-backtrace.log
valgrind.tar.gz

@dengert
Copy link
Member

dengert commented Jan 27, 2020

I think I messed up the valgrind and gdb commands so it did not run the command.

==38500== TO DEBUG THIS PROCESS USING GDB: start GDB like this
==38500== /path/to/gdb gdb
==38500== and then give GDB the following command
==38500== target remote | /usr/lib/valgrind/../../bin/vgdb --pid=38500
==38500== --pid is optional if only one valgrind process is running

I said: "You can leave out the gdb --args if you want." looks like just leave out the "gdb --args"
and valgrind should catch where and what was allocated and then double freed.

@dengert
Copy link
Member

dengert commented Jan 27, 2020

Looks like the double free comes while OpenSC is cleaning up and the card-openpgp.c is cleaning up the blob tree. But it does not show what blob is being freed. If valgrind can't point it out, we may need to use gdb and set break at card-openpgp.c:1166 i.e. sc_file_free(blob->file);

Then at each break:

 print *blob
 x/20xb blob->data
  next
  next 
  next 
 continue 

looks like it fails after 2 times, as the iteration count starts at 99 then goes down so we can catch which one is failing then try and figure out how that blob was created, and how the data could be double freed.

@ghost
Copy link
Author

ghost commented Jan 27, 2020

valgrind.log
gdb.log
How many steps should I continue through? Hope this helps...

@dengert
Copy link
Member

dengert commented Jan 27, 2020

Did this crash?
The previous run crashed after calling pgp_free_blob 3 times.
This one has not crashed after calling 5 times.

Is this some intermittent issue?
Does it crash for EC keys sometimes or every time
Does it crash for RSA?

Can you live with these changes and get SSH working for a few weeks, until I get the Nitrokey.
When I get it, I can try and reproduce your environment.

What commands did you use to create the keys on the token?
What OS and what version of the tools did you use.

Any other developers who have a OpenPGP device with EC keys but no certificates?

@ghost
Copy link
Author

ghost commented Jan 28, 2020

Nope, it didn't.
This is a reproducible issue-every call for rsa key (SSH or just pubkey) spews out the key itself and then sends SIGABRT. No such problem for EC keys at all.
It doesn't concern me very much, as even SSH itself works fine, yet just wanted to finalize this issue =)
The RSA keys were created with GPG some time ago on my Arch Linux laptop, the EC keys were generated directly (gpg generate) on another Nitrokey device (same model) with GPG 2.2.19 like a week ago on another laptop with Arch.

@ghost
Copy link
Author

ghost commented Jan 28, 2020

It takes like 30 iterations to crash
gdb-iter.log

@dengert
Copy link
Member

dengert commented Jan 28, 2020

Pushed another fix for dumb mistake onto #1911

1732         /* clean up anything we may have set in p15pubkey that can not be freed */
1733         if (p15pubkey.algorithm == SC_ALGORITHM_RSA)  {
1734                 p15pubkey.u.rsa.modulus.data  = NULL;
1735                 p15pubkey.u.rsa.modulus.len = 0;
1736                 p15pubkey.u.rsa.modulus.data  = NULL;
1737                 p15pubkey.u.rsa.exponent.len = 0;

Line 1736 should be
p15pubkey.u.rsa.exponent.data = NULL;

sc_pkcs15_erase_pubkey would free p15pubkey.u.rsa.exponent.data because it was not set to NULL, then later when trying to free blob info = ... <pgp34_objects+800>, id = 130 (0x82)

static struct do_info           pgp34_objects[] = {     /**** OpenPGP card spec 3.4 ****/
...
 { 0x0082, SIMPLE,      READ_ALWAYS | WRITE_NEVER, NULL,               NULL        },

Thus the same data would be freed.

@ghost
Copy link
Author

ghost commented Jan 29, 2020

Everything seems to work fine, both RSA and EC. Thanks a lot!

dengert added a commit to dengert/OpenSC that referenced this issue Jan 29, 2020
…SC#1906

The EC Parameters are the way the EC curve is presented to the outside world,
and in most cases is present in a matching certificate in the SPKI.

card-openpgp.c is modified to add the EC named_curve to the PKCS15 public key.
OpenPGP specs only provide this via the "Algorithm Attributes" for the 3 keys
via tags C1, C2 and C3 These contain the OID (not DER encoded) for the EC curve.

PKCS15 has two ways to encode a "pubkey" as it was originally written for RSA.
But other algorithms have parameters. X509 certificates encode the public key
in the SPKI and PKIX requires the parameters to be in the SPKI. PKCS15
allows for using a SPKI as source for a public key.

pgp_get_pubkey_pem will return the DER encoded RSA pubkey as before by
calling sc_pkcs15_encode_pubkey
pgp_get_pubkey_pem will return the DER encoded EC pubkey with parameters by
calling sc_pkcs15_encode_pubkey_as_spki which calls sc_pkcs15_fix_ec_parameters
internally to map DER encoded OID to named_curve.

For readability, "sc_pkcs15_pubkey_t pubkey;" definitions are changed to
"sc_pkcs15_pubkey_t p15pubkey;"

sc_pkcs15_erase_pubkey is used to avoid memory leaks.

 On branch openpgp-ec-pub-curve

 Date:      Tue Jan 21 09:43:56 2020 -0600
 Changes to be committed:
	modified:   src/libopensc/card-openpgp.c
@dengert
Copy link
Member

dengert commented Jan 29, 2020

@unb9rn Squashed all the commits to one and pushed to #1911
Please try this version and if it works make a comment to that effect on #1911

frankmorgner pushed a commit that referenced this issue Feb 1, 2020
The EC Parameters are the way the EC curve is presented to the outside world,
and in most cases is present in a matching certificate in the SPKI.

card-openpgp.c is modified to add the EC named_curve to the PKCS15 public key.
OpenPGP specs only provide this via the "Algorithm Attributes" for the 3 keys
via tags C1, C2 and C3 These contain the OID (not DER encoded) for the EC curve.

PKCS15 has two ways to encode a "pubkey" as it was originally written for RSA.
But other algorithms have parameters. X509 certificates encode the public key
in the SPKI and PKIX requires the parameters to be in the SPKI. PKCS15
allows for using a SPKI as source for a public key.

pgp_get_pubkey_pem will return the DER encoded RSA pubkey as before by
calling sc_pkcs15_encode_pubkey
pgp_get_pubkey_pem will return the DER encoded EC pubkey with parameters by
calling sc_pkcs15_encode_pubkey_as_spki which calls sc_pkcs15_fix_ec_parameters
internally to map DER encoded OID to named_curve.

For readability, "sc_pkcs15_pubkey_t pubkey;" definitions are changed to
"sc_pkcs15_pubkey_t p15pubkey;"

sc_pkcs15_erase_pubkey is used to avoid memory leaks.

 On branch openpgp-ec-pub-curve

 Date:      Tue Jan 21 09:43:56 2020 -0600
 Changes to be committed:
	modified:   src/libopensc/card-openpgp.c
@ghost
Copy link
Author

ghost commented Feb 2, 2020

Oh, I'm sorry for the delay, was out of town. Great to see it was merged. I can confirm everything works fine with binary build from master. Thanks!

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

Successfully merging a pull request may close this issue.

5 participants