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 all 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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@ Entries are listed in reverse chronological order.

# 2.x Series


## 2.0.0-rc.3

* Change: `StaticSecret` serialization and `to_bytes()` no longer returns clamped integers. Clamping is still always done during scalar-point multiplication.

## 2.0.0-rc.2

* Update MSRV to 1.60.
Expand Down
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.

4 changes: 4 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,7 @@ alloc = ["curve25519-dalek/alloc", "serde?/alloc", "zeroize?/alloc"]
precomputed-tables = ["curve25519-dalek/precomputed-tables"]
reusable_secrets = []
static_secrets = []

[patch.crates-io.curve25519-dalek]
git = "https://github.com/dalek-cryptography/curve25519-dalek.git"
rev = "f460ae149b0000695205cc78f560d74a2d3918eb"
61 changes: 20 additions & 41 deletions src/x25519.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@
//! This implements x25519 key exchange as specified by Mike Hamburg
//! and Adam Langley in [RFC7748](https://tools.ietf.org/html/rfc7748).

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

use rand_core::CryptoRng;
use rand_core::RngCore;
Expand Down Expand Up @@ -74,13 +72,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 @@ -94,11 +92,10 @@ impl EphemeralSecret {

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

csprng.fill_bytes(&mut bytes);

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

/// Generate a new [`EphemeralSecret`].
Expand All @@ -111,7 +108,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 @@ -137,14 +134,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 @@ -158,11 +155,10 @@ impl ReusableSecret {

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

csprng.fill_bytes(&mut bytes);

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

/// Generate a new [`ReusableSecret`].
Expand All @@ -176,7 +172,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,16 +195,14 @@ 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

#[cfg(feature = "static_secrets")]
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 @@ -222,11 +216,10 @@ impl StaticSecret {

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

csprng.fill_bytes(&mut bytes);

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

/// Generate a new [`StaticSecret`].
Expand All @@ -238,29 +231,29 @@ 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
}
}

#[cfg(feature = "static_secrets")]
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(bytes)
}
}

#[cfg(feature = "static_secrets")]
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 @@ -373,7 +366,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 @@ -382,17 +375,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: 5 additions & 8 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 @@ -64,8 +62,7 @@ fn serde_bincode_static_secret_matches_from_bytes() {
use bincode;

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

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