Skip to content

Commit

Permalink
Add some key length checks when decoding from base64
Browse files Browse the repository at this point in the history
  • Loading branch information
poljar committed Jul 24, 2023
1 parent 5f93a17 commit 926a867
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 30 deletions.
32 changes: 24 additions & 8 deletions src/types/curve25519.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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] {
Expand All @@ -139,9 +143,17 @@ impl Curve25519PublicKey {

/// Instantiate a Curve25519 public key from an unpadded base64
/// representation.
pub fn from_base64(base64_key: &str) -> Result<Curve25519PublicKey, KeyError> {
let key = base64_decode(base64_key)?;
Self::from_slice(&key)
pub fn from_base64(input: &str) -> Result<Curve25519PublicKey, KeyError> {
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.
Expand All @@ -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,
})
}
}

Expand Down Expand Up @@ -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(..)))
Expand All @@ -251,7 +267,7 @@ mod tests {
let base64_payload = "aaaa";
assert!(matches!(
Curve25519PublicKey::from_base64(base64_payload),
Err(KeyError::InvalidKeyLength(..))
Err(KeyError::InvalidKeyLength { .. })
));
}

Expand Down
60 changes: 42 additions & 18 deletions src/types/ed25519.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

use std::fmt::Display;

use base64::decoded_len_estimate;
use curve25519_dalek::EdwardsPoint;
#[cfg(not(fuzzing))]
use ed25519_dalek::Verifier;
Expand Down Expand Up @@ -170,6 +171,12 @@ impl Default for Ed25519Keypair {
pub struct Ed25519SecretKey(Box<SigningKey>);

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();
Expand Down Expand Up @@ -201,18 +208,25 @@ impl Ed25519SecretKey {
}

/// Try to create a `Ed25519SecretKey` from a base64 encoded string.
pub fn from_base64(key: &str) -> Result<Self, crate::KeyError> {
// TODO: check the length.
let mut bytes = base64_decode(key)?;
let mut key_bytes = [0u8; 32];
pub fn from_base64(input: &str) -> Result<Self, crate::KeyError> {
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`.
Expand Down Expand Up @@ -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<Self, crate::KeyError> {
Ok(Self(VerifyingKey::from_bytes(bytes).map_err(SignatureError::from)?))
Expand All @@ -294,18 +311,25 @@ impl Ed25519PublicKey {

/// Instantiate a Ed25519PublicKey public key from an unpadded base64
/// representation.
pub fn from_base64(base64_key: &str) -> Result<Self, crate::KeyError> {
// TODO: check the length.
let mut bytes = base64_decode(base64_key)?;
let mut key_bytes = [0u8; 32];
pub fn from_base64(input: &str) -> Result<Self, crate::KeyError> {
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
Expand Down
9 changes: 5 additions & 4 deletions src/types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 926a867

Please sign in to comment.