Add ed448 #1121

mikelodder7 · 2025-02-14T18:00:38Z

This adds the Ed448-Goldilocks curve and signing implementation from the latest commit at Mikelodder7/Ed448-Goldilocks.

ed448/rustfmt.toml

dignifiedquire · 2025-02-14T18:22:51Z

ed448/src/curve/montgomery.rs

+// The original file was a part of curve25519-dalek.
+// Copyright (c) 2016-2019 Isis Lovecruft, Henry de Valence
+// Copyright (c) 2020 Kevaundray Wedderburn
+// See LICENSE for licensing information.


lets add a reference to the actual licnes file from curve25519-dalek

dignifiedquire · 2025-02-14T18:25:58Z

@tarcieri @mikelodder7 I am not seeing much in terms of test vectors or the likes, do you happen to know where to find some?

dignifiedquire · 2025-02-14T19:48:10Z

working on a pr to update to the current set of rustcrypto deps

mikelodder7 · 2025-02-14T19:58:16Z

Co authors
@divergentdave
@conradoplg
@stackinspector
@anonion0
@kevaundray

mikelodder7 · 2025-02-14T19:59:22Z

@tarcieri @mikelodder7 I am not seeing much in terms of test vectors or the likes, do you happen to know where to find some?

Here's the Ed448 test vectors

https://github.com/RustCrypto/elliptic-curves/pull/1121/files#diff-16bd59631ab39d33a42cd634a7603565919c4243d9d2b21b3d975d194457f22bR351

dignifiedquire · 2025-02-14T20:27:34Z

created mikelodder7#1

mikelodder7 · 2025-02-14T21:11:01Z

created mikelodder7#1

👀

dkg · 2025-03-04T22:14:47Z

what's the status on this? It would be great to see ed448 under the RustCrypto umbrella.

tarcieri · 2025-03-14T18:10:17Z

@dkg waiting on @mikelodder7 to rebase and get the tests passing

Signed-off-by: Michael Lodder <[email protected]>

stackinspector · 2025-03-17T20:22:23Z

There's a little stuff about my previous contribution that I feel the need to renegotiate before the code gets into a serious place like RustCrypto.
The hex_to_field (example) in test cases, which I originally implemented with macro_rules, passes string literial as a macro argument to hex-literial::hex!. But the maintainer of the original repository preferred hex_to_field as a function. But the problem is that hex-literial::hex! can only accept string literials, and the function hex-literial::decode, which can accept &'static str directly, is an internal API, even though we can do the checks that should be done, the API is theoretically unstable! Some of the other drawbacks I explained in the comment at the time. At final the function version was merged, but I still prefer to switch back to the macro_rules implementation and pass literals.

Signed-off-by: Michael Lodder <[email protected]>

tarcieri · 2025-03-17T21:35:41Z

ed448/Cargo.toml

+homepage = "https://docs.rs/ed448-goldilocks/"
+keywords = ["cryptography", "decaf", "ed448", "ed448-goldilocks"]
+license = "BSD-3-Clause"
+name = "ed448"


If we keep the name as ed448-goldilocks, it will allow the ed448 name to be used for the signature abstraction crate, which would be consistent with the ed25519 crate

(and that also ensures existing users get updates automatically without having to change to a new name)

Another option for a name is curve448. We could extract the EdDSA implementation/instantiation into ed448 at that point, which would follow how curve25519-dalek/ed25519-dalek are structured.

dignifiedquire · 2025-04-05T09:17:39Z

@tarcieri what's the blockers for moving this forward, given CI is green now?

dignifiedquire · 2025-04-05T09:22:31Z

@tarcieri what would be the best place for me to put a x448 implementation based on this?

kevaundray · 2025-04-05T10:58:56Z

@tarcieri what would be the best place for me to put a x448 implementation based on this?

I have an impl here as a reference btw: https://github.com/crate-crypto/x448

dignifiedquire · 2025-04-05T11:12:37Z

@kevaundray very nice, so I guess we just need to update that to the latest version of this 448 crate. Do you want to keep that crate under your management and update it, or should we move it into rustcrypto as well?

