Ed25519 support #120

ctz · 2025-11-23T09:56:54Z

This is work-in-progress Ed25519 support. Much of the work is thanks to @phlip9 in #74

Draft while I do these items:

review coverage
self-review and write this list

fixes #74

Ex: `#define foo() bl edwards25519_decode_alt_mul_p25519` Before this change, the generated rust macro didn't wire up the local label reference properly: ```rust macro_rules! foo { () => { Q!( "bl " Label!("edwards25519_decode_alt_mul_p25519", 0, Before) )} } ``` After this change: ```rust macro_rules! foo { () => { Q!( "bl " Label!("edwards25519_decode_alt_mul_p25519", 4, After) )} } ``` To support this, we now track which macros reference labels. We then track which blocks call these macros. As long as all macro callsites are uniformly before or after the labels they reference, we can safely replace the local label id and search direction in the original macro definition. It was challenging to track macro callsites in the main `RustFormatter` and also difficult to "pre-locate" callsites while dealing with hoisting, so the actual macro fixing happens on the generated Rust code from the `RustFormatter` pass.

codecov · 2025-11-23T10:00:33Z

Codecov Report

❌ Patch coverage is 99.59641% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 99.31%. Comparing base (9796ac5) to head (01938d4).
⚠️ Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
rustls-graviola/src/sign.rs	83.87%	5 Missing ⚠️
graviola/src/mid/ed25519.rs	99.25%	2 Missing ⚠️
graviola/src/error.rs	0.00%	1 Missing ⚠️
graviola/src/high/ed25519.rs	98.96%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #120      +/-   ##
==========================================
+ Coverage   99.14%   99.31%   +0.17%     
==========================================
  Files         171      185      +14     
  Lines       39165    50337   +11172     
==========================================
+ Hits        38830    49994   +11164     
- Misses        335      343       +8

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codspeed-hq · 2025-11-23T10:00:40Z

CodSpeed Performance Report

Merging #120 will not alter performance

_{Comparing jbp-ed25519 (01938d4) with main (9796ac5)}

Summary

✅ 143 untouched
🆕 12 new

Benchmarks breakdown

	Benchmark	`BASE`	`HEAD`	Change
🆕	`aws-lc-rs`	N/A	77.5 µs	N/A
🆕	`dalek`	N/A	112.9 µs	N/A
🆕	`graviola`	N/A	62.5 µs	N/A
🆕	`ring`	N/A	116.3 µs	N/A
🆕	`aws-lc-rs`	N/A	82.7 µs	N/A
🆕	`dalek`	N/A	115.9 µs	N/A
🆕	`graviola`	N/A	62.7 µs	N/A
🆕	`ring`	N/A	122.5 µs	N/A
🆕	`aws-lc-rs`	N/A	195.7 µs	N/A
🆕	`dalek`	N/A	149.8 µs	N/A
🆕	`graviola`	N/A	141 µs	N/A
🆕	`ring`	N/A	233.7 µs	N/A

phlip9

Thanks a ton for bringing this over the finish line! And I apologize for the mess of WIP commits 😅

Figured I would leave some comments as a potential user of both rustls-graviola and graviola::signing::eddsa

phlip9 · 2025-11-25T16:10:42Z

graviola/src/high/ed25519.rs

+
+        pkcs8::Key::decode(bytes, &asn1::oid::id_ed25519, None)
+            .and_then(|k| asn1::OctetString::from_bytes(k.private_key()).map_err(Error::Asn1Error))
+            .and_then(|pk| Self::from_bytes(pk.as_octets()))


would be nice to sanity check that the serialized pubkey in the pkcs8 document matches the derived compressed pubkey

https://github.com/briansmith/ring/blob/main/src/ec/curve25519/ed25519/signing.rs#L136

phlip9 · 2025-11-25T17:58:48Z

graviola/src/high/ed25519.rs

+    pub fn public_key(&self) -> Ed25519VerifyingKey {
+        let _entry = Entry::new_secret();
+        Ed25519VerifyingKey(self.0.verifying_key())
+    }


This is obviously committing to the signing key containing the decompressed verifying key, but it would be huge if this could just return a &Ed25519VerifyingKey and not have to clear registers:

impl Ed25519SigningKey { pub fn public_key(&self) -> &Ed25519VerifyingKey { Ed25519VerifyingKey::from_ref(self.0.verifying_key()) } } /// An Ed25519 verification public key. #[derive(Debug)] #[repr(transparent)] pub struct Ed25519VerifyingKey(ed25519::VerifyingKey); impl Ed25519VerifyingKey { #[inline] const fn from_ref(vk: &ed25519::VerifyingKey) -> &Self { // Assert that both types have the same layout const _: [(); std::mem::size_of::<Self>()] = [(); std::mem::size_of::<ed25519::VerifyingKey>()]; const _: [(); std::mem::align_of::<Self>()] = [(); std::mem::align_of::<ed25519::VerifyingKey>()]; // SAFETY: Both types have the same layout and the newtype uses // #[repr(transparent)], so their references are equivalent. // See <https://docs.rs/ref_cast>. unsafe { &*(vk as *const ed25519::VerifyingKey as *const Self) } } }

phlip9 · 2025-11-25T18:39:35Z

graviola/src/high/ed25519.rs

+            .try_into()
+            .map(|seed| Self(ed25519::SigningKey::from_seed(&seed)))
+            .map_err(|_| Error::WrongLength)
+    }


