From e962bf857c619c5e1988a1ccb11c2792e52e134b Mon Sep 17 00:00:00 2001 From: Oliver Gould Date: Mon, 21 Mar 2022 12:08:32 -0700 Subject: [PATCH] policy: Test identity-parsing logic (#8103) The identity-string parsing logic is currently implemented directly in the ServerAuthorization indexer. In preparation of this being needed in additional modules, this change extracts identity parsing and adds some basic tests. Signed-off-by: Oliver Gould --- policy-controller/core/src/identity_match.rs | 64 ++++++++++++++++++- policy-controller/grpc/src/lib.rs | 2 +- policy-controller/k8s/index/src/lib.rs | 9 +++ .../k8s/index/src/server_authorization.rs | 50 ++++++--------- 4 files changed, 90 insertions(+), 35 deletions(-) diff --git a/policy-controller/core/src/identity_match.rs b/policy-controller/core/src/identity_match.rs index 4600b0740..3431d9378 100644 --- a/policy-controller/core/src/identity_match.rs +++ b/policy-controller/core/src/identity_match.rs @@ -1,10 +1,10 @@ -use std::fmt; +use std::{convert::Infallible, fmt, str::FromStr}; /// Matches a client's mesh identity. #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub enum IdentityMatch { /// An exact match. - Name(String), + Exact(String), /// A suffix match. Suffix(Vec), @@ -12,11 +12,29 @@ pub enum IdentityMatch { // === impl IdentityMatch === +impl FromStr for IdentityMatch { + type Err = Infallible; + + fn from_str(s: &str) -> Result { + if s == "*" { + return Ok(IdentityMatch::Suffix(vec![])); + } + + if s.starts_with("*.") { + return Ok(IdentityMatch::Suffix( + s.split('.').skip(1).map(|s| s.to_string()).collect(), + )); + } + + Ok(IdentityMatch::Exact(s.to_string())) + } +} + impl fmt::Display for IdentityMatch { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { use std::fmt::Write; match self { - Self::Name(name) => name.fmt(f), + Self::Exact(name) => name.fmt(f), Self::Suffix(suffix) => { f.write_char('*')?; for part in suffix { @@ -27,3 +45,43 @@ impl fmt::Display for IdentityMatch { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_star() { + assert_eq!("*".parse(), Ok(IdentityMatch::Suffix(vec![]))); + + assert_eq!( + "*.example.com".parse(), + Ok(IdentityMatch::Suffix(vec![ + "example".to_string(), + "com".to_string() + ])) + ); + assert_eq!( + "*.*.example.com".parse(), + Ok(IdentityMatch::Suffix(vec![ + "*".to_string(), + "example".to_string(), + "com".to_string() + ])) + ); + assert_eq!( + "x.example.com".parse(), + Ok(IdentityMatch::Exact("x.example.com".to_string())) + ); + + assert_eq!( + "**.example.com".parse(), + Ok(IdentityMatch::Exact("**.example.com".to_string())) + ); + + assert_eq!( + "foo.*.example.com".parse(), + Ok(IdentityMatch::Exact("foo.*.example.com".to_string())) + ); + } +} diff --git a/policy-controller/grpc/src/lib.rs b/policy-controller/grpc/src/lib.rs index 9eaa8a3ff..57cb6999b 100644 --- a/policy-controller/grpc/src/lib.rs +++ b/policy-controller/grpc/src/lib.rs @@ -266,7 +266,7 @@ fn to_authz( let identities = identities .iter() .filter_map(|i| match i { - IdentityMatch::Name(n) => Some(proto::Identity { + IdentityMatch::Exact(n) => Some(proto::Identity { name: n.to_string(), }), _ => None, diff --git a/policy-controller/k8s/index/src/lib.rs b/policy-controller/k8s/index/src/lib.rs index 3ff8b3808..c6be7e20d 100644 --- a/policy-controller/k8s/index/src/lib.rs +++ b/policy-controller/k8s/index/src/lib.rs @@ -124,3 +124,12 @@ impl Index { self.namespaces.index.get(ns) } } + +impl ClusterInfo { + fn service_account_identity(&self, ns: &str, sa: &str) -> String { + format!( + "{}.{}.serviceaccount.identity.{}.{}", + sa, ns, self.control_plane_ns, self.identity_domain + ) + } +} diff --git a/policy-controller/k8s/index/src/server_authorization.rs b/policy-controller/k8s/index/src/server_authorization.rs index 4e13f3529..d988d4e0f 100644 --- a/policy-controller/k8s/index/src/server_authorization.rs +++ b/policy-controller/k8s/index/src/server_authorization.rs @@ -235,39 +235,27 @@ fn mk_mtls_authn( return Ok(ClientAuthentication::TlsUnauthenticated); } - let mut identities = Vec::new(); + let ids = mtls + .identities + .into_iter() + .flatten() + .map(|id| match id.parse() { + Ok(id) => id, + Err(e) => match e {}, + }); - for id in mtls.identities.into_iter().flatten() { - if id == "*" { - debug!(suffix = %id, "Authenticated"); - identities.push(IdentityMatch::Suffix(vec![])); - } else if id.starts_with("*.") { - debug!(suffix = %id, "Authenticated"); - let mut parts = id.split('.'); - let star = parts.next(); - debug_assert_eq!(star, Some("*")); - identities.push(IdentityMatch::Suffix( - parts.map(|p| p.to_string()).collect::>(), - )); - } else { - debug!(%id, "Authenticated"); - identities.push(IdentityMatch::Name(id)); - } - } - - for sa in mtls.service_accounts.into_iter().flatten() { - let name = sa.name; - let ns = sa - .namespace - .unwrap_or_else(|| metadata.namespace.clone().unwrap()); - debug!(ns = %ns, serviceaccount = %name, "Authenticated"); - let n = format!( - "{}.{}.serviceaccount.identity.{}.{}", - name, ns, cluster.control_plane_ns, cluster.identity_domain - ); - identities.push(IdentityMatch::Name(n)); - } + let sas = mtls + .service_accounts + .into_iter() + .flatten() + .filter_map(|sa| { + let ns = sa.namespace.as_deref().or(metadata.namespace.as_deref())?; + Some(IdentityMatch::Exact( + cluster.service_account_identity(ns, &sa.name), + )) + }); + let identities = ids.chain(sas).collect::>(); if identities.is_empty() { bail!("authorization authorizes no clients"); }