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

Audit access policy implementation #12846

Merged
merged 2 commits into from
Jul 26, 2024
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
1 change: 1 addition & 0 deletions policy-controller/k8s/api/src/policy/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ pub struct ServerSpec {
pub selector: Selector,
pub port: Port,
pub proxy_protocol: Option<ProxyProtocol>,
pub access_policy: Option<String>,
}

#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize, JsonSchema)]
Expand Down
58 changes: 39 additions & 19 deletions policy-controller/k8s/index/src/defaults.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ pub enum DefaultPolicy {

/// Indicates that all traffic is denied unless explicitly permitted by an authorization policy.
Deny,

/// Indicates that all traffic is let through, but gets audited
Audit,
}

// === impl DefaultPolicy ===
Expand All @@ -47,6 +50,7 @@ impl std::str::FromStr for DefaultPolicy {
cluster_only: true,
}),
"deny" => Ok(Self::Deny),
"audit" => Ok(Self::Audit),
s => Err(anyhow!("invalid mode: {:?}", s)),
}
}
Expand All @@ -72,6 +76,7 @@ impl DefaultPolicy {
cluster_only: true,
} => "cluster-unauthenticated",
Self::Deny => "deny",
Self::Audit => "audit",
}
}

Expand All @@ -80,34 +85,48 @@ impl DefaultPolicy {
config: &ClusterInfo,
) -> HashMap<AuthorizationRef, ClientAuthorization> {
let mut authzs = HashMap::default();
let auth_ref = AuthorizationRef::Default(self.as_str());

if let DefaultPolicy::Allow {
authenticated_only,
cluster_only,
} = self
{
let authentication = if authenticated_only {
ClientAuthentication::TlsAuthenticated(vec![IdentityMatch::Suffix(vec![])])
} else {
ClientAuthentication::Unauthenticated
};
let networks = if cluster_only {
config.networks.iter().copied().map(Into::into).collect()
} else {
vec![
"0.0.0.0/0".parse::<IpNet>().unwrap().into(),
"::/0".parse::<IpNet>().unwrap().into(),
]
};
authzs.insert(
AuthorizationRef::Default(self.as_str()),
ClientAuthorization {
authentication,
networks,
},
auth_ref,
Self::default_client_authz(config, authenticated_only, cluster_only),
);
};
} else if let DefaultPolicy::Audit = self {
authzs.insert(auth_ref, Self::default_client_authz(config, false, false));
}

authzs
}

fn default_client_authz(
config: &ClusterInfo,
authenticated_only: bool,
cluster_only: bool,
) -> ClientAuthorization {
let authentication = if authenticated_only {
ClientAuthentication::TlsAuthenticated(vec![IdentityMatch::Suffix(vec![])])
} else {
ClientAuthentication::Unauthenticated
};
let networks = if cluster_only {
config.networks.iter().copied().map(Into::into).collect()
} else {
vec![
"0.0.0.0/0".parse::<IpNet>().unwrap().into(),
"::/0".parse::<IpNet>().unwrap().into(),
]
};

ClientAuthorization {
authentication,
networks,
}
}
}

