ssh-key: fix parsing comments with whitespace in AuthorizedKeys (#289)

RustCrypto · Sep 5, 2024 · bac2931 · bac2931
1 parent 76ac260
commit bac2931
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 13 deletions.
diff --git a/ssh-key/src/authorized_keys.rs b/ssh-key/src/authorized_keys.rs
@@ -140,25 +140,37 @@ impl str::FromStr for Entry {
     type Err = Error;
 
     fn from_str(line: &str) -> Result<Self> {
-        // TODO(tarcieri): more liberal whitespace handling?
         match line.matches(' ').count() {
             1..=2 => Ok(Self {
                 #[cfg(feature = "alloc")]
                 config_opts: Default::default(),
                 public_key: line.parse()?,
             }),
-            3.. => line
-                .split_once(' ')
-                .map(|(config_opts_str, public_key_str)| {
-                    ConfigOptsIter(config_opts_str).validate()?;
-
-                    Ok(Self {
+            3.. => {
+                // Having >= 3 spaces is ambiguous: it's either a key preceded
+                // by options, or a key with spaces in its comment.  We'll try
+                // parsing as a single key first, then fall back to parsing as
+                // option + key.
+                match line.parse() {
+                    Ok(public_key) => Ok(Self {
                         #[cfg(feature = "alloc")]
-                        config_opts: ConfigOpts(config_opts_str.to_string()),
-                        public_key: public_key_str.parse()?,
-                    })
-                })
-                .ok_or(Error::FormatEncoding)?,
+                        config_opts: Default::default(),
+                        public_key,
+                    }),
+                    Err(_) => line
+                        .split_once(' ')
+                        .map(|(config_opts_str, public_key_str)| {
+                            ConfigOptsIter(config_opts_str).validate()?;
+
+                            Ok(Self {
+                                #[cfg(feature = "alloc")]
+                                config_opts: ConfigOpts(config_opts_str.to_string()),
+                                public_key: public_key_str.parse()?,
+                            })
+                        })
+                        .ok_or(Error::FormatEncoding)?,
+                }
+            }
             _ => Err(Error::FormatEncoding),
         }
     }

diff --git a/ssh-key/tests/authorized_keys.rs b/ssh-key/tests/authorized_keys.rs
@@ -8,7 +8,7 @@ use ssh_key::AuthorizedKeys;
 #[test]
 fn read_example_file() {
     let authorized_keys = AuthorizedKeys::read_file("./tests/examples/authorized_keys").unwrap();
-    assert_eq!(authorized_keys.len(), 4);
+    assert_eq!(authorized_keys.len(), 5);
 
     assert_eq!(authorized_keys[0].config_opts().to_string(), "");
     assert_eq!(
@@ -46,4 +46,10 @@ fn read_example_file() {
         authorized_keys[3].public_key().comment(),
         "[email protected]"
     );
+
+    assert_eq!(authorized_keys[4].public_key().to_string(), "ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBN76zuqnjypL54/w4763l7q1Sn3IBYHptJ5wcYfEWkzeNTvpexr05Z18m2yPT2SWRd1JJ8Aj5TYidG9MdSS5J78= hello world this is a long comment");
+    assert_eq!(
+        authorized_keys[4].public_key().comment(),
+        "hello world this is a long comment"
+    );
 }
diff --git a/ssh-key/tests/examples/authorized_keys b/ssh-key/tests/examples/authorized_keys
@@ -26,3 +26,6 @@ environment="PATH=/bin:/usr/bin" ssh-dss AAAAB3NzaC1kc3MAAACBANw9iSUO2UYhFMssjUg
 
 # Public key which can only be used from certain source addresses and disallows X11 forwarding
 from="10.0.0.?,*.example.com",no-X11-forwarding ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACAQC0WRHtxuxefSJhpIxGq4ibGFgwYnESPm8C3JFM88A1JJLoprenklrd7VJ+VH3Ov/bQwZwLyRU5dRmfR/SWTtIPWs7tToJVayKKDB+/qoXmM5ui/0CU2U4rCdQ6PdaCJdC7yFgpPL8WexjWN06+eSIKYz1AAXbx9rRv1iasslK/KUqtsqzVliagI6jl7FPO2GhRZMcso6LsZGgSxuYf/Lp0D/FcBU8GkeOo1Sx5xEt8H8bJcErtCe4Blb8JxcW6EXO3sReb4z+zcR07gumPgFITZ6hDA8sSNuvo/AlWg0IKTeZSwHHVknWdQqDJ0uczE837caBxyTZllDNIGkBjCIIOFzuTT76HfYc/7CTTGk07uaNkUFXKN79xDiFOX8JQ1ZZMZvGOTwWjuT9CqgdTvQRORbRWwOYv3MH8re9ykw3Ip6lrPifY7s6hOaAKry/nkGPMt40m1TdiW98MTIpooE7W+WXu96ax2l2OJvxX8QR7l+LFlKnkIEEJd/ItF1G22UmOjkVwNASTwza/hlY+8DoVvEmwum/nMgH2TwQT3bTQzF9s9DOJkH4d8p4Mw4gEDjNx0EgUFA91ysCAeUMQQyIvuR8HXXa+VcvhOOO5mmBcVhxJ3qUOJTyDBsT0932Zb4mNtkxdigoVxu+iiwk0vwtvKwGVDYdyMP5EAQeEIP1t0w== [email protected]
+
+# Public key with a comment that contains multiple spaces
+ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBN76zuqnjypL54/w4763l7q1Sn3IBYHptJ5wcYfEWkzeNTvpexr05Z18m2yPT2SWRd1JJ8Aj5TYidG9MdSS5J78= hello world this is a long comment