From c84da370304bcee8f1e8d4d395d88451e40b18b5 Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Tue, 23 Jul 2024 21:25:25 -0400 Subject: [PATCH] rpc: Add support for OAEP-based encryption format (#15058) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR adds support for a new encryption format for exchanging access tokens during the authentication flow. The new format uses Optimal Asymmetric Encryption Padding (OAEP) instead of PKCS#1 v1.5, which is known to be vulnerable to side-channel attacks. **Note: We are not yet encrypting access tokens using the new format, as this is a breaking change between the client and the server. This PR only adds support for it, and makes it so the client and server can decrypt either format moving forward.** This required bumping the RSA key size from 1024 bits to 2048 bits. This is necessary to be able to encode the access token into the ciphertext when using OAEP. This also follows OWASP recommendations: > If ECC is not available and RSA must be used, then ensure that the key is at least 2048 bits. > > — [source](https://cheatsheetseries.owasp.org/cheatsheets/Cryptographic_Storage_Cheat_Sheet.html#algorithms) Release Notes: - N/A --- Cargo.lock | 1 + crates/collab/src/auth.rs | 13 ++++++- crates/rpc/Cargo.toml | 1 + crates/rpc/src/auth.rs | 77 +++++++++++++++++++++++++++++++++------ 4 files changed, 79 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3f6cc254d2..996f3b44a7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8886,6 +8886,7 @@ dependencies = [ "rsa", "serde", "serde_json", + "sha2 0.10.7", "strum", "tracing", "util", diff --git a/crates/collab/src/auth.rs b/crates/collab/src/auth.rs index 14c07742d4..cf0ba6b173 100644 --- a/crates/collab/src/auth.rs +++ b/crates/collab/src/auth.rs @@ -164,10 +164,21 @@ pub fn hash_access_token(token: &str) -> String { /// Encrypts the given access token with the given public key to avoid leaking it on the way /// to the client. pub fn encrypt_access_token(access_token: &str, public_key: String) -> Result { + use rpc::auth::EncryptionFormat; + + /// The encryption format to use for the access token. + /// + /// Currently we're using the original encryption format to avoid + /// breaking compatibility with older clients. + /// + /// Once enough clients are capable of decrypting the newer encryption + /// format we can start encrypting with `EncryptionFormat::V1`. + const ENCRYPTION_FORMAT: EncryptionFormat = EncryptionFormat::V0; + let native_app_public_key = rpc::auth::PublicKey::try_from(public_key).context("failed to parse app public key")?; let encrypted_access_token = native_app_public_key - .encrypt_string(access_token) + .encrypt_string(access_token, ENCRYPTION_FORMAT) .context("failed to encrypt access token with public key")?; Ok(encrypted_access_token) } diff --git a/crates/rpc/Cargo.toml b/crates/rpc/Cargo.toml index ddb23fb3bf..79cb811afa 100644 --- a/crates/rpc/Cargo.toml +++ b/crates/rpc/Cargo.toml @@ -30,6 +30,7 @@ rand.workspace = true rsa.workspace = true serde.workspace = true serde_json.workspace = true +sha2.workspace = true strum.workspace = true tracing = { version = "0.1.34", features = ["log"] } util.workspace = true diff --git a/crates/rpc/src/auth.rs b/crates/rpc/src/auth.rs index c3d16ab0c7..76d95b02ed 100644 --- a/crates/rpc/src/auth.rs +++ b/crates/rpc/src/auth.rs @@ -1,9 +1,29 @@ use anyhow::{Context, Result}; use rand::{thread_rng, Rng as _}; use rsa::pkcs1::{DecodeRsaPublicKey, EncodeRsaPublicKey}; -use rsa::{Pkcs1v15Encrypt, RsaPrivateKey, RsaPublicKey}; +use rsa::traits::PaddingScheme; +use rsa::{Oaep, Pkcs1v15Encrypt, RsaPrivateKey, RsaPublicKey}; +use sha2::Sha256; use std::convert::TryFrom; +fn oaep_sha256_padding() -> impl PaddingScheme { + Oaep::new::() +} + +#[derive(Debug, PartialEq, Eq, Clone, Copy)] +pub enum EncryptionFormat { + /// The original encryption format. + /// + /// This is using [`Pkcs1v15Encrypt`], which is vulnerable to side-channel attacks. + /// As such, we're in the process of phasing it out. + /// + /// See [here](https://people.redhat.com/~hkario/marvin/) for more details. + V0, + + /// The new encryption key format using Optimal Asymmetric Encryption Padding (OAEP) with a SHA-256 digest. + V1, +} + pub struct PublicKey(RsaPublicKey); pub struct PrivateKey(RsaPrivateKey); @@ -11,7 +31,7 @@ pub struct PrivateKey(RsaPrivateKey); /// Generate a public and private key for asymmetric encryption. pub fn keypair() -> Result<(PublicKey, PrivateKey)> { let mut rng = thread_rng(); - let bits = 1024; + let bits = 2048; let private_key = RsaPrivateKey::new(&mut rng, bits)?; let public_key = RsaPublicKey::from(&private_key); Ok((PublicKey(public_key), PrivateKey(private_key))) @@ -30,13 +50,14 @@ pub fn random_token() -> String { impl PublicKey { /// Convert a string to a base64-encoded string that can only be decoded with the corresponding /// private key. - pub fn encrypt_string(&self, string: &str) -> Result { + pub fn encrypt_string(&self, string: &str, format: EncryptionFormat) -> Result { let mut rng = thread_rng(); let bytes = string.as_bytes(); - let encrypted_bytes = self - .0 - .encrypt(&mut rng, PADDING_SCHEME, bytes) - .context("failed to encrypt string with public key")?; + let encrypted_bytes = match format { + EncryptionFormat::V0 => self.0.encrypt(&mut rng, Pkcs1v15Encrypt, bytes), + EncryptionFormat::V1 => self.0.encrypt(&mut rng, oaep_sha256_padding(), bytes), + } + .context("failed to encrypt string with public key")?; let encrypted_string = base64::encode_config(&encrypted_bytes, base64::URL_SAFE); Ok(encrypted_string) } @@ -49,7 +70,12 @@ impl PrivateKey { .context("failed to base64-decode encrypted string")?; let bytes = self .0 - .decrypt(PADDING_SCHEME, &encrypted_bytes) + .decrypt(oaep_sha256_padding(), &encrypted_bytes) + .or_else(|_err| { + // If we failed to decrypt using the new format, try decrypting with the old + // one to handle mismatches between the client and server. + self.0.decrypt(Pkcs1v15Encrypt, &encrypted_bytes) + }) .context("failed to decrypt string with private key")?; let string = String::from_utf8(bytes).context("decrypted content was not valid utf8")?; Ok(string) @@ -78,8 +104,6 @@ impl TryFrom for PublicKey { } } -const PADDING_SCHEME: Pkcs1v15Encrypt = Pkcs1v15Encrypt; - #[cfg(test)] mod tests { use super::*; @@ -99,7 +123,34 @@ mod tests { // * encrypt the token using the public key. let public = PublicKey::try_from(public_string).unwrap(); let token = random_token(); - let encrypted_token = public.encrypt_string(&token).unwrap(); + let encrypted_token = public.encrypt_string(&token, EncryptionFormat::V1).unwrap(); + assert_eq!(token.len(), 64); + assert_ne!(encrypted_token, token); + assert_printable(&token); + assert_printable(&encrypted_token); + + // CLIENT: + // * decrypt the token using the private key. + let decrypted_token = private.decrypt_string(&encrypted_token).unwrap(); + assert_eq!(decrypted_token, token); + } + + #[test] + fn test_generate_encrypt_and_decrypt_token_with_v0_encryption_format() { + // CLIENT: + // * generate a keypair for asymmetric encryption + // * serialize the public key to send it to the server. + let (public, private) = keypair().unwrap(); + let public_string = String::try_from(public).unwrap(); + assert_printable(&public_string); + + // SERVER: + // * parse the public key + // * generate a random token. + // * encrypt the token using the public key. + let public = PublicKey::try_from(public_string).unwrap(); + let token = random_token(); + let encrypted_token = public.encrypt_string(&token, EncryptionFormat::V0).unwrap(); assert_eq!(token.len(), 64); assert_ne!(encrypted_token, token); assert_printable(&token); @@ -130,7 +181,9 @@ mod tests { for _ in 0..5 { let token = random_token(); let (public_key, _) = keypair().unwrap(); - let encrypted_token = public_key.encrypt_string(&token).unwrap(); + let encrypted_token = public_key + .encrypt_string(&token, EncryptionFormat::V1) + .unwrap(); let public_key_str = String::try_from(public_key).unwrap(); assert_printable(&token);