Skip to content

Commit

Permalink
[refactor] hyperledger-iroha#3237: Fix clippy lints
Browse files Browse the repository at this point in the history 
- 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]>
  • Loading branch information
@DCNick3
DCNick3 committed Nov 9, 2023
1 parent c012d65 commit b7f210f
Show file tree
Hide file tree
Showing 13 changed files with 201 additions and 200 deletions.
2 changes: 1 addition & 1 deletion crypto/src/encryption/chacha20poly1305.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down
63 changes: 57 additions & 6 deletions crypto/src/encryption/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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,
Expand All @@ -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)
}
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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 {
Expand All @@ -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() {
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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()
}
Expand Down
12 changes: 8 additions & 4 deletions crypto/src/kex/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand Down
50 changes: 22 additions & 28 deletions crypto/src/kex/x25519.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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;
Expand All @@ -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());
Expand Down
30 changes: 14 additions & 16 deletions crypto/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ pub use blake2;
use derive_more::{DebugCustom, Display};
use error::{Error, NoSuchAlgorithm};
use getset::{CopyGetters, Getters};
// TODO: do not spill the hashes to the crate root
pub use hash::*;
use iroha_macro::ffi_impl_opaque;
use iroha_primitives::const_vec::ConstVec;
Expand All @@ -46,7 +45,6 @@ use parity_scale_codec::{Decode, Encode};
use serde::Deserialize;
use serde::Serialize;
use serde_with::{DeserializeFromStr, SerializeDisplay};
// TODO: do not spill the signatures to the crate root
pub use signature::*;

// Hiding constants is a bad idea. For one, you're forcing the user to
Expand Down Expand Up @@ -233,12 +231,12 @@ impl KeyPair {
};

let (public_key, private_key) = match configuration.algorithm {
Algorithm::Ed25519 => signature::ed25519::Ed25519Sha512.keypair(key_gen_option),
Algorithm::Ed25519 => signature::ed25519::Ed25519Sha512::keypair(key_gen_option),
Algorithm::Secp256k1 => {
signature::secp256k1::EcdsaSecp256k1Sha256::new().keypair(key_gen_option)
signature::secp256k1::EcdsaSecp256k1Sha256::keypair(key_gen_option)
}
Algorithm::BlsNormal => signature::bls::BlsNormal::new().keypair(key_gen_option),
Algorithm::BlsSmall => signature::bls::BlsSmall::new().keypair(key_gen_option),
Algorithm::BlsNormal => signature::bls::BlsNormal::keypair(key_gen_option),
Algorithm::BlsSmall => signature::bls::BlsSmall::keypair(key_gen_option),
}?;

