OpenPGP 3.4+: Enable ed25519/curve25519 support #2286
Conversation
ya-isakov
changed the title
ya-isakov
force-pushed
the
master
branch
2 times, most recently
from
4f82c49 to
c821d47
Compare
|
In regards to: "derive should not be set on public key." I disagree. Other drivers in OpenSC set I can not find directly in PKCS11 if it is appropriate or not. But an EC private key can be used for Sign ECDSA or derive ECDH. X509 cert also have usage and PKCS15 also has usage. Policies may require some EC keys to be used only for signatures and others may only be used for derivation. If you don't allow for a usage of derive on public key, you can not tell from the public key if the private key is allowed for derive. We need a PKCS11 or other authoritative reference. |
|
Of course, @dengert is right. What idiot wrote a test that declares invalid "derive" for ECDH keys?! What Key Usage would you assign to such a key? |
src/libopensc/card-openpgp.c
Outdated
| {{{-1}}, 0} /* This entry must not be touched. */ | ||
| }; | ||
|
|
||
| /* v3.0+ supports: [RFC 4880 & 6637] 0x12 = ECDH, 0x13 = ECDSA */ |
There was a problem hiding this comment.
copy&paste comment which no longer reflects the reality.
There was a problem hiding this comment.
Thank you! And what is reality now? I thought that these RFCs are still valid
|
Appveyor problems are not related to my changes, it seems, but I don't know how to retrigger it... |
Contributions and reviews are always welcomed. |
|
I'm reworking OID comparison, as I don't like the idea of changing the function, which worked before. While researching on it, I've found a bug in parsing of last byte in Algorithm Attribute - if it's 00, which is a mark that no pubkey import is supported, it;s not ignored, and parsed as a part of oid. While fixing that, I realized that parse of Algorithm Attributes in DO 6E should be done, even if DO FA was parsed, as in Yubikey, DO FA entries has no last byte set to FF or 00 at all. So, to be on the safe side, I'm adding the logic that will parse DO 6E, and if algo was added previously, it will update it's keyformat (this part will be added in #2287). Not a fan of this solution, but it's better than to make some wrong assumptions. |
It's better to leave oid comparison as it was before, and drop trailing zero byte after it, when reading from token.
in response to OpenSC#2286 (comment) This removes the error message: fprintf(stderr, " [ ERROR %s ] Derive should not be set on public key\n", PKCS11-base-v2.40says: Table 22, Common Key Attributes Common key attributes: CKA_DERIVE | CK_BBOOL | CK_TRUE if key supports key derivation (i.e., if other keys can be derived from this one (default CK_FALSE) Table 24, Mapping of X.509 key usage flags to Cryptoki attributes for public keys: Key usage flags for public keys in X.509 public key certificates Corresponding cryptoki attributes for public keys. keyAgreement CKA_DERIVE
|
So, I reverted change of OID comparison logic, and instead, strip trailing zero byte, in case if it was read from DOs. Same logic as is used by gnupg (https://github.com/gpg/gnupg/blob/1c16878efd0bcf036f56abef93d64ac41ce9e95b/scd/app-openpgp.c#L5969-L5971 and https://github.com/gpg/gnupg/blob/1c16878efd0bcf036f56abef93d64ac41ce9e95b/scd/app-openpgp.c#L5969-L5971) |
|
@Jakuje I'm happy with algorithm now, so, should I wait for someone with merge rights to merge it? |
4 participants
This MR add support of 25519-series of curves to OpenSC. It extends support added in #1960.
Tested with Yubikey 5 NFC, Firmware 5.2.4.
It includes fix for Yubikey, which returns extra 0 at the end of OID in Algorithm Attribute DO. I've changed sc_compare_oid to return in it found -1 in any OID. This way, all trailing bytes will be ignored for comparison, including Import-Format.
p11test works, the only issue is with test_usages, which complains that derive should not be set on public key. I think, this should be an issue for all ECDH supporting tokens, as it's set in https://github.com/OpenSC/OpenSC/blob/master/src/libopensc/pkcs15-openpgp.c#L480 and https://github.com/OpenSC/OpenSC/blob/master/src/libopensc/pkcs15-openpgp.c#L484.
Checklist
P.S. Right now I'm just checking if version of card is more or equal to 3.4, but it seems, that there is a better way to detect list of supported algorithms, via Algorithm Information DO added in spec version 3.4. I've addressed that in #2287