Merge pull request #465 from str4d/reject-weak-ssh-rsa

Reject weak `ssh-rsa` keys

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

age/i18n/en-US/age.ftl

@@ -94,6 +94,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

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())),
         }
     }

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::from_key_type(key_type),
@@ -84,8 +85,13 @@ fn read_recipients_list<R: io::BufRead>(
             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

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,
                 },

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