[ounsworth]

Creating as a draft. This is very much not ready for review, but this is the easiest way for me to see the diff.

---

Plan

I'm planning to do this feature in stages:

1. Implement x25519, including all the new traits, cli and factory stuff for KeyAgreement. (this PR)
2. Add curve448 as a separate PR, stacked on 1.
3. Add the signature modes Ed25519 and Ed448 as a separate PR stacked on 2.
4. Integrate with David / Luis' ECDSA / ECDH work which I think will be a lot of de-duplicating the core elliptic curve and finite-field stuff into a bouncycastle-math crate, or something. (in theory, this could happen in parallel any time after 1)

---

How to review this PR

1. Start with the crate docs -- build cargo doc -p bouncycastle-ed. The sample code is where an end-user would start, and probably also where a reviewer should start.
2. new KeyAgreement traits in crypto/core/src/traits.rs.
3. Then crypto/ed/src/curve25519.rs. This is the core math. Don't worry about all the commented-out code that's waiting for ed25519; just skip it and it'll be in a future PR.
4. The public API is in crypto/ed/src/x25519.rs.
5. As you'll see in the docs sample code in x25519.rs, actually using the KeyAgreement trait to obtain a shared symmetric key is surprisingly tricky. So I wanted to impl the KEM traits over a DHKEM construction to hide the details, but then the question becomes: which DHKEM construction to use? RFC9180 HPKE is the only standardized one, so I decided to bite the bullet and add a new crate crypto/hpke that provides the generic DHKEM construction, and expose it by impl'ing KEMEcapsulator and KEMDecapsulator on the X25519 struct.
6. There's a file in there, crypto/ed/src/ed_todo.md that tracks all the stuff I've done. It's also tracking the x448, ed25519, and ed448 work that I want to add as separate PRs stacked on top of this one.

---

Open Questions

1. bouncycastle-ed is not a great name for the crate, because technically only the signature variants ed25519 / ed448 use Twisted Edwards curves while x25519 / x448 are Montgomery curves. That said, there doesn't seem to be a generally-accepted overarching term for {x25519, x448, ed25519, ed448}. Maybe xcurves or edcurves? I don't want to split them up into separate crates just because there doesn't exist a good name.

---

TODO before merging

• Delete crypto/ed/src/ed_todo.md.

[dghgit]

In bc-java we've found the formulation completely different - I don't think there's any common code between the Edwards and the Weierstrass form - the basis chosen is usually based on efficiency and simplicity as well, so while it's technically possible, it's unlikely to be worth it. I think we have a Weierstrass representation of Curve 25519 somewhere, but I think it's only their for compatibility (usage wise) with someone else's implementation...

[npajkovsky]

Can you do me a favor and add perf tests?

[dghgit]

sk in this context may vary - in the sense it could be unavailable to the software layer and hardware only. I'd suggest only passing pk, and having the key agreement constructed with sk, and what ever other parameters might be required (for what we're doing it might be useful to be able to apply KDFs as well).

This might actually be a good place for using KeyMaterial as well - ideally we'd make someone do extra work if they want to use a 128 bit secure secret to create a 256 bit shared secret.

[ounsworth]

Yes, I know that you want to suggest a refactor like that, and we'll get to that, but for now I'm making the new KeyAgreement trait match the existing KEMEncapsulator trait.

---

Do you mean having key_agreement(..) -> KeyMaterial instead of returning a DH public key?
I tried this, but:

1. A lot of things really do assume that the output of key_agreement is another public key. For example, the iterated tests in RFC 7748 section 5.2 work that way. Also, I think that group protocols (and maybe Signal / MLS) chain DH's together using the property that the output of DH is a usable public key.
2. If you are going to hash an X25519PublicKey to get a usable shared KeyMaterial, then which KDF do you use? And what do you include in the KDF? RFC7748 suggests all three values: K, K_A, K_B. But since DH is symmetric, the crypto library doesn't know who is Alice and who is Bob, so it's going to get the order of K_A, K_B wrong.
3. To do this robustly, you really need to wrap it in a standardized KEM, both to establish direction, to stardardize the inputs to the KDF, and which KDF to use. That's why I'm also adding a new crate crypto/hpke and using that to impl KEMEncryptor / KEMDecryptor on the X25519 object.

Does that make sense?

Also, I'm still actively building this. I only put up a PR so that I could see the diff. While I appreciate the enthusiasm, maybe everyone should hold off on review until I've finished thinking this through and it's ready to be reviewed.

[dghgit]

No, it doesn't really make sense.

Concerning the suggestion, yes, 6 times, now 7. We've already gotten to that re-factor so I don't really understand why something would be getting designed with the idea that we haven't - PR #170 does this and we even spent the afternoon reviewing it in Prague.

Also, ECCDH only returns one coordinate, at least as defined in FIPS, strictly speaking so does XDH, although the RFC does hint more is possible. It's possible to create multi-party agreement, but not the way that's suggested, it needs a phased approach so you can tell when the right number of parties have come through the door. For most things, such as HPKE, 2 party is enough. I would only start returning public keys if I was trying to implement multi-party up front, which is something, at the moment at least, I would definitely not suggest we do, but if I was we would need an API which reflected the need to provide phases - it's the reason the Java KeyAgreement API has phases.

Concerning KeyMaterial, I noted that seemed to be the first thing being produced in the doc code. The security strength was correct, but we'd be able to take the guess work out if we simply returned it.

[ounsworth]

Can you do me a favor and add perf tests?

Yes that's on my TODO list! That'll be in place before I mark this PR ready for review.

---

UPDATE: done

KeyGen/X25519          
                        time:   [1.2698 ms 1.2719 ms 1.2742 ms]
                        thrpt:  [25.113 Kelem/s 25.159 Kelem/s 25.200 Kelem/s]

KeyAgreement/X25519
                        time:   [1.2705 ms 1.2815 ms 1.2943 ms]
                        thrpt:  [24.724 Kelem/s 24.972 Kelem/s 25.188 Kelem/s]

Encaps/DHKEM(X25519, HKDF-SHA256)
                        time:   [2.6005 ms 2.6078 ms 2.6164 ms]
                        thrpt:  [12.231 Kelem/s 12.271 Kelem/s 12.305 Kelem/s]

Decaps/DHKEM(X25519, HKDF-SHA256)
                        time:   [1.3371 ms 1.3405 ms 1.3445 ms]
                        thrpt:  [23.800 Kelem/s 23.872 Kelem/s 23.933 Kelem/s]

Everything is about the same (basically a single x25519 scalar mult), except the KEM Encaps, which is a KeyGen + a KeyAgreement, so is about 1/2. The DHKEM's also include a round of HKDF to derive the output shared secret. Neat that that is not really noticeable next to the core x25519 op.

[romen]

This felt surprising, is it really needed as part of the public API?

[ounsworth]

Maybe not. Let me try removing it.

[ounsworth]

Removed!

[romen]

All but the doc comment bit on the struct!