From 77765dace11266ef9523301624a01265c6e0f790 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Damir=20Jeli=C4=87?= <poljar@termina.org.uk>
Date: Sat, 18 May 2024 11:02:08 +0200
Subject: [PATCH] fix: Use a constant-time Base64 encoder for secret key
 material

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
---
 Cargo.toml                 |  1 +
 src/megolm/session_keys.rs | 16 +++++++---------
 src/types/ed25519.rs       | 14 +++++++++++---
 src/types/mod.rs           |  2 ++
 4 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index 826df076..8ff86525 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -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"] }
diff --git a/src/megolm/session_keys.rs b/src/megolm/session_keys.rs
index b09ceb0b..d89d2046 100644
--- a/src/megolm/session_keys.rs
+++ b/src/megolm/session_keys.rs
@@ -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.
@@ -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),
@@ -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();
 
@@ -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();
@@ -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();
 
@@ -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();
diff --git a/src/types/ed25519.rs b/src/types/ed25519.rs
index 03e291d0..ce50c2ac 100644
--- a/src/types/ed25519.rs
+++ b/src/types/ed25519.rs
@@ -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;
@@ -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();
 
@@ -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);
 
diff --git a/src/types/mod.rs b/src/types/mod.rs
index 04a269d1..2755c460 100644
--- a/src/types/mod.rs
+++ b/src/types/mod.rs
@@ -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}."