From 6e8cb392ccca7e1115c5b82b0592f5c492143d27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joni=20Lepist=C3=B6?= Date: Tue, 12 Nov 2024 20:11:45 +0200 Subject: [PATCH] Lints and doc updates * Added important lints and fixed documentation accordingly --- README.md | 21 +++++++++++++++---- src/bytearray.rs | 11 ++++++++++ src/cipherstate.rs | 2 ++ src/crypto_impl/x25519.rs | 8 +++---- src/error.rs | 5 +++++ src/handshakepattern.rs | 17 ++++++++++++++- src/handshakestate/dual_layer.rs | 4 ++-- src/handshakestate/nq.rs | 16 ++++++++------ src/handshakestate/pq.rs | 7 +++++-- src/lib.rs | 4 ++++ src/symmetricstate.rs | 8 +++---- src/traits.rs | 36 +++++++++++++++++++++----------- src/transportstate.rs | 30 +++++++++++++------------- 13 files changed, 119 insertions(+), 50 deletions(-) diff --git a/README.md b/README.md index 5c9fa13..1de2bb3 100644 --- a/README.md +++ b/README.md @@ -7,11 +7,11 @@ ⚠️ **Work in progress** ⚠️ -`no_std` compatible, pure Rust implementation of the [Noise protocol framework](https://noiseprotocol.org/noise.html) +`no_std` compatible, pure Rust implementation of the [**Noise protocol framework**](https://noiseprotocol.org/noise.html) with support for [**Post Quantum (PQ) extensions**](https://doi.org/10.1145/3548606.3560577) as presented by Yawning Angel, Benjamin Dowling, Andreas Hülsing, Peter Schwabe, and Fiona Johanna Weber. -Main targets of this crate are correctness, extensibility, and strict `no_std` compatibility +Main targets of this crate are **correctness**, extensibility, and strict `no_std` compatibility and those come with the small drawback of more verbose user experience with some boilerplate. If you don't need PQ functionality and are developing for a regular target, you probably are better off using these instead: @@ -22,7 +22,10 @@ off using these instead: Basis of this implementation relies heavily on the abovementioned crates and I'm extending huge thanks to the developers for their effort! -⚠️ **Warning** ⚠️ This library has not received any formal audit and is still in early phase +⚠️ **Warning** ⚠️ + +* This library has not received any formal audit +* While we enable some cryptographic providers by default, it is up to **you** to get familiar with those and decide if they meet your security and integrity requirements ## Basics @@ -30,7 +33,7 @@ From user perspective, everything in this crate is built around three types: * [`NqHandshake`](https://docs.rs/clatter/latest/clatter/struct.NqHandshake.html) - Classical, non-post-quantum Noise handshake * [`PqHandshake`](https://docs.rs/clatter/latest/clatter/struct.PqHandshake.html) - Post-quantum Noise handshake -* [`DualLayerHandshake`](https://docs.rs/clatter/latest/clatter/struct.DualLayerHandshake.html) - Dual layer handshake, which combines two Noise handshakes +* [`DualLayerHandshake`](https://docs.rs/clatter/latest/clatter/struct.DualLayerHandshake.html) - Dual layer handshake, which combines two Noise handshakes and allows a naive hybrid encryption approach Users will pick and instantiate the desired handshake state machine with the crypto primitives they wish to use (supplied as generic parameters) and complete the handshake using the methods @@ -168,3 +171,13 @@ does not currently implement *SEEC*. * Better documentation * Intuitive Noise pattern parser with `alloc`feature +## Some Coding Quidelines.. + +Let's be the cleanest Noise Rust implementation there is :) + +* `no_std` compatibility required at all times +* No Clippy warnings allowed - ignores allowed only when absolutely justified +* Good hygiene expected: + * Check for integer overflows + * Zeroize sensitive assets on drop + diff --git a/src/bytearray.rs b/src/bytearray.rs index 1d803d7..06de711 100644 --- a/src/bytearray.rs +++ b/src/bytearray.rs @@ -5,12 +5,22 @@ use zeroize::{Zeroize, ZeroizeOnDrop}; /// Simple trait used throughout the codebase to provide /// portable array operations pub trait ByteArray: Sized + Zeroize + PartialEq + Debug { + /// Initialize a new array with zeros fn new_zero() -> Self; + /// Initialize a new array by filling it with the given element fn new_with(_: u8) -> Self; + /// Initialize a new array by copying it from the given slice + /// + /// # Panics + /// * Panics if the slice length does not match this array length fn from_slice(_: &[u8]) -> Self; + /// Array length fn len() -> usize; + /// Borrow this array as a slice fn as_slice(&self) -> &[u8]; + /// Borrow this array as mutable fn as_mut(&mut self) -> &mut [u8]; + /// Clone this array fn clone(&self) -> Self { Self::from_slice(self.as_slice()) } @@ -21,6 +31,7 @@ pub trait ByteArray: Sized + Zeroize + PartialEq + Debug { pub struct SensitiveByteArray(A); impl SensitiveByteArray { + /// Encapsulate the given [`ByteArray`] pub fn new(a: A) -> SensitiveByteArray { Self(a) } diff --git a/src/cipherstate.rs b/src/cipherstate.rs index 503c900..db42c5e 100644 --- a/src/cipherstate.rs +++ b/src/cipherstate.rs @@ -6,7 +6,9 @@ use crate::traits::{Cipher, CryptoComponent}; /// Pair of [`CipherState`] instances for sending and receiving transport messages pub struct CipherStates { + /// Cipher for initiator -> responder communication pub initiator_to_responder: CipherState, + /// Cipher for responder -> initiator communication pub responder_to_initiator: CipherState, } diff --git a/src/crypto_impl/x25519.rs b/src/crypto_impl/x25519.rs index ae4b862..27ad81b 100644 --- a/src/crypto_impl/x25519.rs +++ b/src/crypto_impl/x25519.rs @@ -14,23 +14,23 @@ impl CryptoComponent for X25519 { } impl Dh for X25519 { - type Key = SensitiveByteArray<[u8; 32]>; + type PrivateKey = SensitiveByteArray<[u8; 32]>; type PubKey = [u8; 32]; type Output = SensitiveByteArray<[u8; 32]>; fn genkey( rng: &mut R, - ) -> crate::error::DhResult> { + ) -> crate::error::DhResult> { let s = StaticSecret::random_from_rng(rng); let p = PublicKey::from(&s); Ok(KeyPair { public: Self::PubKey::from_slice(p.as_bytes()), - secret: Self::Key::from_slice(s.as_bytes()), + secret: Self::PrivateKey::from_slice(s.as_bytes()), }) } - fn dh(k: &Self::Key, pk: &Self::PubKey) -> crate::error::DhResult { + fn dh(k: &Self::PrivateKey, pk: &Self::PubKey) -> crate::error::DhResult { let k = StaticSecret::from(**k); let pk = PublicKey::from(*pk); Ok(Self::Output::from_slice(k.diffie_hellman(&pk).as_bytes())) diff --git a/src/error.rs b/src/error.rs index 7a1a579..197900a 100644 --- a/src/error.rs +++ b/src/error.rs @@ -28,6 +28,7 @@ pub enum HandshakeError { Transport(#[from] TransportError), } +/// Handshake operation result type pub type HandshakeResult = Result; /// Errors that can happen during transport operations @@ -41,6 +42,7 @@ pub enum TransportError { Cipher(#[from] CipherError), } +/// Transport operation result type pub type TransportResult = Result; /// Errors that can happen during KEM operations @@ -56,6 +58,7 @@ pub enum KemError { KeyGeneration, } +/// KEM operation result type pub type KemResult = Result; /// Errors that can happen during DH operations @@ -65,6 +68,7 @@ pub enum DhError { KeyGeneration, } +/// DH operation result type pub type DhResult = Result; /// Errors that can happen during Cipher operations @@ -76,4 +80,5 @@ pub enum CipherError { Decrypt, } +/// Cipher operation result type pub type CipherResult = Result; diff --git a/src/handshakepattern.rs b/src/handshakepattern.rs index 3e6ae68..906d5fa 100644 --- a/src/handshakepattern.rs +++ b/src/handshakepattern.rs @@ -5,14 +5,23 @@ use arrayvec::ArrayVec; /// Handshake tokens as defined by the Noise spec and PQNoise paper. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum Token { + /// Initiator ephemeral key E, + /// Initiator static key S, + /// Ephemeral-ephemeral DH EE, + /// Ephemeral-static DH ES, + /// Static-ephemeral DH SE, + /// Static-static DH SS, + /// Ephemeral KEM Ekem, + /// Static KEM Skem, + /// Pre-shared key Psk, } @@ -30,13 +39,19 @@ pub struct HandshakePattern { has_psk: bool, } +/// Handshake message pattern +/// +/// Does not include pre-message patterns #[derive(Clone, Debug)] pub struct MessagePattern { + /// Messages sent by the initiator pub initiator: ArrayVec, 8>, + /// Messages sent by the responder pub responder: ArrayVec, 8>, } impl MessagePattern { + /// Check if the pattern includes pre-shared keys pub fn has_psk(&self) -> bool { self.initiator.iter().flatten().any(|t| *t == Token::Psk) || self.responder.iter().flatten().any(|t| *t == Token::Psk) @@ -46,7 +61,7 @@ impl MessagePattern { impl HandshakePattern { /// Initialize a new handshake pattern /// - /// # Arguments: + /// # Arguments /// * `name` - Pattern name /// * `pre_initiator` - Tokens shared by initiator pre handshake /// * `pre_responder` - Tokens shared by responder pre handshake diff --git a/src/handshakestate/dual_layer.rs b/src/handshakestate/dual_layer.rs index 1edea08..7b97e39 100644 --- a/src/handshakestate/dual_layer.rs +++ b/src/handshakestate/dual_layer.rs @@ -40,11 +40,11 @@ where { /// Initialize a new dual layer handshake /// - /// # Arguments: + /// # Arguments /// * `outer` - Outer handshake, which is completed first /// * `innter` - Inner handshake, which benefits from the security of the outer layer /// - /// # Generic parameters: + /// # Generic parameters /// * `const BUF` - Intermediate decrypt buffer size - Must be large enough to fit all inner handshake messages /// /// # Panics diff --git a/src/handshakestate/nq.rs b/src/handshakestate/nq.rs index ff221dd..84c0100 100644 --- a/src/handshakestate/nq.rs +++ b/src/handshakestate/nq.rs @@ -25,7 +25,8 @@ where { // Internal, we can live with this #[allow(clippy::type_complexity)] - internals: HandshakeInternals<'a, C, H, RNG, DH::Key, DH::PubKey, DH::Key, DH::PubKey>, + internals: + HandshakeInternals<'a, C, H, RNG, DH::PrivateKey, DH::PubKey, DH::PrivateKey, DH::PubKey>, } impl<'a, DH, CIPHER, HASH, RNG> NqHandshake<'a, DH, CIPHER, HASH, RNG> @@ -37,7 +38,7 @@ where { /// Initialize new non-post-quantum handshake /// - /// # Arguments: + /// # Arguments /// * `pattern` - Handshake pattern /// * `prologue` - Optional prologue data for the handshake /// * `initiator` - True if we are the initiator @@ -47,17 +48,20 @@ where /// * `re` - Peer public ephemeral key - Shouldn't usually be provided manually /// * `rng` - RNG to use during the handshake /// - /// # Generic parameters: + /// # Generic parameters /// * `DH` - DH algorithm to use /// * `CIPHER` - Cipher algorithm to use /// * `HASH` - Hashing algorithm to use + /// + /// # Panics + /// * Panics if initialized with a PQ pattern #[allow(clippy::too_many_arguments)] // Okay for now pub fn new( pattern: HandshakePattern, prologue: &[u8], initiator: bool, - s: Option>, - e: Option>, + s: Option>, + e: Option>, rs: Option, re: Option, rng: &'a mut RNG, @@ -168,7 +172,7 @@ where } fn dh( - a: Option<&KeyPair>, + a: Option<&KeyPair>, b: Option<&DH::PubKey>, ) -> HandshakeResult { let a = a.ok_or(HandshakeError::MissingMaterial)?; diff --git a/src/handshakestate/pq.rs b/src/handshakestate/pq.rs index d592842..f94ae75 100644 --- a/src/handshakestate/pq.rs +++ b/src/handshakestate/pq.rs @@ -49,7 +49,7 @@ where { /// Initialize new post-quantum handshake /// - /// # Arguments: + /// # Arguments /// * `pattern` - Handshake pattern /// * `prologue` - Optional prologue data for the handshake /// * `initiator` - True if we are the initiator @@ -59,11 +59,14 @@ where /// * `re` - Peer public ephemeral key - Shouldn't usually be provided manually /// * `rng` - RNG to use during the handshake /// - /// # Generic parameters: + /// # Generic parameters /// * `EKEM` - KEM algorithm to use for ephemeral key encapsulation /// * `SKEM` - KEM algorithm to use for static key encapsulation /// * `CIPHER` - Cipher algorithm to use /// * `HASH` - Hashing algorithm to use + /// + /// # Panics + /// * Panics if initialized with a NQ pattern #[allow(clippy::too_many_arguments)] // Okay for now pub fn new( pattern: HandshakePattern, diff --git a/src/lib.rs b/src/lib.rs index c8f6557..e68dcb0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,4 +1,6 @@ #![cfg_attr(not(feature = "std"), no_std)] +#![warn(missing_docs)] +#![warn(clippy::missing_panics_doc)] //! # Clatter 🔊 //! //! ⚠️ **Work in progress** ⚠️ @@ -186,6 +188,8 @@ pub mod crypto { /// A zeroize-on-drop container for keys #[derive(ZeroizeOnDrop, Clone)] pub struct KeyPair { + /// Public key pub public: P, + /// Secret (private) key pub secret: S, } diff --git a/src/symmetricstate.rs b/src/symmetricstate.rs index 6a1ab0b..cbedea7 100644 --- a/src/symmetricstate.rs +++ b/src/symmetricstate.rs @@ -41,7 +41,7 @@ where } } - /// # Protocol: + /// # Protocol /// ```text /// Sets h = HASH(h || data) /// ``` @@ -52,7 +52,7 @@ where self.h = h.result(); } - /// # Protocol: + /// # Protocol /// ```text /// * Sets ck, temp_k = HKDF(ck, input_key_material, 2). /// * If HASHLEN is 64, then truncates temp_k to 32 bytes. @@ -64,7 +64,7 @@ where self.cipherstate = Some(CipherState::new(&temp_k.as_slice()[..C::key_len()], 0)); } - /// # Protocol: + /// # Protocol /// ```text /// * Sets ck, temp_h, temp_k = HKDF(ck, input_key_material, 3). /// * Calls MixHash(temp_h). @@ -112,7 +112,7 @@ where /// Return [`CipherStates`] for encrypting transport messages /// - /// # Panics: + /// # Panics /// * If no key material has been stablished pub(crate) fn split(&self) -> CipherStates { // This means that we are still in the initial state diff --git a/src/traits.rs b/src/traits.rs index 06f9e77..f898f3b 100644 --- a/src/traits.rs +++ b/src/traits.rs @@ -21,22 +21,31 @@ pub trait CryptoComponent { /// Common trait for all Diffie-Hellman algorithms pub trait Dh: CryptoComponent { - type Key: ByteArray; + /// Private key type + type PrivateKey: ByteArray; + /// Public key type type PubKey: ByteArray; + /// DH output type type Output: ByteArray; /// Generate private key - fn genkey(rng: &mut R) -> DhResult>; + fn genkey( + rng: &mut R, + ) -> DhResult>; /// Perform DH key exchange - fn dh(_: &Self::Key, _: &Self::PubKey) -> DhResult; + fn dh(_: &Self::PrivateKey, _: &Self::PubKey) -> DhResult; } /// Common trait for all key encapsulation mechanisms pub trait Kem: CryptoComponent { + /// Secret key type type SecretKey: ByteArray; + /// Public key type type PubKey: ByteArray; + /// Ciphertext type type Ct: ByteArray; + /// Shared secret type type Ss: ByteArray; /// Generate a keypair @@ -56,7 +65,9 @@ pub trait Kem: CryptoComponent { /// Common trait for all hash algorithms pub trait Hash: CryptoComponent + Default { + /// Hash block type type Block: ByteArray; + /// Hash output type type Output: ByteArray; /// Hash block length @@ -140,6 +151,7 @@ pub trait Hash: CryptoComponent + Default { /// Common trait for all cipher algorithms pub trait Cipher: CryptoComponent { + /// Cipher key type type Key: ByteArray; /// Key length @@ -260,21 +272,21 @@ where { /// Write next handshake message to the given buffer /// - /// # Arguments: + /// # Arguments /// * `payload` - payload to include in the handshake message /// * `out` - destination buffer to write the handshake message to /// - /// # Returns: + /// # Returns /// Number of bytes written to destination buffer /// - /// # Errors: + /// # Errors /// * [`HandshakeError::ErrorState`] - Handshaker encountered an error before and cannot be used anymore /// * [`HandshakeError::InvalidState`] - Handshaker is not in receive state /// * [`HandshakeError::BufferTooSmall`] - Message does not fit into provided destination buffer /// * [`HandshakeError::Dh`] - DH error /// * [`HandshakeError::Cipher`] - Encryption error /// - /// # Panics: + /// # Panics /// If resulting message length is larger than [`crate::constants::MAX_MESSAGE_LEN`] fn write_message(&mut self, payload: &[u8], out: &mut [u8]) -> HandshakeResult { if self.status() == HandshakeStatus::Error { @@ -306,14 +318,14 @@ where /// Read and process next handshake message from given buffer /// - /// # Arguments: + /// # Arguments /// * `message` - handshake message /// * `out` - destination buffer to write the handshake payload /// - /// # Returns: + /// # Returns /// Number of payload bytes written to destination buffer /// - /// # Errors: + /// # Errors /// * [`HandshakeError::ErrorState`] - Handshaker encountered an error before and cannot be used anymore /// * [`HandshakeError::InvalidState`] - Handshaker is not in receive state /// * [`HandshakeError::InvalidMessage`] - Input does not match the next expected message @@ -321,7 +333,7 @@ where /// * [`HandshakeError::Dh`] - DH error /// * [`HandshakeError::Cipher`] - Decryption error /// - /// # Panics: + /// # Panics /// * If message length is larger than [`crate::constants::MAX_MESSAGE_LEN`] fn read_message(&mut self, message: &[u8], out: &mut [u8]) -> HandshakeResult { if message.len() > MAX_MESSAGE_LEN { @@ -357,7 +369,7 @@ where /// Push a PSK to the PSK queue /// - /// # Panics: + /// # Panics /// * If the PSK is not [`crate::constants::PSK_LEN`] bytes /// * If the PSK queue becomes larger than [`crate::constants::MAX_PSKS`] fn push_psk(&mut self, psk: &[u8]); diff --git a/src/transportstate.rs b/src/transportstate.rs index b4b9e90..9a0b796 100644 --- a/src/transportstate.rs +++ b/src/transportstate.rs @@ -10,7 +10,7 @@ use crate::traits::{Cipher, Handshaker, Hash}; /// form of a [`CipherStates`] struct. Users have raw access /// to the keys if needed using the [`Self::take`] method. /// -/// # Sending and receiving messages: +/// # Sending and receiving messages /// * [`Self::send`] /// * [`Self::receive`] /// * [`Self::send_in_place`] @@ -40,18 +40,18 @@ impl TransportState { /// Encrypts data from `msg` and places the resulting ciphertext /// in `buf`, returning the total number of bytes written. /// - /// # Arguments: + /// # Arguments /// * `msg` - Message buffer to encrypt /// * `buf` - Destination buffer to store the encrypted message /// - /// # Returns: + /// # Returns /// * Encrypted ytes written to `buf` /// - /// # Errors: + /// # Errors /// * [`TransportError::BufferTooSmall`] - Resulting message does not fit in `buf` /// * [`TransportError::Cipher`] - Encryption error /// - /// # Panics: + /// # Panics /// * If resulting message length exceeds [`MAX_MESSAGE_LEN`] pub fn send(&mut self, msg: &[u8], buf: &mut [u8]) -> TransportResult { let out_len = msg.len() + C::tag_len(); @@ -80,18 +80,18 @@ impl TransportState { /// returning the total number of bytes the resulting /// ciphertext takes. /// - /// # Arguments: + /// # Arguments /// * `msg` - Message buffer /// * `msg_len` - How many bytes from the beginning of `msg` will be encrypted in-place /// - /// # Returns: + /// # Returns /// * Encrypted bytes written to `msg` /// - /// # Errors: + /// # Errors /// * [`TransportError::BufferTooSmall`] - Resulting message does not fit in `buf` /// * [`TransportError::Cipher`] - Encryption error /// - /// # Panics: + /// # Panics /// * If resulting message length exceeds [`MAX_MESSAGE_LEN`] pub fn send_in_place(&mut self, msg: &mut [u8], msg_len: usize) -> TransportResult { let out_len = msg_len + C::tag_len(); @@ -119,14 +119,14 @@ impl TransportState { /// Decrypts data from `msg` and places the resulting plaintext /// in `buf`, returning the total number of bytes written. /// - /// # Arguments: + /// # Arguments /// * `msg` - Received message buffer /// * `buf` - Destination buffer to store the decrypted message /// - /// # Returns: + /// # Returns /// * Decrypted bytes written to `buf` /// - /// # Errors: + /// # Errors /// * [`TransportError::TooShort`] - Provided message `msg` is too short for decryption /// * [`TransportError::BufferTooSmall`] - Resulting message does not fit in `buf` /// * [`TransportError::Cipher`] - Decryption error @@ -163,14 +163,14 @@ impl TransportState { /// returning the total number of byte the resulting /// plaintext takes. /// - /// # Arguments: + /// # Arguments /// * `msg` - Message buffer /// * `msg_len` - How many bytes from the beginning of `msg` will be decrypted in-place /// - /// # Returns: + /// # Returns /// * Decrypted bytes written to `msg` /// - /// # Errors: + /// # Errors /// * [`TransportError::TooShort`] - Provided message `msg` is too short for decryption /// * [`TransportError::BufferTooSmall`] - Resulting message does not fit in `buf` /// * [`TransportError::Cipher`] - Decryption error