From 1f4a4c11a3e41319d228adc1ed9fc187786f0892 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Wed, 15 Aug 2018 17:11:32 +0000 Subject: [PATCH] change add_*_assign and mul_*_assign to use a byteslice as a tweak, rather than a `SecretKey` This makes more conceptual sense and does not add any new error paths, since even valid `SecretKey`s were able to be invalid tweaks. --- src/key.rs | 72 +++++++++++++++++++++++++++++++++++++++--------------- src/lib.rs | 5 +++- 2 files changed, 56 insertions(+), 21 deletions(-) diff --git a/src/key.rs b/src/key.rs index c5d7b33..285f436 100644 --- a/src/key.rs +++ b/src/key.rs @@ -150,8 +150,16 @@ impl SecretKey { } #[inline] - /// Adds one secret key to another, modulo the curve order - pub fn add_assign(&mut self, other: &SecretKey) -> Result<(), Error> { + /// Adds one secret key to another, modulo the curve order. WIll + /// return an error if the resulting key would be invalid or if + /// the tweak was not a 32-byte length slice. + pub fn add_assign( + &mut self, + other: &[u8], + ) -> Result<(), Error> { + if other.len() != 32 { + return Err(Error::InvalidTweak); + } unsafe { if ffi::secp256k1_ec_privkey_tweak_add( ffi::secp256k1_context_no_precomp, @@ -159,7 +167,7 @@ impl SecretKey { other.as_ptr(), ) != 1 { - Err(InvalidSecretKey) + Err(Error::InvalidTweak) } else { Ok(()) } @@ -167,8 +175,16 @@ impl SecretKey { } #[inline] - /// Multiplies one secret key by another, modulo the curve order - pub fn mul_assign(&mut self, other: &SecretKey) -> Result<(), Error> { + /// Multiplies one secret key by another, modulo the curve order. Will + /// return an error if the resulting key would be invalid or if + /// the tweak was not a 32-byte length slice. + pub fn mul_assign( + &mut self, + other: &[u8], + ) -> Result<(), Error> { + if other.len() != 32 { + return Err(Error::InvalidTweak); + } unsafe { if ffi::secp256k1_ec_privkey_tweak_mul( ffi::secp256k1_context_no_precomp, @@ -176,7 +192,7 @@ impl SecretKey { other.as_ptr(), ) != 1 { - Err(InvalidSecretKey) + Err(Error::InvalidTweak) } else { Ok(()) } @@ -293,28 +309,44 @@ impl PublicKey { #[inline] /// Adds the pk corresponding to `other` to the pk `self` in place - pub fn add_exp_assign(&mut self, secp: &Secp256k1, other: &SecretKey) - -> Result<(), Error> { + /// Will return an error if the resulting key would be invalid or + /// if the tweak was not a 32-byte length slice. + pub fn add_exp_assign( + &mut self, + secp: &Secp256k1, + other: &[u8] + ) -> Result<(), Error> { + if other.len() != 32 { + return Err(Error::InvalidTweak); + } unsafe { if ffi::secp256k1_ec_pubkey_tweak_add(secp.ctx, &mut self.0 as *mut _, other.as_ptr()) == 1 { Ok(()) } else { - Err(InvalidSecretKey) + Err(Error::InvalidTweak) } } } #[inline] /// Muliplies the pk `self` in place by the scalar `other` - pub fn mul_assign(&mut self, secp: &Secp256k1, other: &SecretKey) - -> Result<(), Error> { + /// Will return an error if the resulting key would be invalid or + /// if the tweak was not a 32-byte length slice. + pub fn mul_assign( + &mut self, + secp: &Secp256k1, + other: &[u8], + ) -> Result<(), Error> { + if other.len() != 32 { + return Err(Error::InvalidTweak); + } unsafe { if ffi::secp256k1_ec_pubkey_tweak_mul(secp.ctx, &mut self.0 as *mut _, other.as_ptr()) == 1 { Ok(()) } else { - Err(InvalidSecretKey) + Err(Error::InvalidTweak) } } } @@ -592,13 +624,13 @@ mod test { let (mut sk2, mut pk2) = s.generate_keypair(&mut thread_rng()); assert_eq!(PublicKey::from_secret_key(&s, &sk1), pk1); - assert!(sk1.add_assign(&sk2).is_ok()); - assert!(pk1.add_exp_assign(&s, &sk2).is_ok()); + assert!(sk1.add_assign(&sk2[..]).is_ok()); + assert!(pk1.add_exp_assign(&s, &sk2[..]).is_ok()); assert_eq!(PublicKey::from_secret_key(&s, &sk1), pk1); assert_eq!(PublicKey::from_secret_key(&s, &sk2), pk2); - assert!(sk2.add_assign(&sk1).is_ok()); - assert!(pk2.add_exp_assign(&s, &sk1).is_ok()); + assert!(sk2.add_assign(&sk1[..]).is_ok()); + assert!(pk2.add_exp_assign(&s, &sk1[..]).is_ok()); assert_eq!(PublicKey::from_secret_key(&s, &sk2), pk2); } @@ -610,13 +642,13 @@ mod test { let (mut sk2, mut pk2) = s.generate_keypair(&mut thread_rng()); assert_eq!(PublicKey::from_secret_key(&s, &sk1), pk1); - assert!(sk1.mul_assign(&sk2).is_ok()); - assert!(pk1.mul_assign(&s, &sk2).is_ok()); + assert!(sk1.mul_assign(&sk2[..]).is_ok()); + assert!(pk1.mul_assign(&s, &sk2[..]).is_ok()); assert_eq!(PublicKey::from_secret_key(&s, &sk1), pk1); assert_eq!(PublicKey::from_secret_key(&s, &sk2), pk2); - assert!(sk2.mul_assign(&sk1).is_ok()); - assert!(pk2.mul_assign(&s, &sk1).is_ok()); + assert!(sk2.mul_assign(&sk1[..]).is_ok()); + assert!(pk2.mul_assign(&s, &sk1[..]).is_ok()); assert_eq!(PublicKey::from_secret_key(&s, &sk2), pk2); } diff --git a/src/lib.rs b/src/lib.rs index b75993f..422cb5b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -535,6 +535,8 @@ pub enum Error { InvalidSecretKey, /// Bad recovery id InvalidRecoveryId, + /// Invalid tweak for add_*_assign or mul_*_assign + InvalidTweak, } // Passthrough Debug to Display, since errors should be user-visible @@ -554,7 +556,8 @@ impl error::Error for Error { Error::InvalidPublicKey => "secp: malformed public key", Error::InvalidSignature => "secp: malformed signature", Error::InvalidSecretKey => "secp: malformed or out-of-range secret key", - Error::InvalidRecoveryId => "secp: bad recovery id" + Error::InvalidRecoveryId => "secp: bad recovery id", + Error::InvalidTweak => "secp: bad tweak", } } }