[refactor] hyperledger-iroha#3422: Clean up the API & resurrect ursa …

…tests

Clean up the API a bit:
- hide the implementation details of signatures (they are only used through the PublicKey, PrivateKey and Signature types)
- remove even more unused API functions
- add missing documentation items

Signed-off-by: Nikita Strygin <[email protected]>

crypto/Cargo.toml

@@ -26,7 +26,6 @@ std = [
     "dep:x25519-dalek",
     "dep:rand",
     "dep:rand_chacha",
-    "dep:secp256k1",
     "dep:zeroize",
     "dep:arrayref",
     "dep:aead",
@@ -72,7 +71,6 @@ x25519-dalek = { version = "2.0.0", optional = true, features = ["static_secrets
 rand = { workspace = true, optional = true }
 rand_chacha = { version = "0.3.1", optional = true }
 
-secp256k1 = { version = "0.28.0", features = ["rand", "serde"], optional = true }
 
 zeroize = { version = "1.6.0", optional = true }
 arrayref = { version = "0.3.7", optional = true }

crypto/src/encryption/chacha20poly1305.rs

@@ -7,9 +7,9 @@ use aead::{
 };
 use chacha20poly1305::ChaCha20Poly1305 as SysChaCha20Poly1305;
 
-// use zeroize::Zeroize;
 use super::Encryptor;
 
+/// ChaCha20Poly1305 is a symmetric encryption algorithm that uses the ChaCha20 stream cipher
 #[derive(Debug, Copy, Clone, Eq, PartialEq)]
 pub struct ChaCha20Poly1305 {
     key: GenericArray<u8, U32>,
@@ -35,6 +35,8 @@ impl AeadCore for ChaCha20Poly1305 {
     type CiphertextOverhead = U0;
 }
 
+// false positives: eliding lifetimes here requires an unstable feature `anonymous_lifetime_in_impl_trait`
+#[allow(single_use_lifetimes)]
 impl Aead for ChaCha20Poly1305 {
     fn encrypt<'msg, 'aad>(
         &self,
@@ -57,9 +59,6 @@ impl Aead for ChaCha20Poly1305 {
     }
 }
 
-// default_impl!(ChaCha20Poly1305);
-// drop_impl!(ChaCha20Poly1305);
-
 #[cfg(test)]
 mod tests {
     use std::io::Cursor;
@@ -138,6 +137,7 @@ mod tests {
         assert_eq!(dummytext.to_vec(), plaintext);
     }
 
+    // TODO: this should be tested for, but only after we integrate with secrecy/zeroize
     // #[test]
     // fn zeroed_on_drop() {
     //     let mut aes = ChaCha20Poly1305::new(&ChaCha20Poly1305::key_gen().unwrap());

crypto/src/encryption/mod.rs

@@ -1,5 +1,19 @@
-// TODO: clean up & remove
-#![allow(unused, missing_docs)]
+//! A suite of Authenticated Encryption with Associated Data (AEAD) cryptographic ciphers.
+//!
+//! Each AEAD algorithm provides [`SymmetricEncryptor::encrypt_easy`] and [`SymmetricEncryptor::decrypt_easy`] methods which hides the complexity
+//! of generating a secure nonce of appropriate size with the ciphertext.
+//! The [`SymmetricEncryptor::encrypt_easy`] prepends the nonce to the front of the ciphertext and [`SymmetricEncryptor::decrypt_easy`] expects
+//! the nonce to be prepended to the front of the ciphertext.
+//!
+//! More advanced users may use [`SymmetricEncryptor::encrypt`] and [`SymmetricEncryptor::decrypt`] directly. These two methods require the
+//! caller to supply a nonce with sufficient entropy and should never be reused when encrypting
+//! with the same `key`.
+//!
+//! The convenience struct [`SymmetricEncryptor`] exists to allow users to easily switch between
+//! algorithms by using any algorithm that implements the [`Encryptor`] trait.
+//!
+//! [`ChaCha20Poly1305`] is the only algorithm currently supported,
+//! as it is the only one used by the iroha p2p transport protocol.
 
 mod chacha20poly1305;
 
@@ -15,13 +29,13 @@ pub use self::chacha20poly1305::ChaCha20Poly1305;
 use crate::SessionKey;
 
 // Helpful for generating bytes using the operating system random number generator
-pub fn random_vec(bytes: usize) -> Result<Vec<u8>, Error> {
+fn random_vec(bytes: usize) -> Result<Vec<u8>, Error> {
     let mut value = vec![0u8; bytes];
     OsRng.fill_bytes(value.as_mut_slice());
     Ok(value)
 }
 
-pub fn random_bytes<T: ArrayLength<u8>>() -> Result<GenericArray<u8, T>, Error> {
+fn random_bytes<T: ArrayLength<u8>>() -> Result<GenericArray<u8, T>, Error> {
     Ok(GenericArray::clone_from_slice(
         random_vec(T::to_usize())?.as_slice(),
     ))
@@ -59,27 +73,29 @@ pub struct SymmetricEncryptor<E: Encryptor> {
 }
 
 impl<E: Encryptor> SymmetricEncryptor<E> {
+    /// Create a new [`SymmetricEncryptor`] using the provided `encryptor`
     pub fn new(encryptor: E) -> Self {
         Self { encryptor }
     }
 
+    /// Create a new [`SymmetricEncryptor`] from a [`SessionKey`]
     pub fn new_from_session_key(key: SessionKey) -> Self {
         Self::new(<E as KeyInit>::new(GenericArray::from_slice(&key.0)))
     }
-
+    /// Create a new [`SymmetricEncryptor`] from key bytes
     pub fn new_with_key<A: AsRef<[u8]>>(key: A) -> Result<Self, Error> {
         Ok(Self {
             encryptor: <E as KeyInit>::new(GenericArray::from_slice(key.as_ref())),
         })
     }
 
-    // Encrypt `plaintext` and integrity protect `aad`. The result is the ciphertext.
-    // This method handles safely generating a `nonce` and prepends it to the ciphertext
+    /// Encrypt `plaintext` and integrity protect `aad`. The result is the ciphertext.
+    /// This method handles safely generating a `nonce` and prepends it to the ciphertext
     pub fn encrypt_easy<A: AsRef<[u8]>>(&self, aad: A, plaintext: A) -> Result<Vec<u8>, Error> {
         self.encryptor.encrypt_easy(aad, plaintext)
     }
 
-    // Encrypt `plaintext` and integrity protect `aad`. The result is the ciphertext.
+    /// Encrypt `plaintext` and integrity protect `aad`. The result is the ciphertext.
     pub fn encrypt<A: AsRef<[u8]>>(
         &self,
         nonce: A,
@@ -94,19 +110,19 @@ impl<E: Encryptor> SymmetricEncryptor<E> {
         self.encryptor.encrypt(nonce, payload)
     }
 
-    // Decrypt `ciphertext` using integrity protected `aad`. The result is the plaintext if successful
-    // or an error if the `ciphetext` cannot be decrypted due to tampering, an incorrect `aad` value,
-    // or incorrect key.
-    // `aad` must be the same value used in `encrypt_easy`. Expects the nonce to be prepended to
-    // the `ciphertext`
+    /// Decrypt `ciphertext` using integrity protected `aad`. The result is the plaintext if successful
+    /// or an error if the `ciphetext` cannot be decrypted due to tampering, an incorrect `aad` value,
+    /// or incorrect key.
+    /// `aad` must be the same value used in `encrypt_easy`. Expects the nonce to be prepended to
+    /// the `ciphertext`
     pub fn decrypt_easy<A: AsRef<[u8]>>(&self, aad: A, ciphertext: A) -> Result<Vec<u8>, Error> {
         self.encryptor.decrypt_easy(aad, ciphertext)
     }
 
-    // Decrypt `ciphertext` using integrity protected `aad`. The result is the plaintext if successful
-    // or an error if the `ciphetext` cannot be decrypted due to tampering, an incorrect `aad` value,
-    // or incorrect key.
-    // `aad` must be the same value used in `encrypt_easy`.
+    /// Decrypt `ciphertext` using integrity protected `aad`. The result is the plaintext if successful
+    /// or an error if the `ciphetext` cannot be decrypted due to tampering, an incorrect `aad` value,
+    /// or incorrect key.
+    /// `aad` must be the same value used in `encrypt_easy`.
     pub fn decrypt<A: AsRef<[u8]>>(
         &self,
         nonce: A,
@@ -121,7 +137,7 @@ impl<E: Encryptor> SymmetricEncryptor<E> {
         self.encryptor.decrypt(nonce, payload)
     }
 
-    // Similar to `encrypt_easy` but reads from a stream instead of a slice
+    /// Similar to `encrypt_easy` but reads from a stream instead of a slice
     pub fn encrypt_buffer<A: AsRef<[u8]>, I: Read, O: Write>(
         &self,
         aad: A,
@@ -131,7 +147,7 @@ impl<E: Encryptor> SymmetricEncryptor<E> {
         self.encryptor.encrypt_buffer(aad, plaintext, ciphertext)
     }
 
-    // Similar to `decrypt_easy` but reads from a stream instead of a slice
+    /// Similar to `decrypt_easy` but reads from a stream instead of a slice
     pub fn decrypt_buffer<A: AsRef<[u8]>, I: Read, O: Write>(
         &self,
         aad: A,
@@ -155,6 +171,9 @@ pub trait Encryptor: Aead + KeyInit {
     /// The minimum size that the ciphertext will yield from plaintext
     type MinSize: ArrayLength<u8>;
 
+    /// A simple API to encrypt a message with authenticated associated data.
+    ///
+    /// This API handles nonce generation for you and prepends it in front of the ciphertext. Use [`Encryptor::decrypt_easy`] to decrypt the message encrypted this way.
     fn encrypt_easy<M: AsRef<[u8]>>(&self, aad: M, plaintext: M) -> Result<Vec<u8>, Error> {
         let nonce = Self::nonce_gen()?;
         let payload = Payload {
@@ -167,6 +186,9 @@ pub trait Encryptor: Aead + KeyInit {
         Ok(result)
     }
 
+    /// A simple API to decrypt a message with authenticated associated data.
+    ///
+    /// This API expects the nonce to be prepended to the ciphertext. Use [`Encryptor::encrypt_easy`] to encrypt the message this way.
     fn decrypt_easy<M: AsRef<[u8]>>(&self, aad: M, ciphertext: M) -> Result<Vec<u8>, Error> {
         let ciphertext = ciphertext.as_ref();
         if ciphertext.len() < Self::MinSize::to_usize() {
@@ -182,6 +204,7 @@ pub trait Encryptor: Aead + KeyInit {
         Ok(plaintext)
     }
 
+    /// Same as [`Encryptor::encrypt_easy`] but works with [`std::io`] streams instead of slices
     fn encrypt_buffer<M: AsRef<[u8]>, I: Read, O: Write>(
         &self,
         aad: M,
@@ -194,6 +217,7 @@ pub trait Encryptor: Aead + KeyInit {
         Ok(())
     }
 
+    /// Same as [`Encryptor::decrypt_easy`] but works with [`std::io`] streams instead of slices
     fn decrypt_buffer<M: AsRef<[u8]>, I: Read, O: Write>(
         &self,
         aad: M,
@@ -206,10 +230,12 @@ pub trait Encryptor: Aead + KeyInit {
         Ok(())
     }
 
+    /// Generate a new key for this encryptor
     fn key_gen() -> Result<GenericArray<u8, Self::KeySize>, Error> {
         random_bytes()
     }
 
+    /// Generate a new nonce for this encryptor
     fn nonce_gen() -> Result<GenericArray<u8, Self::NonceSize>, Error> {
         random_bytes()
     }

crypto/src/kex/mod.rs

@@ -1,5 +1,7 @@
-// TODO: clean up & remove
-#![allow(unused, missing_docs)]
+//! A suite of Diffie-Hellman key exchange methods.
+//!
+//! [`X25519Sha256`] is the only key exchange scheme currently supported,
+//! as it is the only one used by the iroha p2p transport protocol.
 
 mod x25519;
 
@@ -28,7 +30,10 @@ pub trait KeyExchangeScheme {
         remote_public_key: &PublicKey,
     ) -> Result<SessionKey, Error>;
 
-    fn shared_secret_size() -> usize;
-    fn public_key_size() -> usize;
-    fn private_key_size() -> usize;
+    /// Size of the shared secret in bytes.
+    const SHARED_SECRET_SIZE: usize;
+    /// Size of the public key in bytes.
+    const PUBLIC_KEY_SIZE: usize;
+    /// Size of the private key in bytes.
+    const PRIVATE_KEY_SIZE: usize;
 }

crypto/src/kex/x25519.rs

@@ -11,6 +11,7 @@ const ALGORITHM: Algorithm = Algorithm::Ed25519;
 use super::KeyExchangeScheme;
 use crate::{Algorithm, Error, KeyGenOption, PrivateKey, PublicKey, SessionKey};
 
+/// Implements the [`KeyExchangeScheme`] using X25519 key exchange and SHA256 hash function.
 #[derive(Copy, Clone)]
 pub struct X25519Sha256;
 
@@ -70,34 +71,15 @@ impl KeyExchangeScheme for X25519Sha256 {
         Ok(SessionKey(ConstVec::new(hash.as_slice().to_vec())))
     }
 
-    fn public_key_size() -> usize {
-        32
-    }
-    fn private_key_size() -> usize {
-        32
-    }
-    fn shared_secret_size() -> usize {
-        32
-    }
+    const SHARED_SECRET_SIZE: usize = 32;
+    const PUBLIC_KEY_SIZE: usize = 32;
+    const PRIVATE_KEY_SIZE: usize = 32;
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
 
-    // #[test]
-    // fn convert_from_sig_keys() {
-    //     use crate::{Ed25519Sha512, SignatureScheme};
-    //     let sig_scheme = Ed25519Sha512::new();
-    //     let (pk, sk) = sig_scheme.keypair(None).unwrap();
-    //     let res = Ed25519Sha512::ver_key_to_key_exchange(&pk);
-    //     assert!(res.is_ok());
-    //     let pk1 = res.unwrap();
-    //     let kex_scheme = X25519Sha256::new();
-    //     let res = kex_scheme.compute_shared_secret(&sk, &pk1);
-    //     assert!(res.is_ok());
-    // }
-
     #[test]
     fn key_exchange() {
         let scheme = X25519Sha256::new();

crypto/src/lib.rs

@@ -233,10 +233,12 @@ impl KeyPair {
         };
 
         let (public_key, private_key) = match configuration.algorithm {
-            Algorithm::Ed25519 => Ed25519Sha512.keypair(key_gen_option),
-            Algorithm::Secp256k1 => EcdsaSecp256k1Sha256::new().keypair(key_gen_option),
-            Algorithm::BlsNormal => BlsNormal::new().keypair(key_gen_option),
-            Algorithm::BlsSmall => BlsSmall::new().keypair(key_gen_option),
+            Algorithm::Ed25519 => signature::ed25519::Ed25519Sha512.keypair(key_gen_option),
+            Algorithm::Secp256k1 => {
+                signature::secp256k1::EcdsaSecp256k1Sha256::new().keypair(key_gen_option)
+            }
+            Algorithm::BlsNormal => signature::bls::BlsNormal::new().keypair(key_gen_option),
+            Algorithm::BlsSmall => signature::bls::BlsSmall::new().keypair(key_gen_option),
         }?;
 
         Ok(Self {
@@ -317,14 +319,37 @@ impl PublicKey {
         let key_gen_option = Some(KeyGenOption::FromPrivateKey(private_key));
 
         let (public_key, _) = match digest_function {
-            Algorithm::Ed25519 => Ed25519Sha512.keypair(key_gen_option),
-            Algorithm::Secp256k1 => EcdsaSecp256k1Sha256::new().keypair(key_gen_option),
-            Algorithm::BlsNormal => BlsNormal::new().keypair(key_gen_option),
-            Algorithm::BlsSmall => BlsSmall::new().keypair(key_gen_option),
+            Algorithm::Ed25519 => signature::ed25519::Ed25519Sha512.keypair(key_gen_option),
+            Algorithm::Secp256k1 => {
+                signature::secp256k1::EcdsaSecp256k1Sha256::new().keypair(key_gen_option)
+            }
+            Algorithm::BlsNormal => signature::bls::BlsNormal::new().keypair(key_gen_option),
+            Algorithm::BlsSmall => signature::bls::BlsSmall::new().keypair(key_gen_option),
         }?;
 
         Ok(public_key)
     }
+
+    /// Construct `PrivateKey` from hex encoded string
+    ///
+    /// # Errors
+    ///
+    /// - If the given payload is not hex encoded
+    /// - If the given payload is not a valid private key
+    #[cfg(feature = "std")]
+    pub fn from_hex(digest_function: Algorithm, payload: &str) -> Result<Self, Error> {
+        let payload = hex_decode(payload)?;
+        let payload = ConstVec::new(payload);
+
+        // NOTE: PrivateKey does some validation by generating a public key from the provided bytes
+        // we can't really do this for PublicKey
+        // this can be solved if the keys used here would be actually aware of the underlying crypto primitive types
+        // instead of just being raw bytes
+        Ok(Self {
+            digest_function,
+            payload,
+        })
+    }
 }
 
 impl FromStr for PublicKey {
@@ -415,7 +440,7 @@ impl PrivateKey {
     /// - If the given payload is not hex encoded
     /// - If the given payload is not a valid private key
     #[cfg(feature = "std")]
-    pub fn from_hex(digest_function: Algorithm, payload: &[u8]) -> Result<Self, Error> {
+    pub fn from_hex(digest_function: Algorithm, payload: &str) -> Result<Self, Error> {
         let payload = hex_decode(payload)?;
         let payload = ConstVec::new(payload);