Skip to content

Commit

Permalink
Limit SharedSecret to 32 byte buffer
Browse files Browse the repository at this point in the history
The `SharedSecret` uses sha256 to hash the secret, this implies the
secret is 32 bytes of data.

Currently we use a buffer of 256 bytes, this seems to be unneeded.

Change the implementation to use a 32 byte buffer. This simplifies the
API and implementation quite considerably.
  • Loading branch information
tcharding committed Feb 16, 2022
1 parent ef59aea commit f9d674a
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 70 deletions.
2 changes: 1 addition & 1 deletion no_std_test/src/main.rs
Expand Up @@ -131,7 +131,7 @@ fn start(_argc: isize, _argv: *const *const u8) -> isize {
y.into()
});
assert_ne!(x_arr, [0u8; 32]);
assert_ne!(&y_arr[..], &[0u8; 32][..]);
assert!(!y_arr.is_empty());

#[cfg(feature = "alloc")]
{
Expand Down
67 changes: 15 additions & 52 deletions src/ecdh.rs
Expand Up @@ -16,7 +16,7 @@
//!

use core::ptr;
use core::ops::{FnMut, Deref};
use core::ops::FnMut;

use key::{SecretKey, PublicKey};
use ffi::{self, CPtr};
Expand All @@ -39,75 +39,33 @@ use secp256k1_sys::types::{c_int, c_uchar, c_void};
/// assert_eq!(sec1, sec2);
/// # }
// ```
#[derive(Copy, Clone)]
pub struct SharedSecret {
data: [u8; 256],
len: usize,
}
impl_raw_debug!(SharedSecret);


// This implementes `From<N>` 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));
#[derive(Copy, Clone, Debug, PartialEq)]
pub struct SharedSecret([u8; 32]);

impl SharedSecret {

/// Creates an empty `SharedSecret`.
pub(crate) fn empty() -> SharedSecret {
SharedSecret {
data: [0u8; 256],
len: 0,
}
SharedSecret([0u8; 32])
}

/// Gets 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()
}

/// Gets the capacity of the underlying data buffer.
pub fn capacity(&self) -> usize {
self.data.len()
}

/// Gets the len of the used data.
pub fn len(&self) -> usize {
self.len
self.0.as_mut_ptr()
}

/// Returns true if the underlying data buffer is empty.
pub fn is_empty(&self) -> bool {
self.data.is_empty()
}

/// Sets the length of the object.
pub(crate) fn set_len(&mut self, len: usize) {
debug_assert!(len <= self.data.len());
self.len = len;
}
}

impl PartialEq for SharedSecret {
fn eq(&self, other: &SharedSecret) -> bool {
self.as_ref() == other.as_ref()
self.0.is_empty()
}
}

impl AsRef<[u8]> for SharedSecret {
fn as_ref(&self) -> &[u8] {
&self.data[..self.len]
}
}

impl Deref for SharedSecret {
type Target = [u8];
fn deref(&self) -> &[u8] {
&self.data[..self.len]
&self.0
}
}


unsafe extern "C" fn c_callback(output: *mut c_uchar, x: *const c_uchar, y: *const c_uchar, _data: *mut c_void) -> c_int {
ptr::copy_nonoverlapping(x, output, 32);
ptr::copy_nonoverlapping(y, output.offset(32), 32);
Expand All @@ -132,7 +90,6 @@ impl SharedSecret {
// The default `secp256k1_ecdh_hash_function_default` should always return 1.
// and the scalar was verified to be valid(0 > scalar > group_order) via the type system
debug_assert_eq!(res, 1);
ss.set_len(32); // The default hash function is SHA256, which is 32 bytes long.
ss
}

Expand Down Expand Up @@ -183,6 +140,12 @@ impl SharedSecret {
}
}

impl From<[u8; 32]> for SharedSecret {
fn from(inner: [u8; 32]) -> SharedSecret {
SharedSecret(inner)
}
}

#[cfg(test)]
#[allow(unused_imports)]
mod tests {
Expand Down Expand Up @@ -226,15 +189,15 @@ mod tests {
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 expect_result: [u8; 32] = [123; 32];
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_eq!(&expect_result[..], &result.0[..]);
assert_ne!(x_out, [0u8; 32]);
assert_ne!(y_out, [0u8; 32]);
}
Expand Down
17 changes: 0 additions & 17 deletions src/macros.rs
Expand Up @@ -26,20 +26,3 @@ 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,
}
}
}
)+
}
}

0 comments on commit f9d674a

Please sign in to comment.