Ok(Self {
Expand Down Expand Up @@ -319,12 +317,12 @@ impl PublicKey {
let key_gen_option = Some(KeyGenOption::FromPrivateKey(private_key));

let (public_key, _) = match digest_function {
Algorithm::Ed25519 => signature::ed25519::Ed25519Sha512.keypair(key_gen_option),
Algorithm::Ed25519 => signature::ed25519::Ed25519Sha512::keypair(key_gen_option),
Algorithm::Secp256k1 => {
signature::secp256k1::EcdsaSecp256k1Sha256::new().keypair(key_gen_option)
signature::secp256k1::EcdsaSecp256k1Sha256::keypair(key_gen_option)
}
Algorithm::BlsNormal => signature::bls::BlsNormal::new().keypair(key_gen_option),
Algorithm::BlsSmall => signature::bls::BlsSmall::new().keypair(key_gen_option),
Algorithm::BlsNormal => signature::bls::BlsNormal::keypair(key_gen_option),
Algorithm::BlsSmall => signature::bls::BlsSmall::keypair(key_gen_option),
}?;

Ok(public_key)
Expand Down Expand Up @@ -687,15 +685,15 @@ mod tests {
.expect("Public key not in mulithash format"),
PrivateKey::from_hex(
Algorithm::Ed25519,
"93CA389FC2979F3F7D2A7F8B76C70DE6D5EAF5FA58D4F93CB8B0FB298D398ACC59C8A4DA1EBB5380F74ABA51F502714652FDCCE9611FAFB9904E4A3C4D382774".as_ref()
"93CA389FC2979F3F7D2A7F8B76C70DE6D5EAF5FA58D4F93CB8B0FB298D398ACC59C8A4DA1EBB5380F74ABA51F502714652FDCCE9611FAFB9904E4A3C4D382774"
).expect("Private key not hex encoded")).is_ok());

assert!(KeyPair::new("ea0161040FCFADE2FC5D9104A9ACF9665EA545339DDF10AE50343249E01AF3B8F885CD5D52956542CCE8105DB3A2EC4006E637A7177FAAEA228C311F907DAAFC254F22667F1A1812BB710C6F4116A1415275D27BB9FB884F37E8EF525CC31F3945E945FA"
.parse()
.expect("Public key not in mulithash format"),
PrivateKey::from_hex(
Algorithm::BlsNormal,
"0000000000000000000000000000000049BF70187154C57B97AF913163E8E875733B4EAF1F3F0689B31CE392129493E9".as_ref()
"0000000000000000000000000000000049BF70187154C57B97AF913163E8E875733B4EAF1F3F0689B31CE392129493E9"
).expect("Private key not hex encoded")).is_ok());
}

Expand Down Expand Up @@ -731,13 +729,13 @@ mod tests {
fn invalid_private_key() {
assert!(PrivateKey::from_hex(
Algorithm::Ed25519,
"0000000000000000000000000000000049BF70187154C57B97AF913163E8E875733B4EAF1F3F0689B31CE392129493E9".as_ref()
"0000000000000000000000000000000049BF70187154C57B97AF913163E8E875733B4EAF1F3F0689B31CE392129493E9"
).is_err());

assert!(
PrivateKey::from_hex(
Algorithm::BlsNormal,
"93CA389FC2979F3F7D2A7F8B76C70DE6D5EAF5FA58D4F93CB8B0FB298D398ACC59C8A4DA1EBB5380F74ABA51F502714652FDCCE9611FAFB9904E4A3C4D382774".as_ref()
"93CA389FC2979F3F7D2A7F8B76C70DE6D5EAF5FA58D4F93CB8B0FB298D398ACC59C8A4DA1EBB5380F74ABA51F502714652FDCCE9611FAFB9904E4A3C4D382774"
).is_err());
}

Expand All @@ -749,15 +747,15 @@ mod tests {
.expect("Public key not in mulithash format"),
PrivateKey::from_hex(
Algorithm::Ed25519,
"3A7991AF1ABB77F3FD27CC148404A6AE4439D095A63591B77C788D53F708A02A1509A611AD6D97B01D871E58ED00C8FD7C3917B6CA61A8C2833A19E000AAC2E4".as_ref()
"3A7991AF1ABB77F3FD27CC148404A6AE4439D095A63591B77C788D53F708A02A1509A611AD6D97B01D871E58ED00C8FD7C3917B6CA61A8C2833A19E000AAC2E4"
).expect("Private key not valid")).is_err());

assert!(KeyPair::new("ea0161040FCFADE2FC5D9104A9ACF9665EA545339DDF10AE50343249E01AF3B8F885CD5D52956542CCE8105DB3A2EC4006E637A7177FAAEA228C311F907DAAFC254F22667F1A1812BB710C6F4116A1415275D27BB9FB884F37E8EF525CC31F3945E945FA"
.parse()
.expect("Public key not in mulithash format"),
PrivateKey::from_hex(
Algorithm::BlsNormal,
"000000000000000000000000000000002F57460183837EFBAC6AA6AB3B8DBB7CFFCFC59E9448B7860A206D37D470CBA3".as_ref()
"000000000000000000000000000000002F57460183837EFBAC6AA6AB3B8DBB7CFFCFC59E9448B7860A206D37D470CBA3"
).expect("Private key not valid")).is_err());
}

Expand Down
Loading

0 comments on commit b7f210f

Please sign in to comment.