From 0a1023f4db0ae2d0944390dc9817bfa946d5e874 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Wed, 14 Apr 2021 20:00:41 +0000 Subject: [PATCH 01/11] 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 02/11] 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 c13e102f9585ab5cb46d7117c56ccfee90ff5c13 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Mon, 13 Sep 2021 21:31:54 +0000 Subject: [PATCH 03/11] Implement optional check for contributory behaviour. --- src/x25519.rs | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/src/x25519.rs b/src/x25519.rs index 538af36..f39e3f5 100644 --- a/src/x25519.rs +++ b/src/x25519.rs @@ -17,6 +17,7 @@ use curve25519_dalek::constants::ED25519_BASEPOINT_TABLE; use curve25519_dalek::montgomery::MontgomeryPoint; use curve25519_dalek::scalar::Scalar; +use curve25519_dalek::traits::IsIdentity; use rand_core::CryptoRng; use rand_core::RngCore; @@ -177,6 +178,43 @@ impl SharedSecret { pub fn as_bytes(&self) -> &[u8; 32] { self.0.as_bytes() } + + /// Ensure in constant-time that this shared secret did not result from a + /// key exchange with non-contributory behaviour. + /// + /// In some more exotic protocols which need to guarantee "contributory" + /// behaviour for both parties, that is, that each party contibuted a public + /// value which increased the security of the resulting shared secret. + /// To take an example protocol attack where this could lead to undesireable + /// results [from Thái "thaidn" Dương](https://vnhacker.blogspot.com/2015/09/why-not-validating-curve25519-public.html): + /// + /// > If Mallory replaces Alice's and Bob's public keys with zero, which is + /// > a valid Curve25519 public key, he would be able to force the ECDH + /// > shared value to be zero, which is the encoding of the point at infinity, + /// > and thus get to dictate some publicly known values as the shared + /// > keys. It still requires an active man-in-the-middle attack to pull the + /// > trick, after which, however, not only Mallory can decode Alice's data, + /// > but everyone too! It is also impossible for Alice and Bob to detect the + /// > intrusion, as they still share the same keys, and can communicate with + /// > each other as normal. + /// + /// The original Curve25519 specification argues that checks for + /// non-contributory behaviour are "unnecessary for Diffie-Hellman". + /// Whether this check is necessary for any particular given protocol is + /// often a matter of debate, which we will not re-hash here, but simply + /// cite some of the [relevant] [public] [discussions]. + /// + /// # Returns + /// + /// Returns `true` if the key exchange was contributory (good), and `false` + /// otherwise (can be bad for some protocols). + /// + /// [relevant]: https://tools.ietf.org/html/rfc7748#page-15 + /// [public]: https://vnhacker.blogspot.com/2015/09/why-not-validating-curve25519-public.html + /// [discussions]: https://vnhacker.blogspot.com/2016/08/the-internet-of-broken-protocols.html + pub fn was_contributory(&self) -> bool { + !self.0.is_identity() + } } /// "Decode" a scalar from a 32-byte array. From 18323afd63d88f38009310015a04b86b99b1c267 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Mon, 13 Sep 2021 22:38:34 +0000 Subject: [PATCH 04/11] Bisect to determine MSRV. --- .github/workflows/rust.yml | 6 +++--- README.md | 4 ++++ src/lib.rs | 4 ++++ 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 8df1723..2ba256c 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -71,18 +71,18 @@ jobs: args: --features "serde" msrv: - name: Current MSRV is 1.54 + name: Current MSRV is 1.41 runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 - uses: actions-rs/toolchain@v1 with: profile: minimal - toolchain: 1.54 + toolchain: 1.41 override: true - uses: actions-rs/cargo@v1 with: - command: test + command: build bench: name: Check that benchmarks compile diff --git a/README.md b/README.md index dee9acc..ce9e7d8 100644 --- a/README.md +++ b/README.md @@ -105,6 +105,10 @@ To install, add the following to your project's `Cargo.toml`: x25519-dalek = "1.1" ``` +# MSRV + +Current MSRV is 1.41 for production builds, and 1.48 for running tests. + # Documentation Documentation is available [here](https://docs.rs/x25519-dalek). diff --git a/src/lib.rs b/src/lib.rs index e5f7bfe..e4990b0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -127,6 +127,10 @@ //! x25519-dalek = "1.1" //! ``` //! +//! # MSRV +//! +//! Current MSRV is 1.41 for production builds, and 1.48 for running tests. +//! //! # Documentation //! //! Documentation is available [here](https://docs.rs/x25519-dalek). From 91babd286ff9faf5625bb8c2f3dbe1a0227ff84b Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Tue, 14 Sep 2021 19:51:02 +0000 Subject: [PATCH 05/11] Add a #[must_use] to the was_contributory check. --- src/x25519.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/x25519.rs b/src/x25519.rs index f39e3f5..b04c2c0 100644 --- a/src/x25519.rs +++ b/src/x25519.rs @@ -212,6 +212,7 @@ impl SharedSecret { /// [relevant]: https://tools.ietf.org/html/rfc7748#page-15 /// [public]: https://vnhacker.blogspot.com/2015/09/why-not-validating-curve25519-public.html /// [discussions]: https://vnhacker.blogspot.com/2016/08/the-internet-of-broken-protocols.html + #[must_use] pub fn was_contributory(&self) -> bool { !self.0.is_identity() } From 3924797b599ee159eb2c6bfb3b10f8411caf5e4a Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Tue, 14 Sep 2021 20:23:38 +0000 Subject: [PATCH 06/11] 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 07/11] 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 08/11] 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 09/11] 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]; From 179986ac672c947af12a423250cfade2eab08329 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Tue, 14 Sep 2021 23:08:22 +0000 Subject: [PATCH 10/11] Update CHANGELOG for 1.2. --- CHANGELOG.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 588b31f..ef8f8d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,24 @@ Entries are listed in reverse chronological order. +# 1.x Series + +## 1.2 + +* Add module documentation for using the bytes-oriented `x25519()` API. +* Add implementation of `zeroize::Zeroize` for `PublicKey`. +* Move unittests to a separate directory. +* Add cargo feature flags `"fiat_u32_backend"` and `"fiat_u64_backend"` for + activating the Fiat crypto field element implementations. +* Fix issue with removed `feature(external_doc)` on nightly compilers. +* Pin `zeroize` to version 1.3 to support a wider range of MSRVs. +* Add CI via Github actions. +* Fix breakage in the serde unittests. +* MSRV is now 1.41 for production and 1.48 for development. +* Add an optional check to `SharedSecret` for contibutory behaviour. +* Add implementation of `ReusableSecret` keys which are non-ephemeral, but which + cannot be serialised to discourage long-term use. + ## 1.1.1 * Fix a typo in the README. @@ -23,6 +41,8 @@ Entries are listed in reverse chronological order. * Remove mention of deprecated `rand_os` crate from examples. * Clarify `EphemeralSecret`/`StaticSecret` distinction in documentation. +# Pre-1.0.0 + ## 0.6.0 * Updates `rand_core` version to `0.5`. From ea047a218fd77a46af9cf48e9376954b646a2536 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Tue, 14 Sep 2021 23:08:36 +0000 Subject: [PATCH 11/11] Bump x25519-dalek version to 1.2. --- Cargo.toml | 2 +- README.md | 2 +- src/lib.rs | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 0ffc6dc..47c47d6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -6,7 +6,7 @@ edition = "2018" # - update html_root_url # - update CHANGELOG # - if any changes were made to README.md, mirror them in src/lib.rs docs -version = "1.1.1" +version = "1.2.0" authors = [ "Isis Lovecruft ", "DebugSteven ", diff --git a/README.md b/README.md index ce9e7d8..c7bb1be 100644 --- a/README.md +++ b/README.md @@ -102,7 +102,7 @@ To install, add the following to your project's `Cargo.toml`: ```toml [dependencies] -x25519-dalek = "1.1" +x25519-dalek = "1" ``` # MSRV diff --git a/src/lib.rs b/src/lib.rs index e4990b0..ef35c62 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -18,7 +18,7 @@ #![cfg_attr(feature = "bench", feature(test))] #![cfg_attr(feature = "nightly", deny(missing_docs))] #![doc(html_logo_url = "https://doc.dalek.rs/assets/dalek-logo-clear.png")] -#![doc(html_root_url = "https://docs.rs/x25519-dalek/1.1.1")] +#![doc(html_root_url = "https://docs.rs/x25519-dalek/1.2.0")] //! # x25519-dalek [![](https://img.shields.io/crates/v/x25519-dalek.svg)](https://crates.io/crates/x25519-dalek) [![](https://docs.rs/x25519-dalek/badge.svg)](https://docs.rs/x25519-dalek) [![](https://travis-ci.org/dalek-cryptography/x25519-dalek.svg?branch=master)](https://travis-ci.org/dalek-cryptography/x25519-dalek) //! @@ -124,7 +124,7 @@ //! //! ```toml //! [dependencies] -//! x25519-dalek = "1.1" +//! x25519-dalek = "1" //! ``` //! //! # MSRV