From 2a2646efd99ea8b8d954fded3538888db0038f28 Mon Sep 17 00:00:00 2001 From: Simon Wicky Date: Tue, 7 May 2024 17:01:27 +0200 Subject: [PATCH] Update crypto crates and fix sphinx issue (#94) * single out crypto changes * phase out curve25519 for x25519 * tidy up tests and warnings * update rand dependencies * update crypto dependencies * bump crate version * cleanup * refactor BLindingFactor into StaticSecret * remove unused export alias --- Cargo.toml | 33 +++---- benches/benchmarks.rs | 3 +- src/crypto/keys.rs | 154 ------------------------------ src/crypto/mod.rs | 37 ++++--- src/header/delays.rs | 4 +- src/header/filler.rs | 14 +-- src/header/keys.rs | 88 ++++++++--------- src/header/mod.rs | 61 ++++++------ src/header/routing/destination.rs | 2 +- src/header/routing/mod.rs | 2 +- src/header/routing/nodes.rs | 2 +- src/packet/builder.rs | 8 +- src/packet/mod.rs | 11 +-- src/route.rs | 5 +- src/surb/mod.rs | 23 +++-- src/test_utils.rs | 13 ++- tests/integration_test.rs | 33 +++---- 17 files changed, 173 insertions(+), 320 deletions(-) delete mode 100644 src/crypto/keys.rs diff --git a/Cargo.toml b/Cargo.toml index a990396..47fcfc6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "sphinx-packet" -version = "0.1.0" +version = "0.2.0" authors = ["Ania Piotrowska ", "Dave Hrycyszyn ", "Jędrzej Stuczyński "] edition = "2018" license = "Apache-2.0" @@ -11,27 +11,28 @@ readme = "README.md" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -aes = { version = "0.7.4", features = ["ctr"] } -bs58 = "0.4.0" -curve25519-dalek = "3.0.0" -hmac = "0.11.0" -digest = "0.9" -log = "0.4" -rand = {version = "0.7.3", features = ["wasm-bindgen"]} -rand_distr = "0.3" -sha2 = "0.9.1" -hkdf = "0.11.0" +aes = "0.8.4" +ctr = "0.9.2" +bs58 = "0.5.1" +x25519-dalek = { version = "2.0.1", features = ["static_secrets", "getrandom"] } +hmac = "0.12.1" +digest = "0.10.7" +log = "0.4.21" +rand = "0.8.5" +rand_distr = "0.4.3" +sha2 = "0.10.8" +hkdf = "0.12.4" lioness = "0.1.2" -arrayref = "0.3.5" +arrayref = "0.3.7" chacha = "0.3.0" blake2 = "0.8.0" # cannot be updated due to outdated dependency inside lioness -byteorder = "1.3.2" -subtle = "2.3.0" +byteorder = "1.5.0" +subtle = "2.4.1" [dev-dependencies] -mockall = "0.10.2" -criterion = "0.3" +mockall = "0.12.1" +criterion = "0.5.1" [[bench]] name = "benchmarks" diff --git a/benches/benchmarks.rs b/benches/benchmarks.rs index efe5f04..5504038 100644 --- a/benches/benchmarks.rs +++ b/benches/benchmarks.rs @@ -18,9 +18,10 @@ use criterion::{black_box, criterion_group, criterion_main, Criterion}; use sphinx_packet::constants::{ DESTINATION_ADDRESS_LENGTH, IDENTIFIER_LENGTH, NODE_ADDRESS_LENGTH, }; -use sphinx_packet::crypto::keygen; + use sphinx_packet::header::delays; use sphinx_packet::route::{Destination, DestinationAddressBytes, Node, NodeAddressBytes}; +use sphinx_packet::test_utils::fixtures::keygen; use sphinx_packet::SphinxPacket; use std::time::Duration; diff --git a/src/crypto/keys.rs b/src/crypto/keys.rs deleted file mode 100644 index 77dee4f..0000000 --- a/src/crypto/keys.rs +++ /dev/null @@ -1,154 +0,0 @@ -// Copyright 2020 Nym Technologies SA -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -// Ideally we would have used pure x25519_dalek implementation and created a wrapper for -// it, but unfortunately it's not an option as what we're doing here is not 'pure' -// x25519, as we require (in form of optimization, TODO: WHICH WE MUST ACTUALLY LEARN IF ITS -// NOT A VULNERABILITY) multiplying scalars together before exponentiation, i.e. -// to obtain g^{xyz} we compute `tmp = x*y*z` followed by g^tmp rather than -// G1 = g^x, G2 = G1^y, G3 = G2^z - -use curve25519_dalek::{ - constants::ED25519_BASEPOINT_TABLE, montgomery::MontgomeryPoint, scalar::Scalar, -}; -use rand::{rngs::OsRng, CryptoRng, RngCore}; - -pub const PRIVATE_KEY_SIZE: usize = 32; -pub const PUBLIC_KEY_SIZE: usize = 32; -pub const SHARED_SECRET_SIZE: usize = PUBLIC_KEY_SIZE; - -// this is specific to our keys being on curve25519; -// if not done, it could introduce attacks involving use -// of small-order points (and some side-channel attacks I think?). -// TODO: could an ECC 'expert' verify those claims? -pub fn clamp_scalar_bytes(mut scalar_bytes: [u8; PRIVATE_KEY_SIZE]) -> Scalar { - scalar_bytes[0] &= 248; - scalar_bytes[31] &= 127; - scalar_bytes[31] |= 64; - - Scalar::from_bits(scalar_bytes) -} - -// TODO: similarly to what x25519_dalek is doing, we should probably -// derive zeroize::Zeroize on drop here -pub struct PrivateKey(Scalar); - -// Because the lint below was renamed in nightly but not in stable, making this problematic in CI -// which tests both, for a brief period we allow renamed lints. -#[allow(renamed_and_removed_lints)] // TODO: remove this in next version -#[allow(clippy::derive_hash_xor_eq)] // TODO: we must be careful about that one if anything changes in the future -#[derive(Copy, Clone, Debug, Hash)] -pub struct PublicKey(MontgomeryPoint); - -// type aliases for easier reasoning -pub type EphemeralSecret = PrivateKey; -pub type SharedSecret = PublicKey; - -impl PrivateKey { - /// Perform a key exchange with another public key - pub fn diffie_hellman(&self, remote_public_key: &PublicKey) -> SharedSecret { - PublicKey(self.0 * remote_public_key.0) - } - - // Do not expose this. It can lead to serious security issues if used incorrectly. - pub(crate) fn clone(&self) -> Self { - PrivateKey(self.0) - } - - // honestly, this method shouldn't really exist, but right now we have no decent - // rng propagation in the library - pub fn new() -> Self { - let mut rng = OsRng; - Self::new_with_rng(&mut rng) - } - - pub fn new_with_rng(rng: &mut R) -> Self { - let mut bytes = [0u8; PRIVATE_KEY_SIZE]; - rng.fill_bytes(&mut bytes); - PrivateKey(clamp_scalar_bytes(bytes)) - } - - pub fn to_bytes(&self) -> [u8; PRIVATE_KEY_SIZE] { - self.0.to_bytes() - } -} - -impl Default for PrivateKey { - fn default() -> Self { - PrivateKey::new() - } -} - -// TODO: is this 'safe' ? -impl<'a, 'b> std::ops::Mul<&'b Scalar> for &'a EphemeralSecret { - type Output = EphemeralSecret; - fn mul(self, rhs: &'b Scalar) -> EphemeralSecret { - PrivateKey(self.0 * rhs) - } -} - -impl<'b> std::ops::MulAssign<&'b Scalar> for EphemeralSecret { - fn mul_assign(&mut self, _rhs: &'b Scalar) { - self.0.mul_assign(_rhs) - } -} - -impl From for EphemeralSecret { - fn from(scalar: Scalar) -> EphemeralSecret { - // TODO: should we ensure it's a valid scalar by performing - // montgomery reduction and/or clamping? - PrivateKey(scalar) - } -} - -impl From<[u8; PRIVATE_KEY_SIZE]> for PrivateKey { - fn from(bytes: [u8; 32]) -> PrivateKey { - // TODO: do we have to clamp it here? - PrivateKey(clamp_scalar_bytes(bytes)) - } -} - -impl PublicKey { - pub fn as_bytes(&self) -> &[u8; PUBLIC_KEY_SIZE] { - self.0.as_bytes() - } -} - -impl<'a> From<&'a PrivateKey> for PublicKey { - fn from(private_key: &'a PrivateKey) -> PublicKey { - // multiplication in edwards using the precomputed ed25519 basepoint table is over 3x quicker - // than multiplication inside montgomery using the curve generator - PublicKey((&ED25519_BASEPOINT_TABLE * &private_key.0).to_montgomery()) - } -} - -impl From<[u8; PUBLIC_KEY_SIZE]> for PublicKey { - fn from(bytes: [u8; PUBLIC_KEY_SIZE]) -> PublicKey { - PublicKey(MontgomeryPoint(bytes)) - } -} - -impl PartialEq for PublicKey { - fn eq(&self, other: &Self) -> bool { - self.0.eq(&other.0) - } -} - -impl Eq for PublicKey {} - -pub fn keygen() -> (PrivateKey, PublicKey) { - let private_key = PrivateKey::new(); - let public_key = PublicKey::from(&private_key); - (private_key, public_key) -} diff --git a/src/crypto/mod.rs b/src/crypto/mod.rs index bca3580..1a15f7f 100644 --- a/src/crypto/mod.rs +++ b/src/crypto/mod.rs @@ -12,22 +12,30 @@ // See the License for the specific language governing permissions and // limitations under the License. -use aes::cipher::{NewCipher, StreamCipher}; -use aes::Aes128Ctr; -use digest::generic_array::{ArrayLength, GenericArray}; -use digest::{BlockInput, FixedOutput, Reset, Update}; -use hmac::{crypto_mac, Hmac, Mac, NewMac}; +use aes::{ + cipher::{KeyIvInit, StreamCipher}, + Aes128, +}; +use digest::{ + block_buffer::Eager, + consts::U256, + core_api::{BlockSizeUser, BufferKindUser, CoreProxy, FixedOutputCore}, + generic_array::GenericArray, + typenum::{IsLess, Le, NonZero}, + CtOutput, HashMarker, +}; +use hmac::{Hmac, Mac}; -pub mod keys; - -// to not break existing imports -pub use keys::*; +//type export and aliasing to keep compatibility +pub use x25519_dalek::PublicKey; +pub type PrivateKey = x25519_dalek::StaticSecret; pub const STREAM_CIPHER_KEY_SIZE: usize = 16; pub const STREAM_CIPHER_INIT_VECTOR: [u8; 16] = [0u8; 16]; -// Type alias for ease of use so that it would not require explicit import of crypto_mac or Hmac -pub type HmacOutput = crypto_mac::Output>; +// Type alias for ease of use +pub type HmacOutput = CtOutput>; +type Aes128Ctr = ctr::Ctr64BE; pub fn generate_pseudorandom_bytes( // TODO: those should use proper generic arrays to begin with!! @@ -49,9 +57,10 @@ pub fn generate_pseudorandom_bytes( /// Compute keyed hmac pub fn compute_keyed_hmac(key: &[u8], data: &[u8]) -> HmacOutput where - D: Update + BlockInput + FixedOutput + Reset + Default + Clone, - D::BlockSize: ArrayLength, - D::OutputSize: ArrayLength, + D: CoreProxy, + D::Core: HashMarker + FixedOutputCore + BufferKindUser + Default + Clone, + ::BlockSize: IsLess, + Le<::BlockSize, U256>: NonZero, { let mut hmac = Hmac::::new_from_slice(key).expect("HMAC should be able to take key of any size!"); diff --git a/src/header/delays.rs b/src/header/delays.rs index 35add6c..e281350 100644 --- a/src/header/delays.rs +++ b/src/header/delays.rs @@ -176,7 +176,7 @@ mod delay_summing { let delay2 = Delay(123); let expected1 = Delay(165); - assert_eq!(expected1, &delay1 + &delay2); + assert_eq!(expected1, delay1 + delay2); let expected2 = Delay(265); let delay3 = Delay(100); @@ -185,7 +185,7 @@ mod delay_summing { #[test] fn works_with_iterator() { - let delays = vec![Delay(42), Delay(123), Delay(100)]; + let delays = [Delay(42), Delay(123), Delay(100)]; let expected = Delay(265); assert_eq!(expected, delays.iter().sum()); diff --git a/src/header/filler.rs b/src/header/filler.rs index 2712f7a..25b13ab 100644 --- a/src/header/filler.rs +++ b/src/header/filler.rs @@ -90,7 +90,7 @@ mod test_creating_pseudorandom_bytes { use crate::header::keys; use super::*; - use crypto::{EphemeralSecret, SharedSecret}; + use x25519_dalek::{PublicKey, StaticSecret}; #[test] fn with_no_keys_it_generates_empty_filler_string() { @@ -102,7 +102,7 @@ mod test_creating_pseudorandom_bytes { #[test] fn with_1_key_it_generates_filler_of_length_1_times_3_times_security_parameter() { - let shared_keys = vec![SharedSecret::from(&EphemeralSecret::new())]; + let shared_keys = [PublicKey::from(&StaticSecret::random())]; let routing_keys: Vec<_> = shared_keys .iter() .map(|&key| keys::RoutingKeys::derive(key)) @@ -114,10 +114,10 @@ mod test_creating_pseudorandom_bytes { #[test] fn with_3_key_it_generates_filler_of_length_3_times_3_times_security_parameter() { - let shared_keys = vec![ - SharedSecret::from(&EphemeralSecret::new()), - SharedSecret::from(&EphemeralSecret::new()), - SharedSecret::from(&EphemeralSecret::new()), + let shared_keys = [ + PublicKey::from(&StaticSecret::random()), + PublicKey::from(&StaticSecret::random()), + PublicKey::from(&StaticSecret::random()), ]; let routing_keys: Vec<_> = shared_keys .iter() @@ -132,7 +132,7 @@ mod test_creating_pseudorandom_bytes { fn panics_with_more_keys_than_the_maximum_path_length() { let shared_keys: Vec<_> = std::iter::repeat(()) .take(constants::MAX_PATH_LENGTH + 1) - .map(|_| SharedSecret::from(&EphemeralSecret::new())) + .map(|_| PublicKey::from(&StaticSecret::random())) .collect(); let routing_keys: Vec<_> = shared_keys .iter() diff --git a/src/header/keys.rs b/src/header/keys.rs index 0c39660..33824b0 100644 --- a/src/header/keys.rs +++ b/src/header/keys.rs @@ -12,40 +12,39 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::convert::TryInto; use std::fmt; use crate::constants::{ BLINDING_FACTOR_SIZE, HKDF_INPUT_SEED, INTEGRITY_MAC_KEY_SIZE, PAYLOAD_KEY_SIZE, ROUTING_KEYS_LENGTH, }; +use crate::crypto; use crate::crypto::STREAM_CIPHER_KEY_SIZE; -use crate::crypto::{self, EphemeralSecret}; use crate::route::Node; -use crypto::SharedSecret; -use curve25519_dalek::scalar::Scalar; use hkdf::Hkdf; use sha2::Sha256; +use x25519_dalek::{PublicKey, StaticSecret}; pub type StreamCipherKey = [u8; STREAM_CIPHER_KEY_SIZE]; pub type HeaderIntegrityMacKey = [u8; INTEGRITY_MAC_KEY_SIZE]; // TODO: perhaps change PayloadKey to a Vec considering it's almost 200 bytes long? // we will lose length assertions but won't need to copy all that data every single function call pub type PayloadKey = [u8; PAYLOAD_KEY_SIZE]; -pub type BlindingFactor = [u8; BLINDING_FACTOR_SIZE]; #[derive(Clone)] pub struct RoutingKeys { pub stream_cipher_key: StreamCipherKey, pub header_integrity_hmac_key: HeaderIntegrityMacKey, pub payload_key: PayloadKey, - pub blinding_factor: BlindingFactor, + pub blinding_factor: StaticSecret, } impl RoutingKeys { // or should this be renamed to 'new'? // Given that everything here except RoutingKeys lives in the `crypto` module, I think // that this one could potentially move most of its functionality there quite profitably. - pub fn derive(shared_key: crypto::SharedSecret) -> Self { + pub fn derive(shared_key: PublicKey) -> Self { let hkdf = Hkdf::::new(None, shared_key.as_bytes()); let mut i = 0; @@ -64,11 +63,10 @@ impl RoutingKeys { payload_key.copy_from_slice(&output[i..i + PAYLOAD_KEY_SIZE]); i += PAYLOAD_KEY_SIZE; - // TODO: we later treat blinding factor as a Scalar, the question is, should it be clamped - // and/or go through montgomery reduction? We kinda need somebody with good ECC knowledge - // to answer this question (and other related ones). - let mut blinding_factor: [u8; BLINDING_FACTOR_SIZE] = Default::default(); - blinding_factor.copy_from_slice(&output[i..i + BLINDING_FACTOR_SIZE]); + //Safety, converting a slice of size BLINDING_FACTOR_SIZE into an array of type [u8; BLINDING_FACTOR_SIZE], hence unwrap is fine + let blinding_factor_bytes: [u8; BLINDING_FACTOR_SIZE] = + output[i..i + BLINDING_FACTOR_SIZE].try_into().unwrap(); + let blinding_factor = StaticSecret::from(blinding_factor_bytes); Self { stream_cipher_key, @@ -100,42 +98,29 @@ impl PartialEq for RoutingKeys { } pub struct KeyMaterial { - pub initial_shared_secret: crypto::SharedSecret, + pub initial_shared_secret: PublicKey, // why this is here? pub routing_keys: Vec, } impl KeyMaterial { // derive shared keys, group elements, blinding factors - pub fn derive(route: &[Node], initial_secret: &EphemeralSecret) -> Self { - let initial_shared_secret = SharedSecret::from(initial_secret); + pub fn derive(route: &[Node], initial_secret: &StaticSecret) -> Self { + let initial_shared_secret = PublicKey::from(initial_secret); let mut routing_keys = Vec::with_capacity(route.len()); - let mut accumulator = initial_secret.clone(); + let mut blinding_factors = vec![initial_secret.clone()]; for (i, node) in route.iter().enumerate() { - // pub^{a * b * ...} - let shared_key = accumulator.diffie_hellman(&node.pub_key); - // let shared_key = Self::compute_shared_key(node.pub_key, &accumulator); + let shared_key = blinding_factors + .iter() + .fold(node.pub_key, |acc, blinding_factor| { + PublicKey::from(blinding_factor.diffie_hellman(&acc).to_bytes()) + }); let node_routing_keys = RoutingKeys::derive(shared_key); // it's not the last iteration if i != route.len() + 1 { - // TODO: do we need to make the reduction here or could we get away with clamping or even nothing at all? - // considering (I *think*) proper reductions will happen during scalar multiplication, i.e. g^x? - // So far it *seems* to produce correct result, but could it be the case it introduces - // some vulnerabilities? Need some ECC expert here. - - // performs montgomery reduction - let blinding_factor_scalar = - &Scalar::from_bytes_mod_order(node_routing_keys.blinding_factor); - // alternatives: - - // 'only' clamps the scalar - // let blinding_factor_scalar = crypto::clamp_scalar_bytes(node_routing_keys.blinding_factor); - - // 'only' makes it 255 bit long - // let blinding_factor_scalar = Scalar::from_bits(node_routing_keys.blinding_factor); - accumulator *= blinding_factor_scalar; + blinding_factors.push(node_routing_keys.blinding_factor.clone()); } routing_keys.push(node_routing_keys); @@ -160,12 +145,11 @@ mod deriving_key_material { #[test] fn it_returns_no_routing_keys() { let empty_route: Vec = vec![]; - let initial_secret = EphemeralSecret::new(); - let hacky_secret_copy = EphemeralSecret::from(initial_secret.to_bytes()); + let initial_secret = StaticSecret::random(); let key_material = KeyMaterial::derive(&empty_route, &initial_secret); assert_eq!(0, key_material.routing_keys.len()); assert_eq!( - SharedSecret::from(&hacky_secret_copy).as_bytes(), + PublicKey::from(&initial_secret).as_bytes(), key_material.initial_shared_secret.as_bytes() ) } @@ -176,12 +160,11 @@ mod deriving_key_material { use super::*; use crate::test_utils::random_node; - fn setup() -> (Vec, EphemeralSecret, KeyMaterial) { + fn setup() -> (Vec, StaticSecret, KeyMaterial) { let route: Vec = vec![random_node(), random_node(), random_node()]; - let initial_secret = EphemeralSecret::new(); - let hacky_secret_copy = EphemeralSecret::from(initial_secret.to_bytes()); + let initial_secret = StaticSecret::random(); let key_material = KeyMaterial::derive(&route, &initial_secret); - (route, hacky_secret_copy, key_material) + (route, initial_secret, key_material) } #[test] @@ -194,7 +177,7 @@ mod deriving_key_material { fn it_returns_correctly_inited_shared_secret() { let (_, initial_secret, key_material) = setup(); assert_eq!( - SharedSecret::from(&initial_secret).as_bytes(), + PublicKey::from(&initial_secret).as_bytes(), key_material.initial_shared_secret.as_bytes() ); } @@ -207,13 +190,18 @@ mod deriving_key_material { // incorrectly blinded shared key through the mixnet in the (unencrypted) // Sphinx packet header. So this test ensures that the accumulator gets incremented // properly on each run through the loop. - let mut expected_accumulator = initial_secret; + let mut expected_accumulator = vec![initial_secret]; for (i, node) in route.iter().enumerate() { - let expected_shared_key = expected_accumulator.diffie_hellman(&node.pub_key); + let expected_shared_key = + expected_accumulator + .iter() + .fold(node.pub_key, |acc, blinding_factor| { + PublicKey::from(blinding_factor.diffie_hellman(&acc).to_bytes()) + }); + let expected_routing_keys = RoutingKeys::derive(expected_shared_key); - expected_accumulator = &expected_accumulator - * &Scalar::from_bytes_mod_order(expected_routing_keys.blinding_factor); + expected_accumulator.push(expected_routing_keys.blinding_factor); let expected_routing_keys = RoutingKeys::derive(expected_shared_key); assert_eq!(expected_routing_keys, key_material.routing_keys[i]) } @@ -227,8 +215,8 @@ mod key_derivation_function { #[test] fn it_expands_the_seed_key_to_expected_length() { - let initial_secret = EphemeralSecret::new(); - let shared_key = SharedSecret::from(&initial_secret); + let initial_secret = StaticSecret::random(); + let shared_key = PublicKey::from(&initial_secret); let routing_keys = RoutingKeys::derive(shared_key); assert_eq!( crypto::STREAM_CIPHER_KEY_SIZE, @@ -238,8 +226,8 @@ mod key_derivation_function { #[test] fn it_returns_the_same_output_for_two_equal_inputs() { - let initial_secret = EphemeralSecret::new(); - let shared_key = SharedSecret::from(&initial_secret); + let initial_secret = StaticSecret::random(); + let shared_key = PublicKey::from(&initial_secret); let routing_keys1 = RoutingKeys::derive(shared_key); let routing_keys2 = RoutingKeys::derive(shared_key); assert_eq!(routing_keys1, routing_keys2); diff --git a/src/header/mod.rs b/src/header/mod.rs index dcec760..e96888d 100644 --- a/src/header/mod.rs +++ b/src/header/mod.rs @@ -13,17 +13,15 @@ // limitations under the License. use crate::constants::HEADER_INTEGRITY_MAC_SIZE; -use crate::crypto; use crate::header::delays::Delay; use crate::header::filler::Filler; -use crate::header::keys::{BlindingFactor, PayloadKey}; +use crate::header::keys::PayloadKey; use crate::header::routing::nodes::ParsedRawRoutingInformation; use crate::header::routing::{EncapsulatedRoutingInformation, ENCRYPTED_ROUTING_INFO_SIZE}; use crate::route::{Destination, DestinationAddressBytes, Node, NodeAddressBytes, SURBIdentifier}; use crate::{Error, ErrorKind, Result}; -use crypto::{EphemeralSecret, PrivateKey, SharedSecret}; -use curve25519_dalek::scalar::Scalar; use keys::RoutingKeys; +use x25519_dalek::{PublicKey, StaticSecret}; pub mod delays; pub mod filler; @@ -37,7 +35,7 @@ pub const HEADER_SIZE: usize = 32 + HEADER_INTEGRITY_MAC_SIZE + ENCRYPTED_ROUTIN #[derive(Debug)] #[cfg_attr(test, derive(Clone))] pub struct SphinxHeader { - pub shared_secret: SharedSecret, + pub shared_secret: PublicKey, pub routing_info: EncapsulatedRoutingInformation, } @@ -50,7 +48,7 @@ impl SphinxHeader { // needs client's secret key, how should we inject this? // needs to deal with SURBs too at some point pub fn new( - initial_secret: &EphemeralSecret, + initial_secret: &StaticSecret, route: &[Node], delays: &[Delay], destination: &Destination, @@ -87,7 +85,7 @@ impl SphinxHeader { /// Prefer normal [process] instead. pub fn process_with_derived_keys( self, - new_blinded_secret: &Option, + new_blinded_secret: &Option, routing_keys: &RoutingKeys, ) -> Result { if !self.routing_info.integrity_mac.verify( @@ -140,14 +138,14 @@ impl SphinxHeader { /// Using the provided shared_secret and node's secret key, derive all routing keys for this hop. pub fn compute_routing_keys( - shared_secret: &SharedSecret, - node_secret_key: &PrivateKey, + shared_secret: &PublicKey, + node_secret_key: &StaticSecret, ) -> RoutingKeys { - let shared_key = node_secret_key.diffie_hellman(shared_secret); + let shared_key = PublicKey::from(node_secret_key.diffie_hellman(shared_secret).to_bytes()); keys::RoutingKeys::derive(shared_key) } - pub fn process(self, node_secret_key: &PrivateKey) -> Result { + pub fn process(self, node_secret_key: &StaticSecret) -> Result { let routing_keys = Self::compute_routing_keys(&self.shared_secret, node_secret_key); if !self.routing_info.integrity_mac.verify( @@ -219,7 +217,7 @@ impl SphinxHeader { let mut shared_secret_bytes = [0u8; 32]; // first 32 bytes represent the shared secret shared_secret_bytes.copy_from_slice(&bytes[..32]); - let shared_secret = SharedSecret::from(shared_secret_bytes); + let shared_secret = PublicKey::from(shared_secret_bytes); // the rest are for the encapsulated routing info let encapsulated_routing_info_bytes = bytes[32..HEADER_SIZE].to_vec(); @@ -234,43 +232,44 @@ impl SphinxHeader { } fn blind_the_shared_secret( - shared_secret: SharedSecret, - blinding_factor: BlindingFactor, - ) -> SharedSecret { - // TODO BEFORE PR: clamping, reduction, etc. - let blinding_factor = Scalar::from_bytes_mod_order(blinding_factor); - let blinder: EphemeralSecret = blinding_factor.into(); + shared_secret: PublicKey, + blinding_factor: StaticSecret, + ) -> PublicKey { // shared_secret * blinding_factor - blinder.diffie_hellman(&shared_secret) + let new_shared_secret = blinding_factor.diffie_hellman(&shared_secret); + PublicKey::from(new_shared_secret.to_bytes()) } } #[cfg(test)] mod create_and_process_sphinx_packet_header { use super::*; - use crate::{constants::NODE_ADDRESS_LENGTH, test_utils::fixtures::destination_fixture}; + use crate::{ + constants::NODE_ADDRESS_LENGTH, + test_utils::fixtures::{destination_fixture, keygen}, + }; use std::time::Duration; #[test] fn it_returns_correct_routing_information_at_each_hop_for_route_of_3_mixnodes() { - let (node1_sk, node1_pk) = crypto::keygen(); + let (node1_sk, node1_pk) = keygen(); let node1 = Node { address: NodeAddressBytes::from_bytes([5u8; NODE_ADDRESS_LENGTH]), pub_key: node1_pk, }; - let (node2_sk, node2_pk) = crypto::keygen(); + let (node2_sk, node2_pk) = keygen(); let node2 = Node { address: NodeAddressBytes::from_bytes([4u8; NODE_ADDRESS_LENGTH]), pub_key: node2_pk, }; - let (node3_sk, node3_pk) = crypto::keygen(); + let (node3_sk, node3_pk) = keygen(); let node3 = Node { address: NodeAddressBytes::from_bytes([2u8; NODE_ADDRESS_LENGTH]), pub_key: node3_pk, }; let route = [node1, node2, node3]; let destination = destination_fixture(); - let initial_secret = EphemeralSecret::new(); + let initial_secret = StaticSecret::random(); let average_delay = 1; let delays = delays::generate_from_average_duration(route.len(), Duration::from_secs(average_delay)); @@ -390,24 +389,24 @@ mod unwrap_routing_information { mod unwrapping_using_previously_derived_keys { use super::*; use crate::constants::NODE_ADDRESS_LENGTH; - use crate::test_utils::fixtures::destination_fixture; + use crate::test_utils::fixtures::{destination_fixture, keygen}; use std::time::Duration; #[test] fn produces_same_result_for_forward_hop() { - let (node1_sk, node1_pk) = crypto::keygen(); + let (node1_sk, node1_pk) = keygen(); let node1 = Node { address: NodeAddressBytes::from_bytes([5u8; NODE_ADDRESS_LENGTH]), pub_key: node1_pk, }; - let (_, node2_pk) = crypto::keygen(); + let (_, node2_pk) = keygen(); let node2 = Node { address: NodeAddressBytes::from_bytes([4u8; NODE_ADDRESS_LENGTH]), pub_key: node2_pk, }; let route = [node1, node2]; let destination = destination_fixture(); - let initial_secret = EphemeralSecret::new(); + let initial_secret = StaticSecret::random(); let average_delay = 1; let delays = delays::generate_from_average_duration(route.len(), Duration::from_secs(average_delay)); @@ -442,14 +441,14 @@ mod unwrapping_using_previously_derived_keys { #[test] fn produces_same_result_for_final_hop() { - let (node1_sk, node1_pk) = crypto::keygen(); + let (node1_sk, node1_pk) = keygen(); let node1 = Node { address: NodeAddressBytes::from_bytes([5u8; NODE_ADDRESS_LENGTH]), pub_key: node1_pk, }; let route = [node1]; let destination = destination_fixture(); - let initial_secret = EphemeralSecret::new(); + let initial_secret = StaticSecret::random(); let average_delay = 1; let delays = delays::generate_from_average_duration(route.len(), Duration::from_secs(average_delay)); @@ -486,7 +485,7 @@ mod converting_header_to_bytes { fn it_is_possible_to_convert_back_and_forth() { let encapsulated_routing_info = encapsulated_routing_information_fixture(); let header = SphinxHeader { - shared_secret: SharedSecret::from(&EphemeralSecret::new()), + shared_secret: PublicKey::from(&StaticSecret::random()), routing_info: encapsulated_routing_info, }; diff --git a/src/header/routing/destination.rs b/src/header/routing/destination.rs index 69282df..cdb6985 100644 --- a/src/header/routing/destination.rs +++ b/src/header/routing/destination.rs @@ -76,7 +76,7 @@ impl FinalRoutingInformation { // return D || I || PAD PaddedFinalRoutingInformation { value: std::iter::once(self.flag) - .chain(self.version.to_bytes().into_iter()) + .chain(self.version.to_bytes()) .chain(self.destination.as_bytes().iter().cloned()) .chain(self.identifier.iter().cloned()) .chain(padding.iter().cloned()) diff --git a/src/header/routing/mod.rs b/src/header/routing/mod.rs index f78a069..ee64664 100644 --- a/src/header/routing/mod.rs +++ b/src/header/routing/mod.rs @@ -289,7 +289,7 @@ mod encapsulating_forward_routing_information { let delay0 = Delay::new_from_nanos(10); let delay1 = Delay::new_from_nanos(20); let delay2 = Delay::new_from_nanos(30); - let delays = [delay0.clone(), delay1.clone(), delay2].to_vec(); + let delays = [delay0, delay1, delay2].to_vec(); let routing_keys = [ routing_keys_fixture(), routing_keys_fixture(), diff --git a/src/header/routing/nodes.rs b/src/header/routing/nodes.rs index 31358ff..87d5881 100644 --- a/src/header/routing/nodes.rs +++ b/src/header/routing/nodes.rs @@ -69,7 +69,7 @@ impl RoutingInformation { .chain(self.version.to_bytes().iter().cloned()) .chain(self.node_address.as_bytes_ref().iter().cloned()) .chain(self.delay.to_bytes().iter().cloned()) - .chain(self.header_integrity_mac.into_inner().into_iter()) + .chain(self.header_integrity_mac.into_inner()) .chain(self.next_routing_information.iter().cloned()) .collect() } diff --git a/src/packet/builder.rs b/src/packet/builder.rs index 9aa9310..3b45546 100644 --- a/src/packet/builder.rs +++ b/src/packet/builder.rs @@ -1,16 +1,16 @@ use crate::{ - crypto::EphemeralSecret, header::{delays::Delay, SphinxHeader}, payload::Payload, route::{Destination, Node}, Result, SphinxPacket, }; +use x25519_dalek::StaticSecret; pub const DEFAULT_PAYLOAD_SIZE: usize = 1024; pub struct SphinxPacketBuilder<'a> { payload_size: usize, - initial_secret: Option<&'a EphemeralSecret>, + initial_secret: Option<&'a StaticSecret>, } impl<'a> SphinxPacketBuilder<'a> { @@ -23,7 +23,7 @@ impl<'a> SphinxPacketBuilder<'a> { self } - pub fn with_initial_secret(mut self, initial_secret: &'a EphemeralSecret) -> Self { + pub fn with_initial_secret(mut self, initial_secret: &'a StaticSecret) -> Self { self.initial_secret = Some(initial_secret); self } @@ -37,7 +37,7 @@ impl<'a> SphinxPacketBuilder<'a> { ) -> Result { let (header, payload_keys) = match self.initial_secret.as_ref() { Some(initial_secret) => SphinxHeader::new(initial_secret, route, delays, destination), - None => SphinxHeader::new(&EphemeralSecret::new(), route, delays, destination), + None => SphinxHeader::new(&StaticSecret::random(), route, delays, destination), }; // no need to check if plaintext has correct length as this check is already performed in payload encapsulation diff --git a/src/packet/mod.rs b/src/packet/mod.rs index 3e98dc9..dcbeccb 100644 --- a/src/packet/mod.rs +++ b/src/packet/mod.rs @@ -1,7 +1,5 @@ -use crate::crypto::keys::SharedSecret; use crate::header::keys::RoutingKeys; use crate::{ - crypto::PrivateKey, header::{self, delays::Delay, HEADER_SIZE}, payload::{Payload, PAYLOAD_OVERHEAD_SIZE}, route::{Destination, DestinationAddressBytes, Node, NodeAddressBytes, SURBIdentifier}, @@ -9,6 +7,7 @@ use crate::{ }; use builder::SphinxPacketBuilder; use header::{ProcessedHeader, SphinxHeader}; +use x25519_dalek::{PublicKey, StaticSecret}; pub mod builder; @@ -20,7 +19,7 @@ pub enum ProcessedPacket { } impl ProcessedPacket { - pub fn shared_secret(&self) -> Option { + pub fn shared_secret(&self) -> Option { match self { ProcessedPacket::ForwardHop(packet, ..) => Some(packet.shared_secret()), ProcessedPacket::FinalHop(..) => None, @@ -45,7 +44,7 @@ impl SphinxPacket { SphinxPacketBuilder::default().build_packet(message, route, destination, delays) } - pub fn shared_secret(&self) -> SharedSecret { + pub fn shared_secret(&self) -> PublicKey { self.header.shared_secret } @@ -62,7 +61,7 @@ impl SphinxPacket { /// Prefer normal [process] instead. pub fn process_with_derived_keys( self, - new_blinded_secret: &Option, + new_blinded_secret: &Option, routing_keys: &RoutingKeys, ) -> Result { let unwrapped_header = self @@ -93,7 +92,7 @@ impl SphinxPacket { } // TODO: we should have some list of 'seen shared_keys' for replay detection, but this should be handled by a mix node - pub fn process(self, node_secret_key: &PrivateKey) -> Result { + pub fn process(self, node_secret_key: &StaticSecret) -> Result { let unwrapped_header = self.header.process(node_secret_key)?; match unwrapped_header { ProcessedHeader::ForwardHop(new_header, next_hop_address, delay, payload_key) => { diff --git a/src/route.rs b/src/route.rs index 0177961..5574d68 100644 --- a/src/route.rs +++ b/src/route.rs @@ -13,7 +13,6 @@ // limitations under the License. use crate::constants::{DESTINATION_ADDRESS_LENGTH, IDENTIFIER_LENGTH, NODE_ADDRESS_LENGTH}; -use crate::crypto; use crate::{Error, ErrorKind, Result}; use std::fmt::{self, Display, Formatter}; @@ -176,11 +175,11 @@ impl Destination { #[derive(Clone, Debug)] pub struct Node { pub address: NodeAddressBytes, - pub pub_key: crypto::PublicKey, + pub pub_key: x25519_dalek::PublicKey, } impl Node { - pub fn new(address: NodeAddressBytes, pub_key: crypto::PublicKey) -> Self { + pub fn new(address: NodeAddressBytes, pub_key: x25519_dalek::PublicKey) -> Self { Self { address, pub_key } } } diff --git a/src/surb/mod.rs b/src/surb/mod.rs index 7a88f57..42f02fb 100644 --- a/src/surb/mod.rs +++ b/src/surb/mod.rs @@ -3,10 +3,11 @@ use crate::header::delays::Delay; use crate::header::keys::PayloadKey; use crate::payload::Payload; use crate::route::{Destination, Node, NodeAddressBytes}; -use crate::{crypto::EphemeralSecret, Error, ErrorKind, Result}; use crate::{header, SphinxPacket}; +use crate::{Error, ErrorKind, Result}; use header::{SphinxHeader, HEADER_SIZE}; use std::fmt; +use x25519_dalek::StaticSecret; /// A Single Use Reply Block (SURB) must have a pre-aggregated Sphinx header, /// the address of the first hop in the route of the SURB, and the key material @@ -51,14 +52,14 @@ impl SURBMaterial { #[allow(non_snake_case)] pub fn construct_SURB(self) -> Result { - let surb_initial_secret = EphemeralSecret::new(); + let surb_initial_secret = StaticSecret::random(); SURB::new(surb_initial_secret, self) } } #[allow(non_snake_case)] impl SURB { - pub fn new(surb_initial_secret: EphemeralSecret, surb_material: SURBMaterial) -> Result { + pub fn new(surb_initial_secret: StaticSecret, surb_material: SURBMaterial) -> Result { let surb_route = surb_material.surb_route; let surb_delays = surb_material.surb_delays; let surb_destination = surb_material.surb_destination; @@ -165,24 +166,26 @@ impl SURB { mod prepare_and_use_process_surb { use super::*; use crate::constants::NODE_ADDRESS_LENGTH; - use crate::crypto; use crate::header::{delays, HEADER_SIZE}; - use crate::{packet::builder::DEFAULT_PAYLOAD_SIZE, test_utils::fixtures::destination_fixture}; + use crate::{ + packet::builder::DEFAULT_PAYLOAD_SIZE, + test_utils::fixtures::{destination_fixture, keygen}, + }; use std::time::Duration; #[allow(non_snake_case)] fn SURB_fixture() -> SURB { - let (_, node1_pk) = crypto::keygen(); + let (_, node1_pk) = keygen(); let node1 = Node { address: NodeAddressBytes::from_bytes([5u8; NODE_ADDRESS_LENGTH]), pub_key: node1_pk, }; - let (_, node2_pk) = crypto::keygen(); + let (_, node2_pk) = keygen(); let node2 = Node { address: NodeAddressBytes::from_bytes([4u8; NODE_ADDRESS_LENGTH]), pub_key: node2_pk, }; - let (_, node3_pk) = crypto::keygen(); + let (_, node3_pk) = keygen(); let node3 = Node { address: NodeAddressBytes::from_bytes([2u8; NODE_ADDRESS_LENGTH]), pub_key: node3_pk, @@ -190,7 +193,7 @@ mod prepare_and_use_process_surb { let surb_route = vec![node1, node2, node3]; let surb_destination = destination_fixture(); - let surb_initial_secret = EphemeralSecret::new(); + let surb_initial_secret = StaticSecret::random(); let surb_delays = delays::generate_from_average_duration(surb_route.len(), Duration::from_secs(3)); @@ -205,7 +208,7 @@ mod prepare_and_use_process_surb { fn returns_error_if_surb_route_empty() { let surb_route = Vec::new(); let surb_destination = destination_fixture(); - let surb_initial_secret = EphemeralSecret::new(); + let surb_initial_secret = StaticSecret::random(); let surb_delays = delays::generate_from_average_duration(surb_route.len(), Duration::from_secs(3)); let expected = ErrorKind::InvalidSURB; diff --git a/src/test_utils.rs b/src/test_utils.rs index 5255ef3..240c84c 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -14,12 +14,13 @@ use crate::{ constants::NODE_ADDRESS_LENGTH, - crypto, route::{Node, NodeAddressBytes}, }; pub mod fixtures { + use x25519_dalek::{PublicKey, StaticSecret}; + use crate::{ constants::{ BLINDING_FACTOR_SIZE, DESTINATION_ADDRESS_LENGTH, HEADER_INTEGRITY_MAC_SIZE, @@ -62,7 +63,7 @@ pub mod fixtures { stream_cipher_key: [1u8; crypto::STREAM_CIPHER_KEY_SIZE], header_integrity_hmac_key: [2u8; INTEGRITY_MAC_KEY_SIZE], payload_key: [3u8; PAYLOAD_KEY_SIZE], - blinding_factor: [4u8; BLINDING_FACTOR_SIZE], + blinding_factor: [4u8; BLINDING_FACTOR_SIZE].into(), } } @@ -84,10 +85,16 @@ pub mod fixtures { integrity_mac: header_integrity_mac_fixture(), } } + + pub fn keygen() -> (StaticSecret, PublicKey) { + let private_key = StaticSecret::random(); + let public_key = PublicKey::from(&private_key); + (private_key, public_key) + } } pub fn random_node() -> Node { - let random_private_key = crypto::PrivateKey::new(); + let random_private_key = x25519_dalek::EphemeralSecret::random(); Node { address: NodeAddressBytes::from_bytes([2u8; NODE_ADDRESS_LENGTH]), pub_key: (&random_private_key).into(), diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 3b336d1..c195eb9 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -14,7 +14,6 @@ extern crate sphinx_packet; -use sphinx_packet::crypto; use sphinx_packet::header::delays; use sphinx_packet::route::{Destination, Node}; use sphinx_packet::SphinxPacket; @@ -25,6 +24,7 @@ use sphinx_packet::SphinxPacket; mod create_and_process_sphinx_packet { use super::*; use sphinx_packet::route::{DestinationAddressBytes, NodeAddressBytes}; + use sphinx_packet::test_utils::fixtures::keygen; use sphinx_packet::{ constants::{ DESTINATION_ADDRESS_LENGTH, IDENTIFIER_LENGTH, NODE_ADDRESS_LENGTH, PAYLOAD_SIZE, @@ -36,17 +36,17 @@ mod create_and_process_sphinx_packet { #[test] fn returns_the_correct_data_at_each_hop_for_route_of_3_mixnodes_without_surb() { - let (node1_sk, node1_pk) = crypto::keygen(); + let (node1_sk, node1_pk) = keygen(); let node1 = Node::new( NodeAddressBytes::from_bytes([5u8; NODE_ADDRESS_LENGTH]), node1_pk, ); - let (node2_sk, node2_pk) = crypto::keygen(); + let (node2_sk, node2_pk) = keygen(); let node2 = Node::new( NodeAddressBytes::from_bytes([4u8; NODE_ADDRESS_LENGTH]), node2_pk, ); - let (node3_sk, node3_pk) = crypto::keygen(); + let (node3_sk, node3_pk) = keygen(); let node3 = Node::new( NodeAddressBytes::from_bytes([2u8; NODE_ADDRESS_LENGTH]), node3_pk, @@ -103,6 +103,7 @@ mod create_and_process_sphinx_packet { mod converting_sphinx_packet_to_and_from_bytes { use super::*; use sphinx_packet::route::{DestinationAddressBytes, NodeAddressBytes}; + use sphinx_packet::test_utils::fixtures::keygen; use sphinx_packet::{ constants::{ DESTINATION_ADDRESS_LENGTH, IDENTIFIER_LENGTH, NODE_ADDRESS_LENGTH, PAYLOAD_SIZE, @@ -114,17 +115,17 @@ mod converting_sphinx_packet_to_and_from_bytes { #[test] fn it_is_possible_to_do_the_conversion_without_data_loss() { - let (node1_sk, node1_pk) = crypto::keygen(); + let (node1_sk, node1_pk) = keygen(); let node1 = Node::new( NodeAddressBytes::from_bytes([5u8; NODE_ADDRESS_LENGTH]), node1_pk, ); - let (node2_sk, node2_pk) = crypto::keygen(); + let (node2_sk, node2_pk) = keygen(); let node2 = Node::new( NodeAddressBytes::from_bytes([4u8; NODE_ADDRESS_LENGTH]), node2_pk, ); - let (node3_sk, node3_pk) = crypto::keygen(); + let (node3_sk, node3_pk) = keygen(); let node3 = Node::new( NodeAddressBytes::from_bytes([2u8; NODE_ADDRESS_LENGTH]), node3_pk, @@ -184,17 +185,17 @@ mod converting_sphinx_packet_to_and_from_bytes { #[test] #[should_panic] fn it_panics_if_data_of_invalid_length_is_provided() { - let (_, node1_pk) = crypto::keygen(); + let (_, node1_pk) = keygen(); let node1 = Node::new( NodeAddressBytes::from_bytes([5u8; NODE_ADDRESS_LENGTH]), node1_pk, ); - let (_, node2_pk) = crypto::keygen(); + let (_, node2_pk) = keygen(); let node2 = Node::new( NodeAddressBytes::from_bytes([4u8; NODE_ADDRESS_LENGTH]), node2_pk, ); - let (_, node3_pk) = crypto::keygen(); + let (_, node3_pk) = keygen(); let node3 = Node::new( NodeAddressBytes::from_bytes([2u8; NODE_ADDRESS_LENGTH]), node3_pk, @@ -219,30 +220,30 @@ mod converting_sphinx_packet_to_and_from_bytes { #[cfg(test)] mod create_and_process_surb { use super::*; - use crypto::EphemeralSecret; use sphinx_packet::route::NodeAddressBytes; use sphinx_packet::surb::{SURBMaterial, SURB}; use sphinx_packet::{ constants::{NODE_ADDRESS_LENGTH, PAYLOAD_SIZE, SECURITY_PARAMETER}, packet::builder::DEFAULT_PAYLOAD_SIZE, - test_utils::fixtures::destination_fixture, + test_utils::fixtures::{destination_fixture, keygen}, ProcessedPacket, }; use std::time::Duration; + use x25519_dalek::StaticSecret; #[test] fn returns_the_correct_data_at_each_hop_for_route_of_3_mixnodes() { - let (node1_sk, node1_pk) = crypto::keygen(); + let (node1_sk, node1_pk) = keygen(); let node1 = Node { address: NodeAddressBytes::from_bytes([5u8; NODE_ADDRESS_LENGTH]), pub_key: node1_pk, }; - let (node2_sk, node2_pk) = crypto::keygen(); + let (node2_sk, node2_pk) = keygen(); let node2 = Node { address: NodeAddressBytes::from_bytes([4u8; NODE_ADDRESS_LENGTH]), pub_key: node2_pk, }; - let (node3_sk, node3_pk) = crypto::keygen(); + let (node3_sk, node3_pk) = keygen(); let node3 = Node { address: NodeAddressBytes::from_bytes([2u8; NODE_ADDRESS_LENGTH]), pub_key: node3_pk, @@ -250,7 +251,7 @@ mod create_and_process_surb { let surb_route = vec![node1, node2, node3]; let surb_destination = destination_fixture(); - let surb_initial_secret = EphemeralSecret::new(); + let surb_initial_secret = StaticSecret::random(); let surb_delays = delays::generate_from_average_duration(surb_route.len(), Duration::from_secs(3));