Validate the supplied store divider

src/by_store/store.rs

@@ -90,10 +90,9 @@ impl StoreConfig {
             config.filename = format!("keyring-{}", config.name);
         }
         if let Some(divider) = mods.get("divider") {
-            // Vault dividers must have a non-alphabetic character so that the
-            // vault's config key is guaranteed not to match any credential's key.
-            if config.divider.chars().all(char::is_alphanumeric) {
-                let err = "must contain a non-alphabetic character".to_string();
+            // A non-alphanumeric divider separates credential keys from the config key.
+            if divider.chars().all(char::is_alphanumeric) {
+                let err = "must contain a non-alphanumeric character".to_string();
                 return Err(Error::Invalid("divider".to_string(), err));
             }
             config.divider = divider.to_string();
@@ -164,7 +163,7 @@ impl Store {
     /// The value of `divider` defaults to `\u{feff}@\u{feff}` which,
     /// when printed as part of a string, looks like `@` because
     /// the BOM character is considered a non-spacing word-joining
-    /// character. The divider _must_ contain a non-alphabetic character.
+    /// character. The divider _must_ contain a non-alphanumeric character.
     ///
     /// For `user-auth-required` and `user-auth-timeout`, see
     /// [the module docs](super#user-authentication).
@@ -395,18 +394,40 @@ mod tests {
 
     #[test]
     fn user_auth_timeout_must_fit_the_keystore_range() {
-        for timeout in ["-1", "2147483648", "soon"] {
+        for timeout in ["-1", "2147483648", "4294967295", "4294967296", "soon"] {
             assert_invalid(
                 &[NAME, REQUIRED, ("user-auth-timeout", timeout)],
                 "user-auth-timeout",
             );
         }
-        for (timeout, seconds) in [("0", 0), ("2147483647", 2147483647)] {
+        for (timeout, seconds) in [("0", 0), ("1", 1), ("2147483647", 2147483647)] {
             let config = parse(&[NAME, REQUIRED, ("user-auth-timeout", timeout)]).unwrap();
             assert_eq!(config.user_auth_timeout, Some(seconds));
         }
     }
 
+    #[test]
+    fn divider_rejects_alphanumeric_and_empty() {
+        for divider in ["abc", "abc123", "123", "é1", ""] {
+            assert_invalid(&[NAME, ("divider", divider)], "divider");
+        }
+    }
+
+    #[test]
+    fn divider_accepts_punctuation() {
+        for divider in ["@", "~", "a@b"] {
+            let config = parse(&[NAME, ("divider", divider)]).unwrap();
+            assert_eq!(config.divider, divider);
+        }
+    }
+
+    #[test]
+    fn unknown_keys_and_malformed_values_are_rejected() {
+        assert_invalid(&[NAME, ("color", "blue")], "color");
+        assert_invalid(&[("name", "")], "name");
+        assert_invalid(&[NAME, ("user-auth-required", "yes")], "user-auth-required");
+    }
+
     #[test]
     fn stored_config_without_user_auth_keeps_its_earlier_format() {
         let stored = r#"{"name":"vault","filename":"keyring-vault","divider":"@"}"#;

src/by_store/vault.rs

@@ -217,10 +217,9 @@ impl Vault {
     }
 
     fn new(config: &StoreConfig) -> Result<Self> {
-        // Vault dividers must have a non-alphabetic character so that the
-        // vault's config key is guaranteed not to match any credential's key.
+        // A non-alphanumeric divider separates credential keys from the config key.
         if config.divider.chars().all(char::is_alphanumeric) {
-            let err = "must contain a non-alphabetic character".to_string();
+            let err = "must contain a non-alphanumeric character".to_string();
             return Err(Error::Invalid("divider".to_string(), err));
         }
         log::debug!("Creating new vault with config {config:?}");

src/tests/store_tests.rs

@@ -6,6 +6,7 @@ use android_log_sys::LogPriority;
 use keyring_core::{Entry, api::CredentialStoreApi};
 
 use super::bad_result;
+use crate::shared_preferences::Context;
 
 pub fn run_tests() -> (usize, usize) {
     let testing = [
@@ -23,6 +24,8 @@ pub fn run_tests() -> (usize, usize) {
         ("exists", exists),
         ("configuration_mismatch", configuration_mismatch),
         ("default_store", default_store),
+        ("oversized_auth_timeout", oversized_auth_timeout),
+        ("direct_invalid_divider", direct_invalid_divider),
     ]
     .iter()
     // user_auth_timeout needs a secure lock screen and a device unlocked in the last 300 seconds.
@@ -461,3 +464,54 @@ fn default_store() -> keyring_core::Result<()> {
     }
     Ok(())
 }
+
+fn oversized_auth_timeout() -> keyring_core::Result<()> {
+    let name = "oversized-timeout-test";
+    let config = crate::by_store::store::StoreConfig {
+        name: name.to_string(),
+        filename: format!("keyring-{name}"),
+        divider: "@".to_string(),
+        user_auth_timeout: Some(u32::MAX),
+    };
+    match crate::Store::new_with_store_config(config) {
+        Err(keyring_core::Error::Invalid(key, _)) if key == "user-auth-timeout" => {}
+        other => {
+            return bad_result(
+                "new_with_store_config",
+                &format!("Invalid(user-auth-timeout), got {other:?}"),
+            );
+        }
+    }
+    // The parser cannot construct this configuration for `Store::delete`.
+    let (vm, context) = crate::android_context()?;
+    let mut env = vm.attach_current_thread().expect("attach_current_thread");
+    let ctx = Context::from_raw(context);
+    let filename = format!("keyring-{name}");
+    ctx.delete_shared_preferences(&mut env, &filename)
+        .expect("delete_shared_preferences");
+    Ok(())
+}
+
+fn direct_invalid_divider() -> keyring_core::Result<()> {
+    let name = "direct-divider-test";
+    let config = crate::by_store::store::StoreConfig {
+        name: name.to_string(),
+        filename: format!("keyring-{name}"),
+        divider: "abc".to_string(),
+        ..Default::default()
+    };
+    match crate::Store::new_with_store_config(config) {
+        Err(keyring_core::Error::Invalid(key, _)) if key == "divider" => {}
+        other => {
+            return bad_result(
+                "new_with_store_config",
+                &format!("Invalid(divider), got {other:?}"),
+            );
+        }
+    }
+    let valid = HashMap::from([("name", name), ("divider", "@")]);
+    if crate::Store::exists(&valid)? {
+        return bad_result("exists", "false after a rejected divider");
+    }
+    Ok(())
+}