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

Create new types for keys and clear values on drop #15

Merged
merged 21 commits into from
Jan 10, 2019
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
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
9 changes: 6 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,19 @@ exclude = [
travis-ci = { repository = "dalek-cryptography/x25519-dalek", branch = "master"}

[dependencies.curve25519-dalek]
version = "^0.19"
version = "^1.0.0-pre.1"
DebugSteven marked this conversation as resolved.
Show resolved Hide resolved
default-features = false

[dependencies.rand_core]
default-features = false
version = "0.2"

[dependencies.clear_on_drop]
version = "0.2"

[dev-dependencies]
criterion = "0.2"
rand = "0.5"
rand = "0.6"

[[bench]]
name = "x25519"
Expand All @@ -38,6 +41,6 @@ harness = false
[features]
default = ["std", "nightly", "u64_backend"]
std = ["curve25519-dalek/std"]
nightly = ["curve25519-dalek/nightly"]
nightly = ["curve25519-dalek/nightly", "clear_on_drop/nightly"]
u64_backend = ["curve25519-dalek/u64_backend"]
u32_backend = ["curve25519-dalek/u32_backend"]
23 changes: 12 additions & 11 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,44 +25,45 @@ up on modern public key cryptography and have learned a nifty trick called
kittens will be able to secretly organise to find their mittens, and then spend
the rest of the afternoon nomming some yummy pie!

First, Alice uses `x25519_dalek::generate_secret()` and then
`x25519_dalek::generate_public()` to produce her secret and public keys:
First, Alice uses `x25519_dalek::EphemeralSecret::new()` and then
`x25519_dalek::EphemeralPublic::from()` to produce her secret and public keys:

```rust
extern crate x25519_dalek;
extern crate rand;

use x25519_dalek::generate_secret;
use x25519_dalek::generate_public;
use x25519_dalek::EphemeralPublic;
use x25519_dalek::EphemeralSecret;
use rand::OsRng;

let mut alice_csprng = OsRng::new().unwrap();
let alice_secret = generate_secret(&mut alice_csprng);
let alice_public = generate_public(&alice_secret);
let alice_secret = EphemeralSecret::new(&mut alice_csprng);
let alice_public = EphemeralPublic::from(&alice_secret);
```

Bob does the same:

```rust
let mut bob_csprng = OsRng::new().unwrap();
let bob_secret = generate_secret(&mut bob_csprng);
let bob_public = generate_public(&bob_secret);
let bob_secret = EphemeralSecret::new(&mut bob_csprng);
let bob_public = EphemeralPublic::from(&bob_secret);
```

Alice meows across the room, telling `alice_public` to Bob, and Bob
loudly meows `bob_public` back to Alice. Alice now computes her
shared secret with Bob by doing:

```rust
use x25519_dalek::diffie_hellman;
use x25519_dalek::EphemeralPublic;
use x25519_dalek::EphemeralSecret;

let shared_secret = diffie_hellman(&alice_secret, &bob_public.as_bytes());
let shared_secret = EphemeralSecret::diffie_hellman(&alice_secret, &bob_public);
```

Similarly, Bob computes the same shared secret by doing:

```rust
let shared_secret = diffie_hellman(&bob_secret, &alice_public.as_bytes());
let shared_secret = EphemeralSecret::diffie_hellman(&bob_secret, &alice_public);
```

Voilá! Alice and Bob can now use their shared secret to encrypt their
Expand Down
16 changes: 9 additions & 7 deletions benches/x25519.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,26 +11,28 @@

#[macro_use]
extern crate criterion;
extern crate curve25519_dalek;
extern crate rand;
extern crate x25519_dalek;

use criterion::Criterion;

use curve25519_dalek::montgomery::MontgomeryPoint;

use rand::OsRng;

use x25519_dalek::generate_public;
use x25519_dalek::generate_secret;
use x25519_dalek::diffie_hellman;
use x25519_dalek::EphemeralPublic;
use x25519_dalek::EphemeralSecret;

fn bench_diffie_hellman(c: &mut Criterion) {
let mut csprng: OsRng = OsRng::new().unwrap();
let alice_secret: [u8; 32] = generate_secret(&mut csprng);
let bob_secret: [u8; 32] = generate_secret(&mut csprng);
let bob_public: [u8; 32] = generate_public(&bob_secret).to_bytes();
let alice_secret: EphemeralSecret = EphemeralSecret::new(&mut csprng);
let bob_secret: EphemeralSecret = EphemeralSecret::new(&mut csprng);
let bob_public: EphemeralPublic = EphemeralPublic::from(&bob_secret);

c.bench_function("diffie_hellman", move |b| {
b.iter(||
diffie_hellman(&alice_secret, &bob_public)
EphemeralSecret::diffie_hellman(&alice_secret, &bob_public)
)
});
}
Expand Down
54 changes: 27 additions & 27 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,21 +32,21 @@
//! incantations, the kittens will be able to secretly organise to find their
//! mittens, and then spend the rest of the afternoon nomming some yummy pie!
//!
//! First, Alice uses `x25519_dalek::generate_secret()` and
//! `x25519_dalek::generate_public()` to produce her secret and public keys:
//! First, Alice uses `x25519_dalek::EphemeralSecret::new()` and
//! `x25519_dalek::EphemeralPublic::from()` to produce her secret and public keys:
//!
//! ```
//! extern crate x25519_dalek;
//! extern crate rand;
//!
//! # fn main() {
//! use x25519_dalek::generate_secret;
//! use x25519_dalek::generate_public;
//! use x25519_dalek::EphemeralPublic;
//! use x25519_dalek::EphemeralSecret;
//! use rand::thread_rng;
//!
//! let mut alice_csprng = thread_rng();
//! let alice_secret = generate_secret(&mut alice_csprng);
//! let alice_public = generate_public(&alice_secret);
//! let alice_secret = EphemeralSecret::new(&mut alice_csprng);
//! let alice_public = EphemeralPublic::from(&alice_secret);
//! # }
//! ```
//!
Expand All @@ -57,13 +57,13 @@
//! # extern crate rand;
//! #
//! # fn main() {
//! # use x25519_dalek::generate_secret;
//! # use x25519_dalek::generate_public;
//! # use x25519_dalek::EphemeralPublic;
//! # use x25519_dalek::EphemeralSecret;
//! # use rand::thread_rng;
//! #
//! let mut bob_csprng = thread_rng();
//! let bob_secret = generate_secret(&mut bob_csprng);
//! let bob_public = generate_public(&bob_secret);
//! let bob_secret = EphemeralSecret::new(&mut bob_csprng);
//! let bob_public = EphemeralPublic::from(&bob_secret);
//! # }
//! ```
//!
Expand All @@ -76,21 +76,20 @@
//! # extern crate rand;
//! #
//! # fn main() {
//! # use x25519_dalek::generate_secret;
//! # use x25519_dalek::generate_public;
//! # use x25519_dalek::EphemeralPublic;
//! # use x25519_dalek::EphemeralSecret;
//! # use rand::thread_rng;
//! #
//! # let mut alice_csprng = thread_rng();
//! # let alice_secret = generate_secret(&mut alice_csprng);
//! # let alice_public = generate_public(&alice_secret);
//! # let alice_secret = EphemeralSecret::new(&mut alice_csprng);
//! # let alice_public = EphemeralPublic::from(&alice_secret);
//! #
//! # let mut bob_csprng = thread_rng();
//! # let bob_secret = generate_secret(&mut bob_csprng);
//! # let bob_public = generate_public(&bob_secret);
//! # let bob_secret = EphemeralSecret::new(&mut bob_csprng);
//! # let bob_public = EphemeralPublic::from(&bob_secret);
//! #
//! use x25519_dalek::diffie_hellman;
//!
//! let shared_secret = diffie_hellman(&alice_secret, &bob_public.as_bytes());
//! #
//! let shared_secret = EphemeralSecret::diffie_hellman(&alice_secret, &bob_public);
//! # }
//! ```
//!
Expand All @@ -101,20 +100,19 @@
//! # extern crate rand;
//! #
//! # fn main() {
//! # use x25519_dalek::diffie_hellman;
//! # use x25519_dalek::generate_secret;
//! # use x25519_dalek::generate_public;
//! # use x25519_dalek::EphemeralPublic;
//! # use x25519_dalek::EphemeralSecret;
//! # use rand::thread_rng;
//! #
//! # let mut alice_csprng = thread_rng();
//! # let alice_secret = generate_secret(&mut alice_csprng);
//! # let alice_public = generate_public(&alice_secret);
//! # let alice_secret = EphemeralSecret::new(&mut alice_csprng);
//! # let alice_public = EphemeralPublic::from(&alice_secret);
//! #
//! # let mut bob_csprng = thread_rng();
//! # let bob_secret = generate_secret(&mut bob_csprng);
//! # let bob_public = generate_public(&bob_secret);
//! # let bob_secret = EphemeralSecret::new(&mut bob_csprng);
//! # let bob_public = EphemeralPublic::from(&bob_secret);
//! #
//! let shared_secret = diffie_hellman(&bob_secret, &alice_public.as_bytes());
//! let shared_secret = EphemeralSecret::diffie_hellman(&bob_secret, &alice_public);
//! # }
//! ```
//!
Expand All @@ -126,6 +124,8 @@
#![cfg_attr(feature = "bench", feature(test))]
#![deny(missing_docs)]

extern crate clear_on_drop;

extern crate curve25519_dalek;

extern crate rand_core;
Expand Down
109 changes: 84 additions & 25 deletions src/x25519.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,93 @@
//! This implements x25519 key exchange as specified by Mike Hamburg
//! and Adam Langley in [RFC7748](https://tools.ietf.org/html/rfc7748).

use core::ops::Mul;

use clear_on_drop::clear::Clear;

use curve25519_dalek::constants::ED25519_BASEPOINT_TABLE;
use curve25519_dalek::montgomery::MontgomeryPoint;
use curve25519_dalek::scalar::Scalar;

use rand_core::RngCore;
use rand_core::CryptoRng;

/// A DH ephemeral public key.
#[repr(C)]
pub struct EphemeralPublic(pub (crate) MontgomeryPoint);
hdevalence marked this conversation as resolved.
Show resolved Hide resolved

/// A DH ephemeral secret key.
#[repr(C)]
#[derive(Default)] // we derive Default in order to use the clear() method in Drop
DebugSteven marked this conversation as resolved.
Show resolved Hide resolved
pub struct EphemeralSecret(pub (crate) Scalar);

/// Overwrite ephemeral secret key material with null bytes when it goes out of scope.
impl Drop for EphemeralSecret {
fn drop(&mut self) {
self.0.clear();
}
}

/// Multiply this `EphemeralPublic` key by a `EphemeralSecret` key.
impl<'a, 'b> Mul<&'b EphemeralSecret> for &'a EphemeralPublic {
type Output = EphemeralPublic;

fn mul(self, secret: &'b EphemeralSecret) -> EphemeralPublic {
EphemeralPublic(self.0 * secret.0)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason to expose this trait impl if the diffie_hellman function exists? My guess is that it's probably not necessary.


impl EphemeralSecret {
/// Utility function to make it easier to call `x25519()` with
/// an ephemeral secret key and montegomery point as input and
/// a shared secret as the output.
pub fn diffie_hellman(&self, their_public: &EphemeralPublic) -> SharedSecret {
SharedSecret(x25519(&self.0, &MontgomeryPoint(*their_public.0.as_bytes())))
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that there's an EphemeralPublic type (which already holds a MontgomeryPoint), there's no need to copy it into a new point and call the x25519 function ... in fact the entire x25519 function could be removed and combined into the diffie_hellman function.

Also, if diffie_hellman takes self rather than &self, then an EphemeralSecret can't be used more than once.

}

/// Generate an x25519 `EphemeralSecret` key.
pub fn new<T>(csprng: &mut T) -> Self
where T: RngCore + CryptoRng
{
let mut bytes = [0u8; 32];

csprng.fill_bytes(&mut bytes);

EphemeralSecret(decode_scalar(&bytes))
}

}

impl<'a> From<&'a EphemeralSecret> for EphemeralPublic {
/// Given an x25519 `EphemeralSecret` key, compute its corresponding
/// `EphemeralPublic` key.
fn from(secret: &'a EphemeralSecret) -> EphemeralPublic {
EphemeralPublic((&ED25519_BASEPOINT_TABLE * &secret.0).to_montgomery())
}

}

/// A DH SharedSecret
#[repr(C)]
#[derive(Default)] // we derive Default in order to use the clear() method in Drop
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it really necessary to derive Default (instead of just calling self.0.clear() in the Drop impl)?

It seems like it would be better not to implement Default, so that the only way to create a SharedSecret is as the output of a DH function.

pub struct SharedSecret(pub (crate) MontgomeryPoint);

/// Overwrite shared secret material with null bytes when it goes out of scope.
impl Drop for SharedSecret {
fn drop(&mut self) {
self.0.clear();
}
}

impl SharedSecret {

/// View this shared secret key as a byte array.
#[inline]
pub fn as_bytes<'a>(&'a self) -> &'a [u8; 32] {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I think these lifetime specifiers might be redundant

&self.0.as_bytes()
}
}

/// "Decode" a scalar from a 32-byte array.
///
/// By "decode" here, what is really meant is applying key clamping by twiddling
Expand All @@ -37,34 +117,13 @@ fn decode_scalar(scalar: &[u8; 32]) -> Scalar {
Scalar::from_bits(s)
}

/// Generate an x25519 secret key.
pub fn generate_secret<T>(csprng: &mut T) -> [u8; 32]
where T: RngCore + CryptoRng
{
let mut bytes = [0u8; 32];
csprng.fill_bytes(&mut bytes);
bytes
}

/// Given an x25519 secret key, compute its corresponding public key.
pub fn generate_public(secret: &[u8; 32]) -> MontgomeryPoint {
(&decode_scalar(secret) * &ED25519_BASEPOINT_TABLE).to_montgomery()
}

/// The x25519 function, as specified in RFC7748.
pub fn x25519(scalar: &Scalar, point: &MontgomeryPoint) -> MontgomeryPoint {
fn x25519(scalar: &Scalar, point: &MontgomeryPoint) -> MontgomeryPoint {
let k: Scalar = decode_scalar(scalar.as_bytes());
hdevalence marked this conversation as resolved.
Show resolved Hide resolved

(&k * point)
(k * point)
}

/// Utility function to make it easier to call `x25519()` with byte arrays as
/// inputs and outputs.
pub fn diffie_hellman(my_secret: &[u8; 32], their_public: &[u8; 32]) -> [u8; 32] {
x25519(&Scalar::from_bits(*my_secret), &MontgomeryPoint(*their_public)).to_bytes()
}


#[cfg(test)]
mod test {
use super::*;
Expand All @@ -73,7 +132,7 @@ mod test {
input_point: &MontgomeryPoint,
expected: &[u8; 32]) {
let result = x25519(&input_scalar, &input_point);

assert_eq!(result.0, *expected);
}

Expand Down Expand Up @@ -152,7 +211,7 @@ mod test {
// 684cf59ba83309552800ef566f2f4d3c1c3887c49360e3875f2eb94d99532c51
// After 1,000,000 iterations:
// 7c3911e0ab2586fd864497297e575e6f3bc601c0883c30df5f4dd2d24f665424

do_iterations!(1);
assert_eq!(k.as_bytes(), &[ 0x42, 0x2c, 0x8e, 0x7a, 0x62, 0x27, 0xd7, 0xbc,
0xa1, 0x35, 0x0b, 0x3e, 0x2b, 0xb7, 0x27, 0x9f,
Expand Down