Skip to content

Commit

Permalink
[refactor] #3422: do not leak error type from aead crate, allows re…
Browse files Browse the repository at this point in the history
…moving dep of iroha_p2p on aead. Also remove buffer-based API from iroha_crypto, it's not used anyways.

Signed-off-by: Nikita Strygin <dcnick3@users.noreply.github.com>
  • Loading branch information
DCNick3 committed Nov 20, 2023
1 parent 4ce1a2c commit 08e28dc
Show file tree
Hide file tree
Showing 8 changed files with 42 additions and 131 deletions.
11 changes: 6 additions & 5 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,6 @@ strum = { version = "0.25.0", default-features = false }
getset = "0.1.2"
hex-literal = "0.4.1"

aead = "0.5.2"

rand = "0.8.5"
warp = { version = "0.3.6", default-features = false }
wasmtime = "13.0.0"
Expand Down
7 changes: 6 additions & 1 deletion crypto/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ std = [
"dep:chacha20poly1305",
"dep:elliptic-curve",
"dep:k256",
"dep:thiserror",
"displaydoc/std",
]
# Force static linking
vendored = ["dep:openssl-sys"]
Expand All @@ -58,6 +60,9 @@ hex = { workspace = true, features = ["alloc", "serde"] }
openssl-sys = { version = "0.9.93", features = ["vendored"], optional = true }
getset = { workspace = true }

thiserror = { version = "1.0.50", optional = true }
displaydoc = { version = "0.2.4", default-features = false }

digest = { version = "0.10.7", optional = true }
blake2 = { version = "0.10.6", optional = true }
sha2 = { version = "0.10.8", optional = true }
Expand All @@ -77,7 +82,7 @@ rand_chacha = { version = "0.3.1", optional = true }
zeroize = { version = "1.6.0", optional = true }
arrayref = { version = "0.3.7", optional = true }

aead = { workspace = true, optional = true }
aead = { version = "0.5.2", optional = true }
chacha20poly1305 = { version = "0.10.1", optional = true }

elliptic-curve = { version = "0.13.6", optional = true }
Expand Down
20 changes: 0 additions & 20 deletions crypto/src/encryption/chacha20poly1305.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,6 @@ impl Aead for ChaCha20Poly1305 {

#[cfg(test)]
mod tests {
use std::io::Cursor;

use super::*;

#[test]
Expand Down Expand Up @@ -119,24 +117,6 @@ mod tests {
assert!(res.is_err());
}

#[test]
fn buffer_works() {
let cipher = ChaCha20Poly1305::new(&ChaCha20Poly1305::key_gen().unwrap());
let aad = b"buffer works".to_vec();
let dummytext = b"Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.";
let mut ciphertext = Vec::new();
let res = cipher.encrypt_buffer(&aad, &mut Cursor::new(dummytext), &mut ciphertext);
assert!(res.is_ok());
let mut plaintext = Vec::new();
let res = cipher.decrypt_buffer(
&aad,
&mut Cursor::new(ciphertext.as_slice()),
&mut plaintext,
);
assert!(res.is_ok());
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() {
Expand Down
104 changes: 26 additions & 78 deletions crypto/src/encryption/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,37 @@

mod chacha20poly1305;

use std::io::{Read, Write};

use aead::{
generic_array::{typenum::Unsigned, ArrayLength, GenericArray},
Aead, Error, KeyInit, Payload,
Aead, Error as AeadError, KeyInit, Payload,
};
use displaydoc::Display;
use rand::{rngs::OsRng, RngCore};
use thiserror::Error;

pub use self::chacha20poly1305::ChaCha20Poly1305;
use crate::SessionKey;

/// An error that can occur during encryption or decryption
#[derive(Error, Display, Debug)]
pub enum Error {
/// Failed to generate nonce for an encryption operation
NonceGeneration(#[source] rand::Error),
/// Failed to encrypt data
Encryption(AeadError),
/// Failed to decrypt data
Decryption(AeadError),
/// Not enough data to decrypt message
NotEnoughData,
}

// 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
.try_fill_bytes(value.as_mut_slice())
// RustCrypto errors don't have any details, can't propagate the error
.map_err(|_| Error)?;
.map_err(Error::NonceGeneration)?;
Ok(value)
}

Expand All @@ -44,13 +57,6 @@ fn random_bytes<T: ArrayLength<u8>>() -> Result<GenericArray<u8, T>, Error> {
))
}

fn read_buffer<I: Read>(buffer: &mut I) -> Result<Vec<u8>, Error> {
let mut v = Vec::new();
let bytes_read = buffer.read_to_end(&mut v).map_err(|_| Error)?;
v.truncate(bytes_read);
Ok(v)
}

/// A generic symmetric encryption wrapper
///
/// # Usage
Expand Down Expand Up @@ -118,7 +124,9 @@ impl<E: Encryptor> SymmetricEncryptor<E> {
msg: plaintext.as_ref(),
aad: aad.as_ref(),
};
self.encryptor.encrypt(nonce, payload)
self.encryptor
.encrypt(nonce, payload)
.map_err(Error::Encryption)
}

/// Decrypt `ciphertext` using integrity protected `aad`. The result is the plaintext if successful
Expand Down Expand Up @@ -153,35 +161,9 @@ impl<E: Encryptor> SymmetricEncryptor<E> {
msg: ciphertext.as_ref(),
aad: aad.as_ref(),
};
self.encryptor.decrypt(nonce, payload)
}

