Skip to content
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.

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

Update to new Scalar API #120

Merged
merged 6 commits into from
Mar 31, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 1 addition & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ rustdoc-args = [
features = ["reusable_secrets", "serde"]

[dependencies]
curve25519-dalek = { version = "4.0.0-rc.0", default-features = false }
curve25519-dalek = { git = "https://github.com/rozbb/curve25519-dalek.git", branch = "scalar-always-reduced", default-features = false }
rand_core = { version = "0.6", default-features = false }
serde = { version = "1", default-features = false, optional = true, features = ["derive"] }
zeroize = { version = "1", default-features = false, optional = true, features = ["zeroize_derive"] }
Expand Down
59 changes: 20 additions & 39 deletions src/x25519.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
//! and Adam Langley in [RFC7748](https://tools.ietf.org/html/rfc7748).

use curve25519_dalek::{
edwards::EdwardsPoint, montgomery::MontgomeryPoint, scalar::Scalar, traits::IsIdentity,
edwards::EdwardsPoint, montgomery::MontgomeryPoint, scalar::clamp_integer, traits::IsIdentity,
};

use rand_core::CryptoRng;
Expand Down Expand Up @@ -75,13 +75,13 @@ impl AsRef<[u8]> for PublicKey {
/// secret is used at most once.
#[cfg_attr(feature = "zeroize", derive(Zeroize))]
#[cfg_attr(feature = "zeroize", zeroize(drop))]
pub struct EphemeralSecret(pub(crate) Scalar);
pub struct EphemeralSecret(pub(crate) [u8; 32]);

impl EphemeralSecret {
/// Perform a Diffie-Hellman key agreement between `self` and
/// `their_public` key to produce a [`SharedSecret`].
pub fn diffie_hellman(self, their_public: &PublicKey) -> SharedSecret {
SharedSecret(self.0 * their_public.0)
SharedSecret(their_public.0.mul_clamped(self.0))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is equivalent because, in the old code, self.0 was the result of Scalar::from_bits_clamped, which was unreduced.

}

/// Generate a new [`EphemeralSecret`] with the supplied RNG.
Expand All @@ -95,11 +95,10 @@ impl EphemeralSecret {

/// Generate a new [`EphemeralSecret`] with the supplied RNG.
pub fn random_from_rng<T: RngCore + CryptoRng>(mut csprng: T) -> Self {
// Generate random bytes. The secret key is the clamping of this.
let mut bytes = [0u8; 32];

csprng.fill_bytes(&mut bytes);

EphemeralSecret(Scalar::from_bits_clamped(bytes))
EphemeralSecret(clamp_integer(bytes))
}

/// Generate a new [`EphemeralSecret`].
Expand All @@ -112,7 +111,7 @@ impl EphemeralSecret {
impl<'a> From<&'a EphemeralSecret> for PublicKey {
/// Given an x25519 [`EphemeralSecret`] key, compute its corresponding [`PublicKey`].
fn from(secret: &'a EphemeralSecret) -> PublicKey {
PublicKey(EdwardsPoint::mul_base(&secret.0).to_montgomery())
PublicKey(EdwardsPoint::mul_base_clamped(secret.0).to_montgomery())
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same reasoning here, and in the multiple copies of this below

}
}

Expand All @@ -138,14 +137,14 @@ impl<'a> From<&'a EphemeralSecret> for PublicKey {
#[cfg_attr(feature = "zeroize", derive(Zeroize))]
#[cfg_attr(feature = "zeroize", zeroize(drop))]
#[derive(Clone)]
pub struct ReusableSecret(pub(crate) Scalar);
pub struct ReusableSecret(pub(crate) [u8; 32]);

#[cfg(feature = "reusable_secrets")]
impl ReusableSecret {
/// Perform a Diffie-Hellman key agreement between `self` and
/// `their_public` key to produce a [`SharedSecret`].
pub fn diffie_hellman(&self, their_public: &PublicKey) -> SharedSecret {
SharedSecret(self.0 * their_public.0)
SharedSecret(their_public.0.mul_clamped(self.0))
}

/// Generate a new [`ReusableSecret`] with the supplied RNG.
Expand All @@ -159,11 +158,10 @@ impl ReusableSecret {

/// Generate a new [`ReusableSecret`] with the supplied RNG.
pub fn random_from_rng<T: RngCore + CryptoRng>(mut csprng: T) -> Self {
// Generate random bytes. The secret key is the clamping of this.
let mut bytes = [0u8; 32];

csprng.fill_bytes(&mut bytes);

ReusableSecret(Scalar::from_bits_clamped(bytes))
ReusableSecret(clamp_integer(bytes))
}

/// Generate a new [`ReusableSecret`].
Expand All @@ -177,7 +175,7 @@ impl ReusableSecret {
impl<'a> From<&'a ReusableSecret> for PublicKey {
/// Given an x25519 [`ReusableSecret`] key, compute its corresponding [`PublicKey`].
fn from(secret: &'a ReusableSecret) -> PublicKey {
PublicKey(EdwardsPoint::mul_base(&secret.0).to_montgomery())
PublicKey(EdwardsPoint::mul_base_clamped(secret.0).to_montgomery())
}
}

Expand All @@ -199,15 +197,13 @@ impl<'a> From<&'a ReusableSecret> for PublicKey {
#[cfg_attr(feature = "zeroize", derive(Zeroize))]
#[cfg_attr(feature = "zeroize", zeroize(drop))]
#[derive(Clone)]
pub struct StaticSecret(
#[cfg_attr(feature = "serde", serde(with = "AllowUnreducedScalarBytes"))] pub(crate) Scalar,
);
pub struct StaticSecret([u8; 32]);
rozbb marked this conversation as resolved.
Show resolved Hide resolved

impl StaticSecret {
/// Perform a Diffie-Hellman key agreement between `self` and
/// `their_public` key to produce a `SharedSecret`.
pub fn diffie_hellman(&self, their_public: &PublicKey) -> SharedSecret {
SharedSecret(self.0 * their_public.0)
SharedSecret(their_public.0.mul_clamped(self.0))
}

/// Generate a new [`StaticSecret`] with the supplied RNG.
Expand All @@ -221,11 +217,10 @@ impl StaticSecret {

/// Generate a new [`StaticSecret`] with the supplied RNG.
pub fn random_from_rng<T: RngCore + CryptoRng>(mut csprng: T) -> Self {
// Generate random bytes. The secret key is the clamping of this.
let mut bytes = [0u8; 32];

csprng.fill_bytes(&mut bytes);

StaticSecret(Scalar::from_bits_clamped(bytes))
StaticSecret(clamp_integer(bytes))
}

/// Generate a new [`StaticSecret`].
Expand All @@ -237,27 +232,27 @@ impl StaticSecret {
/// Extract this key's bytes for serialization.
#[inline]
pub fn to_bytes(&self) -> [u8; 32] {
self.0.to_bytes()
self.0
rozbb marked this conversation as resolved.
Show resolved Hide resolved
}

/// View this key as a byte array.
#[inline]
pub fn as_bytes(&self) -> &[u8; 32] {
self.0.as_bytes()
&self.0
}
}

impl From<[u8; 32]> for StaticSecret {
/// Load a secret key from a byte array.
fn from(bytes: [u8; 32]) -> StaticSecret {
StaticSecret(Scalar::from_bits_clamped(bytes))
StaticSecret(clamp_integer(bytes))
}
}

impl<'a> From<&'a StaticSecret> for PublicKey {
/// Given an x25519 [`StaticSecret`] key, compute its corresponding [`PublicKey`].
fn from(secret: &'a StaticSecret) -> PublicKey {
PublicKey(EdwardsPoint::mul_base(&secret.0).to_montgomery())
PublicKey(EdwardsPoint::mul_base_clamped(secret.0).to_montgomery())
}
}

Expand Down Expand Up @@ -368,7 +363,7 @@ impl AsRef<[u8]> for SharedSecret {
/// assert_eq!(alice_shared, bob_shared);
/// ```
pub fn x25519(k: [u8; 32], u: [u8; 32]) -> [u8; 32] {
(Scalar::from_bits_clamped(k) * MontgomeryPoint(u)).to_bytes()
MontgomeryPoint(u).mul_clamped(k).to_bytes()
}

/// The X25519 basepoint, for use with the bare, byte-oriented x25519
Expand All @@ -377,17 +372,3 @@ pub fn x25519(k: [u8; 32], u: [u8; 32]) -> [u8; 32] {
pub const X25519_BASEPOINT_BYTES: [u8; 32] = [
9, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
];

/// Derived serialization methods will not work on a StaticSecret because x25519 requires
/// non-canonical scalars which are rejected by curve25519-dalek. Thus we provide a way to convert
/// the bytes directly to a scalar using Serde's remote derive functionality.
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
#[cfg_attr(feature = "serde", serde(remote = "Scalar"))]
struct AllowUnreducedScalarBytes(
#[cfg_attr(feature = "serde", serde(getter = "Scalar::to_bytes"))] [u8; 32],
);
impl From<AllowUnreducedScalarBytes> for Scalar {
fn from(bytes: AllowUnreducedScalarBytes) -> Scalar {
Scalar::from_bits_clamped(bytes.0)
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We can remove this because (de)serialization of StaticSecret is all bytes now

13 changes: 6 additions & 7 deletions tests/x25519_tests.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use curve25519_dalek::{edwards::EdwardsPoint, scalar::Scalar};
use curve25519_dalek::edwards::EdwardsPoint;

use x25519_dalek::*;

Expand All @@ -10,11 +10,9 @@ fn byte_basepoint_matches_edwards_scalar_mul() {
scalar_bytes[i] += 2;

let result = x25519(scalar_bytes, X25519_BASEPOINT_BYTES);

let expected = {
let scalar = Scalar::from_bits_clamped(scalar_bytes);
EdwardsPoint::mul_base(&scalar).to_montgomery().to_bytes()
};
let expected = EdwardsPoint::mul_base_clamped(scalar_bytes)
.to_montgomery()
.to_bytes();

assert_eq!(result, expected);
}
Expand Down Expand Up @@ -62,9 +60,10 @@ fn serde_bincode_static_secret_roundtrip() {
#[cfg(feature = "serde")]
fn serde_bincode_static_secret_matches_from_bytes() {
use bincode;
use curve25519_dalek::scalar::clamp_integer;

let expected = StaticSecret::from([0x24; 32]);
let clamped_bytes = Scalar::from_bits_clamped([0x24; 32]).to_bytes();
let clamped_bytes = clamp_integer([0x24; 32]);
let decoded: StaticSecret = bincode::deserialize(&clamped_bytes).unwrap();

assert_eq!(decoded.to_bytes(), expected.to_bytes());
Expand Down