Add serde support for relevant types (#193)

Serde is an important part of the Rust ecosystem, and we want to ensure users
can securely serialize and deserialize types easily and securely. The Serialize
and Deserialize implementations here delegate to constant-time operations
wherever possible.

The phrase "relevant types" above means any type that we expect a user may want
to send outside the application, e.g. to a database or across the network. For
now, this includes:

    - pwhash::PasswordHash
    - hash::Digest
    - auth::Tag
    - kdf::Salt

By not implementing serde for other types, we're helping to ensure that the
secure usage of these types is on the easy path.

Co-authored-by: Vincent Mutolo <[email protected]>

.github/workflows/test.yml

@@ -42,6 +42,12 @@ jobs:
         uses: actions-rs/cargo@v1
         with:
           command: test
+
+      - name: Test debug-mode, all features
+        uses: actions-rs/cargo@v1
+        with:
+          command: test
+          args: --features serde
 
       - name: Test debug-mode, no default features
         uses: actions-rs/cargo@v1
@@ -60,6 +66,12 @@ jobs:
         with:
           command: test
           args: --release
+
+      - name: Test release-mode, all features
+        uses: actions-rs/cargo@v1
+        with:
+          command: test
+          args: --release --features serde
 
       - name: Test release-mode, no default features
         uses: actions-rs/cargo@v1

Cargo.toml

@@ -21,6 +21,7 @@ subtle = { version = "^2.2.2", default-features = false }
 zeroize = { version = "1.1.0", default-features = false }
 getrandom = { version = "0.2.0", optional = true }
 ct-codecs = { version = "1.1.1", optional = true }
+serde = { version = "1.0.124", optional = true }
 
 [features]
 default = [ "safe_api" ]

src/hazardous/ecc/x25519.rs

@@ -66,7 +66,6 @@
 use super::fiat_curve25519_u64;
 use crate::errors::UnknownCryptoError;
 use crate::util::secure_cmp;
-use core::convert::TryFrom;
 use core::ops::{Add, Mul, Sub};
 
 /// The size of a public key used in X25519.
@@ -413,7 +412,7 @@ impl From<[u8; PUBLIC_KEY_SIZE]> for PublicKey {
     }
 }
 