kevaundray · 2025-04-05T11:46:39Z

@kevaundray very nice, so I guess we just need to update that to the latest version of this 448 crate. Do you want to keep that crate under your management and update it, or should we move it into rustcrypto as well?

I think it would make sense to move it to rustcrypto as well for completeness if you haven't made an implementation

tarcieri · 2025-04-07T01:02:29Z

@dignifiedquire I'd like to do a final review and won't have time to do that for about a week

dignifiedquire · 2025-04-09T18:41:33Z

ed448/Cargo.toml

+
+[dependencies]
+crypto-bigint = { version = "0.6.0-rc.3", features = ["hybrid-array"], default-features = false }
+crypto_signature = { version = "2.3.0-pre.6", default-features = false, features = ["digest", "rand_core"], optional = true, package = "signature" }


if we put this on the pre release for signature and [email protected] we will run into the issue that this is going to be unusable as in RustCrypto/signatures#930

Let’s get this merged as-is and follow up on that

It’s also not the same problem as this crate has existing releases with older deps whereas ml-dsa/slh-dsa do not

fyi I made https://github.com/dignifiedquire/cx448/ based on this code, which I will publish for now, so I can use a more up to date version

dignifiedquire · 2025-04-09T18:48:12Z

ed448/Cargo.toml

+elliptic-curve = { version = "0.14.0-rc.0", features = ["arithmetic", "bits", "hash2curve", "jwk", "pkcs8", "pem", "sec1"] }
+pkcs8 = { version = "0.10", features = ["alloc"], optional = true }
+rand_core = { version = "0.9", default-features = false }
+serdect = { version = "0.3.0", optional = true }


looks like this needs to be a prerelease version to compile

I tried updating x448 to use this version and am getting this

error: failed to select a version for `serdect`. ... required by package `elliptic-curve v0.14.0-rc.0` ... which satisfies dependency `elliptic-curve = "^0.14.0-rc.0"` of package `ed448 v0.17.0-pre.0 (https://github.com/mikelodder7/elliptic-curves#363ed9dc)` ... which satisfies git dependency `ed448` of package `x448 v0.6.0 (/Users/dignified/opensource/x448)` versions that meet the requirements `=0.3.0-pre.0` are: 0.3.0-pre.0 all possible versions conflict with previously selected packages. previously selected package `serdect v0.3.0` ... which satisfies dependency `serdect = "^0.3.0"` of package `ed448 v0.17.0-pre.0 (https://github.com/mikelodder7/elliptic-curves#363ed9dc)` ... which satisfies git dependency `ed448` of package `x448 v0.6.0 (/Users/dignified/opensource/x448)`

with these dependencies

ed448 = { git = "https://github.com/mikelodder7/elliptic-curves", features = ["zeroize"] } rand_core = { version = "0.9", default-features = false, features = ["getrandom"] } zeroize = { version = "1.8", default-features = false, optional = true }

For now you'll also need:

[patch.crates-io] elliptic-curve = { git = "https://github.com/RustCrypto/traits.git" }

(also rand_core v0.9 doesn't have a getrandom feature, it's os_rng)

Why not use rand_core 0.8, rand_core introduced more dependencies.

daxpedda

I tried to use this to implement decaf448 for voprf, but wasn't able to get things to match the test vectors (see the specification).

I played around with it for quite a while and eventually got it working, see mikelodder7#2. But unfortunately this broke a bunch of tests, no surprise: I'm not a cryptographer and I don't know what I'm doing.

I also added a test with a single test vector from the OPRF specification just to show how and where it breaks. I'm happy to add a full OPRF test vector suite as in #503, #600 and #964 when it passes.

daxpedda · 2025-04-11T00:38:43Z

ed448/src/field/scalar.rs

+    /// Construct a `Scalar` from a little-endian byte representation.
+    pub fn from_bytes(bytes: &[u8; 56]) -> Scalar {
+        Self(U448::from_le_slice(bytes))
+    }


This can produce invalid Scalars. Just missing modulo ORDER.