somewhat unfortunate that this fn has to become fallible when in principle it's infallible

pub fn from_seed(bytes: &[u8; 32]) -> Self { /* .. */ }

the struggles of API stability 😢

phlip9 · 2025-11-25T18:46:40Z

graviola/src/high/ed25519.rs

+        let _entry = Entry::new_secret();
+        Ed25519VerifyingKey(self.0.verifying_key())
+    }
+}


likewise, if I can get access to the seed after, that would be amazing:

impl Ed25519SigningKey { #[inline] pub fn as_seed(&self) -> &[u8; 32] { /* .. */ } }

this is a problem I have with ring's interface; it doesn't expose the seed, which means I have to duplicate it outside: https://github.com/lexe-app/lexe-public/blob/master/common/src/ed25519.rs#L77

/// An ed25519 secret key and public key. /// /// Applications should always sign with a *key pair* rather than passing in /// the secret key and public key separately, to avoid attacks like /// [attacker controlled pubkey signing](https://github.com/MystenLabs/ed25519-unsafe-libs). pub struct KeyPair { /// The ring key pair for actually signing things. key_pair: ring::signature::Ed25519KeyPair, /// Unfortunately, [`ring`] doesn't expose the `seed` after construction, /// so we need to hold on to the seed if we ever need to serialize the key /// pair later. seed: [u8; 32], }

phlip9 · 2025-11-25T19:10:06Z

graviola/src/mid/ed25519.rs

+
+impl SigningKey {
+    pub(crate) fn from_seed(seed: &[u8; 32]) -> Self {
+        let _entry = low::Entry::new_secret();


does this still need let _entry = low::Entry::new_secret();?

phlip9 · 2025-11-25T19:14:24Z

graviola/src/mid/ed25519.rs

+        low::ct::into_public(sig)
+    }
+
+    pub(crate) fn verifying_key(&self) -> VerifyingKey {


this can probably return a &VerifyingKey now

phlip9 · 2025-11-25T19:16:26Z

graviola/src/mid/ed25519.rs

+
+        let mut sig = [0u8; 64];
+        sig[0..32].clone_from_slice(&sig_r.0);
+        sig[32..64].clone_from_slice(&s.to_le_bytes());


minor: the compiler specializes this correctly, but .copy_from_slice(..) is less surprising as a reader

phlip9 · 2025-11-25T19:17:22Z

graviola/src/mid/ed25519.rs

+        low::zeroise(&mut self.prefix);
+        // clearly not necessary, but makes zeroisation easier to test
+        low::zeroise(&mut self.verifying_key.point.0);
+        low::zeroise(&mut self.verifying_key.bytes);


minor: could gate zeroising public data on #[cfg(debug_assertions)]

phlip9 · 2025-11-25T21:02:05Z

graviola/src/mid/ed25519.rs

+
+    /// `PureEd25519` signature verification
+    pub(crate) fn verify(&self, sig: &[u8; 64], msg: &[u8]) -> Result<(), Error> {
+        let _entry = low::Entry::new_public();


likewise, does this need entry if it's not a public API?

ctz and others added 25 commits November 23, 2025 09:32

parse-asm: reformat

14a03b9

ed25519: x86_64: edwards25519_decode codegen

8a65063

ed25519: aarch64: edwards25519_decode codegen

ad41eb0

ed25519: tiny test for edwards25519_decode to sanity check

88c7637

ed25519: add edwards25519_encode in Rust

336638d

ed25519: x86_64: edwards25519_scalarmulbase codegen

5dd2ce1

ed25519: aarch64: edwards25519_scalarmulbase codegen

316e7c5

ed25519: x86_64: bignum_mod_n25519 codegen

7531836

ed25519: aarch64: bignum_mod_n25519 codegen

e2f0d62

ed25519: x86_64: bignum_madd_n25519 codegen

cc8398f

ed25519: aarch64: bignum_madd_n25519 codegen

bf7e9a1

ed25519: x86_64: bignum_neg_p25519 codegen

5b715aa

ed25519: aarch64: bignum_neg_p25519 codegen

c63bd0b

ed25519: x86_64: edwards25519_scalarmuldouble codegen

cb564a1

ed25519: aarch64: edwards25519_scalarmuldouble codegen

4aaaa24

Mid-level ed25519 implementation

be7c0b4

High-level ed25519 API

a1be2c6

rustls-graviola: support ed25519 verification

84da13a

rustls-graviola: support ed25519 signing

8d552eb

Enable ed25519 in pairwise TLS tests

b646b29

Wycheproof tests for ed25519

4f271a3

ed25519 zeroing test

fd3240c

Crosscheck test for ed25519

f82d9f5

Benchmarks for ed25519

01938d4

ctz marked this pull request as draft November 23, 2025 09:57

phlip9 reviewed Nov 25, 2025

View reviewed changes

phlip9 mentioned this pull request Nov 25, 2025

ed25519: {x86_64, aarch64}: codegen required s2n-bignum assembly #74

Closed

Ed25519 support #120

Are you sure you want to change the base?

Ed25519 support #120

Uh oh!

Conversation

ctz commented Nov 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Nov 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

codspeed-hq bot commented Nov 23, 2025

Merging #120 will not alter performance

Summary

Benchmarks breakdown

Uh oh!

phlip9 left a comment

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!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ctz commented Nov 23, 2025 •

edited

Loading

codecov bot commented Nov 23, 2025 •

edited

Loading