Ensure that all public keys are being checked when deserialized

src/key_exchange/traits.rs

@@ -4,22 +4,21 @@
 // LICENSE file in the root directory of this source tree.
 
 use crate::{
+    ciphersuite::CipherSuite,
     errors::{PakeError, ProtocolError},
     group::Group,
     hash::Hash,
     keypair::Key,
 };
 use rand::{CryptoRng, RngCore};
-
-use std::convert::TryFrom;
 use zeroize::Zeroize;
 
 pub trait KeyExchange<D: Hash, G: Group> {
-    type KE1State: for<'r> TryFrom<&'r [u8], Error = PakeError> + ToBytesWithPointers + Zeroize;
+    type KE1State: FromBytes + ToBytesWithPointers + Zeroize;
-    type KE2State: for<'r> TryFrom<&'r [u8], Error = PakeError> + ToBytesWithPointers + Zeroize;
+    type KE2State: FromBytes + ToBytesWithPointers + Zeroize;
-    type KE1Message: for<'r> TryFrom<&'r [u8], Error = PakeError> + ToBytes;
+    type KE1Message: FromBytes + ToBytes;
-    type KE2Message: for<'r> TryFrom<&'r [u8], Error = PakeError> + ToBytes;
+    type KE2Message: FromBytes + ToBytes;
-    type KE3Message: for<'r> TryFrom<&'r [u8], Error = PakeError> + ToBytes;
+    type KE3Message: FromBytes + ToBytes;
 
     fn generate_ke1<R: RngCore + CryptoRng>(
         info: Vec<u8>,
@@ -60,6 +59,10 @@ pub trait KeyExchange<D: Hash, G: Group> {
     fn ke2_message_size() -> usize;
 }
 
+pub trait FromBytes: Sized {
+    fn from_bytes<CS: CipherSuite>(input: &[u8]) -> Result<Self, PakeError>;
+}
+
 pub trait ToBytes {
     fn to_bytes(&self) -> Vec<u8>;
 }

src/key_exchange/tripledh.rs

@@ -5,13 +5,14 @@
 
 //! An implementation of the Triple Diffie-Hellman key exchange protocol
 use crate::{
+    ciphersuite::CipherSuite,
     errors::{
         utils::{check_slice_size, check_slice_size_atleast},
         InternalPakeError, PakeError, ProtocolError,
     },
     group::Group,
     hash::Hash,
-    key_exchange::traits::{KeyExchange, ToBytes, ToBytesWithPointers},
+    key_exchange::traits::{FromBytes, KeyExchange, ToBytes, ToBytesWithPointers},
     keypair::{Key, KeyPair, SizedBytesExt},
     serialization::{serialize, tokenize},
 };
@@ -26,8 +27,6 @@ use hmac::{Hmac, Mac, NewMac};
 use rand::{CryptoRng, RngCore};
 use zeroize::Zeroize;
 
-use std::convert::TryFrom;
-
 const KEY_LEN: usize = 32;
 pub(crate) type NonceLen = U32;
 
@@ -253,10 +252,8 @@ pub struct Ke1Message {
     pub(crate) client_e_pk: Key,
 }
 
-impl TryFrom<&[u8]> for Ke1State {
+impl FromBytes for Ke1State {
-    type Error = PakeError;
+    fn from_bytes<CS: CipherSuite>(bytes: &[u8]) -> Result<Self, PakeError> {
-
-    fn try_from(bytes: &[u8]) -> Result<Self, Self::Error> {
         let nonce_len = NonceLen::to_usize();
         let checked_bytes = check_slice_size_atleast(bytes, KEY_LEN + nonce_len, "ke1_state")?;
 
@@ -298,22 +295,24 @@ impl ToBytes for Ke1Message {
     }
 }
 
-impl TryFrom<&[u8]> for Ke1Message {
+impl FromBytes for Ke1Message {
-    type Error = PakeError;
+    fn from_bytes<CS: CipherSuite>(ke1_message_bytes: &[u8]) -> Result<Self, PakeError> {
-
-    fn try_from(ke1_message_bytes: &[u8]) -> Result<Self, Self::Error> {
         let nonce_len = NonceLen::to_usize();
         let checked_nonce =
             check_slice_size_atleast(ke1_message_bytes, nonce_len, "ke1_message nonce")?;
 
         let (info, remainder) = tokenize(&checked_nonce[nonce_len..], 2)?;
 
-        let checked_client_e_pk = check_slice_size(&remainder, KEY_LEN, "ke1_message client_e_pk")?;
+        // Check the public key bytes
+        let unchecked_client_e_pk =
+            check_slice_size(&remainder, KEY_LEN, "ke1_message client_e_pk")?;
+        let client_e_pk =
+            KeyPair::<CS::Group>::check_public_key(Key::from_bytes(unchecked_client_e_pk)?)?;
 
         Ok(Self {
             client_nonce: GenericArray::clone_from_slice(&checked_nonce[..nonce_len]),
             info,
-            client_e_pk: Key::from_bytes(checked_client_e_pk)?,
+            client_e_pk,
         })
     }
 }
@@ -367,10 +366,8 @@ pub struct Ke2Message<HashLen: ArrayLength<u8>> {
     mac: GenericArray<u8, HashLen>,
 }
 
-impl<HashLen: ArrayLength<u8>> TryFrom<&[u8]> for Ke2State<HashLen> {
+impl<HashLen: ArrayLength<u8>> FromBytes for Ke2State<HashLen> {
-    type Error = PakeError;
+    fn from_bytes<CS: CipherSuite>(input: &[u8]) -> Result<Self, PakeError> {
-
-    fn try_from(input: &[u8]) -> Result<Self, Self::Error> {
         let hash_len = HashLen::to_usize();
         let checked_bytes = check_slice_size(input, 3 * hash_len, "ke2_state")?;
 
@@ -401,23 +398,27 @@ impl<HashLen: ArrayLength<u8>> Ke2Message<HashLen> {
     }
 }
 
-impl<HashLen: ArrayLength<u8>> TryFrom<&[u8]> for Ke2Message<HashLen> {
+impl<HashLen: ArrayLength<u8>> FromBytes for Ke2Message<HashLen> {
-    type Error = PakeError;
+    fn from_bytes<CS: CipherSuite>(input: &[u8]) -> Result<Self, PakeError> {
-
-    fn try_from(input: &[u8]) -> Result<Self, Self::Error> {
         let nonce_len = NonceLen::to_usize();
         let checked_nonce = check_slice_size_atleast(input, nonce_len, "ke2_message nonce")?;
-        let checked_server_e_pk = check_slice_size_atleast(
+
+        let unchecked_server_e_pk = check_slice_size_atleast(
             &checked_nonce[nonce_len..],
             KEY_LEN,
             "ke2_message server_e_pk",
         )?;
-        let (e_info, remainder) = tokenize(&checked_server_e_pk[KEY_LEN..], 2)?;
+        let (e_info, remainder) = tokenize(&unchecked_server_e_pk[KEY_LEN..], 2)?;
         let checked_mac = check_slice_size(&remainder, HashLen::to_usize(), "ke1_message mac")?;
 
+        // Check the public key bytes
+        let server_e_pk = KeyPair::<CS::Group>::check_public_key(Key::from_bytes(
+            &unchecked_server_e_pk[..KEY_LEN],
+        )?)?;
+
         Ok(Self {
             server_nonce: GenericArray::clone_from_slice(&checked_nonce[..nonce_len]),
-            server_e_pk: Key::from_bytes(&checked_server_e_pk[..KEY_LEN])?,
+            server_e_pk,
             e_info,
             mac: GenericArray::clone_from_slice(checked_mac),
         })
@@ -455,10 +456,8 @@ impl<HashLen: ArrayLength<u8>> ToBytes for Ke3Message<HashLen> {
     }
 }
 
-impl<HashLen: ArrayLength<u8>> TryFrom<&[u8]> for Ke3Message<HashLen> {
+impl<HashLen: ArrayLength<u8>> FromBytes for Ke3Message<HashLen> {
-    type Error = PakeError;
+    fn from_bytes<CS: CipherSuite>(bytes: &[u8]) -> Result<Self, PakeError> {
-
-    fn try_from(bytes: &[u8]) -> Result<Self, Self::Error> {
         let checked_bytes = check_slice_size(bytes, HashLen::to_usize(), "ke3_message")?;
 
         Ok(Self {

src/messages.rs

@@ -13,12 +13,11 @@ use crate::{
         PakeError, ProtocolError,
     },
     group::Group,
-    key_exchange::traits::{KeyExchange, ToBytes},
+    key_exchange::traits::{FromBytes, KeyExchange, ToBytes},
     keypair::{Key, KeyPair, SizedBytesExt},
 };
 use generic_array::{typenum::Unsigned, GenericArray};
 use generic_bytes::SizedBytes;
-use std::convert::TryFrom;
 
 // Messages
 // =========
@@ -75,11 +74,11 @@ impl<CS: CipherSuite> RegistrationResponse<CS> {
         // correct subgroup
         let arr = GenericArray::from_slice(&checked_slice[..elem_len]);
         let beta = CS::Group::from_element_slice(arr)?;
+        let server_s_pk =
+            KeyPair::<CS::Group>::check_public_key(Key::from_bytes(&checked_slice[elem_len..])?)?
+                .to_vec();
 
-        Ok(Self {
+        Ok(Self { server_s_pk, beta })
-            server_s_pk: checked_slice[elem_len..].to_vec(),
-            beta,
-        })
     }
 }
 
@@ -138,10 +137,7 @@ pub struct CredentialRequest<CS: CipherSuite> {
 impl<CS: CipherSuite> CredentialRequest<CS> {
     /// Serialization into bytes
     pub fn serialize(&self) -> Vec<u8> {
-        let mut credential_request: Vec<u8> = Vec::new();
+        [self.alpha.to_arr().to_vec(), self.ke1_message.to_bytes()].concat()
-        credential_request.extend_from_slice(&self.alpha.to_arr());
-        credential_request.extend_from_slice(&self.ke1_message.to_bytes());
-        credential_request
     }
 
     /// Deserialization from bytes
@@ -156,7 +152,7 @@ impl<CS: CipherSuite> CredentialRequest<CS> {
         let alpha = CS::Group::from_element_slice(arr)?;
 
         let ke1_message =
-            <CS::KeyExchange as KeyExchange<CS::Hash, CS::Group>>::KE1Message::try_from(
+            <CS::KeyExchange as KeyExchange<CS::Hash, CS::Group>>::KE1Message::from_bytes::<CS>(
                 &checked_slice[elem_len..],
             )?;
 
@@ -223,7 +219,7 @@ impl<CS: CipherSuite> CredentialResponse<CS> {
         let checked_remainder =
             check_slice_size_atleast(&remainder, ke2_message_size, "login_second_message_bytes")?;
         let ke2_message =
-            <CS::KeyExchange as KeyExchange<CS::Hash, CS::Group>>::KE2Message::try_from(
+            <CS::KeyExchange as KeyExchange<CS::Hash, CS::Group>>::KE2Message::from_bytes::<CS>(
                 checked_remainder,
             )?;
 
@@ -253,7 +249,9 @@ impl<CS: CipherSuite> CredentialFinalization<CS> {
     /// Deserialization from bytes
     pub fn deserialize(input: &[u8]) -> Result<Self, ProtocolError> {
         let ke3_message =
-            <CS::KeyExchange as KeyExchange<CS::Hash, CS::Group>>::KE3Message::try_from(input)?;
+            <CS::KeyExchange as KeyExchange<CS::Hash, CS::Group>>::KE3Message::from_bytes::<CS>(
+                input,
+            )?;
         Ok(Self { ke3_message })
     }
 }

src/opaque.rs

@@ -11,7 +11,7 @@ use crate::{
     errors::{utils::check_slice_size_atleast, InternalPakeError, PakeError, ProtocolError},
     group::Group,
     hash::Hash,
-    key_exchange::traits::{KeyExchange, ToBytesWithPointers},
+    key_exchange::traits::{FromBytes, KeyExchange, ToBytesWithPointers},
     keypair::{Key, KeyPair, SizedBytesExt},
     map_to_curve::GroupWithMapToCurve,
     oprf,
@@ -24,7 +24,7 @@ use digest::Digest;
 use generic_array::{typenum::Unsigned, GenericArray};
 use generic_bytes::SizedBytes;
 use rand::{CryptoRng, RngCore};
-use std::{convert::TryFrom, marker::PhantomData};
+use std::marker::PhantomData;
 use zeroize::Zeroize;
 
 // Registration
@@ -432,9 +432,10 @@ impl<CS: CipherSuite> ClientLogin<CS> {
         let (serialized_credential_request, remainder) = tokenize(&checked_slice[scalar_len..], 2)?;
         let (ke1_state_bytes, password) = tokenize(&remainder, 2)?;
 
-        let ke1_state = <CS::KeyExchange as KeyExchange<CS::Hash, CS::Group>>::KE1State::try_from(
+        let ke1_state =
-            &ke1_state_bytes[..],
+            <CS::KeyExchange as KeyExchange<CS::Hash, CS::Group>>::KE1State::from_bytes::<CS>(
-        )?;
+                &ke1_state_bytes[..],
+            )?;
         Ok(Self {
             token: oprf::Token {
                 data: password,
@@ -707,9 +708,9 @@ impl<CS: CipherSuite> ServerLogin<CS> {
     pub fn deserialize(bytes: &[u8]) -> Result<Self, ProtocolError> {
         Ok(Self {
             _cs: PhantomData,
-            ke2_state: <CS::KeyExchange as KeyExchange<CS::Hash, CS::Group>>::KE2State::try_from(
+            ke2_state: <CS::KeyExchange as KeyExchange<CS::Hash, CS::Group>>::KE2State::from_bytes::<
-                bytes,
+                CS,
-            )?,
+            >(bytes)?,
         })
     }
 

src/serialization/tests.rs

@@ -8,7 +8,7 @@ use crate::{
     envelope::{Envelope, InnerEnvelopeMode},
     group::Group,
     key_exchange::{
-        traits::{KeyExchange, ToBytes},
+        traits::{FromBytes, KeyExchange, ToBytes},
         tripledh::{NonceLen, TripleDH},
     },
     opaque::*,
@@ -23,7 +23,6 @@ use proptest::{collection::vec, prelude::*};
 use rand::{rngs::OsRng, RngCore};
 
 use sha2::Digest;
-use std::convert::TryFrom;
 
 struct Default;
 impl CipherSuite for Default {
@@ -294,9 +293,10 @@ fn ke1_message_roundtrip() {
         &client_e_kp.public(),
     ]
     .concat();
-    let reg =
+    let reg = <TripleDH as KeyExchange<sha2::Sha512, RistrettoPoint>>::KE1Message::from_bytes::<
-        <TripleDH as KeyExchange<sha2::Sha512, RistrettoPoint>>::KE1Message::try_from(&ke1m[..])
+        Default,
-            .unwrap();
+    >(&ke1m[..])
+    .unwrap();
     let reg_bytes = reg.to_bytes();
     assert_eq!(reg_bytes, ke1m);
 }
@@ -321,9 +321,10 @@ fn ke2_message_roundtrip() {
     ]
     .concat();
 
-    let reg =
+    let reg = <TripleDH as KeyExchange<sha2::Sha512, RistrettoPoint>>::KE2Message::from_bytes::<
-        <TripleDH as KeyExchange<sha2::Sha512, RistrettoPoint>>::KE2Message::try_from(&ke2m[..])
+        Default,
-            .unwrap();
+    >(&ke2m[..])
+    .unwrap();
     let reg_bytes = reg.to_bytes();
     assert_eq!(reg_bytes, ke2m);
 }
@@ -336,9 +337,10 @@ fn ke3_message_roundtrip() {
 
     let ke3m: Vec<u8> = [&mac[..]].concat();
 
-    let reg =
+    let reg = <TripleDH as KeyExchange<sha2::Sha512, RistrettoPoint>>::KE3Message::from_bytes::<
-        <TripleDH as KeyExchange<sha2::Sha512, RistrettoPoint>>::KE3Message::try_from(&ke3m[..])
+        Default,
-            .unwrap();
+    >(&ke3m[..])
+    .unwrap();
     let reg_bytes = reg.to_bytes();
     assert_eq!(reg_bytes, ke3m);
 }