From 5f93474512e2b7a190a3bc71a33536f4dc384463 Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Tue, 28 May 2019 20:48:26 +0300 Subject: [PATCH 1/7] Added the preallocated FFI --- src/ffi.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/ffi.rs b/src/ffi.rs index dbcfbc4..4a750f1 100644 --- a/src/ffi.rs +++ b/src/ffi.rs @@ -144,6 +144,16 @@ extern "C" { // Contexts pub fn secp256k1_context_create(flags: c_uint) -> *mut Context; + pub fn secp256k1_context_preallocated_size(flags: c_uint) -> usize; + + pub fn secp256k1_context_preallocated_create(prealloc: *mut c_void, flags: c_uint) -> *mut Context; + + pub fn secp256k1_context_preallocated_destroy(cx: *mut Context); + + pub fn secp256k1_context_preallocated_clone_size(cx: *const Context) -> usize; + + pub fn secp256k1_context_preallocated_clone(cx: *const Context, prealloc: *mut c_void) -> *mut Context; + pub fn secp256k1_context_clone(cx: *mut Context) -> *mut Context; pub fn secp256k1_context_destroy(cx: *mut Context); From b4b52a98589255c7bc281edb426697d5a42e0a66 Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Thu, 4 Jul 2019 17:38:03 -0400 Subject: [PATCH 2/7] Moved the context specific traits/enums into a separate file with `std` gate --- src/context.rs | 122 +++++++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 101 ++++++++++------------------------------ 2 files changed, 145 insertions(+), 78 deletions(-) create mode 100644 src/context.rs diff --git a/src/context.rs b/src/context.rs new file mode 100644 index 0000000..92d09cf --- /dev/null +++ b/src/context.rs @@ -0,0 +1,122 @@ +use core::marker::PhantomData; +use {ffi, types::{c_uint, c_void}, Error, Secp256k1, }; + +#[cfg(feature = "std")] +pub use self::std_only::*; + +/// A trait for all kinds of Context's that let's you define the exact flags and a function to deallocate memory. +/// * DO NOT * implement it for your own types. +pub unsafe trait Context { + /// Flags for the ffi. + const FLAGS: c_uint; + /// A constant description of the context. + const DESCRIPTION: &'static str; + /// A function to deallocate the memory when the context is dropped. + fn deallocate(ptr: *mut [u8]); +} + +/// Marker trait for indicating that an instance of `Secp256k1` can be used for signing. +pub trait Signing: Context {} + +/// Marker trait for indicating that an instance of `Secp256k1` can be used for verification. +pub trait Verification: Context {} + + +#[cfg(feature = "std")] +mod std_only { + use super::*; + + /// Represents the set of capabilities needed for signing. + pub enum SignOnly {} + + /// Represents the set of capabilities needed for verification. + pub enum VerifyOnly {} + + /// Represents the set of all capabilities. + pub enum All {} + + impl Signing for SignOnly {} + impl Signing for All {} + + impl Verification for VerifyOnly {} + impl Verification for All {} + + unsafe impl Context for SignOnly { + const FLAGS: c_uint = ffi::SECP256K1_START_SIGN; + const DESCRIPTION: &'static str = "signing only"; + + fn deallocate(ptr: *mut [u8]) { + let _ = unsafe { Box::from_raw(ptr) }; + } + } + + unsafe impl Context for VerifyOnly { + const FLAGS: c_uint = ffi::SECP256K1_START_VERIFY; + const DESCRIPTION: &'static str = "verification only"; + + fn deallocate(ptr: *mut [u8]) { + let _ = unsafe { Box::from_raw(ptr) }; + } + } + + unsafe impl Context for All { + const FLAGS: c_uint = VerifyOnly::FLAGS | SignOnly::FLAGS; + const DESCRIPTION: &'static str = "all capabilities"; + + fn deallocate(ptr: *mut [u8]) { + let _ = unsafe { Box::from_raw(ptr) }; + } + } + + impl Secp256k1 { + fn gen_new() -> Secp256k1 { + let buf = vec![0u8; Self::preallocate_size()].into_boxed_slice(); + let ptr = Box::into_raw(buf); + Secp256k1 { + ctx: unsafe { ffi::secp256k1_context_preallocated_create(ptr as *mut c_void, C::FLAGS) }, + phantom: PhantomData, + buf: ptr, + } + } + } + + impl Secp256k1 { + /// Creates a new Secp256k1 context with all capabilities + pub fn new() -> Secp256k1 { + Secp256k1::gen_new() + } + } + + impl Secp256k1 { + /// Creates a new Secp256k1 context that can only be used for signing + pub fn signing_only() -> Secp256k1 { + Secp256k1::gen_new() + } + } + + impl Secp256k1 { + /// Creates a new Secp256k1 context that can only be used for verification + pub fn verification_only() -> Secp256k1 { + Secp256k1::gen_new() + } + } + + impl Default for Secp256k1 { + fn default() -> Self { + Self::new() + } + } + + impl Clone for Secp256k1 { + fn clone(&self) -> Secp256k1 { + let buf = vec![0u8; unsafe { (&*self.buf).len() }].into_boxed_slice(); + let ptr = Box::into_raw(buf); + Secp256k1 { + ctx: unsafe { ffi::secp256k1_context_preallocated_create(ptr as *mut c_void, C::FLAGS) }, + phantom: PhantomData, + buf: ptr, + } + } + } + +} diff --git a/src/lib.rs b/src/lib.rs index 0947ce7..d6ace6a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -148,6 +148,7 @@ use core::{fmt, ptr, str}; #[macro_use] mod macros; mod types; +mod context; pub mod constants; pub mod ecdh; pub mod ffi; @@ -157,6 +158,7 @@ pub mod recovery; pub use key::SecretKey; pub use key::PublicKey; +pub use context::*; use core::marker::PhantomData; use core::ops::Deref; @@ -463,6 +465,9 @@ pub enum Error { InvalidRecoveryId, /// Invalid tweak for add_*_assign or mul_*_assign InvalidTweak, + /// Didn't pass enough memory to context creation with preallocated memory + NotEnoughMemory, + } impl Error { @@ -475,6 +480,7 @@ impl Error { Error::InvalidSecretKey => "secp: malformed or out-of-range secret key", Error::InvalidRecoveryId => "secp: bad recovery id", Error::InvalidTweak => "secp: bad tweak", + Error::NotEnoughMemory => "secp: not enough memory allocated", } } } @@ -491,48 +497,21 @@ impl std::error::Error for Error { fn description(&self) -> &str { self.as_str() } } -/// Marker trait for indicating that an instance of `Secp256k1` can be used for signing. -pub trait Signing {} - -/// Marker trait for indicating that an instance of `Secp256k1` can be used for verification. -pub trait Verification {} - -/// Represents the set of capabilities needed for signing. -pub struct SignOnly {} - -/// Represents the set of capabilities needed for verification. -pub struct VerifyOnly {} - -/// Represents the set of all capabilities. -pub struct All {} - -impl Signing for SignOnly {} -impl Signing for All {} - -impl Verification for VerifyOnly {} -impl Verification for All {} /// The secp256k1 engine, used to execute all signature operations -pub struct Secp256k1 { +pub struct Secp256k1 { ctx: *mut ffi::Context, - phantom: PhantomData + phantom: PhantomData, + buf: *mut [u8], } // The underlying secp context does not contain any references to memory it does not own -unsafe impl Send for Secp256k1 {} +unsafe impl Send for Secp256k1 {} // The API does not permit any mutation of `Secp256k1` objects except through `&mut` references -unsafe impl Sync for Secp256k1 {} +unsafe impl Sync for Secp256k1 {} -impl Clone for Secp256k1 { - fn clone(&self) -> Secp256k1 { - Secp256k1 { - ctx: unsafe { ffi::secp256k1_context_clone(self.ctx) }, - phantom: self.phantom - } - } -} -impl PartialEq for Secp256k1 { +impl PartialEq for Secp256k1 { fn eq(&self, _other: &Secp256k1) -> bool { true } } @@ -566,60 +545,21 @@ impl Deref for SerializedSignature { impl Eq for SerializedSignature {} -impl Eq for Secp256k1 { } +impl Eq for Secp256k1 { } -impl Drop for Secp256k1 { +impl Drop for Secp256k1 { fn drop(&mut self) { - unsafe { ffi::secp256k1_context_destroy(self.ctx); } + C::deallocate(self.buf) } } -impl fmt::Debug for Secp256k1 { +impl fmt::Debug for Secp256k1 { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "", self.ctx) + write!(f, "", self.ctx, C::DESCRIPTION) } } -impl fmt::Debug for Secp256k1 { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "", self.ctx) - } -} - -impl fmt::Debug for Secp256k1 { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "", self.ctx) - } -} - -impl Secp256k1 { - /// Creates a new Secp256k1 context with all capabilities - pub fn new() -> Secp256k1 { - Secp256k1 { ctx: unsafe { ffi::secp256k1_context_create(ffi::SECP256K1_START_SIGN | ffi::SECP256K1_START_VERIFY) }, phantom: PhantomData } - } -} - -impl Default for Secp256k1 { - fn default() -> Self { - Self::new() - } -} - -impl Secp256k1 { - /// Creates a new Secp256k1 context that can only be used for signing - pub fn signing_only() -> Secp256k1 { - Secp256k1 { ctx: unsafe { ffi::secp256k1_context_create(ffi::SECP256K1_START_SIGN) }, phantom: PhantomData } - } -} - -impl Secp256k1 { - /// Creates a new Secp256k1 context that can only be used for verification - pub fn verification_only() -> Secp256k1 { - Secp256k1 { ctx: unsafe { ffi::secp256k1_context_create(ffi::SECP256K1_START_VERIFY) }, phantom: PhantomData } - } -} - -impl Secp256k1 { +impl Secp256k1 { /// Getter for the raw pointer to the underlying secp256k1 context. This /// shouldn't be needed with normal usage of the library. It enables @@ -629,6 +569,11 @@ impl Secp256k1 { &self.ctx } + /// Uses the ffi `secp256k1_context_preallocated_size` to check the memory size needed for a context + pub fn preallocate_size() -> usize { + unsafe { ffi::secp256k1_context_preallocated_size(C::FLAGS) } + } + /// (Re)randomizes the Secp256k1 context for cheap sidechannel resistance; /// see comment in libsecp256k1 commit d2275795f by Gregory Maxwell. Requires /// compilation with "rand" feature. From 1c0ae7d4ba700e32b7dd8760e5a8865df830a179 Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Thu, 4 Jul 2019 17:39:37 -0400 Subject: [PATCH 3/7] Added structs and functions for manual allocation --- src/context.rs | 87 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 86 insertions(+), 1 deletion(-) diff --git a/src/context.rs b/src/context.rs index 92d09cf..81f0308 100644 --- a/src/context.rs +++ b/src/context.rs @@ -1,5 +1,5 @@ use core::marker::PhantomData; -use {ffi, types::{c_uint, c_void}, Error, Secp256k1, }; +use {ffi, types::{c_uint, c_void}, Error, Secp256k1}; #[cfg(feature = "std")] pub use self::std_only::*; @@ -21,6 +21,20 @@ pub trait Signing: Context {} /// Marker trait for indicating that an instance of `Secp256k1` can be used for verification. pub trait Verification: Context {} +/// Represents the set of capabilities needed for signing with a user preallocated memory. +pub struct SignOnlyPreallocated<'buf> { + phantom: PhantomData<&'buf ()>, +} + +/// Represents the set of capabilities needed for verification with a user preallocated memory. +pub struct VerifyOnlyPreallocated<'buf> { + phantom: PhantomData<&'buf ()>, +} + +/// Represents the set of all capabilities with a user preallocated memory. +pub struct AllPreallocated<'buf> { + phantom: PhantomData<&'buf ()>, +} #[cfg(feature = "std")] mod std_only { @@ -120,3 +134,74 @@ mod std_only { } } + +impl<'buf> Signing for SignOnlyPreallocated<'buf> {} +impl<'buf> Signing for AllPreallocated<'buf> {} + +impl<'buf> Verification for VerifyOnlyPreallocated<'buf> {} +impl<'buf> Verification for AllPreallocated<'buf> {} + +unsafe impl<'buf> Context for SignOnlyPreallocated<'buf> { + const FLAGS: c_uint = ffi::SECP256K1_START_SIGN; + const DESCRIPTION: &'static str = "signing only"; + + fn deallocate(ptr: *mut [u8]) { + let _ = ptr; + } +} + +unsafe impl<'buf> Context for VerifyOnlyPreallocated<'buf> { + const FLAGS: c_uint = ffi::SECP256K1_START_VERIFY; + const DESCRIPTION: &'static str = "verification only"; + + fn deallocate(ptr: *mut [u8]) { + let _ = ptr; + } +} + +unsafe impl<'buf> Context for AllPreallocated<'buf> { + const FLAGS: c_uint = SignOnlyPreallocated::FLAGS | VerifyOnlyPreallocated::FLAGS; + const DESCRIPTION: &'static str = "all capabilities"; + + fn deallocate(ptr: *mut [u8]) { + let _ = ptr; + } +} + +impl<'buf, C: Context + 'buf> Secp256k1 { + fn preallocated_gen_new(buf: &'buf mut [u8]) -> Result, Error> { + if buf.len() < Self::preallocate_size() { + return Err(Error::NotEnoughMemory); + } + Ok(Secp256k1 { + ctx: unsafe { + ffi::secp256k1_context_preallocated_create( + buf.as_mut_ptr() as *mut c_void, + AllPreallocated::FLAGS) + }, + phantom: PhantomData, + buf: buf as *mut [u8], + }) + } +} + +impl<'buf> Secp256k1> { + /// Creates a new Secp256k1 context with all capabilities + pub fn preallocated_new(buf: &'buf mut [u8]) -> Result>, Error> { + Secp256k1::preallocated_gen_new(buf) + } +} + +impl<'buf> Secp256k1> { + /// Creates a new Secp256k1 context that can only be used for signing + pub fn preallocated_new(buf: &'buf mut [u8]) -> Result>, Error> { + Secp256k1::preallocated_gen_new(buf) + } +} + +impl<'buf> Secp256k1> { + /// Creates a new Secp256k1 context that can only be used for verification + pub fn preallocated_new(buf: &'buf mut [u8]) -> Result>, Error> { + Secp256k1::preallocated_gen_new(buf) + } +} From 9186f0223ab3c35e220435415d2cd2312e795579 Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Thu, 4 Jul 2019 18:09:55 -0400 Subject: [PATCH 4/7] Added preallocation size functions and added a test for the preallocation --- src/context.rs | 26 +++++++++++++++++++++----- src/lib.rs | 26 +++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 6 deletions(-) diff --git a/src/context.rs b/src/context.rs index 81f0308..9c5ac4b 100644 --- a/src/context.rs +++ b/src/context.rs @@ -84,7 +84,7 @@ mod std_only { impl Secp256k1 { fn gen_new() -> Secp256k1 { - let buf = vec![0u8; Self::preallocate_size()].into_boxed_slice(); + let buf = vec![0u8; Self::preallocate_size_gen()].into_boxed_slice(); let ptr = Box::into_raw(buf); Secp256k1 { ctx: unsafe { ffi::secp256k1_context_preallocated_create(ptr as *mut c_void, C::FLAGS) }, @@ -170,14 +170,14 @@ unsafe impl<'buf> Context for AllPreallocated<'buf> { impl<'buf, C: Context + 'buf> Secp256k1 { fn preallocated_gen_new(buf: &'buf mut [u8]) -> Result, Error> { - if buf.len() < Self::preallocate_size() { + if buf.len() < Self::preallocate_size_gen() { return Err(Error::NotEnoughMemory); } Ok(Secp256k1 { ctx: unsafe { ffi::secp256k1_context_preallocated_create( buf.as_mut_ptr() as *mut c_void, - AllPreallocated::FLAGS) + C::FLAGS) }, phantom: PhantomData, buf: buf as *mut [u8], @@ -190,18 +190,34 @@ impl<'buf> Secp256k1> { pub fn preallocated_new(buf: &'buf mut [u8]) -> Result>, Error> { Secp256k1::preallocated_gen_new(buf) } + /// Uses the ffi `secp256k1_context_preallocated_size` to check the memory size needed for a context + pub fn preallocate_size() -> usize { + Self::preallocate_size_gen() + } } impl<'buf> Secp256k1> { /// Creates a new Secp256k1 context that can only be used for signing - pub fn preallocated_new(buf: &'buf mut [u8]) -> Result>, Error> { + pub fn preallocated_signing_only(buf: &'buf mut [u8]) -> Result>, Error> { Secp256k1::preallocated_gen_new(buf) } + + /// Uses the ffi `secp256k1_context_preallocated_size` to check the memory size needed for the context + #[inline] + pub fn preallocate_signing_size() -> usize { + Self::preallocate_size_gen() + } } impl<'buf> Secp256k1> { /// Creates a new Secp256k1 context that can only be used for verification - pub fn preallocated_new(buf: &'buf mut [u8]) -> Result>, Error> { + pub fn preallocated_verification_only(buf: &'buf mut [u8]) -> Result>, Error> { Secp256k1::preallocated_gen_new(buf) } + + /// Uses the ffi `secp256k1_context_preallocated_size` to check the memory size needed for the context + #[inline] + pub fn preallocate_verification_size() -> usize { + Self::preallocate_size_gen() + } } diff --git a/src/lib.rs b/src/lib.rs index d6ace6a..05f2fdc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -570,7 +570,7 @@ impl Secp256k1 { } /// Uses the ffi `secp256k1_context_preallocated_size` to check the memory size needed for a context - pub fn preallocate_size() -> usize { + pub(crate) fn preallocate_size_gen() -> usize { unsafe { ffi::secp256k1_context_preallocated_size(C::FLAGS) } } @@ -695,6 +695,30 @@ mod tests { }); } + #[test] + fn test_preallocation() { + let mut buf_ful = vec![0u8; Secp256k1::preallocate_size()]; + let mut buf_sign = vec![0u8; Secp256k1::preallocate_signing_size()]; + let mut buf_vfy = vec![0u8; Secp256k1::preallocate_verification_size()]; +// + let full = Secp256k1::preallocated_new(&mut buf_ful).unwrap(); + let sign = Secp256k1::preallocated_signing_only(&mut buf_sign).unwrap(); + let vrfy = Secp256k1::preallocated_verification_only(&mut buf_vfy).unwrap(); + +// drop(buf_vfy); // The buffer can't get dropped before the context. +// println!("{:?}", buf_ful[5]); // Can't even read the data thanks to the borrow checker. + + let (sk, pk) = full.generate_keypair(&mut thread_rng()); + let msg = Message::from_slice(&[2u8; 32]).unwrap(); + // Try signing + assert_eq!(sign.sign(&msg, &sk), full.sign(&msg, &sk)); + let sig = full.sign(&msg, &sk); + + // Try verifying + assert!(vrfy.verify(&msg, &sig, &pk).is_ok()); + assert!(full.verify(&msg, &sig, &pk).is_ok()); + } + #[test] fn capabilities() { let sign = Secp256k1::signing_only(); From 811e8d24e954f780ec9405095fe4dfefd10453b7 Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Thu, 4 Jul 2019 18:46:51 -0400 Subject: [PATCH 5/7] Removed context_create/destroy/clone and scratch_create/destroy/clone functions. --- depend/secp256k1/include/secp256k1.h | 67 ---------------------------- depend/secp256k1/src/scratch.h | 2 - depend/secp256k1/src/scratch_impl.h | 25 ----------- depend/secp256k1/src/secp256k1.c | 39 ---------------- depend/secp256k1/src/util.h | 16 ------- src/context.rs | 8 ++-- src/ffi.rs | 6 --- src/lib.rs | 3 +- 8 files changed, 6 insertions(+), 160 deletions(-) diff --git a/depend/secp256k1/include/secp256k1.h b/depend/secp256k1/include/secp256k1.h index 3e90b1b..78ac6bf 100644 --- a/depend/secp256k1/include/secp256k1.h +++ b/depend/secp256k1/include/secp256k1.h @@ -188,51 +188,6 @@ typedef int (*secp256k1_nonce_function)( */ SECP256K1_API extern const secp256k1_context *secp256k1_context_no_precomp; -/** Create a secp256k1 context object (in dynamically allocated memory). - * - * This function uses malloc to allocate memory. It is guaranteed that malloc is - * called at most once for every call of this function. If you need to avoid dynamic - * memory allocation entirely, see the functions in secp256k1_preallocated.h. - * - * Returns: a newly created context object. - * In: flags: which parts of the context to initialize. - * - * See also secp256k1_context_randomize. - */ -SECP256K1_API secp256k1_context* secp256k1_context_create( - unsigned int flags -) SECP256K1_WARN_UNUSED_RESULT; - -/** Copy a secp256k1 context object (into dynamically allocated memory). - * - * This function uses malloc to allocate memory. It is guaranteed that malloc is - * called at most once for every call of this function. If you need to avoid dynamic - * memory allocation entirely, see the functions in secp256k1_preallocated.h. - * - * Returns: a newly created context object. - * Args: ctx: an existing context to copy (cannot be NULL) - */ -SECP256K1_API secp256k1_context* secp256k1_context_clone( - const secp256k1_context* ctx -) SECP256K1_ARG_NONNULL(1) SECP256K1_WARN_UNUSED_RESULT; - -/** Destroy a secp256k1 context object (created in dynamically allocated memory). - * - * The context pointer may not be used afterwards. - * - * The context to destroy must have been created using secp256k1_context_create - * or secp256k1_context_clone. If the context has instead been created using - * secp256k1_context_preallocated_create or secp256k1_context_preallocated_clone, the - * behaviour is undefined. In that case, secp256k1_context_preallocated_destroy must - * be used instead. - * - * Args: ctx: an existing context to destroy, constructed using - * secp256k1_context_create or secp256k1_context_clone - */ -SECP256K1_API void secp256k1_context_destroy( - secp256k1_context* ctx -); - /** Set a callback function to be called when an illegal argument is passed to * an API call. It will only trigger for violations that are mentioned * explicitly in the header. @@ -301,28 +256,6 @@ SECP256K1_API void secp256k1_context_set_error_callback( const void* data ) SECP256K1_ARG_NONNULL(1); -/** Create a secp256k1 scratch space object. - * - * Returns: a newly created scratch space. - * Args: ctx: an existing context object (cannot be NULL) - * In: size: amount of memory to be available as scratch space. Some extra - * (<100 bytes) will be allocated for extra accounting. - */ -SECP256K1_API SECP256K1_WARN_UNUSED_RESULT secp256k1_scratch_space* secp256k1_scratch_space_create( - const secp256k1_context* ctx, - size_t size -) SECP256K1_ARG_NONNULL(1); - -/** Destroy a secp256k1 scratch space. - * - * The pointer may not be used afterwards. - * Args: ctx: a secp256k1 context object. - * scratch: space to destroy - */ -SECP256K1_API void secp256k1_scratch_space_destroy( - const secp256k1_context* ctx, - secp256k1_scratch_space* scratch -) SECP256K1_ARG_NONNULL(1); /** Parse a variable-length public key into the pubkey object. * diff --git a/depend/secp256k1/src/scratch.h b/depend/secp256k1/src/scratch.h index 77b35d1..814f1b1 100644 --- a/depend/secp256k1/src/scratch.h +++ b/depend/secp256k1/src/scratch.h @@ -21,9 +21,7 @@ typedef struct secp256k1_scratch_space_struct { size_t max_size; } secp256k1_scratch; -static secp256k1_scratch* secp256k1_scratch_create(const secp256k1_callback* error_callback, size_t max_size); -static void secp256k1_scratch_destroy(const secp256k1_callback* error_callback, secp256k1_scratch* scratch); /** Returns an opaque object used to "checkpoint" a scratch space. Used * with `secp256k1_scratch_apply_checkpoint` to undo allocations. */ diff --git a/depend/secp256k1/src/scratch_impl.h b/depend/secp256k1/src/scratch_impl.h index 4cee700..1c42b07 100644 --- a/depend/secp256k1/src/scratch_impl.h +++ b/depend/secp256k1/src/scratch_impl.h @@ -10,31 +10,6 @@ #include "util.h" #include "scratch.h" -static secp256k1_scratch* secp256k1_scratch_create(const secp256k1_callback* error_callback, size_t size) { - const size_t base_alloc = ((sizeof(secp256k1_scratch) + ALIGNMENT - 1) / ALIGNMENT) * ALIGNMENT; - void *alloc = checked_malloc(error_callback, base_alloc + size); - secp256k1_scratch* ret = (secp256k1_scratch *)alloc; - if (ret != NULL) { - memset(ret, 0, sizeof(*ret)); - memcpy(ret->magic, "scratch", 8); - ret->data = (void *) ((char *) alloc + base_alloc); - ret->max_size = size; - } - return ret; -} - -static void secp256k1_scratch_destroy(const secp256k1_callback* error_callback, secp256k1_scratch* scratch) { - if (scratch != NULL) { - VERIFY_CHECK(scratch->alloc_size == 0); /* all checkpoints should be applied */ - if (memcmp(scratch->magic, "scratch", 8) != 0) { - secp256k1_callback_call(error_callback, "invalid scratch space"); - return; - } - memset(scratch->magic, 0, sizeof(scratch->magic)); - free(scratch); - } -} - static size_t secp256k1_scratch_checkpoint(const secp256k1_callback* error_callback, const secp256k1_scratch* scratch) { if (memcmp(scratch->magic, "scratch", 8) != 0) { secp256k1_callback_call(error_callback, "invalid scratch space"); diff --git a/depend/secp256k1/src/secp256k1.c b/depend/secp256k1/src/secp256k1.c index 6954e1b..44e7961 100644 --- a/depend/secp256k1/src/secp256k1.c +++ b/depend/secp256k1/src/secp256k1.c @@ -136,17 +136,6 @@ secp256k1_context* secp256k1_context_preallocated_create(void* prealloc, unsigne return (secp256k1_context*) ret; } -secp256k1_context* secp256k1_context_create(unsigned int flags) { - size_t const prealloc_size = secp256k1_context_preallocated_size(flags); - secp256k1_context* ctx = (secp256k1_context*)checked_malloc(&default_error_callback, prealloc_size); - if (EXPECT(secp256k1_context_preallocated_create(ctx, flags) == NULL, 0)) { - free(ctx); - return NULL; - } - - return ctx; -} - secp256k1_context* secp256k1_context_preallocated_clone(const secp256k1_context* ctx, void* prealloc) { size_t prealloc_size; secp256k1_context* ret; @@ -161,17 +150,6 @@ secp256k1_context* secp256k1_context_preallocated_clone(const secp256k1_context* return ret; } -secp256k1_context* secp256k1_context_clone(const secp256k1_context* ctx) { - secp256k1_context* ret; - size_t prealloc_size; - - VERIFY_CHECK(ctx != NULL); - prealloc_size = secp256k1_context_preallocated_clone_size(ctx); - ret = (secp256k1_context*)checked_malloc(&ctx->error_callback, prealloc_size); - ret = secp256k1_context_preallocated_clone(ctx, ret); - return ret; -} - void secp256k1_context_preallocated_destroy(secp256k1_context* ctx) { ARG_CHECK_NO_RETURN(ctx != secp256k1_context_no_precomp); if (ctx != NULL) { @@ -180,13 +158,6 @@ void secp256k1_context_preallocated_destroy(secp256k1_context* ctx) { } } -void secp256k1_context_destroy(secp256k1_context* ctx) { - if (ctx != NULL) { - secp256k1_context_preallocated_destroy(ctx); - free(ctx); - } -} - void secp256k1_context_set_illegal_callback(secp256k1_context* ctx, void (*fun)(const char* message, void* data), const void* data) { ARG_CHECK_NO_RETURN(ctx != secp256k1_context_no_precomp); if (fun == NULL) { @@ -205,16 +176,6 @@ void secp256k1_context_set_error_callback(secp256k1_context* ctx, void (*fun)(co ctx->error_callback.data = data; } -secp256k1_scratch_space* secp256k1_scratch_space_create(const secp256k1_context* ctx, size_t max_size) { - VERIFY_CHECK(ctx != NULL); - return secp256k1_scratch_create(&ctx->error_callback, max_size); -} - -void secp256k1_scratch_space_destroy(const secp256k1_context *ctx, secp256k1_scratch_space* scratch) { - VERIFY_CHECK(ctx != NULL); - secp256k1_scratch_destroy(&ctx->error_callback, scratch); -} - static int secp256k1_pubkey_load(const secp256k1_context* ctx, secp256k1_ge* ge, const secp256k1_pubkey* pubkey) { if (sizeof(secp256k1_ge_storage) == 64) { /* When the secp256k1_ge_storage type is exactly 64 byte, use its diff --git a/depend/secp256k1/src/util.h b/depend/secp256k1/src/util.h index 9deb61b..9d750d9 100644 --- a/depend/secp256k1/src/util.h +++ b/depend/secp256k1/src/util.h @@ -68,22 +68,6 @@ static SECP256K1_INLINE void secp256k1_callback_call(const secp256k1_callback * #define VERIFY_SETUP(stmt) #endif -static SECP256K1_INLINE void *checked_malloc(const secp256k1_callback* cb, size_t size) { - void *ret = malloc(size); - if (ret == NULL) { - secp256k1_callback_call(cb, "Out of memory"); - } - return ret; -} - -static SECP256K1_INLINE void *checked_realloc(const secp256k1_callback* cb, void *ptr, size_t size) { - void *ret = realloc(ptr, size); - if (ret == NULL) { - secp256k1_callback_call(cb, "Out of memory"); - } - return ret; -} - #if defined(__BIGGEST_ALIGNMENT__) #define ALIGNMENT __BIGGEST_ALIGNMENT__ #else diff --git a/src/context.rs b/src/context.rs index 9c5ac4b..4eb5e81 100644 --- a/src/context.rs +++ b/src/context.rs @@ -123,12 +123,12 @@ mod std_only { impl Clone for Secp256k1 { fn clone(&self) -> Secp256k1 { - let buf = vec![0u8; unsafe { (&*self.buf).len() }].into_boxed_slice(); - let ptr = Box::into_raw(buf); + let clone_size = unsafe {ffi::secp256k1_context_preallocated_clone_size(self.ctx)}; + let ptr_buf = Box::into_raw(vec![0u8; clone_size].into_boxed_slice()); Secp256k1 { - ctx: unsafe { ffi::secp256k1_context_preallocated_create(ptr as *mut c_void, C::FLAGS) }, + ctx: unsafe { ffi::secp256k1_context_preallocated_clone(self.ctx, ptr_buf as *mut c_void) }, phantom: PhantomData, - buf: ptr, + buf: ptr_buf, } } } diff --git a/src/ffi.rs b/src/ffi.rs index 4a750f1..ba136bb 100644 --- a/src/ffi.rs +++ b/src/ffi.rs @@ -142,8 +142,6 @@ extern "C" { pub static secp256k1_context_no_precomp: *const Context; // Contexts - pub fn secp256k1_context_create(flags: c_uint) -> *mut Context; - pub fn secp256k1_context_preallocated_size(flags: c_uint) -> usize; pub fn secp256k1_context_preallocated_create(prealloc: *mut c_void, flags: c_uint) -> *mut Context; @@ -154,10 +152,6 @@ extern "C" { pub fn secp256k1_context_preallocated_clone(cx: *const Context, prealloc: *mut c_void) -> *mut Context; - pub fn secp256k1_context_clone(cx: *mut Context) -> *mut Context; - - pub fn secp256k1_context_destroy(cx: *mut Context); - pub fn secp256k1_context_randomize(cx: *mut Context, seed32: *const c_uchar) -> c_int; diff --git a/src/lib.rs b/src/lib.rs index 05f2fdc..d82e359 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -549,7 +549,8 @@ impl Eq for Secp256k1 { } impl Drop for Secp256k1 { fn drop(&mut self) { - C::deallocate(self.buf) + unsafe { ffi::secp256k1_context_preallocated_destroy(self.ctx) }; + C::deallocate(self.buf); } } From 49f0cc1c46eebf507ebe038da246e4e547eadb40 Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Thu, 4 Jul 2019 19:03:13 -0400 Subject: [PATCH 6/7] Updated the fuzzing dummy functions --- src/context.rs | 5 ++++- src/ffi.rs | 29 ++++++++++++++++++++--------- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/context.rs b/src/context.rs index 4eb5e81..aa35d1e 100644 --- a/src/context.rs +++ b/src/context.rs @@ -1,5 +1,8 @@ use core::marker::PhantomData; -use {ffi, types::{c_uint, c_void}, Error, Secp256k1}; +use ffi; +use types::{c_uint, c_void}; +use Error; +use Secp256k1; #[cfg(feature = "std")] pub use self::std_only::*; diff --git a/src/ffi.rs b/src/ffi.rs index ba136bb..42062f7 100644 --- a/src/ffi.rs +++ b/src/ffi.rs @@ -324,7 +324,7 @@ mod fuzz_dummy { extern crate std; use types::*; use ffi::*; - use self::std::ptr; + use self::std::{ptr, mem}; use self::std::boxed::Box; extern "C" { @@ -335,20 +335,31 @@ mod fuzz_dummy { // Contexts /// Creates a dummy context, tracking flags to ensure proper calling semantics - pub unsafe fn secp256k1_context_create(flags: c_uint) -> *mut Context { + pub unsafe fn secp256k1_context_preallocated_create(_ptr: *mut c_void, flags: c_uint) -> *mut Context { let b = Box::new(Context(flags as i32)); Box::into_raw(b) } - /// Copies a dummy context - pub unsafe fn secp256k1_context_clone(cx: *mut Context) -> *mut Context { - let b = Box::new(Context((*cx).0)); - Box::into_raw(b) + /// Return dummy size of context struct. + pub unsafe fn secp256k1_context_preallocated_size(_flags: c_uint) -> usize { + mem::size_of::() } - /// Frees a dummy context - pub unsafe fn secp256k1_context_destroy(cx: *mut Context) { - Box::from_raw(cx); + /// Return dummy size of context struct. + pub unsafe fn secp256k1_context_preallocated_clone_size(cx: *mut Context) -> usize { + mem::size_of::() + } + + /// Copies a dummy context + pub unsafe fn secp256k1_context_preallocated_clone(cx: *const Context, prealloc: *mut c_void) -> *mut Context { + let ret = prealloc as *mut Context; + *ret = (*cx).clone(); + ret + } + + /// "Destroys" a dummy context + pub unsafe fn secp256k1_context_preallocated_destroy(cx: *mut Context) { + (*cx).0 = 0; } /// Asserts that cx is properly initialized From 96ca40faedfe2c2de988f65873a9baaf9eb7d415 Mon Sep 17 00:00:00 2001 From: Elichai Turkel Date: Thu, 4 Jul 2019 21:18:36 -0400 Subject: [PATCH 7/7] Exposed generic functions to create the Context --- src/context.rs | 20 +++++++++++--------- src/lib.rs | 4 ++-- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/context.rs b/src/context.rs index aa35d1e..5dec624 100644 --- a/src/context.rs +++ b/src/context.rs @@ -7,7 +7,7 @@ use Secp256k1; #[cfg(feature = "std")] pub use self::std_only::*; -/// A trait for all kinds of Context's that let's you define the exact flags and a function to deallocate memory. +/// A trait for all kinds of Context's that Lets you define the exact flags and a function to deallocate memory. /// * DO NOT * implement it for your own types. pub unsafe trait Context { /// Flags for the ffi. @@ -86,7 +86,8 @@ mod std_only { } impl Secp256k1 { - fn gen_new() -> Secp256k1 { + /// Lets you create a context in a generic manner(sign/verify/all) + pub fn gen_new() -> Secp256k1 { let buf = vec![0u8; Self::preallocate_size_gen()].into_boxed_slice(); let ptr = Box::into_raw(buf); Secp256k1 { @@ -148,8 +149,8 @@ unsafe impl<'buf> Context for SignOnlyPreallocated<'buf> { const FLAGS: c_uint = ffi::SECP256K1_START_SIGN; const DESCRIPTION: &'static str = "signing only"; - fn deallocate(ptr: *mut [u8]) { - let _ = ptr; + fn deallocate(_ptr: *mut [u8]) { + // Allocated by the user } } @@ -157,8 +158,8 @@ unsafe impl<'buf> Context for VerifyOnlyPreallocated<'buf> { const FLAGS: c_uint = ffi::SECP256K1_START_VERIFY; const DESCRIPTION: &'static str = "verification only"; - fn deallocate(ptr: *mut [u8]) { - let _ = ptr; + fn deallocate(_ptr: *mut [u8]) { + // Allocated by the user } } @@ -166,13 +167,14 @@ unsafe impl<'buf> Context for AllPreallocated<'buf> { const FLAGS: c_uint = SignOnlyPreallocated::FLAGS | VerifyOnlyPreallocated::FLAGS; const DESCRIPTION: &'static str = "all capabilities"; - fn deallocate(ptr: *mut [u8]) { - let _ = ptr; + fn deallocate(_ptr: *mut [u8]) { + // Allocated by the user } } impl<'buf, C: Context + 'buf> Secp256k1 { - fn preallocated_gen_new(buf: &'buf mut [u8]) -> Result, Error> { + /// Lets you create a context with preallocated buffer in a generic manner(sign/verify/all) + pub fn preallocated_gen_new(buf: &'buf mut [u8]) -> Result, Error> { if buf.len() < Self::preallocate_size_gen() { return Err(Error::NotEnoughMemory); } diff --git a/src/lib.rs b/src/lib.rs index d82e359..6651bc2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -570,8 +570,8 @@ impl Secp256k1 { &self.ctx } - /// Uses the ffi `secp256k1_context_preallocated_size` to check the memory size needed for a context - pub(crate) fn preallocate_size_gen() -> usize { + /// Returns the required memory for a preallocated context buffer in a generic manner(sign/verify/all) + pub fn preallocate_size_gen() -> usize { unsafe { ffi::secp256k1_context_preallocated_size(C::FLAGS) } }