impl std::fmt::Display for DefaultPolicy {
Expand Down Expand Up @@ -140,6 +159,7 @@ mod test {
authenticated_only: false,
cluster_only: true,
},
DefaultPolicy::Audit,
] {
assert_eq!(
default.to_string().parse::<DefaultPolicy>().unwrap(),
Expand Down
4 changes: 4 additions & 0 deletions policy-controller/k8s/index/src/inbound/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1753,6 +1753,10 @@ impl PolicyIndex {
authzs.insert(reference, authz);
}

if let Some(p) = server.access_policy {
authzs.extend(p.default_authzs(&self.cluster_info));
}

authzs
}

Expand Down
4 changes: 3 additions & 1 deletion policy-controller/k8s/index/src/inbound/server.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::ClusterInfo;
use crate::{ClusterInfo, DefaultPolicy};
use linkerd_policy_controller_core::inbound::ProxyProtocol;
use linkerd_policy_controller_k8s_api::{
self as k8s, policy::server::Port, policy::server::Selector,
Expand All @@ -11,6 +11,7 @@ pub(crate) struct Server {
pub selector: Selector,
pub port_ref: Port,
pub protocol: ProxyProtocol,
pub access_policy: Option<DefaultPolicy>,
}

impl Server {
Expand All @@ -20,6 +21,7 @@ impl Server {
selector: srv.spec.selector,
port_ref: srv.spec.port,
protocol: proxy_protocol(srv.spec.proxy_protocol, cluster),
access_policy: srv.spec.access_policy.and_then(|p| p.parse().ok()),
}
}
}
Expand Down
11 changes: 10 additions & 1 deletion policy-controller/k8s/index/src/inbound/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ struct TestConfig {
_tracing: tracing::subscriber::DefaultGuard,
}

const DEFAULTS: [DefaultPolicy; 5] = [
const DEFAULTS: [DefaultPolicy; 6] = [
DefaultPolicy::Deny,
DefaultPolicy::Allow {
authenticated_only: true,
Expand All @@ -63,6 +63,7 @@ const DEFAULTS: [DefaultPolicy; 5] = [
authenticated_only: false,
cluster_only: true,
},
DefaultPolicy::Audit,
];

pub fn mk_pod_with_containers(
Expand Down Expand Up @@ -121,6 +122,7 @@ fn mk_server(
port,
selector: k8s::policy::server::Selector::Pod(pod_labels.into_iter().collect()),
proxy_protocol,
access_policy: None,
},
}
}
Expand Down Expand Up @@ -177,6 +179,13 @@ fn mk_default_policy(
networks: cluster_nets,
},
)),
DefaultPolicy::Audit => Some((
AuthorizationRef::Default("audit"),
ClientAuthorization {
authentication: ClientAuthentication::Unauthenticated,
networks: all_nets,
},
)),
}
.into_iter()
.collect()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ fn authenticated_annotated() {
authenticated_only: true,
},
DefaultPolicy::Deny => DefaultPolicy::Deny,
DefaultPolicy::Audit => DefaultPolicy::Audit,
};
InboundServer {
reference: ServerRef::Default(policy.as_str()),
Expand Down
1 change: 1 addition & 0 deletions policy-controller/k8s/status/src/tests/routes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ fn make_server(
port,
selector: linkerd_k8s_api::server::Selector::Pod(pod_labels.into_iter().collect()),
proxy_protocol,
access_policy: None,
},
}
}
9 changes: 8 additions & 1 deletion policy-controller/src/admission.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,8 @@ impl Validate<MeshTLSAuthenticationSpec> for Admission {

#[async_trait::async_trait]
impl Validate<ServerSpec> for Admission {
/// Checks that `spec` doesn't select the same pod/ports as other existing Servers
/// Checks that `spec` doesn't select the same pod/ports as other existing Servers, and that
/// `accessPolicy` contains a valid value
//
// TODO(ver) this isn't rigorous about detecting servers that select the same port if one port
// specifies a numeric port and the other specifies the port's name.
Expand All @@ -346,6 +347,12 @@ impl Validate<ServerSpec> for Admission {
}
}

if let Some(policy) = spec.access_policy {
policy
.parse::<index::DefaultPolicy>()
.map_err(|err| anyhow!("Invalid 'accessPolicy' field: {err}"))?;
}

Ok(())
}
}
Expand Down
3 changes: 2 additions & 1 deletion policy-test/src/web.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ pub fn pod(ns: &str) -> k8s::Pod {
}
}

pub fn server(ns: &str) -> k8s::policy::Server {
pub fn server(ns: &str, access_policy: Option<String>) -> k8s::policy::Server {
k8s::policy::Server {
metadata: k8s::ObjectMeta {
namespace: Some(ns.to_string()),
Expand All @@ -46,6 +46,7 @@ pub fn server(ns: &str) -> k8s::policy::Server {
)))),
port: k8s::policy::server::Port::Name("http".to_string()),
proxy_protocol: Some(k8s::policy::server::ProxyProtocol::Http1),
access_policy,
},
}
}
Expand Down
23 changes: 23 additions & 0 deletions policy-test/tests/admit_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ async fn accepts_valid() {
selector: Selector::Pod(api::labels::Selector::default()),
port: Port::Number(80.try_into().unwrap()),
proxy_protocol: None,
access_policy: None,
},
})
.await;
Expand All @@ -34,6 +35,7 @@ async fn accepts_server_updates() {
selector: Selector::Pod(api::labels::Selector::from_iter(Some(("app", "test")))),
port: Port::Number(80.try_into().unwrap()),
proxy_protocol: None,
access_policy: None,
},
};

Expand All @@ -60,6 +62,7 @@ async fn rejects_identitical_pod_selector() {
selector: Selector::Pod(api::labels::Selector::from_iter(Some(("app", "test")))),
port: Port::Number(80.try_into().unwrap()),
proxy_protocol: None,
access_policy: None,
};

let api = kube::Api::namespaced(client, &ns);
Expand Down Expand Up @@ -106,6 +109,7 @@ async fn rejects_all_pods_selected() {
selector: Selector::Pod(api::labels::Selector::from_iter(Some(("app", "test")))),
port: Port::Number(80.try_into().unwrap()),
proxy_protocol: Some(ProxyProtocol::Http2),
access_policy: None,
},
};
api.create(&kube::api::PostParams::default(), &test0)
Expand All @@ -123,6 +127,7 @@ async fn rejects_all_pods_selected() {
port: Port::Number(80.try_into().unwrap()),
// proxy protocol doesn't factor into the selection
proxy_protocol: Some(ProxyProtocol::Http1),
access_policy: None,
},
};
api.create(&kube::api::PostParams::default(), &test1)
Expand Down Expand Up @@ -179,3 +184,21 @@ async fn rejects_invalid_proxy_protocol() {
})
.await;
}

#[tokio::test(flavor = "current_thread")]
async fn rejects_invalid_access_policy() {
admission::rejects(|ns| Server {
metadata: api::ObjectMeta {
namespace: Some(ns),
name: Some("test".to_string()),
..Default::default()
},
spec: ServerSpec {
selector: Selector::Pod(api::labels::Selector::default()),
port: Port::Number(80.try_into().unwrap()),
proxy_protocol: None,
access_policy: Some("foobar".to_string()),
},
})
.await;
}
Loading
Loading