From 275d8f0b96d6a3e30415b98012343a4bb9ce7250 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 25 Jan 2024 05:53:05 +0000 Subject: [PATCH] Reject weak `ssh-rsa` keys --- age/CHANGELOG.md | 9 ++++++++- age/i18n/en-US/age.ftl | 2 ++ age/src/cli_common/error.rs | 10 +++++++++- age/src/cli_common/recipients.rs | 10 ++++++++-- age/src/ssh/recipient.rs | 18 +++++++++++++----- rage/CHANGELOG.md | 1 + 6 files changed, 41 insertions(+), 9 deletions(-) diff --git a/age/CHANGELOG.md b/age/CHANGELOG.md index 8aa2fbce..ab76a5d0 100644 --- a/age/CHANGELOG.md +++ b/age/CHANGELOG.md @@ -42,8 +42,11 @@ to 1.0.0 are beta releases. - `MissingRecipientsFile` - `MultipleStdin` - `RsaModulusTooLarge` + - `RsaModulusTooSmall` - `age::ssh`: - - `ParseRecipientKeyError` has a new variant `RsaModulusTooLarge`. + - `ParseRecipientKeyError` has new variants: + - `RsaModulusTooLarge` + - `RsaModulusTooSmall` - The following trait implementations now return `Err(ParseRecipientKeyError::RsaModulusTooLarge)` instead of `Err(ParseRecipientKeyError::Unsupported(_))` when encountering an RSA @@ -59,6 +62,10 @@ to 1.0.0 are beta releases. before it will accept a response. - `UiCallbacks::request_public_string` no longer prepends the description to the response string. +- Weak `ssh-rsa` public keys that are smaller than 2048 bits are now rejected + from all string-parsing APIs. The `Recipient::SshRsa` enum variant can still + be manually constructed with such keys; this will be fixed in a future crate + refactor. ## [0.9.2] - 2023-06-12 ### Added diff --git a/age/i18n/en-US/age.ftl b/age/i18n/en-US/age.ftl index f482825e..f98395b6 100644 --- a/age/i18n/en-US/age.ftl +++ b/age/i18n/en-US/age.ftl @@ -86,6 +86,8 @@ err-read-rsa-modulus-too-large = keys of at most {$max_size} bits, to prevent a Denial of Service (DoS) condition when encrypting to untrusted public keys. +err-read-rsa-modulus-too-small = RSA key size is too small. + err-stream-last-chunk-empty = Last STREAM chunk is empty. Please report this, and/or try an older {-rage} version. ## Encrypted identities diff --git a/age/src/cli_common/error.rs b/age/src/cli_common/error.rs index 5242a8de..0ae80f29 100644 --- a/age/src/cli_common/error.rs +++ b/age/src/cli_common/error.rs @@ -34,10 +34,14 @@ pub enum ReadError { MissingRecipientsFile(String), /// Standard input was used by multiple files. MultipleStdin, - /// A recipient is an `ssh-rsa`` public key with a modulus larger than we support. + /// A recipient is an `ssh-rsa` public key with a modulus larger than we support. #[cfg(feature = "ssh")] #[cfg_attr(docsrs, doc(cfg(feature = "ssh")))] RsaModulusTooLarge, + /// A recipient is a weak `ssh-rsa` public key with a modulus smaller than 2048 bits. + #[cfg(feature = "ssh")] + #[cfg_attr(docsrs, doc(cfg(feature = "ssh")))] + RsaModulusTooSmall, /// The given identity file contains an SSH key that we know how to parse, but that we /// do not support. #[cfg(feature = "ssh")] @@ -98,6 +102,10 @@ impl fmt::Display for ReadError { wfl!(f, "err-read-rsa-modulus-too-large", max_size = 4096) } #[cfg(feature = "ssh")] + ReadError::RsaModulusTooSmall => { + wfl!(f, "err-read-rsa-modulus-too-small") + } + #[cfg(feature = "ssh")] ReadError::UnsupportedKey(filename, k) => k.display(f, Some(filename.as_str())), } } diff --git a/age/src/cli_common/recipients.rs b/age/src/cli_common/recipients.rs index 6792b9e4..80c46206 100644 --- a/age/src/cli_common/recipients.rs +++ b/age/src/cli_common/recipients.rs @@ -33,6 +33,7 @@ where ParseRecipientKeyError::Ignore => Ok(None), ParseRecipientKeyError::Invalid(_) => invalid(), ParseRecipientKeyError::RsaModulusTooLarge => Err(ReadError::RsaModulusTooLarge), + ParseRecipientKeyError::RsaModulusTooSmall => Err(ReadError::RsaModulusTooSmall), ParseRecipientKeyError::Unsupported(key_type) => Err(ReadError::UnsupportedKey( filename.to_string(), UnsupportedKey::Type(key_type), @@ -84,8 +85,13 @@ fn read_recipients_list( continue; } else if let Err(e) = parse_recipient(filename, line, recipients, plugin_recipients) { #[cfg(feature = "ssh")] - if matches!(e, ReadError::UnsupportedKey(_, _)) { - return Err(io::Error::new(io::ErrorKind::InvalidData, e.to_string()).into()); + match e { + ReadError::RsaModulusTooLarge + | ReadError::RsaModulusTooSmall + | ReadError::UnsupportedKey(_, _) => { + return Err(io::Error::new(io::ErrorKind::InvalidData, e.to_string()).into()); + } + _ => (), } // Return a line number in place of the line, so we don't leak the file diff --git a/age/src/ssh/recipient.rs b/age/src/ssh/recipient.rs index 0f796441..26088ee4 100644 --- a/age/src/ssh/recipient.rs +++ b/age/src/ssh/recipient.rs @@ -16,7 +16,7 @@ use nom::{ IResult, }; use rand::rngs::OsRng; -use rsa::Oaep; +use rsa::{traits::PublicKeyParts, Oaep}; use sha2::Sha256; use std::fmt; use x25519_dalek::{EphemeralSecret, PublicKey as X25519PublicKey, StaticSecret}; @@ -44,6 +44,7 @@ pub enum Recipient { pub(crate) enum ParsedRecipient { Supported(Recipient), RsaModulusTooLarge, + RsaModulusTooSmall, Unsupported(String), } @@ -56,8 +57,10 @@ pub enum ParseRecipientKeyError { Ignore, /// The string is not a valid SSH recipient. Invalid(&'static str), - /// The string is an ssh-rsa public key with a modulus larger than we support. + /// The string is an `ssh-rsa` public key with a modulus larger than we support. RsaModulusTooLarge, + /// The string is a weak `ssh-rsa` public key with a modulus smaller than 2048 bits. + RsaModulusTooSmall, /// The string is a parseable value that corresponds to an unsupported SSH key type. Unsupported(String), } @@ -72,6 +75,9 @@ impl std::str::FromStr for Recipient { Ok((_, ParsedRecipient::RsaModulusTooLarge)) => { Err(ParseRecipientKeyError::RsaModulusTooLarge) } + Ok((_, ParsedRecipient::RsaModulusTooSmall)) => { + Err(ParseRecipientKeyError::RsaModulusTooSmall) + } Ok((_, ParsedRecipient::Unsupported(key_type))) => { Err(ParseRecipientKeyError::Unsupported(key_type)) } @@ -201,9 +207,11 @@ fn ssh_rsa_pubkey(max_size: usize) -> impl Fn(&str) -> IResult<&str, ParsedRecip map_opt( str_while_encoded(BASE64_STANDARD_NO_PAD), |ssh_key| match read_ssh::rsa_pubkey(max_size)(&ssh_key) { - Ok((_, Some(pk))) => { - Some(ParsedRecipient::Supported(Recipient::SshRsa(ssh_key, pk))) - } + Ok((_, Some(pk))) => Some(if pk.n().bits() < 2048 { + ParsedRecipient::RsaModulusTooSmall + } else { + ParsedRecipient::Supported(Recipient::SshRsa(ssh_key, pk)) + }), Ok((_, None)) => Some(ParsedRecipient::RsaModulusTooLarge), Err(_) => None, }, diff --git a/rage/CHANGELOG.md b/rage/CHANGELOG.md index ab30774b..e97638de 100644 --- a/rage/CHANGELOG.md +++ b/rage/CHANGELOG.md @@ -29,6 +29,7 @@ to 1.0.0 are beta releases. ### Fixed - OpenSSH private keys passed to `-i/--identity` that contain invalid public keys are no longer ignored when encrypting, and instead cause an error. +- Weak `ssh-rsa` public keys that are smaller than 2048 bits are now rejected. - `rage-keygen` no longer overwrites existing key files with the `-o/--output` flag. This was its behaviour prior to 0.6.0, but was unintentionally changed when `rage` was modified to overwrite existing files. Key file overwriting can