[refactor] hyperledger-iroha#3237: Fix clippy lints

- make the signature implementations not use the &self, they are all stateless anyway
- remove redundant Result returns
- add docs on errors

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

crypto/Cargo.toml

@@ -21,6 +21,7 @@ std = [
     "dep:hkdf",
     "dep:amcl",
     "dep:amcl_wrapper",
+    "dep:signature",
     "dep:ed25519-dalek",
     "dep:curve25519-dalek",
     "dep:x25519-dalek",
@@ -64,6 +65,7 @@ hkdf = { version = "0.12.3", optional = true }
 amcl = { version = "0.2.0", optional = true, default-features = false, features = ["secp256k1"] }
 amcl_wrapper = { version = "0.4.0", optional = true }
 
+signature = { version = "2.1.0", optional = true }
 ed25519-dalek = { version = "2.0.0", optional = true, features = ["rand_core"] }
 curve25519-dalek = { version = "4.1.1", optional = true }
 x25519-dalek = { version = "2.0.0", optional = true, features = ["static_secrets"] }

crypto/src/encryption/chacha20poly1305.rs

@@ -9,7 +9,7 @@ use chacha20poly1305::ChaCha20Poly1305 as SysChaCha20Poly1305;
 
 use super::Encryptor;
 
-/// ChaCha20Poly1305 is a symmetric encryption algorithm that uses the ChaCha20 stream cipher
+/// `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>,

crypto/src/encryption/mod.rs

@@ -31,7 +31,10 @@ use crate::SessionKey;
 // Helpful for generating bytes using the operating system random number generator
 fn random_vec(bytes: usize) -> Result<Vec<u8>, Error> {
     let mut value = vec![0u8; bytes];
-    OsRng.fill_bytes(value.as_mut_slice());
+    OsRng
+        .try_fill_bytes(value.as_mut_slice())
+        // RustCrypto errors don't have any details, can't propagate the error
+        .map_err(|_| Error)?;
     Ok(value)
 }
 
@@ -79,23 +82,31 @@ impl<E: Encryptor> SymmetricEncryptor<E> {
     }
 
     /// Create a new [`SymmetricEncryptor`] from a [`SessionKey`]
-    pub fn new_from_session_key(key: SessionKey) -> Self {
+    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 {
+    pub fn new_with_key<A: AsRef<[u8]>>(key: A) -> Self {
+        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
+    ///
+    /// # Errors
+    ///
+    /// This function will return an error if nonce generation or encryption fails
     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.
+    ///
+    /// # Errors
+    ///
+    /// This function will return an error if encryption fails
     pub fn encrypt<A: AsRef<[u8]>>(
         &self,
         nonce: A,
@@ -115,6 +126,10 @@ impl<E: Encryptor> SymmetricEncryptor<E> {
     /// or incorrect key.
     /// `aad` must be the same value used in `encrypt_easy`. Expects the nonce to be prepended to
     /// the `ciphertext`
+    ///
+    /// # Errors
+    ///
+    /// This function will return an error if decryption fails
     pub fn decrypt_easy<A: AsRef<[u8]>>(&self, aad: A, ciphertext: A) -> Result<Vec<u8>, Error> {
         self.encryptor.decrypt_easy(aad, ciphertext)
     }
@@ -123,6 +138,10 @@ impl<E: Encryptor> SymmetricEncryptor<E> {
     /// 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`.
+    ///
+    /// # Errors
+    ///
+    /// This function will return an error if decryption fails
     pub fn decrypt<A: AsRef<[u8]>>(
         &self,
         nonce: A,
@@ -138,6 +157,10 @@ impl<E: Encryptor> SymmetricEncryptor<E> {
     }
 
     /// Similar to `encrypt_easy` but reads from a stream instead of a slice
+    ///
+    /// # Errors
+    ///
+    /// This function will return an error if reading the buffer, nonce generation, encryption or writing the buffer fails
     pub fn encrypt_buffer<A: AsRef<[u8]>, I: Read, O: Write>(
         &self,
         aad: A,
@@ -148,6 +171,10 @@ impl<E: Encryptor> SymmetricEncryptor<E> {
     }
 
     /// Similar to `decrypt_easy` but reads from a stream instead of a slice
+    ///
+    /// # Errors
+    ///
+    /// This function will return an error if reading the buffer, decryption or writing the buffer fails
     pub fn decrypt_buffer<A: AsRef<[u8]>, I: Read, O: Write>(
         &self,
         aad: A,
@@ -174,6 +201,10 @@ pub trait Encryptor: Aead + KeyInit {
     /// 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.
+    ///
+    /// # Errors
+    ///
+    /// This function will return an error if nonce generation or encryption fails
     fn encrypt_easy<M: AsRef<[u8]>>(&self, aad: M, plaintext: M) -> Result<Vec<u8>, Error> {
         let nonce = Self::nonce_gen()?;
         let payload = Payload {
@@ -189,6 +220,10 @@ pub trait Encryptor: Aead + KeyInit {
     /// 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.
+    ///
+    /// # Errors
+    ///
+    /// This function will return an error if decryption fails
     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() {
@@ -200,11 +235,15 @@ pub trait Encryptor: Aead + KeyInit {
             msg: &ciphertext[Self::NonceSize::to_usize()..],
             aad: aad.as_ref(),
         };
-        let plaintext = self.decrypt(&nonce, payload)?;
+        let plaintext = self.decrypt(nonce, payload)?;
         Ok(plaintext)
     }
 
     /// Same as [`Encryptor::encrypt_easy`] but works with [`std::io`] streams instead of slices
+    ///
+    /// # Errors
+    ///
+    /// This function will return an error if reading the buffer, nonce generation, encryption or writing the buffer fails
     fn encrypt_buffer<M: AsRef<[u8]>, I: Read, O: Write>(
         &self,
         aad: M,
@@ -218,6 +257,10 @@ pub trait Encryptor: Aead + KeyInit {
     }
 
     /// Same as [`Encryptor::decrypt_easy`] but works with [`std::io`] streams instead of slices
+    ///
+    /// # Errors
+    ///
+    /// This function will return an error if reading the buffer, decryption or writing the buffer fails
     fn decrypt_buffer<M: AsRef<[u8]>, I: Read, O: Write>(
         &self,
         aad: M,
@@ -231,11 +274,19 @@ pub trait Encryptor: Aead + KeyInit {
     }
 
     /// Generate a new key for this encryptor
+    ///
+    /// # Errors
+    ///
+    /// This function will return an error if the operating system random number generator fails
     fn key_gen() -> Result<GenericArray<u8, Self::KeySize>, Error> {
         random_bytes()
     }
 
     /// Generate a new nonce for this encryptor
+    ///
+    /// # Errors
+    ///
+    /// This function will return an error if the operating system random number generator fails
     fn nonce_gen() -> Result<GenericArray<u8, Self::NonceSize>, Error> {
         random_bytes()
     }

crypto/src/kex/mod.rs

@@ -16,10 +16,14 @@ pub trait KeyExchangeScheme {
     fn new() -> Self;
     /// Create new keypairs. If
     /// `options` is None, the keys are generated ephemerally from the `OsRng`
-    /// `options` is UseSeed, the keys are generated ephemerally from the sha256 hash of the seed which is
-    ///     then used to seed the ChaChaRng
-    /// `options` is FromPrivateKey, the corresponding public key is returned. This should be used for
+    /// `options` is `UseSeed`, the keys are generated ephemerally from the sha256 hash of the seed which is
+    ///     then used to seed the `ChaChaRng`
+    /// `options` is `FromPrivateKey`, the corresponding public key is returned. This should be used for
     ///     static Diffie-Hellman and loading a long-term key.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if the key generation fails.
     fn keypair(&self, options: Option<KeyGenOption>) -> Result<(PublicKey, PrivateKey), Error>;
     /// Compute the diffie-hellman shared secret.
     /// `local_private_key` is the key generated from calling `keypair` while
@@ -28,7 +32,7 @@ pub trait KeyExchangeScheme {
         &self,
         local_private_key: &PrivateKey,
         remote_public_key: &PublicKey,
-    ) -> Result<SessionKey, Error>;
+    ) -> SessionKey;
 
     /// Size of the shared secret in bytes.
     const SHARED_SECRET_SIZE: usize;

crypto/src/kex/x25519.rs

@@ -20,27 +20,25 @@ impl KeyExchangeScheme for X25519Sha256 {
         Self
     }
 
-    fn keypair(&self, option: Option<KeyGenOption>) -> Result<(PublicKey, PrivateKey), Error> {
+    fn keypair(&self, mut option: Option<KeyGenOption>) -> Result<(PublicKey, PrivateKey), Error> {
         let (pk, sk) = match option {
-            Some(mut o) => match o {
-                KeyGenOption::UseSeed(ref mut s) => {
-                    let hash = sha2::Sha256::digest(s.as_slice());
-                    s.zeroize();
-                    let mut rng = ChaChaRng::from_seed(*array_ref!(hash.as_slice(), 0, 32));
-                    let sk = StaticSecret::random_from_rng(&mut rng);
-                    let pk = X25519PublicKey::from(&sk);
-                    (pk, sk)
-                }
-                KeyGenOption::FromPrivateKey(ref s) => {
-                    assert_eq!(s.digest_function, ALGORITHM);
-                    let sk = StaticSecret::from(*array_ref!(&s.payload, 0, 32));
-                    let pk = X25519PublicKey::from(&sk);
-                    (pk, sk)
-                }
-            },
+            Some(KeyGenOption::UseSeed(ref mut s)) => {
+                let hash = sha2::Sha256::digest(s.as_slice());
+                s.zeroize();
+                let rng = ChaChaRng::from_seed(*array_ref!(hash.as_slice(), 0, 32));
+                let sk = StaticSecret::random_from_rng(rng);
+                let pk = X25519PublicKey::from(&sk);
+                (pk, sk)
+            }
+            Some(KeyGenOption::FromPrivateKey(ref s)) => {
+                assert_eq!(s.digest_function, ALGORITHM);
+                let sk = StaticSecret::from(*array_ref!(&s.payload, 0, 32));
+                let pk = X25519PublicKey::from(&sk);
+                (pk, sk)
+            }
             None => {
-                let mut rng = OsRng::default();
-                let sk = StaticSecret::random_from_rng(&mut rng);
+                let rng = OsRng;
+                let sk = StaticSecret::random_from_rng(rng);
                 let pk = X25519PublicKey::from(&sk);
                 (pk, sk)
             }
@@ -61,14 +59,14 @@ impl KeyExchangeScheme for X25519Sha256 {
         &self,
         local_private_key: &PrivateKey,
         remote_public_key: &PublicKey,
-    ) -> Result<SessionKey, Error> {
+    ) -> SessionKey {
         assert_eq!(local_private_key.digest_function, ALGORITHM);
         assert_eq!(remote_public_key.digest_function, ALGORITHM);
         let sk = StaticSecret::from(*array_ref!(&local_private_key.payload, 0, 32));
         let pk = X25519PublicKey::from(*array_ref!(&remote_public_key.payload, 0, 32));
         let shared_secret = sk.diffie_hellman(&pk);
         let hash = sha2::Sha256::digest(shared_secret.as_bytes());
-        Ok(SessionKey(ConstVec::new(hash.as_slice().to_vec())))
+        SessionKey(ConstVec::new(hash.as_slice().to_vec()))
     }
 
     const SHARED_SECRET_SIZE: usize = 32;
@@ -86,15 +84,11 @@ mod tests {
         let res = scheme.keypair(None);
         assert!(res.is_ok());
         let (pk, sk) = res.unwrap();
-        let res = scheme.compute_shared_secret(&sk, &pk);
-        assert!(res.is_ok());
+        let _res = scheme.compute_shared_secret(&sk, &pk);
         let res = scheme.keypair(None);
-        assert!(res.is_ok());
         let (pk1, sk1) = res.unwrap();
-        let res = scheme.compute_shared_secret(&sk1, &pk);
-        assert!(res.is_ok());
-        let res = scheme.compute_shared_secret(&sk, &pk1);
-        assert!(res.is_ok());
+        let _res = scheme.compute_shared_secret(&sk1, &pk);
+        let _res = scheme.compute_shared_secret(&sk, &pk1);
 
         let res = scheme.keypair(Some(KeyGenOption::FromPrivateKey(sk.clone())));
         assert!(res.is_ok());