Add key validation code

cli/tests/integration/object_store.rs

@@ -155,3 +155,105 @@ async fn object_store_bad_configs() -> TestResult {
 
     Ok(())
 }
+
+#[tokio::test(flavor = "multi_thread")]
+async fn object_store_bad_key_values() -> TestResult {
+    const BAD_1_FASTLY_TOML: &str = r#"
+        name = "object-store-test"
+        description = "object store test"
+        authors = ["Jill Bryson <[email protected]>", "Rose McDowall <[email protected]>"]
+        language = "rust"
+        [local_server]
+        object_store.store_one = [{key = "", data = "This is some data"}]
+    "#;
+    match Test::using_fixture("object_store.wasm").using_fastly_toml(BAD_1_FASTLY_TOML) {
+        Err(e) => assert_eq!("invalid configuration for 'store_one': Invalid `key` value used: Keys for objects cannot be empty.", &e.to_string()),
+        _ => panic!(),
+    }
+
+    const BAD_2_FASTLY_TOML: &str = r#"
+        name = "object-store-test"
+        description = "object store test"
+        authors = ["Jill Bryson <[email protected]>", "Rose McDowall <[email protected]>"]
+        language = "rust"
+        [local_server]
+        object_store.store_one = [{key = "LOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOoooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong,looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong,keeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeEEEEEEEEEEEEEEEEEEEEEEEEEEEEEeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeey", data = "This is some data"}]
+    "#;
+    match Test::using_fixture("object_store.wasm").using_fastly_toml(BAD_2_FASTLY_TOML) {
+        Err(e) => assert_eq!(
+            "invalid configuration for 'store_one': Invalid `key` value used: Keys for objects cannot be over 1024 bytes in size.",
+            &e.to_string()
+        ),
+        _ => panic!(),
+    }
+
+    const BAD_3_FASTLY_TOML: &str = r#"
+        name = "object-store-test"
+        description = "object store test"
+        authors = ["Jill Bryson <[email protected]>", "Rose McDowall <[email protected]>"]
+        language = "rust"
+        [local_server]
+        object_store.store_one = [{key = ".well-known/acme-challenge/wheeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee", data = "This is some data"}]
+    "#;
+    match Test::using_fixture("object_store.wasm").using_fastly_toml(BAD_3_FASTLY_TOML) {
+        Err(e) => assert_eq!(
+            "invalid configuration for 'store_one': Invalid `key` value used: Keys for objects cannot start with `.well-known/acme-challenge`.",
+            &e.to_string()
+        ),
+        _ => panic!(),
+    }
+
+    const BAD_4_FASTLY_TOML: &str = r#"
+        name = "object-store-test"
+        description = "object store test"
+        authors = ["Jill Bryson <[email protected]>", "Rose McDowall <[email protected]>"]
+        language = "rust"
+        [local_server]
+        object_store.store_one = [{key = "its.me.the.dot.key", data = "This is some data"}]
+    "#;
+    match Test::using_fixture("object_store.wasm").using_fastly_toml(BAD_4_FASTLY_TOML) {
+        Err(e) => assert_eq!("invalid configuration for 'store_one': Invalid `key` value used: Keys for objects cannot contain a `.`.", &e.to_string()),
+        _ => panic!(),
+    }
+
+    const BAD_5_FASTLY_TOML: &str = r#"
+        name = "object-store-test"
+        description = "object store test"
+        authors = ["Jill Bryson <[email protected]>", "Rose McDowall <[email protected]>"]
+        language = "rust"
+        [local_server]
+        object_store.store_one = [{key = "double..dippin..dots", data = "This is some data"}]
+    "#;
+    match Test::using_fixture("object_store.wasm").using_fastly_toml(BAD_5_FASTLY_TOML) {
+        Err(e) => assert_eq!("invalid configuration for 'store_one': Invalid `key` value used: Keys for objects cannot contain a `..`.", &e.to_string()),
+        _ => panic!(),
+    }
+
+    const BAD_6_FASTLY_TOML: &str = r#"
+        name = "object-store-test"
+        description = "object store test"
+        authors = ["Jill Bryson <[email protected]>", "Rose McDowall <[email protected]>"]
+        language = "rust"
+        [local_server]
+        object_store.store_one = [{key = "carriage\rreturn", data = "This is some data"}]
+    "#;
+    match Test::using_fixture("object_store.wasm").using_fastly_toml(BAD_6_FASTLY_TOML) {
+        Err(e) => assert_eq!("invalid configuration for 'store_one': Invalid `key` value used: Keys for objects cannot contain a `\r`.", &e.to_string()),
+        _ => panic!(),
+    }
+
+    const BAD_7_FASTLY_TOML: &str = r#"
+        name = "object-store-test"
+        description = "object store test"
+        authors = ["Jill Bryson <[email protected]>", "Rose McDowall <[email protected]>"]
+        language = "rust"
+        [local_server]
+        object_store.store_one = [{key = "newlines\nin\nthis\neconomy?", data = "This is some data"}]
+    "#;
+    match Test::using_fixture("object_store.wasm").using_fastly_toml(BAD_7_FASTLY_TOML) {
+        Err(e) => assert_eq!("invalid configuration for 'store_one': Invalid `key` value used: Keys for objects cannot contain a `\n`.", &e.to_string()),
+        _ => panic!(),
+    }
+
+    Ok(())
+}

lib/src/config/object_store.rs

