From 0a1023f4db0ae2d0944390dc9817bfa946d5e874 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Wed, 14 Apr 2021 20:00:41 +0000 Subject: [PATCH 1/6] Implement reused secret keys for Noise protocol. --- src/x25519.rs | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/x25519.rs b/src/x25519.rs index 538af36..c1c2aa4 100644 --- a/src/x25519.rs +++ b/src/x25519.rs @@ -95,6 +95,46 @@ impl<'a> From<&'a EphemeralSecret> for PublicKey { } } +/// A Diffie-Hellman secret key which may be used more than once, but is +/// purposefully not serialiseable in order to discourage key-reuse. This is +/// implemented to facilitate protocols such as Noise (e.g. Noise IK key usage, +/// etc.) and X3DH which require an "ephemeral" key to conduct the +/// Diffie-Hellman operation multiple times throughout the protocol, while the +/// protocol run at a higher level is only conducted once per key. +/// +/// If you're uncertain about whether you should use this, then you likely +/// should not be using this. Our strongly recommended advice is to use +/// [`EphemeralSecret`] at all times, as that type enforces at compile-time that +/// secret keys are never reused, which can have very serious security +/// implications for many protocols. +#[derive(Zeroize)] +#[zeroize(drop)] +pub struct NonSerializeableSecret(pub(crate) Scalar); + +impl NonSerializeableSecret { + /// 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) + } + + /// Generate a non-serializeable x25519 key. + pub fn new(mut csprng: T) -> Self { + let mut bytes = [0u8; 32]; + + csprng.fill_bytes(&mut bytes); + + NonSerializeableSecret(clamp_scalar(bytes)) + } +} + +impl<'a> From<&'a NonSerializeableSecret> for PublicKey { + /// Given an x25519 [`NonSerializeableSecret`] key, compute its corresponding [`PublicKey`]. + fn from(secret: &'a NonSerializeableSecret) -> PublicKey { + PublicKey((&ED25519_BASEPOINT_TABLE * &secret.0).to_montgomery()) + } +} + /// A Diffie-Hellman secret key that can be used to compute multiple [`SharedSecret`]s. /// /// This type is identical to the [`EphemeralSecret`] type, except that the From 27c73fdb5763a0d0c76548f0b934455b66483326 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Tue, 20 Apr 2021 21:33:32 +0000 Subject: [PATCH 2/6] Feature gate reusable secrets and make the name more intuitive. --- Cargo.toml | 3 ++- src/x25519.rs | 15 +++++++++------ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 16aa97c..41127dd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,7 +30,7 @@ travis-ci = { repository = "dalek-cryptography/x25519-dalek", branch = "master"} [package.metadata.docs.rs] #rustdoc-args = ["--html-in-header", ".cargo/registry/src/github.com-1ecc6299db9ec823/curve25519-dalek-1.0.1/docs/assets/rustdoc-include-katex-header.html"] -features = ["nightly"] +features = ["nightly", "reusable_secrets", "serde"] [dependencies] curve25519-dalek = { version = "3", default-features = false } @@ -53,5 +53,6 @@ default = ["std", "u64_backend"] serde = ["our_serde", "curve25519-dalek/serde"] std = ["curve25519-dalek/std"] nightly = ["curve25519-dalek/nightly"] +reusable_secrets = [] u64_backend = ["curve25519-dalek/u64_backend"] u32_backend = ["curve25519-dalek/u32_backend"] diff --git a/src/x25519.rs b/src/x25519.rs index c1c2aa4..030e49b 100644 --- a/src/x25519.rs +++ b/src/x25519.rs @@ -107,11 +107,13 @@ impl<'a> From<&'a EphemeralSecret> for PublicKey { /// [`EphemeralSecret`] at all times, as that type enforces at compile-time that /// secret keys are never reused, which can have very serious security /// implications for many protocols. +#[cfg(feature = "reusable_secrets")] #[derive(Zeroize)] #[zeroize(drop)] -pub struct NonSerializeableSecret(pub(crate) Scalar); +pub struct ReusableSecret(pub(crate) Scalar); -impl NonSerializeableSecret { +#[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 { @@ -124,13 +126,14 @@ impl NonSerializeableSecret { csprng.fill_bytes(&mut bytes); - NonSerializeableSecret(clamp_scalar(bytes)) + ReusableSecret(clamp_scalar(bytes)) } } -impl<'a> From<&'a NonSerializeableSecret> for PublicKey { - /// Given an x25519 [`NonSerializeableSecret`] key, compute its corresponding [`PublicKey`]. - fn from(secret: &'a NonSerializeableSecret) -> PublicKey { +#[cfg(feature = "reusable_secrets")] +impl<'a> From<&'a ReusableSecret> for PublicKey { + /// Given an x25519 [`ReusableSecret`] key, compute its corresponding [`PublicKey`]. + fn from(secret: &'a ReusableSecret) -> PublicKey { PublicKey((&ED25519_BASEPOINT_TABLE * &secret.0).to_montgomery()) } } From 3924797b599ee159eb2c6bfb3b10f8411caf5e4a Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Tue, 14 Sep 2021 20:23:38 +0000 Subject: [PATCH 3/6] Add note to StaticSecret that EphemeralSecret is recommended. --- src/x25519.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/x25519.rs b/src/x25519.rs index 030e49b..5a342e8 100644 --- a/src/x25519.rs +++ b/src/x25519.rs @@ -102,6 +102,8 @@ impl<'a> From<&'a EphemeralSecret> for PublicKey { /// Diffie-Hellman operation multiple times throughout the protocol, while the /// protocol run at a higher level is only conducted once per key. /// +/// # Warning +/// /// If you're uncertain about whether you should use this, then you likely /// should not be using this. Our strongly recommended advice is to use /// [`EphemeralSecret`] at all times, as that type enforces at compile-time that @@ -153,6 +155,14 @@ impl<'a> From<&'a ReusableSecret> for PublicKey { /// ``` /// since the only difference between the two is that [`StaticSecret`] does not enforce at /// compile-time that the key is only used once. +/// +/// # Warning +/// +/// If you're uncertain about whether you should use this, then you likely +/// should not be using this. Our strongly recommended advice is to use +/// [`EphemeralSecret`] at all times, as that type enforces at compile-time that +/// secret keys are never reused, which can have very serious security +/// implications for many protocols. #[cfg_attr(feature = "serde", serde(crate = "our_serde"))] #[cfg_attr( feature = "serde", From 588e48f8f2bf5c3fc66e62aae17ee23f666de12f Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Tue, 14 Sep 2021 20:24:12 +0000 Subject: [PATCH 4/6] Make ReusableSecret derive Clone. --- src/x25519.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/x25519.rs b/src/x25519.rs index 5a342e8..166f923 100644 --- a/src/x25519.rs +++ b/src/x25519.rs @@ -110,7 +110,7 @@ impl<'a> From<&'a EphemeralSecret> for PublicKey { /// secret keys are never reused, which can have very serious security /// implications for many protocols. #[cfg(feature = "reusable_secrets")] -#[derive(Zeroize)] +#[derive(Clone, Zeroize)] #[zeroize(drop)] pub struct ReusableSecret(pub(crate) Scalar); From edb9ec984ed12c6d037b38545d1927f6df328eda Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Tue, 14 Sep 2021 22:33:31 +0000 Subject: [PATCH 5/6] Document that ReusableSecret is preferrable for Noise protocols. --- src/x25519.rs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/x25519.rs b/src/x25519.rs index 166f923..7478f77 100644 --- a/src/x25519.rs +++ b/src/x25519.rs @@ -102,6 +102,10 @@ impl<'a> From<&'a EphemeralSecret> for PublicKey { /// Diffie-Hellman operation multiple times throughout the protocol, while the /// protocol run at a higher level is only conducted once per key. /// +/// Similarly to [`EphemeralSecret`], this type does _not_ have serialisation +/// methods, in order to discourage long-term usage of secret key material. (For +/// long-term secret keys, see [`StaticSecret`].) +/// /// # Warning /// /// If you're uncertain about whether you should use this, then you likely @@ -147,15 +151,6 @@ impl<'a> From<&'a ReusableSecret> for PublicKey { /// serialization methods to save and load key material. This means that the secret may be used /// multiple times (but does not *have to be*). /// -/// Some protocols, such as Noise, already handle the static/ephemeral distinction, so the -/// additional guarantees provided by [`EphemeralSecret`] are not helpful or would cause duplicate -/// code paths. In this case, it may be useful to -/// ```rust,ignore -/// use x25519_dalek::StaticSecret as SecretKey; -/// ``` -/// since the only difference between the two is that [`StaticSecret`] does not enforce at -/// compile-time that the key is only used once. -/// /// # Warning /// /// If you're uncertain about whether you should use this, then you likely From eef4de41c00f3416345bce3575a5b383c721fd6f Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Tue, 14 Sep 2021 22:34:41 +0000 Subject: [PATCH 6/6] Disambiguate what kind of key in docstring. --- src/x25519.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/x25519.rs b/src/x25519.rs index 7478f77..1146961 100644 --- a/src/x25519.rs +++ b/src/x25519.rs @@ -126,7 +126,7 @@ impl ReusableSecret { SharedSecret(&self.0 * their_public.0) } - /// Generate a non-serializeable x25519 key. + /// Generate a non-serializeable x25519 [`ReuseableSecret`] key. pub fn new(mut csprng: T) -> Self { let mut bytes = [0u8; 32];