Merge branch 'mathias-CRP-1934-remove-string-conversions-for-csp-types' into 'master'

cleanup(crypto): CRP-1934: Remove string conversions for SecretKeyBytes

Remove conversions to and from base64-encoded strings for `ic_crypto_internal_multi_sig_bls12381::types::SecretKeyBytes`, `ic_crypto_internal_threshold_sig_bls12381::types::SecretKeyBytes`, and `ic_crypto_internal_basic_sig_ed25519::types::SecretKeyBytes`, based on [the discussion in MR 10314](https://gitlab.com/dfinity-lab/public/ic/-/merge_requests/10314#note_1262953398). 

See merge request dfinity-lab/public/ic!12370

Author: mbjorkqvist

rs/crypto/internal/crypto_lib/basic_sig/ed25519/src/types/conversions.rs

@@ -1,5 +1,4 @@
 use super::*;
-use ic_crypto_secrets_containers::SecretArray;
 use ic_types::crypto::{AlgorithmId, CryptoError};
 use std::convert::TryFrom;
 
@@ -8,39 +7,6 @@ pub mod protobuf;
 #[cfg(test)]
 mod tests;
 
-impl From<SecretKeyBytes> for String {
-    fn from(val: SecretKeyBytes) -> Self {
-        base64::encode(val.0.expose_secret())
-    }
-}
-impl TryFrom<&str> for SecretKeyBytes {
-    type Error = CryptoError;
-
-    fn try_from(key: &str) -> Result<Self, CryptoError> {
-        let mut key = base64::decode(key).map_err(|e| CryptoError::MalformedSecretKey {
-            algorithm: AlgorithmId::Ed25519,
-            internal_error: format!("Key is not a valid base64 encoded string: {}", e),
-        })?;
-        if key.len() != SecretKeyBytes::SIZE {
-            return Err(CryptoError::MalformedSecretKey {
-                algorithm: AlgorithmId::Ed25519,
-                internal_error: "Key length is incorrect".to_string(),
-            });
-        }
-        let mut buffer = [0u8; SecretKeyBytes::SIZE];
-        buffer.copy_from_slice(&key);
-        key.zeroize();
-        let ret = SecretKeyBytes(SecretArray::new_and_zeroize_argument(&mut buffer));
-        Ok(ret)
-    }
-}
-impl TryFrom<&String> for SecretKeyBytes {
-    type Error = CryptoError;
-    fn try_from(signature: &String) -> Result<Self, CryptoError> {
-        Self::try_from(signature as &str)
-    }
-}
-
 impl From<PublicKeyBytes> for String {
     fn from(val: PublicKeyBytes) -> Self {
         base64::encode(&val.0[..])

rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/types/conversions.rs

@@ -90,35 +90,3 @@ impl From<&CombinedSignature> for CombinedSignatureBytes {
         CombinedSignatureBytes(signature.serialize())
     }
 }
-impl From<SecretKeyBytes> for String {
-    fn from(val: SecretKeyBytes) -> Self {
-        base64::encode(&val.0.expose_secret())
-    }
-}
-impl TryFrom<&str> for SecretKeyBytes {
-    type Error = CryptoError;
-
-    fn try_from(key: &str) -> Result<Self, CryptoError> {
-        let key = base64::decode(key).map_err(|e| CryptoError::MalformedSecretKey {
-            algorithm: AlgorithmId::MultiBls12_381,
-            internal_error: format!("Key is not a valid base64 encoded string: {}", e),
-        })?;
-        if key.len() != SecretKeyBytes::SIZE {
-            return Err(CryptoError::MalformedSecretKey {
-                algorithm: AlgorithmId::MultiBls12_381,
-                internal_error: "Key length is incorrect".to_string(),
-            });
-        }
-        let mut buffer = [0u8; SecretKeyBytes::SIZE];
-        buffer.copy_from_slice(&key);
-        Ok(SecretKeyBytes(
-            ic_crypto_secrets_containers::SecretArray::new_and_zeroize_argument(&mut buffer),
-        ))
-    }
-}
-impl TryFrom<&String> for SecretKeyBytes {
-    type Error = CryptoError;
-    fn try_from(signature: &String) -> Result<Self, CryptoError> {
-        Self::try_from(signature as &str)
-    }
-}

rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/types/conversions/tests.rs

@@ -53,12 +53,6 @@ proptest! {
         let reconstructed_pops: Result<Vec<Pop>, CryptoError> = bytes.map(|bytes| Pop::try_from(&bytes)).collect();
         assert_eq!(Ok(pops), reconstructed_pops);
     }
-    #[test]
-    fn secret_key_base64_serde(key in arbitrary::secret_key()) {
-        let bytes = SecretKeyBytes::from(&key);
-        let base64: String = bytes.clone().into();
-        assert_eq!(Ok(bytes), SecretKeyBytes::try_from(&base64));
-    }
 }
 
 #[test]
@@ -85,15 +79,3 @@ fn conversion_to_combined_signature_fails_gracefully() {
     let converted: Result<CombinedSignature, _> = CombinedSignature::try_from(&bad_point);
     assert!(converted.is_err());
 }
-#[test]
-fn base64_decoding_secret_key_fails_gracefully() {
-    let bad_base64 = "leNiiKeMW5l8UQUf=SrVRevJbAFoQafWg4X09pBCcI2cP5kKTmW/j97hGyCX5PtXrB+0HKj/rmJ8r4uiapMNZt3ecy4aQTELqJl/2f2UKV+uyFNrjt4WuP7NOlt89SXua".to_string();
-    let converted = SecretKeyBytes::try_from(&bad_base64);
-    assert!(converted.is_err());
-}
-#[test]
-fn decoding_wrong_length_secret_key_fails_gracefully() {
-    let bad_value = "lUn1Nh1inf3zBhf3okqo/q7vyNhKcEnHwJ0dLqMatUk1poY5rf80rBJOunirUU3zAJF96i/cLCzoD3wEvGusbr2OqO+zFI143BCwCo1OGhgEULSqvRm6kxGigqPhp+Ma".to_string();
-    let converted = SecretKeyBytes::try_from(&bad_value);
-    assert!(converted.is_err());
-}

rs/crypto/internal/crypto_lib/multi_sig/bls12_381/src/types/generic_traits/tests.rs

@@ -26,7 +26,7 @@ proptest! {
     #[test]
     fn debug_should_redact_secretkey_bytes(secret_key_bytes: SecretKeyBytes) {
         let debug_str = format!("{:?}", secret_key_bytes);
-        let raw_str: String = secret_key_bytes.into();
+        let raw_str = base64::encode(&secret_key_bytes.0.expose_secret());
         assert!(!debug_str.contains(&raw_str));
         assert_eq!(debug_str, "REDACTED");
     }

rs/crypto/internal/crypto_lib/threshold_sig/bls12_381/src/types/conversions.rs

@@ -123,39 +123,6 @@ impl TryFrom<&CombinedSignatureBytes> for CombinedSignature {
     }
 }
 
-impl From<SecretKeyBytes> for String {
-    fn from(bytes: SecretKeyBytes) -> String {
-        base64::encode(&bytes.0.expose_secret())
-    }
-}
-impl TryFrom<&str> for SecretKeyBytes {
-    type Error = CryptoError;
-
-    fn try_from(string: &str) -> Result<Self, CryptoError> {
-        let bytes = base64::decode(string).map_err(|e| CryptoError::MalformedSecretKey {
-            algorithm: AlgorithmId::ThresBls12_381,
-            internal_error: format!("Secret key is not a valid base64 encoded string: {}", e),
-        })?;
-        if bytes.len() != SecretKeyBytes::SIZE {
-            return Err(CryptoError::MalformedSecretKey {
-                algorithm: AlgorithmId::ThresBls12_381,
-                internal_error: "Secret key length is incorrect".to_string(),
-            });
-        }
-        let mut buffer = [0u8; SecretKeyBytes::SIZE];
-        buffer.copy_from_slice(&bytes);
-        Ok(SecretKeyBytes(
-            ic_crypto_secrets_containers::SecretArray::new_and_zeroize_argument(&mut buffer),
-        ))
-    }
-}
-impl TryFrom<&String> for SecretKeyBytes {
-    type Error = CryptoError;
-    fn try_from(string: &String) -> Result<Self, CryptoError> {
-        Self::try_from(string as &str)
-    }
-}
-
 impl From<IndividualSignatureBytes> for String {
     fn from(bytes: IndividualSignatureBytes) -> String {
         base64::encode(&bytes.0[..])

rs/crypto/internal/crypto_lib/threshold_sig/bls12_381/src/types/conversions/tests.rs

@@ -53,15 +53,6 @@ proptest! {
         assert_eq!(signature_bytes, serialised, "Parsing followed by serailizing produced a value different from the starting value.");
     }
 
-    /// Verifies that stringifying SecretKeyBytes and paring them again yields the original.
-    #[test]
-    #[allow(clippy::unnecessary_operation)] // Clippy believes that these tests are unnecessary.
-    fn proptest_secret_key_stringifying_and_parsing_should_be_inverse(secret_key: SecretKeyBytes) {
-        let string = String::from(secret_key.clone());
-        let parsed = SecretKeyBytes::try_from(&string).expect("Failed to parse stringified secret key");
-        assert_eq!(secret_key, parsed, "Stringifying followed by parsing produced a value different from the starting value.");
-    }
-
     /// Verifies that stringifying PublicKeyBytes and paring them again yields the original.
     #[test]
     #[allow(clippy::unnecessary_operation)] // Clippy believes that these tests are unnecessary.
@@ -158,24 +149,6 @@ fn test_invalid_combined_signature_fails_to_parse() {
     }
 }
 
-/// Verifies that parsing invalid base64 SecretKeyBytes fails
-#[test]
-fn test_snowman_is_not_valid_secret_key() {
-    match SecretKeyBytes::try_from(SNOWMAN) {
-        Err(CryptoError::MalformedSecretKey { .. }) => (),
-        other => panic!("Expected a MalformedSecretKey error.  Got: {:?}", other),
-    }
-}
-
-/// Verifies that parsing invalid base64 SecretKeyBytes fails
-#[test]
-fn test_base64_snowman_is_not_valid_secret_key() {
-    match SecretKeyBytes::try_from(SNOWCODE) {
-        Err(CryptoError::MalformedSecretKey { .. }) => (),
-        other => panic!("Expected a MalformedSecretKey error.  Got: {:?}", other),
-    }
-}
-
 /// Verifies that parsing invalid base64 PublicKeyBytes fails
 #[test]
 fn test_snowman_is_not_valid_public_key() {

rs/crypto/internal/crypto_lib/threshold_sig/bls12_381/src/types/generic_traits/tests.rs

@@ -24,7 +24,7 @@ proptest! {
     #[test]
     fn debug_should_redact_secretkey_bytes(secret_key_bytes: SecretKeyBytes) {
         let debug_str = format!("{:?}", secret_key_bytes);
-        let raw_str = String::from(secret_key_bytes);
+        let raw_str = base64::encode(&secret_key_bytes.0.expose_secret());
         assert!(!debug_str.contains(&raw_str));
         assert_eq!(debug_str, "REDACTED");
     }