mirrord policy for enforcing header pattern (metalbear-co#3048)

* Naive Implementation * Ops * Ops 2 * deref str for filter * Tiny * Docs * Changelog * Tiny * Ops * Ops * Apply suggestions from code review Co-authored-by: Michał Smolarek <[email protected]> * Make it so no protocol break * Apply suggestions from code review Co-authored-by: Michał Smolarek <[email protected]> * Update to current behavior * Update --------- Co-authored-by: Michał Smolarek <[email protected]>
Razz4780 · Feb 5, 2025 · 1429a36 · 1429a36
1 parent 12dcea9
commit 1429a36
Show file tree

Hide file tree

Showing 10 changed files with 83 additions and 8 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/changelog.d/+operator-policy-http-filter.internal.md b/changelog.d/+operator-policy-http-filter.internal.md
@@ -0,0 +1 @@
+Add mirrord policy support for specifying pattern requirment for header filter when performing steal-with-filter.
diff --git a/mirrord/intproxy/src/proxies/incoming/subscriptions.rs b/mirrord/intproxy/src/proxies/incoming/subscriptions.rs
@@ -258,9 +258,12 @@ impl SubscriptionsManager {
             }
 
             Err(
-                ref response_error @ ResponseError::Forbidden {
+                ref response_error @ (ResponseError::Forbidden {
                     ref blocked_action, ..
-                },
+                }
+                | ResponseError::ForbiddenWithReason {
+                    ref blocked_action, ..
+                }),
             ) => {
                 tracing::warn!(%response_error, "Port subscribe blocked by policy");
 

diff --git a/mirrord/layer/src/error.rs b/mirrord/layer/src/error.rs
@@ -281,7 +281,8 @@ impl From<HookError> for i64 {
                 ResponseError::PortAlreadyStolen(_port) => libc::EINVAL,
                 ResponseError::NotImplemented => libc::EINVAL,
                 ResponseError::StripPrefix(_) => libc::EINVAL,
-                err @ ResponseError::Forbidden { .. } => {
+                err @ (ResponseError::Forbidden { .. }
+                | ResponseError::ForbiddenWithReason { .. }) => {
                     graceful_exit!(
                         "Stopping mirrord run. Please adjust your mirrord configuration.\n{err}"
                     );

diff --git a/mirrord/operator/src/crd/policy.rs b/mirrord/operator/src/crd/policy.rs
@@ -63,6 +63,10 @@ pub struct MirrordPolicySpec {
     /// the user config.
     #[serde(default)]
     pub fs: FsPolicy,
+
+    /// Fine grained control over network features like specifying required HTTP filters.
+    #[serde(default)]
+    pub network: NetworkPolicy,
 }
 
 /// Custom cluster-wide resource for policies that limit what mirrord features users can use.
@@ -100,6 +104,9 @@ pub struct MirrordClusterPolicySpec {
     /// the user config.
     #[serde(default)]
     pub fs: FsPolicy,
+
+    #[serde(default)]
+    pub network: NetworkPolicy,
 }
 
 /// Policy for controlling environment variables access from mirrord instances.
@@ -150,6 +157,41 @@ pub struct FsPolicy {
     pub not_found: HashSet<String>,
 }
 
+/// Network operations policy that partialy mimics the mirrord network config.
+#[derive(Clone, Default, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)]
+#[serde(rename_all = "camelCase")]
+pub struct NetworkPolicy {
+    #[serde(default)]
+    pub incoming: IncomingNetworkPolicy,
+}
+
+/// Incoming network operations policy that partialy mimics the mirrord `network.incoming` config.
+#[derive(Clone, Default, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)]
+#[serde(rename_all = "camelCase")]
+pub struct IncomingNetworkPolicy {
+    #[serde(default)]
+    pub http_filter: HttpFilterPolicy,
+}
+
+/// Http filter policy that allows to specify requirements for the HTTP filter used in a session.
+#[derive(Clone, Default, Debug, Deserialize, Eq, PartialEq, Serialize, JsonSchema)]
+#[serde(rename_all = "camelCase")]
+pub struct HttpFilterPolicy {
+    /// Require the user's header filter to match this regex, if such filter is provided.
+    ///
+    /// This works in tandem with the `steal-without-filter` block
+    /// to require that the user specifies a header filter for the network steal feature.
+    ///
+    /// # Composed filters
+    ///
+    /// When the user requests an `all_of` HTTP filter, at least one of the nested filters
+    /// must be a header filter that matches this regex. At least one nested filter is required.
+    ///
+    /// When the user requests an `any_of` HTTP filter, all nested header filters must match this
+    /// regex. At least one nested header filter is required.
+    pub header_filter: Option<String>,
+}
+
 #[test]
 fn check_one_api_group() {
     use kube::Resource;

diff --git a/mirrord/protocol/Cargo.toml b/mirrord/protocol/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "mirrord-protocol"
-version = "1.16.1"
+version = "1.17.0"
 authors.workspace = true
 description.workspace = true
 documentation.workspace = true

diff --git a/mirrord/protocol/src/error.rs b/mirrord/protocol/src/error.rs
@@ -59,7 +59,7 @@ pub enum ResponseError {
     #[error("Operation is not yet supported by mirrord.")]
     NotImplemented,
 
-    #[error("{blocked_action} is forbidden by {} for this target (your organization does not allow you to use this mirrord feature with the chosen target).", policy_name_string(.policy_name.clone()))]
+    #[error("{blocked_action} is forbidden by {} for this target (your organization does not allow you to use this mirrord feature with the chosen target).", policy_name_string(.policy_name.as_deref()))]
     Forbidden {
         blocked_action: BlockedAction,
         policy_name: Option<String>,
@@ -70,6 +70,13 @@ pub enum ResponseError {
 
     #[error("File has to be opened locally!")]
     OpenLocal,
+
+    #[error("{blocked_action} is forbidden by {} for this target ({reason}).", policy_name_string(.policy_name.as_deref()))]
+    ForbiddenWithReason {
+        blocked_action: BlockedAction,
+        policy_name: Option<String>,
+        reason: String,
+    },
 }
 
 impl From<StripPrefixError> for ResponseError {
@@ -79,7 +86,7 @@ impl From<StripPrefixError> for ResponseError {
 }
 
 /// If some then the name with a trailing space, else empty string.
-fn policy_name_string(policy_name: Option<String>) -> String {
+fn policy_name_string(policy_name: Option<&str>) -> String {
     if let Some(name) = policy_name {
         format!("the mirrord policy \"{name}\"")
     } else {
@@ -91,8 +98,13 @@ fn policy_name_string(policy_name: Option<String>) -> String {
 pub static MIRROR_BLOCK_VERSION: LazyLock<VersionReq> =
     LazyLock::new(|| ">=1.12.0".parse().expect("Bad Identifier"));
 
+/// Minimal mirrord-protocol version that allows [`ResponseError::Forbidden`] to have `reason`
+/// member.
+pub static MIRROR_POLICY_REASON_VERSION: LazyLock<VersionReq> =
+    LazyLock::new(|| ">=1.17.0".parse().expect("Bad Identifier"));
+
 /// All the actions that can be blocked by the operator, to identify the blocked feature in a
-/// [`ResponseError::Forbidden`] message.
+/// [`ResponseError::Forbidden`] or [`ResponseError::ForbiddenWithReason`] message.
 #[derive(Encode, Decode, Debug, PartialEq, Clone, Eq, Error)]
 pub enum BlockedAction {
     Steal(StealType),

diff --git a/mirrord/protocol/src/tcp.rs b/mirrord/protocol/src/tcp.rs
@@ -155,6 +155,14 @@ impl Display for Filter {
     }
 }
 
+impl std::ops::Deref for Filter {
+    type Target = str;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
 /// Describes different types of HTTP filtering available
 #[derive(Encode, Decode, Debug, PartialEq, Eq, Clone)]
 pub enum HttpFilter {

diff --git a/tests/src/operator/policies.rs b/tests/src/operator/policies.rs
@@ -121,6 +121,7 @@ fn block_steal_without_qualifiers() -> PolicyTestCase {
                 block: vec![BlockedFeature::Steal],
                 env: Default::default(),
                 fs: Default::default(),
+                network: Default::default(),
             },
         ),
         service_b_can_steal: No,
@@ -141,6 +142,7 @@ fn block_steal_with_path_pattern() -> PolicyTestCase {
                 block: vec![BlockedFeature::Steal],
                 env: Default::default(),
                 fs: Default::default(),
+                network: Default::default(),
             },
         ),
         service_b_can_steal: EvenWithoutFilter,
@@ -161,6 +163,7 @@ fn block_unfiltered_steal_with_path_pattern() -> PolicyTestCase {
                 block: vec![BlockedFeature::StealWithoutFilter],
                 env: Default::default(),
                 fs: Default::default(),
+                network: Default::default(),
             },
         ),
         service_b_can_steal: EvenWithoutFilter,
@@ -181,6 +184,7 @@ fn block_unfiltered_steal_with_deployment_path_pattern() -> PolicyTestCase {
                 block: vec![BlockedFeature::StealWithoutFilter],
                 env: Default::default(),
                 fs: Default::default(),
+                network: Default::default(),
             },
         ),
         service_a_can_steal: OnlyWithFilter,
@@ -207,6 +211,7 @@ fn block_steal_with_label_selector() -> PolicyTestCase {
                 block: vec![BlockedFeature::Steal],
                 env: Default::default(),
                 fs: Default::default(),
+                network: Default::default(),
             },
         ),
         service_b_can_steal: EvenWithoutFilter,
@@ -234,6 +239,7 @@ fn block_steal_with_unmatching_policy() -> PolicyTestCase {
                 block: vec![BlockedFeature::Steal],
                 env: Default::default(),
                 fs: Default::default(),
+                network: Default::default(),
             },
         ),
         service_b_can_steal: EvenWithoutFilter,
@@ -376,6 +382,7 @@ pub async fn create_cluster_policy_and_try_to_mirror(
                 block: vec![BlockedFeature::Mirror],
                 env: Default::default(),
                 fs: Default::default(),
+                network: Default::default(),
             },
         ),
     )

diff --git a/tests/src/operator/policies/fs.rs b/tests/src/operator/policies/fs.rs
@@ -48,6 +48,7 @@ pub async fn create_namespaced_fs_policy_and_try_file_open(
                     local: HashSet::from_iter(vec!["file\\.local".to_string()]),
                     not_found: HashSet::from_iter(vec!["file\\.not-found".to_string()]),
                 },
+                network: Default::default(),
             },
         ),
         &service.namespace,
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Add mirrord policy support for specifying pattern requirment for header filter when performing steal-with-filter.