Merge pull request #51 from rust-bitcoin/2018-08-message

Various slice-handling ergonomic improvements

src/key.rs

@@ -150,8 +150,16 @@ impl SecretKey {
     }
 
     #[inline]
-    /// Adds one secret key to another, modulo the curve order
+    /// Adds one secret key to another, modulo the curve order. WIll
-    pub fn add_assign(&mut self, other: &SecretKey) -> Result<(), Error> {
+    /// 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
+    /// Multiplies one secret key by another, modulo the curve order. Will
-    pub fn mul_assign(&mut self, other: &SecretKey) -> Result<(), Error> {
+    /// 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<C: Verification>(&mut self, secp: &Secp256k1<C>, other: &SecretKey)
+    /// Will return an error if the resulting key would be invalid or
-                         -> Result<(), Error> {
+    /// if the tweak was not a 32-byte length slice.
+    pub fn add_exp_assign<C: Verification>(
+        &mut self,
+        secp: &Secp256k1<C>,
+        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<C: Verification>(&mut self, secp: &Secp256k1<C>, other: &SecretKey)
+    /// Will return an error if the resulting key would be invalid or
-                         -> Result<(), Error> {
+    /// if the tweak was not a 32-byte length slice.
+    pub fn mul_assign<C: Verification>(
+        &mut self,
+        secp: &Secp256k1<C>,
+        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!(sk1.add_assign(&sk2[..]).is_ok());
-        assert!(pk1.add_exp_assign(&s, &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!(sk2.add_assign(&sk1[..]).is_ok());
-        assert!(pk2.add_exp_assign(&s, &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!(sk1.mul_assign(&sk2[..]).is_ok());
-        assert!(pk1.mul_assign(&s, &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!(sk2.mul_assign(&sk1[..]).is_ok());
-        assert!(pk2.mul_assign(&s, &sk1).is_ok());
+        assert!(pk2.mul_assign(&s, &sk1[..]).is_ok());
         assert_eq!(PublicKey::from_secret_key(&s, &sk2), pk2);
     }
 

src/lib.rs

@@ -205,6 +205,14 @@ fn from_str(s: &str) -> Result<Signature, Error> {
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]
 pub struct RecoverableSignature(ffi::RecoverableSignature);
 
+/// Trait describing something that promises to be a 32-byte random number; in particular,
+/// it has negligible probability of being zero or overflowing the group order. Such objects
+/// may be converted to `Message`s without any error paths.
+pub trait ThirtyTwoByteHash {
+    /// Converts the object into a 32-byte array
+    fn into_32(self) -> [u8; 32];
+}
+
 impl RecoveryId {
 #[inline]
 /// Allows library users to create valid recovery IDs from i32.
@@ -504,6 +512,10 @@ impl Message {
     /// Converts a `MESSAGE_SIZE`-byte slice to a message object
     #[inline]
     pub fn from_slice(data: &[u8]) -> Result<Message, Error> {
+        if data == &[0; constants::MESSAGE_SIZE] {
+            return Err(Error::InvalidMessage);
+        }
+
         match data.len() {
             constants::MESSAGE_SIZE => {
                 let mut ret = [0; constants::MESSAGE_SIZE];
@@ -515,10 +527,10 @@ impl Message {
     }
 }
 
-/// Creates a message from a `MESSAGE_SIZE` byte array
+impl<T: ThirtyTwoByteHash> From<T> for Message {
-impl From<[u8; constants::MESSAGE_SIZE]> for Message {
+    /// Converts a 32-byte hash directly to a message without error paths
-    fn from(buf: [u8; constants::MESSAGE_SIZE]) -> Message {
+    fn from(t: T) -> Message {
-        Message(buf)
+        Message(t.into_32())
     }
 }
 
@@ -538,6 +550,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
@@ -557,7 +571,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",
         }
     }
 }
@@ -990,17 +1005,16 @@ mod tests {
         s.randomize(&mut thread_rng());
 
         // Wild keys: 1, CURVE_ORDER - 1
-        // Wild msgs: 0, 1, CURVE_ORDER - 1, CURVE_ORDER
+        // Wild msgs: 1, CURVE_ORDER - 1
         let mut wild_keys = [[0; 32]; 2];
-        let mut wild_msgs = [[0; 32]; 4];
+        let mut wild_msgs = [[0; 32]; 2];
 
         wild_keys[0][0] = 1;
-        wild_msgs[1][0] = 1;
+        wild_msgs[0][0] = 1;
 
         use constants;
         wild_keys[1][..].copy_from_slice(&constants::CURVE_ORDER[..]);
         wild_msgs[1][..].copy_from_slice(&constants::CURVE_ORDER[..]);
-        wild_msgs[2][..].copy_from_slice(&constants::CURVE_ORDER[..]);
 
         wild_keys[1][0] -= 1;
         wild_msgs[1][0] -= 1;
@@ -1079,7 +1093,11 @@ mod tests {
                    Err(InvalidMessage));
         assert_eq!(Message::from_slice(&[0; constants::MESSAGE_SIZE + 1]),
                    Err(InvalidMessage));
-        assert!(Message::from_slice(&[0; constants::MESSAGE_SIZE]).is_ok());
+        assert_eq!(
+            Message::from_slice(&[0; constants::MESSAGE_SIZE]),
+            Err(InvalidMessage)
+        );
+        assert!(Message::from_slice(&[1; constants::MESSAGE_SIZE]).is_ok());
     }
 
     #[test]