From 9759cb07f5dc0742ea85912981251a14444c85ca Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Sun, 10 Nov 2019 00:02:44 +0200 Subject: [PATCH 1/7] Replace SharedSecret with a more generic alternative --- secp256k1-sys/src/lib.rs | 23 +------- secp256k1-sys/src/macros.rs | 1 + src/ecdh.rs | 111 ++++++++++++++++++++---------------- src/macros.rs | 17 ++++++ 4 files changed, 81 insertions(+), 71 deletions(-) diff --git a/secp256k1-sys/src/lib.rs b/secp256k1-sys/src/lib.rs index 3d189f5e3..c7b2196d9 100644 --- a/secp256k1-sys/src/lib.rs +++ b/secp256k1-sys/src/lib.rs @@ -72,7 +72,7 @@ pub type EcdhHashFn = unsafe extern "C" fn( x: *const c_uchar, y: *const c_uchar, data: *mut c_void, -); +) -> c_int; /// A Secp256k1 context, containing various precomputed values and such /// needed to do elliptic curve computations. If you create one of these @@ -134,25 +134,6 @@ impl Default for Signature { } } -/// Library-internal representation of an ECDH shared secret -#[repr(C)] -pub struct SharedSecret([c_uchar; 32]); -impl_array_newtype!(SharedSecret, c_uchar, 32); -impl_raw_debug!(SharedSecret); - -impl SharedSecret { - /// Create a new (zeroed) signature usable for the FFI interface - pub fn new() -> SharedSecret { SharedSecret([0; 32]) } - /// Create a new (uninitialized) signature usable for the FFI interface - #[deprecated(since = "0.15.3", note = "Please use the new function instead")] - pub unsafe fn blank() -> SharedSecret { SharedSecret::new() } -} - -impl Default for SharedSecret { - fn default() -> Self { - SharedSecret::new() - } -} #[cfg(not(feature = "fuzztarget"))] extern "C" { @@ -296,7 +277,7 @@ extern "C" { #[cfg_attr(not(feature = "external-symbols"), link_name = "rustsecp256k1_v0_1_0_ecdh")] pub fn secp256k1_ecdh( cx: *const Context, - output: *mut SharedSecret, + output: *mut c_uchar, pubkey: *const PublicKey, privkey: *const c_uchar, hashfp: EcdhHashFn, diff --git a/secp256k1-sys/src/macros.rs b/secp256k1-sys/src/macros.rs index 76c45bd5c..3263ff161 100644 --- a/secp256k1-sys/src/macros.rs +++ b/secp256k1-sys/src/macros.rs @@ -144,6 +144,7 @@ macro_rules! impl_array_newtype { } } +#[macro_export] macro_rules! impl_raw_debug { ($thing:ident) => { impl ::core::fmt::Debug for $thing { diff --git a/src/ecdh.rs b/src/ecdh.rs index ebe38b94f..4b08dbb64 100644 --- a/src/ecdh.rs +++ b/src/ecdh.rs @@ -16,83 +16,94 @@ //! Support for shared secret computations //! -use core::{ops, ptr}; +use core::ptr; +use core::ops::Deref; use key::{SecretKey, PublicKey}; use ffi::{self, CPtr}; /// A tag used for recovering the public key from a compact signature -#[derive(Copy, Clone, PartialEq, Eq, Debug)] -pub struct SharedSecret(ffi::SharedSecret); +#[derive(Copy, Clone)] +pub struct SharedSecret { + data: [u8; 256], + len: usize, +} +impl_raw_debug!(SharedSecret); + + +// This implementes `From` for all `[u8; N]` arrays from 128bits(16 byte) to 2048bits allowing known hash lengths. +// Lower than 128 bits isn't resistant to collisions any more. +impl_from_array_len!(SharedSecret, 256, (16 20 28 32 48 64 96 128 256)); impl SharedSecret { - /// Creates a new shared secret from a pubkey and secret key - #[inline] - pub fn new(point: &PublicKey, scalar: &SecretKey) -> SharedSecret { - unsafe { - let mut ss = ffi::SharedSecret::new(); - let res = ffi::secp256k1_ecdh( - ffi::secp256k1_context_no_precomp, - &mut ss, - point.as_c_ptr(), - scalar.as_c_ptr(), - ffi::secp256k1_ecdh_hash_function_default, - ptr::null_mut(), - ); - debug_assert_eq!(res, 1); - SharedSecret(ss) + + /// Create an empty SharedSecret + pub(crate) fn empty() -> SharedSecret { + SharedSecret { + data: [0u8; 256], + len: 0, } } - /// Obtains a raw pointer suitable for use with FFI functions - #[inline] - pub fn as_ptr(&self) -> *const ffi::SharedSecret { - &self.0 as *const _ + /// Get a pointer to the underlying data with the specified capacity. + pub(crate) fn get_data_mut_ptr(&mut self) -> *mut u8 { + self.data.as_mut_ptr() } -} -/// Creates a new shared secret from a FFI shared secret -impl From for SharedSecret { - #[inline] - fn from(ss: ffi::SharedSecret) -> SharedSecret { - SharedSecret(ss) + /// Get the capacity of the underlying data buffer. + pub fn capacity(&self) -> usize { + self.data.len() } -} + /// Get the len of the used data. + pub fn len(&self) -> usize { + self.len + } -impl ops::Index for SharedSecret { - type Output = u8; - - #[inline] - fn index(&self, index: usize) -> &u8 { - &self.0[index] + /// Set the length of the object. + pub(crate) fn set_len(&mut self, len: usize) { + self.len = len; } } -impl ops::Index> for SharedSecret { - type Output = [u8]; - - #[inline] - fn index(&self, index: ops::Range) -> &[u8] { - &self.0[index] +impl PartialEq for SharedSecret { + fn eq(&self, other: &SharedSecret) -> bool { + &self.data[..self.len] == &other.data[..other.len] } } -impl ops::Index> for SharedSecret { - type Output = [u8]; +impl AsRef<[u8]> for SharedSecret { + fn as_ref(&self) -> &[u8] { + &self.data[..self.len] + } +} - #[inline] - fn index(&self, index: ops::RangeFrom) -> &[u8] { - &self.0[index.start..] +impl Deref for SharedSecret { + type Target = [u8]; + fn deref(&self) -> &[u8] { + &self.data[..self.len] } } -impl ops::Index for SharedSecret { - type Output = [u8]; +impl SharedSecret { + /// Creates a new shared secret from a pubkey and secret key #[inline] - fn index(&self, _: ops::RangeFull) -> &[u8] { - &self.0[..] + pub fn new(point: &PublicKey, scalar: &SecretKey) -> SharedSecret { + let mut ss = SharedSecret::empty(); + let res = unsafe { + ffi::secp256k1_ecdh( + ffi::secp256k1_context_no_precomp, + ss.get_data_mut_ptr(), + point.as_c_ptr(), + scalar.as_c_ptr(), + ffi::secp256k1_ecdh_hash_function_default, + ptr::null_mut(), + ) + }; + debug_assert_eq!(res, 1); // The default `secp256k1_ecdh_hash_function_default` should always return 1. + ss.set_len(32); // The default hash function is SHA256, which is 32 bytes long. + ss } } diff --git a/src/macros.rs b/src/macros.rs index bf8fb0c19..9cf9ba6d3 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -27,6 +27,23 @@ macro_rules! impl_pretty_debug { } } +macro_rules! impl_from_array_len { + ($thing:ident, $capacity:tt, ($($N:tt)+)) => { + $( + impl From<[u8; $N]> for $thing { + fn from(arr: [u8; $N]) -> Self { + let mut data = [0u8; $capacity]; + data[..$N].copy_from_slice(&arr); + $thing { + data, + len: $N, + } + } + } + )+ + } +} + #[cfg(feature="serde")] /// Implements `Serialize` and `Deserialize` for a type `$t` which represents /// a newtype over a byte-slice over length `$len`. Type `$t` must implement From af8fa21a20d7d747a9bafcd8c4f1f3fedb51675e Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Sun, 10 Nov 2019 00:03:44 +0200 Subject: [PATCH 2/7] Add 'new_with_hash' function to SharedSecret --- src/ecdh.rs | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 57 insertions(+), 2 deletions(-) diff --git a/src/ecdh.rs b/src/ecdh.rs index 4b08dbb64..ca23ede0a 100644 --- a/src/ecdh.rs +++ b/src/ecdh.rs @@ -17,10 +17,11 @@ //! use core::ptr; -use core::ops::Deref; +use core::ops::{FnMut, Deref}; use key::{SecretKey, PublicKey}; use ffi::{self, CPtr}; +use secp256k1_sys::types::{c_int, c_uchar, c_void}; /// A tag used for recovering the public key from a compact signature #[derive(Copy, Clone)] @@ -68,7 +69,7 @@ impl SharedSecret { impl PartialEq for SharedSecret { fn eq(&self, other: &SharedSecret) -> bool { - &self.data[..self.len] == &other.data[..other.len] + self.as_ref() == other.as_ref() } } @@ -86,6 +87,22 @@ impl Deref for SharedSecret { } +unsafe extern "C" fn hash_callback(output: *mut c_uchar, x: *const c_uchar, y: *const c_uchar, data: *mut c_void) -> c_int + where F: FnMut([u8; 32], [u8; 32]) -> SharedSecret { + let callback: &mut F = &mut *(data as *mut F); + + let mut x_arr = [0; 32]; + let mut y_arr = [0; 32]; + ptr::copy_nonoverlapping(x, x_arr.as_mut_ptr(), 32); + ptr::copy_nonoverlapping(y, y_arr.as_mut_ptr(), 32); + + let secret = callback(x_arr, y_arr); + ptr::copy_nonoverlapping(secret.as_ptr(), output as *mut u8, secret.len()); + + secret.len() as c_int +} + + impl SharedSecret { /// Creates a new shared secret from a pubkey and secret key #[inline] @@ -105,6 +122,44 @@ impl SharedSecret { ss.set_len(32); // The default hash function is SHA256, which is 32 bytes long. ss } + + /// Creates a new shared secret from a pubkey and secret key with applied custom hash function + /// # Examples + /// ``` + /// # use secp256k1::ecdh::SharedSecret; + /// # use secp256k1::{Secp256k1, PublicKey, SecretKey}; + /// # fn sha2(_a: &[u8], _b: &[u8]) -> [u8; 32] {[0u8; 32]} + /// # let secp = Secp256k1::signing_only(); + /// # let secret_key = SecretKey::from_slice(&[3u8; 32]).unwrap(); + /// # let secret_key2 = SecretKey::from_slice(&[7u8; 32]).unwrap(); + /// # let public_key = PublicKey::from_secret_key(&secp, &secret_key2); + /// + /// let secret = SharedSecret::new_with_hash(&public_key, &secret_key, |x,y| { + /// let hash: [u8; 32] = sha2(&x,&y); + /// hash.into() + /// }); + /// + /// ``` + pub fn new_with_hash(point: &PublicKey, scalar: &SecretKey, mut hash_function: F) -> SharedSecret + where F: FnMut([u8; 32], [u8; 32]) -> SharedSecret + { + let mut ss = SharedSecret::empty(); + let hashfp: ffi::EcdhHashFn = hash_callback::; + + let res = unsafe { + ffi::secp256k1_ecdh( + ffi::secp256k1_context_no_precomp, + ss.get_data_mut_ptr(), + point.as_ptr(), + scalar.as_ptr(), + hashfp, + &mut hash_function as *mut F as *mut c_void, + ) + }; + debug_assert!(res >= 16); // 128 bit is the minimum for a secure hash function and the minimum we let users. + ss.set_len(res as usize); + ss + } } #[cfg(test)] From f80428258cd6f16b3f949229a0e6c8e2de67b8ee Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Sun, 10 Nov 2019 00:04:05 +0200 Subject: [PATCH 3/7] Add tests for the new SharedSecret::new_with_hash() function --- src/ecdh.rs | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/ecdh.rs b/src/ecdh.rs index ca23ede0a..3f9799089 100644 --- a/src/ecdh.rs +++ b/src/ecdh.rs @@ -180,6 +180,36 @@ mod tests { assert_eq!(sec1, sec2); assert!(sec_odd != sec2); } + + #[test] + fn ecdh_with_hash() { + let s = Secp256k1::signing_only(); + let (sk1, pk1) = s.generate_keypair(&mut thread_rng()); + let (sk2, pk2) = s.generate_keypair(&mut thread_rng()); + + let sec1 = SharedSecret::new_with_hash(&pk1, &sk2, |x,_| x.into()); + let sec2 = SharedSecret::new_with_hash(&pk2, &sk1, |x,_| x.into()); + let sec_odd = SharedSecret::new_with_hash(&pk1, &sk1, |x,_| x.into()); + assert_eq!(sec1, sec2); + assert_ne!(sec_odd, sec2); + } + + #[test] + fn ecdh_with_hash_callback() { + let s = Secp256k1::signing_only(); + let (sk1, pk1) = s.generate_keypair(&mut thread_rng()); + let expect_result: [u8; 64] = [123; 64]; + let mut x_out = [0u8; 32]; + let mut y_out = [0u8; 32]; + let result = SharedSecret::new_with_hash(&pk1, &sk1, | x, y | { + x_out = x; + y_out = y; + expect_result.into() + }); + assert_eq!(&expect_result[..], &result[..]); + assert_ne!(x_out, [0u8; 32]); + assert_ne!(y_out, [0u8; 32]); + } } #[cfg(all(test, feature = "unstable"))] From ca8ea924185a20b2c3ded1e62a365653e148c1d5 Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Sun, 10 Nov 2019 12:05:52 +0200 Subject: [PATCH 4/7] Fixed secp256k1_ecdh fuzztarget --- secp256k1-sys/src/lib.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/secp256k1-sys/src/lib.rs b/secp256k1-sys/src/lib.rs index c7b2196d9..b11b5eff7 100644 --- a/secp256k1-sys/src/lib.rs +++ b/secp256k1-sys/src/lib.rs @@ -440,7 +440,7 @@ mod fuzz_dummy { use self::std::{ptr, mem}; use self::std::boxed::Box; use types::*; - use ::{Signature, Context, NonceFn, EcdhHashFn, PublicKey, SharedSecret, + use ::{Signature, Context, NonceFn, EcdhHashFn, PublicKey, SECP256K1_START_NONE, SECP256K1_START_VERIFY, SECP256K1_START_SIGN, SECP256K1_SER_COMPRESSED, SECP256K1_SER_UNCOMPRESSED}; @@ -769,7 +769,7 @@ mod fuzz_dummy { /// Sets out to point[0..16]||scalar[0..16] pub unsafe fn secp256k1_ecdh( cx: *const Context, - out: *mut SharedSecret, + out: *mut c_uchar, point: *const PublicKey, scalar: *const c_uchar, _hashfp: EcdhHashFn, @@ -782,13 +782,13 @@ mod fuzz_dummy { ptr::copy(scalar, scalar_prefix[..].as_mut_ptr(), 16); if (*point).0[0..16] > scalar_prefix[0..16] { - (*out).0[0..16].copy_from_slice(&(*point).0[0..16]); - ptr::copy(scalar, (*out).0[16..32].as_mut_ptr(), 16); + ptr::copy((*point).as_ptr(), out, 16); + ptr::copy(scalar, out.offset(16), 16); } else { - ptr::copy(scalar, (*out).0[0..16].as_mut_ptr(), 16); - (*out).0[16..32].copy_from_slice(&(*point).0[0..16]); + ptr::copy(scalar, out, 16); + ptr::copy((*point).as_ptr(), out.offset(16), 16); } - (*out).0[16] = 0x00; // result should always be a valid secret key + (*out.offset(16)) = 0x00; // result should always be a valid secret key 1 } } From 124c1f3c7c50f153d8cf57d8482b2e2e5443eae7 Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Sun, 10 Nov 2019 13:24:53 +0200 Subject: [PATCH 5/7] feature gate new_with_hash with std only, added catch_unwind --- src/ecdh.rs | 57 ++++++++++++++++++++++++++++++++++++++++------------- src/lib.rs | 4 +++- 2 files changed, 46 insertions(+), 15 deletions(-) diff --git a/src/ecdh.rs b/src/ecdh.rs index 3f9799089..4b7b95aef 100644 --- a/src/ecdh.rs +++ b/src/ecdh.rs @@ -22,6 +22,7 @@ use core::ops::{FnMut, Deref}; use key::{SecretKey, PublicKey}; use ffi::{self, CPtr}; use secp256k1_sys::types::{c_int, c_uchar, c_void}; +use Error; /// A tag used for recovering the public key from a compact signature #[derive(Copy, Clone)] @@ -63,6 +64,7 @@ impl SharedSecret { /// Set the length of the object. pub(crate) fn set_len(&mut self, len: usize) { + debug_assert!(len <= self.data.len()); self.len = len; } } @@ -87,19 +89,29 @@ impl Deref for SharedSecret { } +#[cfg(feature = "std")] unsafe extern "C" fn hash_callback(output: *mut c_uchar, x: *const c_uchar, y: *const c_uchar, data: *mut c_void) -> c_int where F: FnMut([u8; 32], [u8; 32]) -> SharedSecret { - let callback: &mut F = &mut *(data as *mut F); - let mut x_arr = [0; 32]; - let mut y_arr = [0; 32]; - ptr::copy_nonoverlapping(x, x_arr.as_mut_ptr(), 32); - ptr::copy_nonoverlapping(y, y_arr.as_mut_ptr(), 32); + use std::panic::catch_unwind; + let res = catch_unwind(|| { + let callback: &mut F = &mut *(data as *mut F); - let secret = callback(x_arr, y_arr); - ptr::copy_nonoverlapping(secret.as_ptr(), output as *mut u8, secret.len()); + let mut x_arr = [0; 32]; + let mut y_arr = [0; 32]; + ptr::copy_nonoverlapping(x, x_arr.as_mut_ptr(), 32); + ptr::copy_nonoverlapping(y, y_arr.as_mut_ptr(), 32); - secret.len() as c_int + let secret = callback(x_arr, y_arr); + ptr::copy_nonoverlapping(secret.as_ptr(), output as *mut u8, secret.len()); + + secret.len() as c_int + }); + if let Ok(len) = res { + len + } else { + -1 + } } @@ -140,7 +152,8 @@ impl SharedSecret { /// }); /// /// ``` - pub fn new_with_hash(point: &PublicKey, scalar: &SecretKey, mut hash_function: F) -> SharedSecret + #[cfg(feature = "std")] + pub fn new_with_hash(point: &PublicKey, scalar: &SecretKey, mut hash_function: F) -> Result where F: FnMut([u8; 32], [u8; 32]) -> SharedSecret { let mut ss = SharedSecret::empty(); @@ -156,9 +169,12 @@ impl SharedSecret { &mut hash_function as *mut F as *mut c_void, ) }; + if res == -1 { + return Err(Error::CallbackPanicked); + } debug_assert!(res >= 16); // 128 bit is the minimum for a secure hash function and the minimum we let users. ss.set_len(res as usize); - ss + Ok(ss) } } @@ -167,6 +183,7 @@ mod tests { use rand::thread_rng; use super::SharedSecret; use super::super::Secp256k1; + use Error; #[test] fn ecdh() { @@ -187,9 +204,9 @@ mod tests { let (sk1, pk1) = s.generate_keypair(&mut thread_rng()); let (sk2, pk2) = s.generate_keypair(&mut thread_rng()); - let sec1 = SharedSecret::new_with_hash(&pk1, &sk2, |x,_| x.into()); - let sec2 = SharedSecret::new_with_hash(&pk2, &sk1, |x,_| x.into()); - let sec_odd = SharedSecret::new_with_hash(&pk1, &sk1, |x,_| x.into()); + let sec1 = SharedSecret::new_with_hash(&pk1, &sk2, |x,_| x.into()).unwrap(); + let sec2 = SharedSecret::new_with_hash(&pk2, &sk1, |x,_| x.into()).unwrap(); + let sec_odd = SharedSecret::new_with_hash(&pk1, &sk1, |x,_| x.into()).unwrap(); assert_eq!(sec1, sec2); assert_ne!(sec_odd, sec2); } @@ -205,11 +222,23 @@ mod tests { x_out = x; y_out = y; expect_result.into() - }); + }).unwrap(); assert_eq!(&expect_result[..], &result[..]); assert_ne!(x_out, [0u8; 32]); assert_ne!(y_out, [0u8; 32]); } + + #[test] + fn ecdh_with_hash_callback_panic() { + let s = Secp256k1::signing_only(); + let (sk1, pk1) = s.generate_keypair(&mut thread_rng()); + let mut res = [0u8; 48]; + let result = SharedSecret::new_with_hash(&pk1, &sk1, | x, _ | { + res.copy_from_slice(&x); // res.len() != x.len(). this will panic. + res.into() + }); + assert_eq!(result, Err(Error::CallbackPanicked)); + } } #[cfg(all(test, feature = "unstable"))] diff --git a/src/lib.rs b/src/lib.rs index 74254f21c..8b613726b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -496,7 +496,8 @@ pub enum Error { InvalidTweak, /// Didn't pass enough memory to context creation with preallocated memory NotEnoughMemory, - + /// The callback has panicked. + CallbackPanicked, } impl Error { @@ -510,6 +511,7 @@ impl Error { Error::InvalidRecoveryId => "secp: bad recovery id", Error::InvalidTweak => "secp: bad tweak", Error::NotEnoughMemory => "secp: not enough memory allocated", + Error::CallbackPanicked => "secp: a callback passed has panicked", } } } From 5619f2a5df194a1d217adcb68afa0157ac0ddfca Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Thu, 28 Nov 2019 00:21:51 +0200 Subject: [PATCH 6/7] Add an unsafe variant of new_with_has called new_with_hash_no_panic --- src/ecdh.rs | 118 +++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 84 insertions(+), 34 deletions(-) diff --git a/src/ecdh.rs b/src/ecdh.rs index 4b7b95aef..166907bdf 100644 --- a/src/ecdh.rs +++ b/src/ecdh.rs @@ -89,24 +89,26 @@ impl Deref for SharedSecret { } -#[cfg(feature = "std")] -unsafe extern "C" fn hash_callback(output: *mut c_uchar, x: *const c_uchar, y: *const c_uchar, data: *mut c_void) -> c_int +unsafe fn callback_logic(output: *mut c_uchar, x: *const c_uchar, y: *const c_uchar, data: *mut c_void) -> c_int where F: FnMut([u8; 32], [u8; 32]) -> SharedSecret { + let callback: &mut F = &mut *(data as *mut F); + + let mut x_arr = [0; 32]; + let mut y_arr = [0; 32]; + ptr::copy_nonoverlapping(x, x_arr.as_mut_ptr(), 32); + ptr::copy_nonoverlapping(y, y_arr.as_mut_ptr(), 32); - use std::panic::catch_unwind; - let res = catch_unwind(|| { - let callback: &mut F = &mut *(data as *mut F); + let secret = callback(x_arr, y_arr); + ptr::copy_nonoverlapping(secret.as_ptr(), output as *mut u8, secret.len()); - let mut x_arr = [0; 32]; - let mut y_arr = [0; 32]; - ptr::copy_nonoverlapping(x, x_arr.as_mut_ptr(), 32); - ptr::copy_nonoverlapping(y, y_arr.as_mut_ptr(), 32); + secret.len() as c_int +} - let secret = callback(x_arr, y_arr); - ptr::copy_nonoverlapping(secret.as_ptr(), output as *mut u8, secret.len()); +#[cfg(feature = "std")] +unsafe extern "C" fn hash_callback_catch_unwind(output: *mut c_uchar, x: *const c_uchar, y: *const c_uchar, data: *mut c_void) -> c_int + where F: FnMut([u8; 32], [u8; 32]) -> SharedSecret { - secret.len() as c_int - }); + let res = ::std::panic::catch_unwind(||callback_logic::(output, x, y, data)); if let Ok(len) = res { len } else { @@ -114,6 +116,11 @@ unsafe extern "C" fn hash_callback(output: *mut c_uchar, x: *const c_uchar, y } } +unsafe extern "C" fn hash_callback_unsafe(output: *mut c_uchar, x: *const c_uchar, y: *const c_uchar, data: *mut c_void) -> c_int + where F: FnMut([u8; 32], [u8; 32]) -> SharedSecret { + callback_logic::(output, x, y, data) +} + impl SharedSecret { /// Creates a new shared secret from a pubkey and secret key @@ -135,6 +142,29 @@ impl SharedSecret { ss } + fn new_with_callback_internal(point: &PublicKey, scalar: &SecretKey, mut closure: F, callback: ffi::EcdhHashFn) -> Result + where F: FnMut([u8; 32], [u8; 32]) -> SharedSecret { + let mut ss = SharedSecret::empty(); + + let res = unsafe { + ffi::secp256k1_ecdh( + ffi::secp256k1_context_no_precomp, + ss.get_data_mut_ptr(), + point.as_ptr(), + scalar.as_ptr(), + callback, + &mut closure as *mut F as *mut c_void, + ) + }; + if res == -1 { + return Err(Error::CallbackPanicked); + } + debug_assert!(res >= 16); // 128 bit is the minimum for a secure hash function and the minimum we let users. + ss.set_len(res as usize); + Ok(ss) + + } + /// Creates a new shared secret from a pubkey and secret key with applied custom hash function /// # Examples /// ``` @@ -153,28 +183,42 @@ impl SharedSecret { /// /// ``` #[cfg(feature = "std")] - pub fn new_with_hash(point: &PublicKey, scalar: &SecretKey, mut hash_function: F) -> Result - where F: FnMut([u8; 32], [u8; 32]) -> SharedSecret - { - let mut ss = SharedSecret::empty(); - let hashfp: ffi::EcdhHashFn = hash_callback::; + pub fn new_with_hash(point: &PublicKey, scalar: &SecretKey, hash_function: F) -> Result + where F: FnMut([u8; 32], [u8; 32]) -> SharedSecret { + Self::new_with_callback_internal(point, scalar, hash_function, hash_callback_catch_unwind::) + } - let res = unsafe { - ffi::secp256k1_ecdh( - ffi::secp256k1_context_no_precomp, - ss.get_data_mut_ptr(), - point.as_ptr(), - scalar.as_ptr(), - hashfp, - &mut hash_function as *mut F as *mut c_void, - ) - }; - if res == -1 { - return Err(Error::CallbackPanicked); - } - debug_assert!(res >= 16); // 128 bit is the minimum for a secure hash function and the minimum we let users. - ss.set_len(res as usize); - Ok(ss) + /// Creates a new shared secret from a pubkey and secret key with applied custom hash function + /// Note that this function is the same as [`new_with_hash`] + /// + /// # Safety + /// The function doesn't wrap the callback with [`catch_unwind`] + /// so if the callback panics it will panic through an FFI boundray which is [`Undefined Behavior`] + /// If possible you should use [`new_with_hash`] which does wrap the callback with [`catch_unwind`] so is safe to use. + /// + /// [`catch_unwind`]: https://doc.rust-lang.org/std/panic/fn.catch_unwind.html + /// [`Undefined Behavior`]: https://doc.rust-lang.org/nomicon/ffi.html#ffi-and-panics + /// [`new_with_hash`]: #method.new_with_hash + /// # Examples + /// ``` + /// # use secp256k1::ecdh::SharedSecret; + /// # use secp256k1::{Secp256k1, PublicKey, SecretKey}; + /// # fn sha2(_a: &[u8], _b: &[u8]) -> [u8; 32] {[0u8; 32]} + /// # let secp = Secp256k1::signing_only(); + /// # let secret_key = SecretKey::from_slice(&[3u8; 32]).unwrap(); + /// # let secret_key2 = SecretKey::from_slice(&[7u8; 32]).unwrap(); + /// # let public_key = PublicKey::from_secret_key(&secp, &secret_key2); + // + /// let secret = unsafe { SharedSecret::new_with_hash_no_panic(&public_key, &secret_key, |x,y| { + /// let hash: [u8; 32] = sha2(&x,&y); + /// hash.into() + /// })}; + /// + /// + /// ``` + pub unsafe fn new_with_hash_no_panic(point: &PublicKey, scalar: &SecretKey, hash_function: F) -> Result + where F: FnMut([u8; 32], [u8; 32]) -> SharedSecret { + Self::new_with_callback_internal(point, scalar, hash_function, hash_callback_unsafe::) } } @@ -223,7 +267,13 @@ mod tests { y_out = y; expect_result.into() }).unwrap(); + let result_unsafe = unsafe {SharedSecret::new_with_hash_no_panic(&pk1, &sk1, | x, y | { + x_out = x; + y_out = y; + expect_result.into() + }).unwrap()}; assert_eq!(&expect_result[..], &result[..]); + assert_eq!(result, result_unsafe); assert_ne!(x_out, [0u8; 32]); assert_ne!(y_out, [0u8; 32]); } From 92c42ca9e68464896bc190fcfe51f6bc5067a71f Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Thu, 28 Nov 2019 00:42:15 +0200 Subject: [PATCH 7/7] Add ECDH to the no-std tests --- no_std_test/src/main.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/no_std_test/src/main.rs b/no_std_test/src/main.rs index 5cbdce1ad..d40d8e397 100644 --- a/no_std_test/src/main.rs +++ b/no_std_test/src/main.rs @@ -55,6 +55,7 @@ use core::panic::PanicInfo; use secp256k1::rand::{self, RngCore}; use secp256k1::serde::Serialize; use secp256k1::*; +use secp256k1::ecdh::SharedSecret; use serde_cbor::de; use serde_cbor::ser::SliceWrite; @@ -102,6 +103,16 @@ fn start(_argc: isize, _argv: *const *const u8) -> isize { let new_sig: Signature = de::from_mut_slice(&mut cbor_ser[..size]).unwrap(); assert_eq!(sig, new_sig); + let _ = SharedSecret::new(&public_key, &secret_key); + let mut x_arr = [0u8; 32]; + let y_arr = unsafe { SharedSecret::new_with_hash_no_panic(&public_key, &secret_key, |x,y| { + x_arr = x; + y.into() + })}.unwrap(); + assert_ne!(x_arr, [0u8; 32]); + assert_ne!(&y_arr[..], &[0u8; 32][..]); + + unsafe { libc::printf("Verified Successfully!\n\0".as_ptr() as _) }; 0 }