Skip to content

Commit

Permalink
fix: Use a constant-time Base64 encoder for secret key material
Browse files Browse the repository at this point in the history
This patch fixes a security issue around a side-channel vulnerability[1]
when decoding secret key material using Base64.

In some circumstances an attacker can obtain information about secret
secret key material via a controlled-channel and side-channel attack.

This patch avoids the side-channel by switching to the base64ct crate
for the encoding, and more importantly, the decoding of secret key
material.

[1]: https://arxiv.org/abs/2108.04600
  • Loading branch information
poljar committed May 20, 2024
1 parent f0a897b commit 77765da
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 12 deletions.
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ low-level-api = []
aes = "0.8.4"
arrayvec = { version = "0.7.4", features = ["serde"] }
base64 = "0.22.1"
base64ct = { version = "1.6.0", features = ["std", "alloc"] }
cbc = { version = "0.1.2", features = ["std"] }
chacha20poly1305 = "0.10.1"
curve25519-dalek = { version = "4.1.2", default-features = false, features = ["zeroize"] }
Expand Down
16 changes: 7 additions & 9 deletions src/megolm/session_keys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,13 @@

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

use base64ct::Encoding;
use serde::{Deserialize, Serialize};
use thiserror::Error;
use zeroize::Zeroize;

use super::ratchet::Ratchet;
use crate::{
utilities::{base64_decode, base64_encode},
Ed25519PublicKey, Ed25519Signature, SignatureError,
};
use crate::{Ed25519PublicKey, Ed25519Signature, SignatureError};

/// Error type describing failure modes for the `SessionKey` and
/// `ExportedSessionKey` decoding.
Expand All @@ -36,7 +34,7 @@ pub enum SessionKeyDecodeError {
Read(#[from] std::io::Error),
/// The encoded session key wasn't valid base64.
#[error("The session key wasn't valid base64: {0}")]
Base64(#[from] base64::DecodeError),
Base64(#[from] base64ct::Error),
/// The signature on the session key was invalid.
#[error("The signature on the session key was invalid: {0}")]
Signature(#[from] SignatureError),
Expand Down Expand Up @@ -93,7 +91,7 @@ impl ExportedSessionKey {
pub fn to_base64(&self) -> String {
let mut bytes = self.to_bytes();

let ret = base64_encode(&bytes);
let ret = base64ct::Base64Unpadded::encode_string(&bytes);

bytes.zeroize();

Expand All @@ -102,7 +100,7 @@ impl ExportedSessionKey {

/// Deserialize the `ExportedSessionKey` from base64 encoded string.
pub fn from_base64(key: &str) -> Result<Self, SessionKeyDecodeError> {
let mut bytes = base64_decode(key)?;
let mut bytes = base64ct::Base64Unpadded::decode_vec(key)?;
let ret = Self::from_bytes(&bytes);

bytes.zeroize();
Expand Down Expand Up @@ -268,7 +266,7 @@ impl SessionKey {
/// to a string using unpadded base64 as the encoding.
pub fn to_base64(&self) -> String {
let mut bytes = self.to_bytes();
let ret = base64_encode(&bytes);
let ret = base64ct::Base64Unpadded::encode_string(&bytes);

bytes.zeroize();

Expand All @@ -277,7 +275,7 @@ impl SessionKey {

/// Deserialize the `SessionKey` from base64 encoded string.
pub fn from_base64(key: &str) -> Result<Self, SessionKeyDecodeError> {
let mut bytes = base64_decode(key)?;
let mut bytes = base64ct::Base64Unpadded::decode_vec(key)?;
let ret = Self::from_bytes(&bytes);

bytes.zeroize();
Expand Down
14 changes: 11 additions & 3 deletions src/types/ed25519.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use std::fmt::Display;

use base64::decoded_len_estimate;
use base64ct::Encoding;
use curve25519_dalek::EdwardsPoint;
#[cfg(not(fuzzing))]
use ed25519_dalek::Verifier;
Expand Down Expand Up @@ -226,7 +227,7 @@ impl Ed25519SecretKey {
/// otherwise an unintentional copy of the key might exist in memory.
pub fn to_base64(&self) -> String {
let mut bytes = self.to_bytes();
let ret = base64_encode(bytes.as_ref());
let ret = base64ct::Base64Unpadded::encode_string(bytes.as_ref());

bytes.zeroize();

Expand All @@ -242,9 +243,16 @@ impl Ed25519SecretKey {
length: decoded_len_estimate(input.len()),
})
} else {
let mut bytes = base64_decode(input)?;
let mut key_bytes = [0u8; 32];
// Ed25519 secret keys can sometimes be encoded with padding, don't ask me why.
// This means that if the unpadded decoding fails, we have to attempt the padded
// one.
let mut bytes = if let Ok(bytes) = base64ct::Base64Unpadded::decode_vec(input) {
bytes
} else {
base64ct::Base64::decode_vec(input)?
};

let mut key_bytes = [0u8; 32];
key_bytes.copy_from_slice(&bytes);
let key = Self::from_slice(&key_bytes);

Expand Down
2 changes: 2 additions & 0 deletions src/types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ impl KeyId {
pub enum KeyError {
#[error("Failed decoding a public key from base64: {}", .0)]
Base64Error(#[from] base64::DecodeError),
#[error("Failed to decode a private key from base64: {}", .0)]
Base64PrivateKey(#[from] base64ct::Error),
#[error(
"Failed decoding {key_type} key from base64: \
Invalid number of bytes for {key_type}, expected {expected_length}, got {length}."
Expand Down

0 comments on commit 77765da

Please sign in to comment.