/// 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,
plaintext: &mut I,
ciphertext: &mut O,
) -> Result<(), Error> {
self.encryptor.encrypt_buffer(aad, plaintext, ciphertext)
}

/// 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,
ciphertext: &mut I,
plaintext: &mut O,
) -> Result<(), Error> {
self.encryptor.decrypt_buffer(aad, ciphertext, plaintext)
self.encryptor
.decrypt(nonce, payload)
.map_err(Error::Decryption)
}
}

Expand Down Expand Up @@ -211,7 +193,7 @@ pub trait Encryptor: Aead + KeyInit {
msg: plaintext.as_ref(),
aad: aad.as_ref(),
};
let ciphertext = self.encrypt(&nonce, payload)?;
let ciphertext = self.encrypt(&nonce, payload).map_err(Error::Encryption)?;
let mut result = nonce.to_vec();
result.extend_from_slice(ciphertext.as_slice());
Ok(result)
Expand All @@ -227,52 +209,18 @@ pub trait Encryptor: Aead + KeyInit {
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() {
return Err(Error);
return Err(Error::NotEnoughData);
}

let nonce = GenericArray::from_slice(&ciphertext[..Self::NonceSize::to_usize()]);
let payload = Payload {
msg: &ciphertext[Self::NonceSize::to_usize()..],
aad: aad.as_ref(),
};
let plaintext = self.decrypt(nonce, payload)?;
let plaintext = self.decrypt(nonce, payload).map_err(Error::Decryption)?;
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,
plaintext: &mut I,
ciphertext: &mut O,
) -> Result<(), Error> {
let p = read_buffer(plaintext)?;
let c = self.encrypt_easy(aad.as_ref(), p.as_slice())?;
ciphertext.write_all(c.as_slice()).map_err(|_| Error)?;
Ok(())
}

/// 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,
ciphertext: &mut I,
plaintext: &mut O,
) -> Result<(), Error> {
let c = read_buffer(ciphertext)?;
let p = self.decrypt_easy(aad.as_ref(), c.as_slice())?;
plaintext.write_all(p.as_slice()).map_err(|_| Error)?;
Ok(())
}

/// Generate a new key for this encryptor
///
/// # Errors
Expand Down
1 change: 0 additions & 1 deletion p2p/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ tokio = { workspace = true, features = ["rt-multi-thread", "macros", "io-util",
futures = { workspace = true, features = ["alloc"] }
async-trait = { workspace = true }
parity-scale-codec = { workspace = true, features = ["derive"] }
aead = { workspace = true }
thiserror = { workspace = true }
displaydoc = { workspace = true }
derive_more = { workspace = true }
Expand Down
24 changes: 3 additions & 21 deletions p2p/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use iroha_crypto::{
Blake2bVar,
},
encryption::ChaCha20Poly1305,
error::Error as CryptoError,
kex::X25519Sha256,
};
pub use network::message::*;
Expand Down Expand Up @@ -55,32 +54,15 @@ pub enum Error {
/// Parity Scale codec error
ParityScale(#[from] parity_scale_codec::Error),
/// Failed to create keys
Keys(#[source] CryptographicError),
Keys(#[from] iroha_crypto::error::Error),
/// Symmetric encryption has failed
SymmetricEncryption(#[from] iroha_crypto::encryption::Error),
/// Failed to parse socket address
Addr(#[from] AddrParseError),
/// Connection reset by peer in the middle of message transfer
ConnectionResetByPeer,
}

/// Error in the cryptographic processes.
#[derive(derive_more::From, Debug, Error, displaydoc::Display)]
pub enum CryptographicError {
/// Decryption failed
#[from(ignore)]
Decrypt(aead::Error),
/// Encryption failed
#[from(ignore)]
Encrypt(aead::Error),
/// Cryptography error
Crypto(CryptoError),
}

impl<T: Into<CryptographicError>> From<T> for Error {
fn from(err: T) -> Self {
Self::Keys(err.into())
}
}

impl From<io::Error> for Error {
fn from(e: io::Error) -> Self {
Self::Io(std::sync::Arc::new(e))
Expand Down
4 changes: 1 addition & 3 deletions p2p/src/peer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use tokio::{
sync::{mpsc, oneshot},
};

use crate::{boilerplate::*, CryptographicError, Error};
use crate::{boilerplate::*, Error};

/// Max length of message handshake in bytes excluding first message length byte.
pub const MAX_HANDSHAKE_LENGTH: u8 = 255;
Expand Down Expand Up @@ -700,7 +700,6 @@ mod cryptographer {
pub fn decrypt(&self, data: &[u8]) -> Result<Vec<u8>, Error> {
self.encryptor
.decrypt_easy(DEFAULT_AAD.as_ref(), data)
.map_err(CryptographicError::Decrypt)
.map_err(Into::into)
}

Expand All @@ -711,7 +710,6 @@ mod cryptographer {
pub fn encrypt(&self, data: &[u8]) -> Result<Vec<u8>, Error> {
self.encryptor
.encrypt_easy(DEFAULT_AAD.as_ref(), data)
.map_err(CryptographicError::Encrypt)
.map_err(Into::into)
}

Expand Down

0 comments on commit 08e28dc

Please sign in to comment.