daxpedda · 2025-04-11T00:42:10Z

ed448/src/field/scalar.rs

+    pub fn hash<X>(msg: &[u8], dst: &[u8]) -> Self
+    where
+        X: for<'a> ExpandMsg<'a>,
+    {
+        let mut random_bytes = Array::<u8, U84>::default();
+        let dst = [dst];
+        let mut expander =
+            X::expand_message(&[msg], &dst, random_bytes.len()).expect("invalid dst");
+        expander.fill_bytes(&mut random_bytes);
+        Self::from_okm(&random_bytes)
+    }


The specification says to use 64 bytes here. An public method taking 64 bytes, like Scalar::from_bytes_mod_order_wide(), would probably be useful as well.

My assessment was incorrect here, the specification part I linked here is not related to the hash_to_field implementation.

daxpedda · 2025-04-11T00:47:08Z

ed448/src/constants.rs

+pub const DECAF_BASEPOINT: DecafPoint = DecafPoint(curve::twedwards::extended::ExtendedPoint {
+    X: TWISTED_EDWARDS_BASE_POINT.X,
+    Y: TWISTED_EDWARDS_BASE_POINT.Y,
+    Z: TWISTED_EDWARDS_BASE_POINT.Z,
+    T: TWISTED_EDWARDS_BASE_POINT.T,
+});


While I have no idea why the twisted curve is used for the internal representation, at least the public DecafPoint::GENERATOR has to be correct.

Suggested change

pub const DECAF_BASEPOINT: DecafPoint = DecafPoint(curve::twedwards::extended::ExtendedPoint {

X: TWISTED_EDWARDS_BASE_POINT.X,

Y: TWISTED_EDWARDS_BASE_POINT.Y,

Z: TWISTED_EDWARDS_BASE_POINT.Z,

T: TWISTED_EDWARDS_BASE_POINT.T,

});

pub const DECAF_BASEPOINT: DecafPoint = DecafPoint(curve::twedwards::extended::ExtendedPoint {

X: FieldElement(ConstMontyType::new(&U448::from_be_hex("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa955555555555555555555555555555555555555555555555555555555"))),

Y: FieldElement(ConstMontyType::new(&U448::from_be_hex("ae05e9634ad7048db359d6205086c2b0036ed7a035884dd7b7e36d728ad8c4b80d6565833a2a3098bbbcb2bed1cda06bdaeafbcdea9386ed"))),

Z: FieldElement(ConstMontyType::new(&U448::from_u64(1))),

T: FieldElement(ConstMontyType::new(&U448::from_be_hex("696d84643374bace9d70983a12aa9d461da74d2d5c35e8d97ba72c3aba4450a5d29274229bd22c1d5e3a6474ee4ffb0e7a9e200a28eee402"))),

});

I got the definition of the base point from the specification.

This is covered in mikelodder7#2 (comment) as well.

daxpedda · 2025-04-11T20:35:10Z

ed448/src/field/scalar.rs

