From 926a8679f19cf85d709977b59d9c85a37e2fb3e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Mon, 24 Jul 2023 09:25:54 +0200 Subject: [PATCH] Add some key length checks when decoding from base64 --- src/types/curve25519.rs | 32 ++++++++++++++++------ src/types/ed25519.rs | 60 ++++++++++++++++++++++++++++------------- src/types/mod.rs | 9 ++++--- 3 files changed, 71 insertions(+), 30 deletions(-) diff --git a/src/types/curve25519.rs b/src/types/curve25519.rs index 125ed9de..fd051f19 100644 --- a/src/types/curve25519.rs +++ b/src/types/curve25519.rs @@ -14,6 +14,7 @@ use std::fmt::Display; +use base64::decoded_len_estimate; use matrix_pickle::{Decode, DecodeError}; use rand::thread_rng; use serde::{Deserialize, Serialize}; @@ -115,6 +116,9 @@ impl Curve25519PublicKey { /// The number of bytes a Curve25519 public key has. pub const LENGTH: usize = 32; + const BASE64_LENGTH: usize = 43; + const PADDED_BASE64_LENGTH: usize = 44; + /// Convert this public key to a byte array. #[inline] pub fn to_bytes(&self) -> [u8; Self::LENGTH] { @@ -139,9 +143,17 @@ impl Curve25519PublicKey { /// Instantiate a Curve25519 public key from an unpadded base64 /// representation. - pub fn from_base64(base64_key: &str) -> Result { - let key = base64_decode(base64_key)?; - Self::from_slice(&key) + pub fn from_base64(input: &str) -> Result { + if input.len() != Self::BASE64_LENGTH && input.len() != Self::PADDED_BASE64_LENGTH { + Err(KeyError::InvalidKeyLength { + key_type: "Curve25519", + expected_length: Self::LENGTH, + length: decoded_len_estimate(input.len()), + }) + } else { + let key = base64_decode(input)?; + Self::from_slice(&key) + } } /// Try to create a `Curve25519PublicKey` from a slice of bytes. @@ -154,7 +166,11 @@ impl Curve25519PublicKey { Ok(Self::from(key)) } else { - Err(KeyError::InvalidKeyLength(key_len)) + Err(KeyError::InvalidKeyLength { + key_type: "Curve25519", + expected_length: Self::LENGTH, + length: key_len, + }) } } @@ -230,16 +246,16 @@ mod tests { let base64_payload = "a"; assert!(matches!( Curve25519PublicKey::from_base64(base64_payload), - Err(KeyError::Base64Error(DecodeError::InvalidLength)) + Err(KeyError::InvalidKeyLength { .. }) )); - let base64_payload = "a "; + let base64_payload = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA "; assert!(matches!( Curve25519PublicKey::from_base64(base64_payload), Err(KeyError::Base64Error(DecodeError::InvalidByte(..))) )); - let base64_payload = "aZ"; + let base64_payload = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAZ"; assert!(matches!( Curve25519PublicKey::from_base64(base64_payload), Err(KeyError::Base64Error(DecodeError::InvalidLastSymbol(..))) @@ -251,7 +267,7 @@ mod tests { let base64_payload = "aaaa"; assert!(matches!( Curve25519PublicKey::from_base64(base64_payload), - Err(KeyError::InvalidKeyLength(..)) + Err(KeyError::InvalidKeyLength { .. }) )); } diff --git a/src/types/ed25519.rs b/src/types/ed25519.rs index a624e72f..a175649c 100644 --- a/src/types/ed25519.rs +++ b/src/types/ed25519.rs @@ -14,6 +14,7 @@ use std::fmt::Display; +use base64::decoded_len_estimate; use curve25519_dalek::EdwardsPoint; #[cfg(not(fuzzing))] use ed25519_dalek::Verifier; @@ -170,6 +171,12 @@ impl Default for Ed25519Keypair { pub struct Ed25519SecretKey(Box); impl Ed25519SecretKey { + /// The number of bytes a Ed25519 secret key has. + pub const LENGTH: usize = ed25519_dalek::SECRET_KEY_LENGTH; + + const BASE64_LENGTH: usize = 43; + const PADDED_BASE64_LENGTH: usize = 44; + /// Create a new random `Ed25519SecretKey`. pub fn new() -> Self { let mut rng = thread_rng(); @@ -201,18 +208,25 @@ impl Ed25519SecretKey { } /// Try to create a `Ed25519SecretKey` from a base64 encoded string. - pub fn from_base64(key: &str) -> Result { - // TODO: check the length. - let mut bytes = base64_decode(key)?; - let mut key_bytes = [0u8; 32]; + pub fn from_base64(input: &str) -> Result { + if input.len() != Self::BASE64_LENGTH && input.len() != Self::PADDED_BASE64_LENGTH { + Err(crate::KeyError::InvalidKeyLength { + key_type: "Ed25519", + expected_length: ed25519_dalek::SECRET_KEY_LENGTH, + length: decoded_len_estimate(input.len()), + }) + } else { + let mut bytes = base64_decode(input)?; + let mut key_bytes = [0u8; 32]; - key_bytes.copy_from_slice(&bytes); - let key = Self::from_slice(&key_bytes); + key_bytes.copy_from_slice(&bytes); + let key = Self::from_slice(&key_bytes); - bytes.zeroize(); - key_bytes.zeroize(); + bytes.zeroize(); + key_bytes.zeroize(); - Ok(key) + Ok(key) + } } /// Get the public key that matches this `Ed25519SecretKey`. @@ -282,6 +296,9 @@ impl Ed25519PublicKey { /// The number of bytes a Ed25519 public key has. pub const LENGTH: usize = PUBLIC_KEY_LENGTH; + const BASE64_LENGTH: usize = 43; + const PADDED_BASE64_LENGTH: usize = 44; + /// Try to create a `Ed25519PublicKey` from a slice of bytes. pub fn from_slice(bytes: &[u8; 32]) -> Result { Ok(Self(VerifyingKey::from_bytes(bytes).map_err(SignatureError::from)?)) @@ -294,18 +311,25 @@ impl Ed25519PublicKey { /// Instantiate a Ed25519PublicKey public key from an unpadded base64 /// representation. - pub fn from_base64(base64_key: &str) -> Result { - // TODO: check the length. - let mut bytes = base64_decode(base64_key)?; - let mut key_bytes = [0u8; 32]; + pub fn from_base64(input: &str) -> Result { + if input.len() != Self::BASE64_LENGTH && input.len() != Self::PADDED_BASE64_LENGTH { + Err(crate::KeyError::InvalidKeyLength { + key_type: "Ed25519", + expected_length: Self::LENGTH, + length: decoded_len_estimate(input.len()), + }) + } else { + let mut bytes = base64_decode(input)?; + let mut key_bytes = [0u8; 32]; - key_bytes.copy_from_slice(&bytes); - let key = Self::from_slice(&key_bytes); + key_bytes.copy_from_slice(&bytes); + let key = Self::from_slice(&key_bytes); - bytes.zeroize(); - key_bytes.zeroize(); + bytes.zeroize(); + key_bytes.zeroize(); - key + key + } } /// Serialize a Ed25519PublicKey public key to an unpadded base64 diff --git a/src/types/mod.rs b/src/types/mod.rs index 614e44c9..1e1b1e2e 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -46,10 +46,11 @@ impl KeyId { pub enum KeyError { #[error("Failed decoding a public key from base64: {}", .0)] Base64Error(#[from] base64::DecodeError), - #[error("Failed decoding curve25519 key from base64: \ - Invalid number of bytes for curve25519, expected {}, got {}.", - Curve25519PublicKey::LENGTH, .0)] - InvalidKeyLength(usize), + #[error( + "Failed decoding {key_type} key from base64: \ + Invalid number of bytes for {key_type}, expected {expected_length}, got {length}." + )] + InvalidKeyLength { key_type: &'static str, expected_length: usize, length: usize }, #[error(transparent)] Signature(#[from] SignatureError), /// At least one of the keys did not have contributory behaviour and the