Skip to content

Optionally implement zerocopy traits on relevant curve25519_dalek and x25519_dalek types #825

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Optionally implement zerocopy traits on relevant curve25519_dalek and x25519_dalek types #825

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3b678f5
Add optional support for deriving zerocopy traits
danderson Sep 5, 2025
1d6e8c3
Add changelog entries for the zerocopy feature
danderson Sep 5, 2025
6c6f2f7
Add features table to the x25519-dalek crate.
danderson Sep 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions curve25519-dalek/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ major series.

## 5.x series

## 5.0.0-pre.2

* Add optional integration with [`zerocopy`][zerocopy], controlled by the `zerocopy` feature.

[zerocopy]: https://docs.rs/zerocopy/latest/zerocopy/

## 5.0.0-pre.1

* Rename `Scalar::batch_invert` -> `Scalar::invert_batch` for consistency. Also make it no-alloc.
Expand Down
7 changes: 6 additions & 1 deletion curve25519-dalek/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ features = [
"digest",
"legacy_compatibility",
"group-bits",
"zerocopy",
]

[dev-dependencies]
Expand Down Expand Up @@ -68,6 +69,9 @@ serde = { version = "1.0", default-features = false, optional = true, features =
"derive",
] }
zeroize = { version = "1", default-features = false, optional = true }
zerocopy = { version = "0.8", default-features = false, optional = true, features = [
"derive"
] }

[target.'cfg(target_arch = "x86_64")'.dependencies]
cpufeatures = "0.2.17"
Expand All @@ -77,12 +81,13 @@ fiat-crypto = { version = "0.3.0", default-features = false }

[features]
default = ["alloc", "precomputed-tables", "zeroize"]
alloc = ["zeroize?/alloc"]
alloc = ["zerocopy?/alloc", "zeroize?/alloc"]
precomputed-tables = []
legacy_compatibility = []
group = ["dep:group", "rand_core"]
group-bits = ["group", "ff/bits"]
digest = ["dep:digest"]
zerocopy = ["dep:zerocopy"]

[target.'cfg(all(not(curve25519_dalek_backend = "fiat"), not(curve25519_dalek_backend = "serial"), target_arch = "x86_64"))'.dependencies]
curve25519-dalek-derive = "0.1"
Expand Down
2 changes: 2 additions & 0 deletions curve25519-dalek/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ curve25519-dalek = ">= 5.0, < 5.2"
| `legacy_compatibility`| | Enables `Scalar::from_bits`, which allows the user to build unreduced scalars whose arithmetic is broken. Do not use this unless you know what you're doing. |
| `group` | | Enables external `group` and `ff` crate traits. |
| `group-bits` | | Enables `group` and impls `ff::PrimeFieldBits` for `Scalar`. |
| `zerocopy` | | Derives [`zerocopy`][zerocopy] traits on `MontgomeryPoint`. |

To disable the default features when using `curve25519-dalek` as a dependency,
add `default-features = false` to the dependency in your `Cargo.toml`. To
Expand Down Expand Up @@ -313,4 +314,5 @@ contributions.
[semver]: https://semver.org/spec/v2.0.0.html
[rngcorestd]: https://github.com/rust-random/rand/tree/7aa25d577e2df84a5156f824077bb7f6bdf28d97/rand_core#crate-features
[zeroize-trait]: https://docs.rs/zeroize/latest/zeroize/trait.Zeroize.html
[zerocopy]: https://docs.rs/zerocopy/latest/zerocopy/
[SIMD backend]: #simd-backend
9 changes: 9 additions & 0 deletions curve25519-dalek/src/montgomery.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,15 @@ const FE_C2: FieldElement = FieldElement::from_limbs([
/// Curve25519 or its twist.
#[derive(Copy, Clone, Debug, Default)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
#[cfg_attr(
feature = "zerocopy",
derive(
zerocopy::FromBytes,
zerocopy::IntoBytes,
zerocopy::Immutable,
zerocopy::KnownLayout
)
)]
pub struct MontgomeryPoint(pub [u8; 32]);

/// Equality of `MontgomeryPoint`s is defined mod p.
Expand Down
6 changes: 6 additions & 0 deletions x25519-dalek/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@ Entries are listed in reverse chronological order.

# 3.x Series

## 3.0.0-pre.1

* Add optional integration with [`zerocopy`][zerocopy], controlled by the `zerocopy` feature.

[zerocopy]: https://docs.rs/zerocopy/latest/zerocopy/

## 3.0.0-pre.0

* Update edition to 2024
Expand Down
8 changes: 6 additions & 2 deletions x25519-dalek/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ rustdoc-args = [
"--cfg",
"docsrs",
]
features = ["os_rng", "reusable_secrets", "serde", "static_secrets"]
features = ["os_rng", "reusable_secrets", "serde", "static_secrets", "zeroize", "zerocopy"]

[dependencies]
curve25519-dalek = { version = "=5.0.0-pre.1", default-features = false }
Expand All @@ -48,6 +48,9 @@ serde = { version = "1", default-features = false, optional = true, features = [
"derive",
] }
zeroize = { version = "1", default-features = false, optional = true }
zerocopy = { version = "0.8", default-features = false, optional = true, features = [
"derive"
] }