+}
+
+impl FromOkm for Scalar {
+    type Length = U84;


This should be 64 bytes as well, see #1121 (comment).

As pointed out in #1121 (comment), this is wrong.

Main motivation was that `decaf448` (RustCrypto/elliptic-curves#1121) requires `ExpandMsgXof` instead of `ExpandMsgXmd`. I was briefly tempted to add a `type Hash` to the `ExpandMsg` trait to be able to write `type ExpandMsg = ExpandMsg<Hash = Self::Hash>`. Otherwise it is possible to add a different hash in the `ExpandMsg` type: ```rust impl VoprfParamters for Fun128 { type Hash = Sha512; type ExpandMsg = ExpandMsgXmd<Sha256>; ... } ``` But I thought its a bit much. Let me know if you want me to add that.

baloo · 2025-05-29T04:26:19Z

I took a swab at a rebase of this PR in #1208 just because I needed ed448. I ended up converting some of it to primeorder.

Hopefully that can help with getting this merged.

tarcieri · 2025-05-29T13:43:18Z

I ended up converting some of it to primeorder

That's a little weird since ed(wards)448 isn't a prime order curve

mikelodder7 mentioned this pull request Feb 14, 2025

upstream this repo to RustCrypto crate-crypto/Ed448-Goldilocks#35

Open

dignifiedquire reviewed Feb 14, 2025

View reviewed changes

ed448/rustfmt.toml Outdated Show resolved Hide resolved

dignifiedquire reviewed Feb 14, 2025

View reviewed changes

hko-s mentioned this pull request Feb 22, 2025

Add support for Ed448 when there's a usable Rust crate rpgp/rpgp#418

Closed

tarcieri self-requested a review March 4, 2025 22:46

mikelodder7 added 3 commits March 17, 2025 11:36

add ed448

6434c72

Signed-off-by: Michael Lodder <[email protected]>

update rustfmt.toml

6485a34

Signed-off-by: Michael Lodder <[email protected]>

fixes to scalar

2778ed8

Signed-off-by: Michael Lodder <[email protected]>

mikelodder7 force-pushed the master branch from 1037ac1 to 2778ed8 Compare March 17, 2025 17:37

mikelodder7 added 2 commits March 17, 2025 13:05

fix tests and compile issues

5150f16

Signed-off-by: Michael Lodder <[email protected]>

add ed448 workflow

6b1a0d2

Signed-off-by: Michael Lodder <[email protected]>

mikelodder7 force-pushed the master branch from 5ef69d9 to 6b1a0d2 Compare March 17, 2025 19:08

mikelodder7 added 2 commits March 17, 2025 13:37

update to no_std

6d0823a

Signed-off-by: Michael Lodder <[email protected]>

clean up warnings

5dd6659

Signed-off-by: Michael Lodder <[email protected]>

mikelodder7 added 2 commits March 17, 2025 14:36

update workflows

6ec5b9c

Signed-off-by: Michael Lodder <[email protected]>

clippy

363ed9d

Signed-off-by: Michael Lodder <[email protected]>

tarcieri reviewed Mar 17, 2025

View reviewed changes

dignifiedquire mentioned this pull request Apr 5, 2025

feat: implement PQC draft 08 rpgp/rpgp#518

Merged

12 tasks

dignifiedquire reviewed Apr 9, 2025

View reviewed changes

daxpedda reviewed Apr 11, 2025

View reviewed changes

This was referenced Apr 11, 2025

Add type ExpandMsg to VoprfParameters RustCrypto/traits#1814

Merged

Add decaf448 support facebook/voprf#139

Draft

daxpedda mentioned this pull request May 4, 2025

SIGMA-I Key Exchange facebook/opaque-ke#378

Merged

baloo mentioned this pull request May 29, 2025

Add ed448 #1208

Closed

Add ed448 #1121

Are you sure you want to change the base?

Add ed448 #1121

Uh oh!

Conversation

mikelodder7 commented Feb 14, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dignifiedquire commented Feb 14, 2025

Uh oh!

dignifiedquire commented Feb 14, 2025

Uh oh!

mikelodder7 commented Feb 14, 2025

Uh oh!

mikelodder7 commented Feb 14, 2025

Uh oh!

dignifiedquire commented Feb 14, 2025

Uh oh!

mikelodder7 commented Feb 14, 2025

Uh oh!

dkg commented Mar 4, 2025

Uh oh!

tarcieri commented Mar 14, 2025

Uh oh!

stackinspector commented Mar 17, 2025

Uh oh!

tarcieri Mar 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tarcieri Mar 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dignifiedquire commented Apr 5, 2025

Uh oh!

dignifiedquire commented Apr 5, 2025

Uh oh!

kevaundray commented Apr 5, 2025

Uh oh!

dignifiedquire commented Apr 5, 2025

Uh oh!

kevaundray commented Apr 5, 2025

Uh oh!

tarcieri commented Apr 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daxpedda left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daxpedda Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

tarcieri Mar 17, 2025 •

edited

Loading

tarcieri Mar 17, 2025 •

edited

Loading

daxpedda Apr 11, 2025 •

edited

Loading

baloo commented May 29, 2025 •

edited

Loading