@@ -87,7 +87,16 @@ impl TryFrom<Table> for ObjectStoreConfig {
                         .to_vec(),
                 };
                 obj_store
-                    .insert(ObjectStoreKey::new(store), ObjectKey::new(key), bytes)
+                    .insert(
+                        ObjectStoreKey::new(store),
+                        ObjectKey::new(key).map_err(|err| {
+                            FastlyConfigError::InvalidObjectStoreDefinition {
+                                name: store.to_string(),
+                                err: err.into(),
+                            }
+                        })?,
+                        bytes,
+                    )
                     .expect("Lock was not poisoned");
             }
         }

lib/src/error.rs

@@ -109,6 +109,9 @@ pub enum Error {
 
     #[error("Object Store '{0}' does not exist")]
     UnknownObjectStore(String),
+
+    #[error("Invalid Object Store `key` value used: {0}.")]
+    ObjectStoreKeyValidationError(#[from] crate::object_store::KeyValidationError),
 }
 
 impl Error {
@@ -157,7 +160,8 @@ impl Error {
             | Error::Utf8Expected(_)
             | Error::BackendNameRegistryError(_)
             | Error::HttpError(_)
-            | Error::UnknownObjectStore(_) => FastlyStatus::Error,
+            | Error::UnknownObjectStore(_)
+            | Error::ObjectStoreKeyValidationError(_) => FastlyStatus::Error,
         }
     }
 
@@ -418,6 +422,8 @@ pub enum ObjectStoreConfigError {
     NotATable,
     #[error("There was an error when manipulating the ObjectStore: {0}.")]
     ObjectStoreError(#[from] crate::object_store::ObjectStoreError),
+    #[error("Invalid `key` value used: {0}.")]
+    KeyValidationError(#[from] crate::object_store::KeyValidationError),
 }
 
 /// Errors related to the downstream request.

lib/src/object_store.rs

@@ -90,8 +90,10 @@ impl ObjectStoreKey {
 pub struct ObjectKey(String);
 
 impl ObjectKey {
-    pub fn new(key: impl ToString) -> Self {
-        Self(key.to_string())
+    pub fn new(key: impl ToString) -> Result<Self, KeyValidationError> {
+        let key = key.to_string();
+        is_valid_key(&key)?;
+        Ok(Self(key))
     }
 }
 
@@ -112,3 +114,53 @@ impl From<&ObjectStoreError> for FastlyStatus {
         }
     }
 }
+
+/// Keys in the Object Store must follow the following rules:
+///
+///   * Keys can contain any sequence of valid Unicode characters, of length 1-1024 bytes when
+///     UTF-8 encoded.
+///   * Keys cannot contain Carriage Return or Line Feed characters.
+///   * Keys cannot start with `.well-known/acme-challenge/`.
+///   * Keys cannot be named `.` or `..`.
+fn is_valid_key(key: &str) -> Result<(), KeyValidationError> {
+    let len = key.as_bytes().len();
+    if len < 1 {
+        return Err(KeyValidationError::EmptyKey);
+    } else if len > 1024 {
+        return Err(KeyValidationError::Over1024Bytes);
+    }
+
+    if key.starts_with(".well-known/acme-challenge") {
+        return Err(KeyValidationError::StartsWithWellKnown);
+    }
+
+    if key.contains("..") {
+        return Err(KeyValidationError::ContainsDotDot);
+    } else if key.contains(".") {
+        return Err(KeyValidationError::ContainsDot);
+    } else if key.contains("\r") {
+        return Err(KeyValidationError::ContainsCarriageReturn);
+    } else if key.contains("\n") {
+        return Err(KeyValidationError::ContainsLineFeed);
+    }
+
+    Ok(())
+}
+
+#[derive(Debug, thiserror::Error)]
+pub enum KeyValidationError {
+    #[error("Keys for objects cannot be empty")]
+    EmptyKey,
+    #[error("Keys for objects cannot be over 1024 bytes in size")]
+    Over1024Bytes,
+    #[error("Keys for objects cannot start with `.well-known/acme-challenge`")]
+    StartsWithWellKnown,
+    #[error("Keys for objects cannot contain a `.`")]
+    ContainsDot,
+    #[error("Keys for objects cannot contain a `..`")]
+    ContainsDotDot,
+    #[error("Keys for objects cannot contain a `\r`")]
+    ContainsCarriageReturn,
+    #[error("Keys for objects cannot contain a `\n`")]
+    ContainsLineFeed,
+}

lib/src/wiggle_abi/obj_store_impl.rs

@@ -34,7 +34,7 @@ impl FastlyObjectStore for Session {
         opt_body_handle_out: &GuestPtr<BodyHandle>,
     ) -> Result<(), Error> {
         let store = self.get_obj_store_key(store).unwrap();
-        let key = ObjectKey::new(&*key.as_str()?);
+        let key = ObjectKey::new(&*key.as_str()?)?;
         match self.obj_lookup(store, &key) {
             Ok(obj) => {
                 let new_handle = self.insert_body(Body::from(obj));
@@ -55,7 +55,7 @@ impl FastlyObjectStore for Session {
         body_handle: BodyHandle,
     ) -> Result<(), Error> {
         let store = self.get_obj_store_key(store).unwrap().clone();
-        let key = ObjectKey::new(&*key.as_str()?);
+        let key = ObjectKey::new(&*key.as_str()?)?;
         let bytes = self.take_body(body_handle)?.read_into_vec().await?;
         self.obj_insert(store, key, bytes)?;