[dev-dependencies]
bincode = "1"
Expand All @@ -63,7 +66,8 @@ default = ["alloc", "precomputed-tables", "zeroize"]
os_rng = ["rand_core/os_rng"]
zeroize = ["dep:zeroize", "curve25519-dalek/zeroize"]
serde = ["dep:serde", "curve25519-dalek/serde"]
alloc = ["curve25519-dalek/alloc", "serde?/alloc", "zeroize?/alloc"]
alloc = ["curve25519-dalek/alloc", "serde?/alloc", "zerocopy?/alloc", "zeroize?/alloc"]
precomputed-tables = ["curve25519-dalek/precomputed-tables"]
reusable_secrets = []
static_secrets = []
zerocopy = ["dep:zerocopy", "curve25519-dalek/zerocopy"]
13 changes: 13 additions & 0 deletions x25519-dalek/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,17 @@ Performance is a secondary goal behind correctness, safety, and clarity, but we

Further instructions and details regarding backends can be found in the [curve25519-dalek docs](https://github.com/dalek-cryptography/curve25519-dalek#backends).

# Feature flags

| Feature | Default? | Description |
|:---------------------|:--------:|:---------------------------------------------------------------------------|
| `zeroize` | ✓ | Enables the [`zeroize`][zeroize-trait] trait on key types, and implements `ZeroizeOnDrop` for secrets. |
| `reusable_secrets` | | Enables `ReusableSecret`, a Diffie-Hellman secret key that can be used more than once, but is not serializable. |
| `static_secrets` | | Enables `StaticSecret`, a Diffie-Hellman secret key that can be used more than once, and is serializable. |
| `os_rng` | | Enables `EphemeralSecret::random`, `ReusableSecret::random` and `StaticSecret::random` helpers that implicitly use the OS RNG. `random_from_rng` with an explicitly provided RNG is available without this feature. |
| `serde` | | Enables `serde` serialization/deserialization for `PublicKey` and `StaticSecret`. |
| `zerocopy` | | Enables [`zerocopy`][zerocopy] serialization/deserialization for `PublicKey` and `StaticSecret`. |

# Note

This code matches the [RFC7748][rfc7748] test vectors.
Expand All @@ -140,3 +151,5 @@ copyright © Amy Wibowo ([@sailorhg](https://twitter.com/sailorhg))

[fiat]: https://github.com/mit-plv/fiat-crypto
[crypto_box]: https://github.com/RustCrypto/nacl-compat/tree/master/crypto_box
[zeroize-trait]: https://docs.rs/zeroize/latest/zeroize/trait.Zeroize.html
[zerocopy]: https://docs.rs/zerocopy/latest/zerocopy/
18 changes: 18 additions & 0 deletions x25519-dalek/src/x25519.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,15 @@ use zeroize::{Zeroize, ZeroizeOnDrop};
/// (in this crate) does *not* automatically happen, but either must be derived
/// for Drop or explicitly called.
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
#[cfg_attr(
feature = "zerocopy",
derive(
zerocopy::FromBytes,
zerocopy::IntoBytes,
zerocopy::Immutable,
zerocopy::KnownLayout
)
)]
#[derive(PartialEq, Eq, Hash, Copy, Clone, Debug)]
pub struct PublicKey(pub(crate) MontgomeryPoint);

Expand Down Expand Up @@ -198,6 +207,15 @@ impl ZeroizeOnDrop for ReusableSecret {}
/// implications for many protocols.
#[cfg(feature = "static_secrets")]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
#[cfg_attr(
feature = "zerocopy",
derive(
zerocopy::FromBytes,
zerocopy::IntoBytes,
zerocopy::Immutable,
zerocopy::KnownLayout
)
)]
#[derive(Clone)]
pub struct StaticSecret([u8; 32]);
Comment on lines +210 to 220
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Impls on the public key types are fine, but impls on secrets are slightly worrisome.

One scenario is someone may unintentionally pass the wrong type and could accidentally expose key material because both the public and secret types impl the same trait.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same concern applies to the serde traits, no? By definition, StaticSecret is transformable into a form that can be stored, that's why it exists as a separate (riskier) type than ReusableSecret. You could also as_bytes() the key and pass it to anything that handles bytes and the type system won't stop you.

I can see an argument for only allowing serialization via explicit, non-trait methods in order to make it harder to accidentally trigger serialization, but in that case the same argument should apply to the serde derivations?

Copy link
Contributor

@tarcieri tarcieri Sep 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, FWIW the @RustCrypto crates don't implement the serde traits on secrets, though I guess that happened in curve25519-dalek

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For my purposes, zerocopy is most useful for serializing data into network packets, and indeed in that case it would be quite bad to serialize a cleartext secret key :)

I'm okay with either outcome, either consistency with the serde traits or treating the serde traits as a historical mistake (I assume we can't remove them without a breaking semver release). In the latter case I can remove the derivations from StaticSecret.

... although tbh that means this entire change, with a new cargo feature and all the plumbing, is effectively to support zerocopy on a single type (x25519 PublicKey), and maybe we're back to the discussion of "is this even worth it" :)


Expand Down