Improve string validation (#1497)

server/lib/src/value.rs

@@ -54,19 +54,32 @@ lazy_static! {
         m.insert("administrator");
         m
     };
+
+    /// Only lowercase+numbers, with limited chars.
     pub static ref INAME_RE: Regex = {
         #[allow(clippy::expect_used)]
         Regex::new("^[a-z][a-z0-9-_\\.]+$").expect("Invalid Iname regex found")
-        // Only lowercase+numbers, with limited chars.
     };
     pub static ref NSUNIQUEID_RE: Regex = {
         #[allow(clippy::expect_used)]
         Regex::new("^[0-9a-fA-F]{8}-[0-9a-fA-F]{8}-[0-9a-fA-F]{8}-[0-9a-fA-F]{8}$").expect("Invalid Nsunique regex found")
     };
+
+    /// Must not contain whitespace.
     pub static ref OAUTHSCOPE_RE: Regex = {
         #[allow(clippy::expect_used)]
         Regex::new("^[0-9a-zA-Z_]+$").expect("Invalid oauthscope regex found")
-        // Must not contain whitespace.
+    };
+
+    pub static ref SINGLELINE_RE: Regex = {
+        #[allow(clippy::expect_used)]
+        Regex::new("[\n\r\t]").expect("Invalid singleline regex found")
+    };
+
+    pub static ref ESCAPES_RE: Regex = {
+        #[allow(clippy::expect_used)]
+        Regex::new(r"\x1b\[([\x30-\x3f]*[\x20-\x2f]*[\x40-\x7e])")
+            .expect("Invalid escapes regex found")
     };
 }
 
@@ -337,7 +350,6 @@ pub enum PartialValue {
     UiHint(UiHint),
     Passkey(Uuid),
     DeviceKey(Uuid),
-    TrustedDeviceEnrollment(Uuid),
     // The label, if any.
 }
 
@@ -707,7 +719,6 @@ impl PartialValue {
             PartialValue::Address(a) => a.to_string(),
             PartialValue::PhoneNumber(a) => a.to_string(),
             PartialValue::IntentToken(u) => u.clone(),
-            PartialValue::TrustedDeviceEnrollment(u) => u.as_hyphenated().to_string(),
             PartialValue::UiHint(u) => (*u as u16).to_string(),
         }
     }
@@ -847,7 +858,6 @@ pub enum Value {
     Passkey(Uuid, String, PasskeyV4),
     DeviceKey(Uuid, String, DeviceKeyV4),
 
-    TrustedDeviceEnrollment(Uuid),
     Session(Uuid, Session),
     ApiToken(Uuid, ApiToken),
     Oauth2Session(Uuid, Oauth2Session),
@@ -1546,13 +1556,6 @@ impl Value {
         }
     }
 