-impl TryFrom<&PrivateKey> for PublicKey {
+impl core::convert::TryFrom<&PrivateKey> for PublicKey {
     type Error = UnknownCryptoError;
 
     fn try_from(private_key: &PrivateKey) -> Result<Self, Self::Error> {

src/high_level/pwhash.rs

@@ -96,6 +96,12 @@ use crate::{
 use ct_codecs::{Base64NoPadding, Decoder, Encoder};
 use zeroize::Zeroizing;
 
+#[cfg(feature = "serde")]
+use serde::{
+    de::{self, Deserialize, Deserializer},
+    ser::{Serialize, Serializer},
+};
+
 /// The length of the salt used for password hashing.
 pub const SALT_LENGTH: usize = 16;
 
@@ -362,6 +368,33 @@ impl core::fmt::Debug for PasswordHash {
 
 impl_ct_partialeq_trait!(PasswordHash, unprotected_as_bytes);
 
+#[cfg(feature = "serde")]
+/// `PasswordHash` serializes as would a [`String`](std::string::String). Note that
+/// the serialized type likely does not have the same protections that orion
+/// provides, such as constant-time operations. A good rule of thumb is to only
+/// serialize these types for storage. Don't operate on the serialized types.
+impl Serialize for PasswordHash {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: Serializer,
+    {
+        let encoded_string = self.unprotected_as_encoded();
+        serializer.serialize_str(encoded_string)
+    }
+}
+
+#[cfg(feature = "serde")]
+/// `PasswordHash` deserializes from a [`String`](std::string::String).
+impl<'de> Deserialize<'de> for PasswordHash {
+    fn deserialize<D>(deserializer: D) -> Result<PasswordHash, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        let encoded_str = String::deserialize(deserializer)?;
+        PasswordHash::from_encoded(&encoded_str).map_err(de::Error::custom)
+    }
+}
+
 #[must_use = "SECURITY WARNING: Ignoring a Result can have real security implications."]
 /// Hash a password using Argon2i.
 pub fn hash_password(
@@ -450,6 +483,28 @@ mod public {
         assert_eq!(debug, expected);
     }
 
+    #[cfg(feature = "serde")]
+    mod test_serde_impls {
+        use super::*;
+
+        #[test]
+        fn test_valid_deserialization() {
+            let encoded_hash = "$argon2i$v=19$m=65536,t=3,p=1$c29tZXNhbHRzb21lc2FsdA$fRsRY9PAt5H+qAKuXRzL0/6JbFShsCd62W5aHzESk/c";
+            let expected = PasswordHash::from_encoded(encoded_hash).unwrap();
+            let deserialized: PasswordHash =
+                serde_json::from_str(format!("\"{}\"", encoded_hash).as_str()).unwrap();
+            assert_eq!(deserialized, expected);
+        }
+
+        #[test]
+        fn test_valid_serialization() {
+            let encoded_hash = "$argon2i$v=19$m=65536,t=3,p=1$c29tZXNhbHRzb21lc2FsdA$fRsRY9PAt5H+qAKuXRzL0/6JbFShsCd62W5aHzESk/c";
+            let hash = PasswordHash::from_encoded(encoded_hash).unwrap();
+            let serialized: String = serde_json::to_string(&hash).unwrap();
+            assert_eq!(serialized, format!("\"{}\"", encoded_hash));
+        }
+    }
+
     /// The tests herein were generated with the CLI tool from the reference implementation at:
     /// https://github.com/P-H-C/phc-winner-argon2/commit/62358ba2123abd17fccf2a108a301d4b52c01a7c
     mod test_encoding_from_ref {

src/typedefs.rs

@@ -85,6 +85,59 @@ macro_rules! impl_normal_debug_trait (($name:ident) => (
     }
 ));
 
+/// Macro that implements the `serde::{Serialize, Deserialize}` traits.
+#[cfg(feature = "serde")]
+macro_rules! impl_serde_traits (($name:ident, $bytes_function:ident) => (
+    /// This type tries to serialize as a `&[u8]` would. Note that the serialized
+    /// type likely does not have the same protections that orion provides, such
+    /// as constant-time operations. A good rule of thumb is to only serialize
+    /// these types for storage. Don't operate on the serialized types.
+    impl serde::Serialize for $name {
+        fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+        where
+            S: serde::ser::Serializer,
+        {
+            let bytes: &[u8] = self.$bytes_function();
+            bytes.serialize(serializer)
+        }
+    }
+
+    /// This type tries to deserialize as a `Vec<u8>` would. If it succeeds, the digest
+    /// will be built using `Self::from_slice`.
+    ///
+    /// Note that **this allocates** once to store the referenced bytes on the heap.
+    impl<'de> serde::Deserialize<'de> for $name {
+        fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+        where
+            D: serde::de::Deserializer<'de>,
+        {
+            let bytes = Vec::<u8>::deserialize(deserializer)?;
+            std::convert::TryFrom::try_from(bytes.as_slice()).map_err(serde::de::Error::custom)
+        }
+    }
+));
+
+#[cfg(test)]
+#[cfg(feature = "serde")]
+macro_rules! test_serde_impls (($name:ident, $gen_length:expr) => (
+    #[test]
+    fn test_serde_serialized_equivalence_to_bytes_fn() {
+        let bytes = &[38u8; $gen_length][..];
+        let orion_type = $name::from_slice(bytes).unwrap();
+        let serialized_from_bytes = serde_json::to_value(bytes).unwrap();
+        let serialized_from_orion_type = serde_json::to_value(&orion_type).unwrap();
+        assert_eq!(serialized_from_bytes, serialized_from_orion_type);
+    }
+
+    #[test]
+    fn test_serde_deserialized_equivalence_to_bytes_fn() {
+        let bytes = &[38u8; $gen_length][..];
+        let serialized_from_bytes = serde_json::to_value(bytes).unwrap();
+        let orion_type: $name = serde_json::from_value(serialized_from_bytes).unwrap();
+        assert_eq!(orion_type, bytes);
+    }
+));
+
 /// Macro that implements the `Drop` trait on a object called `$name` which has
 /// a field `value`. This `Drop` will zero out the field `value` when the
 /// objects destructor is called.
@@ -127,6 +180,18 @@ macro_rules! impl_from_trait (($name:ident, $size:expr) => (
     }
 ));
 
+/// Macro that implements `TryFrom<&[u8]>` on an object called `$name` that
+/// implements the method `from_slice`.
+macro_rules! impl_try_from_trait (($name:ident) => (
+    /// Delegates to `from_slice` implementation
+    impl core::convert::TryFrom<&[u8]> for $name {
+        type Error = UnknownCryptoError;
+        fn try_from(slice: &[u8]) -> Result<Self, Self::Error> {
+            Self::from_slice(slice)
+        }
+    }
+));
+
 ///
 /// Function implementation macros
 
@@ -564,8 +629,12 @@ macro_rules! construct_public {
 
         impl_ct_partialeq_trait!($name, as_ref);
         impl_normal_debug_trait!($name);
+        impl_try_from_trait!($name);
         impl_asref_trait!($name);
 
+        #[cfg(feature = "serde")]
+        impl_serde_traits!($name, as_ref);
+
         impl $name {
             func_from_slice!($name, $lower_bound, $upper_bound);
             func_len!();
@@ -582,6 +651,9 @@ macro_rules! construct_public {
             test_as_bytes_and_get_length!($name, $lower_bound, $upper_bound, as_ref);
             test_partial_eq!($name, $upper_bound);
 
+            #[cfg(feature = "serde")]
+            test_serde_impls!($name, $upper_bound);
+
             #[cfg(test)]
             #[cfg(feature = "safe_api")]
             mod tests_with_std {
@@ -672,6 +744,10 @@ macro_rules! construct_tag {
 
         impl_omitted_debug_trait!($name);
         impl_ct_partialeq_trait!($name, unprotected_as_bytes);
+        impl_try_from_trait!($name);
+
+        #[cfg(feature = "serde")]
+        impl_serde_traits!($name, unprotected_as_bytes);
 
         impl $name {
             func_from_slice!($name, $lower_bound, $upper_bound);
@@ -690,6 +766,9 @@ macro_rules! construct_tag {
             test_as_bytes_and_get_length!($name, $lower_bound, $upper_bound, unprotected_as_bytes);
             test_partial_eq!($name, $upper_bound);
 
+            #[cfg(feature = "serde")]
+            test_serde_impls!($name, $upper_bound);
+
             #[cfg(test)]
             #[cfg(feature = "safe_api")]
             mod tests_with_std {
@@ -870,6 +949,10 @@ macro_rules! construct_salt_variable_size {
         impl_default_trait!($name, $default_size);
         impl_ct_partialeq_trait!($name, as_ref);
         impl_asref_trait!($name);
+        impl_try_from_trait!($name);
+
+        #[cfg(feature = "serde")]
+        impl_serde_traits!($name, as_ref);
 
         impl $name {
             func_from_slice_variable_size!($name);
@@ -887,6 +970,9 @@ macro_rules! construct_salt_variable_size {
             test_generate_variable!($name);
             test_partial_eq!($name, $default_size);
             test_normal_debug!($name, $default_size);
+
+            #[cfg(feature = "serde")]
+            test_serde_impls!($name, $default_size);
         }
     );
 }