Update namespaces parsing

obelisk · May 4, 2024 · 1487bc2 · 1487bc2
1 parent f529293
commit 1487bc2
Show file tree

Hide file tree

Showing 4 changed files with 105 additions and 20 deletions.
diff --git a/src/ssh/allowed_signer.rs b/src/ssh/allowed_signer.rs
@@ -61,7 +61,6 @@ impl AllowedSigner {
     pub fn from_string(s: &str) -> Result<AllowedSigner> {
         let (mut head, mut rest) = s.split_once(char::is_whitespace)
             .ok_or(Error::InvalidAllowedSigner("missing key data".to_string()))?;
-        rest = rest.trim_start();
 
         let principals: Vec<&str> = head.split(',').collect();
         if principals.iter().any(|p| p.is_empty()) {
@@ -74,9 +73,11 @@ impl AllowedSigner {
         let mut valid_after = None;
         let mut valid_before = None;
 
+        // We need to trim here and below since split_once(char::is_whitespace) treats
+        // consecutive whitespaces as separate delimiters
+        rest = rest.trim_start();
         (head, rest) = rest.split_once(char::is_whitespace)
                 .ok_or(Error::InvalidAllowedSigner("missing key data".to_string()))?;
-        rest = rest.trim_start();
         loop {
             // Check if this is a valid option
             let (option_key, option_value) = match head.split_once('=') {
@@ -89,12 +90,14 @@ impl AllowedSigner {
                     if namespaces.is_some() {
                         return Err(Error::InvalidAllowedSigner("multiple \"namespaces\" clauses".to_string()));
                     }
-                    let option_value = dequote_option(option_value)
-                        .map_err(|_| Error::InvalidAllowedSigner("invalid \"namespaces\" clause".to_string()))?;
-                    let namespaces_inner: Vec<&str> = option_value.split(',')
+
+                    let (namespaces_value, rest_) = parse_namespaces(option_value, &mut rest)?;
+                    rest = rest_;
+                    let namespaces_value: Vec<&str> = namespaces_value.split(',')
+                        .filter(|e| !e.is_empty())
                         .collect();
                     namespaces = Some(
-                        namespaces_inner.iter()
+                        namespaces_value.iter()
                             .map(|s| s.to_string())
                             .collect()
                     );
@@ -117,13 +120,14 @@ impl AllowedSigner {
                 _ => break,
             };
 
+            rest = rest.trim_start();
             (head, rest) = rest.split_once(char::is_whitespace)
                 .ok_or(Error::InvalidAllowedSigner("missing key data".to_string()))?;
-            rest = rest.trim_start();
         }
 
         let kt = head;
 
+        rest = rest.trim_start();
         (head, rest) = match rest.split_once(char::is_whitespace) {
             Some(v) => v,
             None => (rest, ""),
@@ -242,20 +246,47 @@ fn parse_timestamp(s: &str) -> Result<u64> {
     Ok(s.parse::<u64>().map_err(|_| Error::InvalidFormat)?)
 }
 
-/// Remove enclosing pair of double quotes if applicable.
-/// If enclosing double quotes are malformed, return error.
-/// If the content inside the quotes contains quotes, return error.
-fn dequote_option(s: &str) -> Result<String> {
-    let mut s = s.to_string();
-    if s.starts_with('"') &&  s.ends_with('"') {
-        s = s[1..s.len() - 1].to_string();
-    } else if s.starts_with('"') || s.ends_with('"') {
-        return Err(Error::InvalidFormat);
+/// Parse the namespaces value.
+/// We have `namespaces=<first_part> <rest>`.
+/// If `first_part` starts with " but does not have a closing ", we will try to find the closing "
+/// in `rest`.
+fn parse_namespaces<'a>(first_part: &str, rest: &'a str) -> Result<(String, &'a str)> {
+    let mut rest = rest;
+    if !first_part.starts_with('"') {
+        if first_part.contains('"') {
+            return Err(Error::InvalidAllowedSigner("invalid \"namespaces\" clause".to_string()));
+        } else {
+            return Ok((first_part.to_string(), rest));
+        }
     }
 
-    if s.contains('"') {
-        return Err(Error::InvalidFormat);
+    // Here, we begins dequoting
+    // First, we remove the opening "
+    let (_, first_part) = first_part.split_once('"')
+        .ok_or(Error::InvalidAllowedSigner("invalid \"namespaces\" clause".to_string()))?;
+
+    // We find the closing " in first_part
+    let namespaces_value = if let Some(v) = first_part.split_once('"') {
+        if !v.1.is_empty() {
+            return Err(Error::InvalidAllowedSigner("invalid \"namespaces\" clause".to_string()));
+        }
+        v.0.to_string()
+    } else {
+        let (second_part, rest_) = rest.split_once('"')
+            .ok_or(Error::InvalidAllowedSigner("invalid \"namespaces\" clause".to_string()))?;
+        // There must be spaces after the closing "
+        if !rest_.starts_with(char::is_whitespace) {
+            return Err(Error::InvalidAllowedSigner("invalid \"namespaces\" clause".to_string()));
+        }
+        rest = rest_.trim_start();
+
+        format!("{} {}", first_part, second_part)
+    };
+
+    // There should be no " after dequoting
+    if namespaces_value.contains('"') {
+        return Err(Error::InvalidAllowedSigner("invalid \"namespaces\" clause".to_string()));
     }
 
-    Ok(s)
+    Ok((namespaces_value, rest))
 }
diff --git a/tests/allowed-signer.rs b/tests/allowed-signer.rs
@@ -26,6 +26,7 @@ fn parse_good_allowed_signer_with_options() {
     let allowed_signer =
         "mitchell@confurious.io,mitchel2@confurious.io cert-authority namespaces=\"thanh,mitchell\" valid-before=\"123\" ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIDO0VQD9TIdICZLWFWwtf7s8/aENve8twGTEmNV0myh5";
     let allowed_signer = AllowedSigner::from_string(allowed_signer);
+    println!("{:?}", allowed_signer);
     assert!(allowed_signer.is_ok());
     let allowed_signer = allowed_signer.unwrap();
     assert_eq!(allowed_signer.key.fingerprint().to_string(), "SHA256:QAtqtvvCePelMMUNPP7madH2zNa1ATxX1nt9L/0C5+M");
@@ -63,6 +64,48 @@ fn parse_good_allowed_signer_with_consecutive_spaces() {
     assert_eq!(allowed_signer.valid_before, Some(123u64));
 }
 
+#[test]
+fn parse_good_allowed_signer_with_empty_namespaces() {
+    let allowed_signer =
+        "mitchell@confurious.io,mitchel2@confurious.io cert-authority namespaces=\"thanh,,mitchell\" valid-before=\"123\" ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIDO0VQD9TIdICZLWFWwtf7s8/aENve8twGTEmNV0myh5";
+    let allowed_signer = AllowedSigner::from_string(allowed_signer);
+    assert!(allowed_signer.is_ok());
+    let allowed_signer = allowed_signer.unwrap();
+    assert_eq!(allowed_signer.key.fingerprint().to_string(), "SHA256:QAtqtvvCePelMMUNPP7madH2zNa1ATxX1nt9L/0C5+M");
+    assert_eq!(
+        allowed_signer.principals,
+        vec!["mitchell@confurious.io".to_string(), "mitchel2@confurious.io".to_string()],
+    );
+    assert!(allowed_signer.cert_authority);
+    assert_eq!(
+        allowed_signer.namespaces, 
+        Some(vec!["thanh".to_string(), "mitchell".to_string()])
+    );
+    assert!(allowed_signer.valid_after.is_none());
+    assert_eq!(allowed_signer.valid_before, Some(123u64));
+}
+
+#[test]
+fn parse_good_allowed_signer_with_space_in_namespaces() {
+    let allowed_signer =
+        "mitchell@confurious.io,mitchel2@confurious.io cert-authority namespaces=\"thanh,mitchell   tech\" valid-before=\"123\" ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIDO0VQD9TIdICZLWFWwtf7s8/aENve8twGTEmNV0myh5";
+    let allowed_signer = AllowedSigner::from_string(allowed_signer);
+    assert!(allowed_signer.is_ok());
+    let allowed_signer = allowed_signer.unwrap();
+    assert_eq!(allowed_signer.key.fingerprint().to_string(), "SHA256:QAtqtvvCePelMMUNPP7madH2zNa1ATxX1nt9L/0C5+M");
+    assert_eq!(
+        allowed_signer.principals,
+        vec!["mitchell@confurious.io".to_string(), "mitchel2@confurious.io".to_string()],
+    );
+    assert!(allowed_signer.cert_authority);
+    assert_eq!(
+        allowed_signer.namespaces, 
+        Some(vec!["thanh".to_string(), "mitchell   tech".to_string()])
+    );
+    assert!(allowed_signer.valid_after.is_none());
+    assert_eq!(allowed_signer.valid_before, Some(123u64));
+}
+
 #[test]
 fn parse_bad_allowed_signer_with_wrong_key_type() {
     let allowed_signer =

diff --git a/tests/allowed-signers.rs b/tests/allowed-signers.rs
@@ -5,7 +5,7 @@ fn parse_good_allowed_signers() {
     let allowed_signers = AllowedSigners::from_path("tests/allowed_signers/good_allowed_signers");
     assert!(allowed_signers.is_ok());
     let AllowedSigners(allowed_signers) = allowed_signers.unwrap();
-    assert_eq!(allowed_signers.len(), 2);
+    assert_eq!(allowed_signers.len(), 3);
 
     assert_eq!(
         allowed_signers[0].key.fingerprint().to_string(),
@@ -31,4 +31,14 @@ fn parse_good_allowed_signers() {
     );
     assert!(allowed_signers[1].valid_after.is_none());
     assert_eq!(allowed_signers[1].valid_before, Some(123u64));
+
+    assert_eq!(
+        allowed_signers[2].namespaces, 
+        Some(vec![
+            "thanh".to_string(),
+            " ".to_string(),
+            "mitchell mitchell".to_string(),
+            " andrew   andrew".to_string(),
+        ]),
+    );
 }
diff --git a/tests/allowed_signers/good_allowed_signers b/tests/allowed_signers/good_allowed_signers
@@ -3,3 +3,4 @@ mitchell@confurious.io ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIDO0VQD9TIdICZLWFWwtf
 # Empty lines are also ignored
 
 mitchell@confurious.io,mitchel2@confurious.io cert-authority namespaces="thanh,#mitchell" valid-before="123" ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIDO0VQD9TIdICZLWFWwtf7s8/aENve8twGTEmNV0myh5  # End of line comment is ignored
+mitchell@confurious.io,mitchel2@confurious.io cert-authority namespaces="thanh, ,mitchell mitchell, andrew   andrew" valid-before="123" ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIDO0VQD9TIdICZLWFWwtf7s8/aENve8twGTEmNV0myh5
Original file line number	Diff line number	Diff line change
Expand Up		@@ -3,3 +3,4 @@ mitchell@confurious.io ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIDO0VQD9TIdICZLWFWwtf
		# Empty lines are also ignored

		mitchell@confurious.io,mitchel2@confurious.io cert-authority namespaces="thanh,#mitchell" valid-before="123" ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIDO0VQD9TIdICZLWFWwtf7s8/aENve8twGTEmNV0myh5 # End of line comment is ignored
		mitchell@confurious.io,mitchel2@confurious.io cert-authority namespaces="thanh, ,mitchell mitchell, andrew andrew" valid-before="123" ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIDO0VQD9TIdICZLWFWwtf7s8/aENve8twGTEmNV0myh5