-    pub fn to_trusteddeviceenrollment(self) -> Option<(Uuid, ())> {
-        match self {
-            Value::TrustedDeviceEnrollment(u) => Some((u, ())),
-            _ => None,
-        }
-    }
-
     pub fn to_session(self) -> Option<(Uuid, Session)> {
         match self {
             Value::Session(u, s) => Some((u, s)),
@@ -1591,34 +1594,83 @@ impl Value {
         }
     }
 
-    // !!! relocate to value set !!!
     pub(crate) fn validate(&self) -> bool {
         // Validate that extra-data constraints on the type exist and are
         // valid. IE json filter is really a filter, or cred types have supplemental
         // data.
         match &self {
-            Value::Iname(s) => Value::validate_iname(s),
+            // String security is required here
-            /*
+            Value::Utf8(s)
-            Value::Cred(_) => match &self.data {
+            | Value::Iutf8(s)
-                Some(v) => matches!(v.as_ref(), DataValue::Cred(_)),
+            | Value::Cred(s, _)
-                None => false,
+            | Value::PublicBinary(s, _)
-            },
+            | Value::IntentToken(s, _)
-            */
+            | Value::Passkey(_, s, _)
-            Value::SshKey(_, key) => SshPublicKey::from_string(key).is_ok(),
+            | Value::DeviceKey(_, s, _)
+            | Value::TotpSecret(s, _) => {
+                Value::validate_str_escapes(s) && Value::validate_singleline(s)
+            }
+
+            Value::Spn(a, b) => {
+                Value::validate_str_escapes(a)
+                    && Value::validate_str_escapes(b)
+                    && Value::validate_singleline(a)
+                    && Value::validate_singleline(b)
+            }
+
+            Value::Iname(s) => {
+                Value::validate_str_escapes(s)
+                    && Value::validate_iname(s)
+                    && Value::validate_singleline(s)
+            }
+
+            Value::SshKey(s, key) => {
+                SshPublicKey::from_string(key).is_ok()
+                    && Value::validate_str_escapes(s)
+                    && Value::validate_singleline(s)
+            }
+
+            Value::ApiToken(_, at) => {
+                Value::validate_str_escapes(&at.label) && Value::validate_singleline(&at.label)
+            }
+
+            // These have stricter validators so not needed.
             Value::Nsuniqueid(s) => NSUNIQUEID_RE.is_match(s),
             Value::DateTime(odt) => odt.offset() == time::UtcOffset::UTC,
             Value::EmailAddress(mail, _) => validator::validate_email(mail.as_str()),
-            // PartialValue::Url validated through parsing.
             Value::OauthScope(s) => OAUTHSCOPE_RE.is_match(s),
             Value::OauthScopeMap(_, m) => m.iter().all(|s| OAUTHSCOPE_RE.is_match(s)),
-            _ => true,
+
+            Value::PhoneNumber(_, _) => true,
+            Value::Address(_) => true,
+
+            Value::Uuid(_)
+            | Value::Bool(_)
+            | Value::Syntax(_)
+            | Value::Index(_)
+            | Value::Refer(_)
+            | Value::JsonFilt(_)
+            | Value::SecretValue(_)
+            | Value::Uint32(_)
+            | Value::Url(_)
+            | Value::Cid(_)
+            | Value::PrivateBinary(_)
+            | Value::RestrictedString(_)
+            | Value::JwsKeyEs256(_)
+            | Value::Session(_, _)
+            | Value::Oauth2Session(_, _)
+            | Value::JwsKeyRs256(_)
+            | Value::UiHint(_) => true,
         }
     }
 
     pub(crate) fn validate_iname(s: &str) -> bool {
         match Uuid::parse_str(s) {
             // It is a uuid, disallow.
-            Ok(_) => false,
+            Ok(_) => {
+                warn!("iname values may not contain uuids");
+                false
+            }
             // Not a uuid, check it against the re.
             Err(_) => {
                 if !INAME_RE.is_match(s) {
@@ -1633,6 +1685,31 @@ impl Value {
             }
         }
     }
+
+    pub(crate) fn validate_singleline(s: &str) -> bool {
+        if !SINGLELINE_RE.is_match(s) {
+            true
+        } else {
+            warn!(
+                "value contains invalid whitespace chars forbidden by \"{}\"",
+                *SINGLELINE_RE
+            );
+            false
+        }
+    }
+
+    pub(crate) fn validate_str_escapes(s: &str) -> bool {
+        // Look for and prevent certain types of string escapes and injections.
+        if !ESCAPES_RE.is_match(s) {
+            true
+        } else {
+            warn!(
+                "value contains invalid escape chars forbidden by \"{}\"",
+                *ESCAPES_RE
+            );
+            false
+        }
+    }
 }
 
 #[cfg(test)]
@@ -1867,4 +1944,21 @@ mod tests {
         assert!(val2.is_some());
         assert!(val3.is_some());
     }
+
+    #[test]
+    fn test_singleline() {
+        assert!(Value::validate_singleline("no new lines"));
+
+        assert!(!Value::validate_singleline("contains a \n new line"));
+        assert!(!Value::validate_singleline("contains a \r return feed"));
+        assert!(!Value::validate_singleline("contains a \t tab char"));
+    }
+
+    #[test]
+    fn test_str_escapes() {
+        assert!(Value::validate_str_escapes("safe str"));
+        assert!(Value::validate_str_escapes("🙃 emoji are 👍"));
+
+        assert!(!Value::validate_str_escapes("naughty \x1b[31mred"));
+    }
 }

server/lib/src/valueset/binary.rs

@@ -245,7 +245,9 @@ impl ValueSetT for ValueSetPublicBinary {
     }
 
     fn validate(&self, _schema_attr: &SchemaAttribute) -> bool {
-        true
+        self.map
+            .iter()
+            .all(|(s, _)| Value::validate_str_escapes(s) && Value::validate_singleline(s))
     }
 
     fn to_proto_string_clone_iter(&self) -> Box<dyn Iterator<Item = String> + '_> {

server/lib/src/valueset/cred.rs

@@ -119,7 +119,9 @@ impl ValueSetT for ValueSetCredential {
     }
 
     fn validate(&self, _schema_attr: &SchemaAttribute) -> bool {
-        true
+        self.map
+            .iter()
+            .all(|(s, _)| Value::validate_str_escapes(s) && Value::validate_singleline(s))
     }
 
     fn to_proto_string_clone_iter(&self) -> Box<dyn Iterator<Item = String> + '_> {
@@ -333,7 +335,9 @@ impl ValueSetT for ValueSetIntentToken {
     }
 
     fn validate(&self, _schema_attr: &SchemaAttribute) -> bool {
-        true
+        self.map
+            .iter()
+            .all(|(s, _)| Value::validate_str_escapes(s) && Value::validate_singleline(s))
     }
 
     fn to_proto_string_clone_iter(&self) -> Box<dyn Iterator<Item = String> + '_> {
@@ -540,7 +544,9 @@ impl ValueSetT for ValueSetPasskey {
     }
 
     fn validate(&self, _schema_attr: &SchemaAttribute) -> bool {
-        true
+        self.map
+            .iter()
+            .all(|(_, (s, _))| Value::validate_str_escapes(s) && Value::validate_singleline(s))
     }
 
     fn to_proto_string_clone_iter(&self) -> Box<dyn Iterator<Item = String> + '_> {
@@ -724,7 +730,9 @@ impl ValueSetT for ValueSetDeviceKey {
     }
 
     fn validate(&self, _schema_attr: &SchemaAttribute) -> bool {
-        true
+        self.map
+            .iter()
+            .all(|(_, (s, _))| Value::validate_str_escapes(s) && Value::validate_singleline(s))
     }
 
     fn to_proto_string_clone_iter(&self) -> Box<dyn Iterator<Item = String> + '_> {

server/lib/src/valueset/iname.rs

@@ -102,7 +102,11 @@ impl ValueSetT for ValueSetIname {
     }
 
     fn validate(&self, _schema_attr: &SchemaAttribute) -> bool {
-        self.set.iter().all(|s| Value::validate_iname(s.as_str()))
+        self.set.iter().all(|s| {
+            Value::validate_str_escapes(s)
+                && Value::validate_singleline(s)
+                && Value::validate_iname(s.as_str())
+        })
     }
 
     fn to_proto_string_clone_iter(&self) -> Box<dyn Iterator<Item = String> + '_> {

server/lib/src/valueset/iutf8.rs

@@ -103,7 +103,9 @@ impl ValueSetT for ValueSetIutf8 {
     }
 
     fn validate(&self, _schema_attr: &SchemaAttribute) -> bool {
-        true
+        self.set
+            .iter()
+            .all(|s| Value::validate_str_escapes(s) && Value::validate_singleline(s))
     }
 
     fn to_proto_string_clone_iter(&self) -> Box<dyn Iterator<Item = String> + '_> {

server/lib/src/valueset/mod.rs

@@ -583,7 +583,6 @@ pub fn from_result_value_iter(
         | Value::Passkey(_, _, _)
         | Value::DeviceKey(_, _, _)
         | Value::TotpSecret(_, _)
-        | Value::TrustedDeviceEnrollment(_)
         | Value::Session(_, _)
         | Value::ApiToken(_, _)
         | Value::Oauth2Session(_, _)
@@ -646,7 +645,7 @@ pub fn from_value_iter(mut iter: impl Iterator<Item = Value>) -> Result<ValueSet
         Value::Oauth2Session(u, m) => ValueSetOauth2Session::new(u, m),
         Value::UiHint(u) => ValueSetUiHint::new(u),
         Value::TotpSecret(l, t) => ValueSetTotpSecret::new(l, t),
-        Value::PhoneNumber(_, _) | Value::TrustedDeviceEnrollment(_) => {
+        Value::PhoneNumber(_, _) => {
             debug_assert!(false);
             return Err(OperationError::InvalidValueState);
         }

server/lib/src/valueset/session.rs

@@ -1088,7 +1088,9 @@ impl ValueSetT for ValueSetApiToken {
     }
 
     fn validate(&self, _schema_attr: &SchemaAttribute) -> bool {
-        true
+        self.map.iter().all(|(_, at)| {
+            Value::validate_str_escapes(&at.label) && Value::validate_singleline(&at.label)
+        })
     }
 
     fn to_proto_string_clone_iter(&self) -> Box<dyn Iterator<Item = String> + '_> {

server/lib/src/valueset/spn.rs

@@ -96,7 +96,12 @@ impl ValueSetT for ValueSetSpn {
     }
 
     fn validate(&self, _schema_attr: &SchemaAttribute) -> bool {
-        true
+        self.set.iter().all(|(a, b)| {
+            Value::validate_str_escapes(a)
+                && Value::validate_str_escapes(b)
+                && Value::validate_singleline(a)
+                && Value::validate_singleline(b)
+        })
     }
 
     fn to_proto_string_clone_iter(&self) -> Box<dyn Iterator<Item = String> + '_> {

server/lib/src/valueset/ssh.rs

@@ -7,6 +7,8 @@ use crate::repl::proto::ReplAttrV1;
 use crate::schema::SchemaAttribute;
 use crate::valueset::{DbValueSetV2, ValueSet};
 
+use sshkeys::PublicKey as SshPublicKey;
+
 #[derive(Debug, Clone)]
 pub struct ValueSetSshKey {
     map: BTreeMap<String, String>,
@@ -102,7 +104,11 @@ impl ValueSetT for ValueSetSshKey {
     }
 
     fn validate(&self, _schema_attr: &SchemaAttribute) -> bool {
-        true
+        self.map.iter().all(|(s, key)| {
+            SshPublicKey::from_string(key).is_ok()
+                && Value::validate_str_escapes(s)
+                && Value::validate_singleline(s)
+        })
     }
 
     fn to_proto_string_clone_iter(&self) -> Box<dyn Iterator<Item = String> + '_> {

server/lib/src/valueset/utf8.rs

@@ -88,7 +88,9 @@ impl ValueSetT for ValueSetUtf8 {
     }
 
     fn validate(&self, _schema_attr: &SchemaAttribute) -> bool {
-        true
+        self.set
+            .iter()
+            .all(|s| Value::validate_str_escapes(s) && Value::validate_singleline(s))
     }
 
     fn to_proto_string_clone_iter(&self) -> Box<dyn Iterator<Item = String> + '_> {