Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mirrord policy for enforcing header pattern #3048

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions changelog.d/+operator-policy-http-filter.internal.md
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.
7 changes: 5 additions & 2 deletions mirrord/intproxy/src/proxies/incoming/subscriptions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down
3 changes: 2 additions & 1 deletion mirrord/layer/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
);
Expand Down
42 changes: 42 additions & 0 deletions mirrord/operator/src/crd/policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
DmitryDodzin marked this conversation as resolved.
Show resolved Hide resolved
}

/// Custom cluster-wide resource for policies that limit what mirrord features users can use.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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>,
Razz4780 marked this conversation as resolved.
Show resolved Hide resolved
}

#[test]
fn check_one_api_group() {
use kube::Resource;
Expand Down
2 changes: 1 addition & 1 deletion mirrord/protocol/Cargo.toml
Original file line number Diff line number Diff line change
@@ -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
Expand Down
18 changes: 15 additions & 3 deletions mirrord/protocol/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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),
Expand Down
8 changes: 8 additions & 0 deletions mirrord/protocol/src/tcp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
7 changes: 7 additions & 0 deletions tests/src/operator/policies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
},
),
)
Expand Down
1 change: 1 addition & 0 deletions tests/src/operator/policies/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading