OpenPGP: Support for Curve25519, Ed25519, X448 and Ed448

[vanitasvitae]

Hey!

As far as I can tell, BC is missing support for some algorithms. Also, there are ambiguities regarding the used curve OIDs. Here are my observations, note that I'm not an expert, so my observations might be wrong:

BCs Curve Constants

I'm comparing the curve OIDs to those from crypto-refresh-05:

RFC 4880 Algo Name	BC OID Constant	Value
Curve25519 (ECDH)	CryptlibObjectIdentifiers.curvey25519	1.3.6.1.4.1.3029.1.5.1
not used	EdECObjectIdentifiers.id_X25519	1.3.101.110
X448 (ECDH)	EdECObjectIdentifiers.id_X448	1.3.101.111
not used	EdECObjectIdentifiers.id_Ed25519	1.3.101.112
Ed448 (EdDSA)	EdECObjectIdentifiers.id_Ed448	1.3.101.113
Ed25519 (EdDSA)	GNUObjectIdentifiers.Ed25519	1.3.6.1.4.1.11591.15.1

JcaPGPKeyConverter

Algorithm	getPrivateKey()	getPublicKey()	getPrivateBCPGKey()	getPublicBCPGKey()
Curve25519 (ECDH)	✅ 1	✅ 2	Probably? 3	✔️
X448 (ECDH)	❌	❌	Probably not? 3	❌
Ed25519 (EdDSA)	✔️ 4	✔️ 5	Maybe? 6	✔️
Ed448 (EdDSA)	❌	❌	Probably not? 6	❌

BcPGPKeyConverter

Algorithm	getPrivateKey()	getPublicKey()	getPrivateBCPGKey()	getPublicBCPGKey()
Curve25519 (ECDH)	✅ 7	✅ 8	✅ 9	✅ 10
X448 (ECDH)	❌	❌	❌	✔️
Ed25519 (EdDSA)	✅ 11	✔️ 12	✔️	✔️
Ed448 (EdDSA)	✔️	✔️	✔️	✔️

Footnotes

1. Comments mention XDH, refer to EdECObjectIdentifiers.id_X25519, should use CryptlibObjectIdentifiers.curvey25519? (src) ↩

2. Refers to EdECObjectIdentifiers.id_X25519 in method call, should use CryptlibObjectIdentifiers.curvey25519? (src) ↩

3. Source is not clear, does not compare OIDs (src) ↩ ↩²

4. Refers to EdECObjectIdentifiers.id_Ed25519, should use GNUObjectIdentifiers.Ed25519? (src) ↩

5. Refers to EdECObjectIdentifiers.id_Ed25519, should use CryptlibObjectIdentifiers.curvey25519? (src) ↩

6. Source is not clear, does not compare OIDs (src) ↩ ↩²

7. Comments mention X25519, refers to EdECObjectIdentifiers.id_X25519, should use CryptlibObjectIdentifiers.curvey25519? (src) ↩

8. Refers to EdECObjectIdentifiers.id_X25519, should use CryptlibObjectIdentifiers.curvey25519? (src) ↩

9. Comment and code mention "X25519", not clear if it uses Ed25519 ↩

10. pubkey instanceof X25519PublicKeyParameters, is this correct for Curve25519? ↩

11. Refers to EdECObjectIdentifiers.id_Ed25519, should use GNUObjectIdentifiers.Ed25519? (src) ↩

12. If statement is hard to read (src) ↩

[vanitasvitae]

Although ECDH over Curve25519 and EdDSA over Ed25519 apparently use the wrong OIDs in some cases, working with those keys does not present any obvious problems for now. I'm still pointing it out here, as I cannot follow the code.

[vanitasvitae]

The fix implemented for #1087 by the way also uses the wrong curve names.
PGPUtil.getCurveName(oid) does not recognize 1.3.6.1.4.1.11591.15.1, which is the OID for Ed25519. Instead, the map used in that method contains unrelated curves afaict.

I will reopen #1087

[vanitasvitae]

Any news on this?
In an attempt to tackle this myself, I tried replacing faulty OIDs with the ones from the standards document, but that broke everything :D

So this is above my head unfortunately :( Let me know if I can help this advance in any way though :)

[vanitasvitae]

I'm reusing this issue tracker to report about some observations I made.

I'm currently working on making sure that X448, Ed448 are ready to be used with OpenPGP.
There are some places in the code, where support appears to be lacking, especially in the *PgpKeyConverter classes.

The PGPKeyConverter base class does not support X448 in implGetKdfParameters(). Here, we need to return SHA512, AES256 according to https://www.ietf.org/archive/id/draft-ietf-openpgp-crypto-refresh-13.html#name-algorithm-specific-fields-for-x

Both the JcaPgpKeyConverter and BcPgpKeyConverter class trip over X448 keys (see also #1584 ).
I noticed, that oftentimes the nature of a passed in key is determined by use of instanceof and I wonder, why we don't use the algorithm id instead?

In some places, the current implementation returns an instance of the new dedicated BCPGKey classes intended for X448/X25519/Ed448/Ed25519, even if the algorithm ID is ECDH or EdDSA_LEGACY. In my experimental branch I fixed this, such that the dedicated classes are only used together with the dedicated algorithm IDs.

In a test where I convert between JcaPgpKeyPair and BcPgpKeyPair objects I noticed, that the underlying keypair implementation oftentimes changes in a way that makes detecting the actual curve very hard. For example, an X25519 key may have a sun.security.ec.XDHPublicKeyImpl key which suddenly no longer reports "X25519" as algorithm name so matching on "X2" no longer works, but instead returns "XDH" only, so we have to inspect the length of the keys encoding, etc.

I will continue to work on my experimental branch and write more tests to ensure correct functionality and then reimplement my patches in a clean, comprehensible way to create a PR.

[vanitasvitae]

I created #1658 to add X448 support to PGPKeyConverter.implGetKdfParameters()

[vanitasvitae]

In a test where I convert between JcaPgpKeyPair and BcPgpKeyPair objects I noticed, that the underlying keypair implementation oftentimes changes in a way that makes detecting the actual curve very hard. For example, an X25519 key may have a sun.security.ec.XDHPublicKeyImpl key which suddenly no longer reports "X25519" as algorithm name so matching on "X2" no longer works, but instead returns "XDH" only, so we have to inspect the length of the keys encoding, etc.

I figured out, that the underlying key class changes depending on whether JcaPGPKeyConverter.setProvider(new BouncyCastleProvider()) was called or not.

For X25519, X448, Ed25519, Ed448 I got key conversion working, regardless of which provider is used. However, for EC keys over prime256v1, P-256, conversion fails without the BC provider.