From 35a137797008dbe0babc701d9b9d50d9c28ebd33 Mon Sep 17 00:00:00 2001 From: Zahari Dichev Date: Thu, 4 Jan 2024 10:52:52 +0000 Subject: [PATCH] policy: allow spiffe ids in MeshTLSAuthentication Signed-off-by: Zahari Dichev --- Cargo.lock | 1 + .../policy/meshtls-authentication.yaml | 1 - policy-controller/Cargo.toml | 1 + policy-controller/src/admission.rs | 8 +- policy-controller/src/lib.rs | 1 + policy-controller/src/validation.rs | 245 ++++++++++++++++++ 6 files changed, 255 insertions(+), 2 deletions(-) create mode 100644 policy-controller/src/validation.rs diff --git a/Cargo.lock b/Cargo.lock index d3a442f4a15a8..cf4c90f1bf8a7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1181,6 +1181,7 @@ dependencies = [ "linkerd-policy-controller-k8s-status", "openssl", "parking_lot", + "regex", "serde", "serde_json", "thiserror", diff --git a/charts/linkerd-crds/templates/policy/meshtls-authentication.yaml b/charts/linkerd-crds/templates/policy/meshtls-authentication.yaml index 6840d5ee6d1db..58ee815f59ec7 100644 --- a/charts/linkerd-crds/templates/policy/meshtls-authentication.yaml +++ b/charts/linkerd-crds/templates/policy/meshtls-authentication.yaml @@ -49,7 +49,6 @@ spec: minItems: 1 items: type: string - pattern: '^(\*|[a-z0-9]([-a-z0-9]*[a-z0-9])?)(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$' identityRefs: type: array minItems: 1 diff --git a/policy-controller/Cargo.toml b/policy-controller/Cargo.toml index 5bf353d43ba9d..06f868ead63da 100644 --- a/policy-controller/Cargo.toml +++ b/policy-controller/Cargo.toml @@ -33,6 +33,7 @@ serde_json = "1" thiserror = "1" tokio-stream = { version = "0.1", features = ["sync"] } tracing = "0.1" +regex = "1" [dependencies.clap] version = "4" diff --git a/policy-controller/src/admission.rs b/policy-controller/src/admission.rs index 09515126bdb4f..63eb41cf2f33b 100644 --- a/policy-controller/src/admission.rs +++ b/policy-controller/src/admission.rs @@ -1,3 +1,4 @@ +use super::validation; use crate::k8s::{ labels, policy::{ @@ -296,7 +297,12 @@ fn validate_identity_ref(id: &NamespacedTargetRef) -> Result<()> { #[async_trait::async_trait] impl Validate for Admission { async fn validate(self, _ns: &str, _name: &str, spec: MeshTLSAuthenticationSpec) -> Result<()> { - // The CRD validates identity strings, but does not validate identity references. + for id in spec.identities.iter().flatten() { + if let Err(err) = validation::validate_identity(id) { + bail!("id {} is invalid: {}", id, err); + } + } + for id in spec.identity_refs.iter().flatten() { validate_identity_ref(id)?; } diff --git a/policy-controller/src/lib.rs b/policy-controller/src/lib.rs index 4fbff73604151..8efbb1cb99052 100644 --- a/policy-controller/src/lib.rs +++ b/policy-controller/src/lib.rs @@ -2,6 +2,7 @@ #![forbid(unsafe_code)] mod admission; pub mod index_list; +mod validation; pub use self::admission::Admission; use anyhow::Result; use linkerd_policy_controller_core::inbound::{ diff --git a/policy-controller/src/validation.rs b/policy-controller/src/validation.rs new file mode 100644 index 0000000000000..6e759b44701b9 --- /dev/null +++ b/policy-controller/src/validation.rs @@ -0,0 +1,245 @@ +use anyhow::Result; +use thiserror::Error; + +use regex::Regex; + +const SCHEME_PREFIX: &str = "spiffe://"; +const VALID_TRUST_DOMAIN_CHARS: &str = "abcdefghijklmnopqrstuvwxyz0123456789-._"; +const VALID_PATH_SEGMENT_CHARS: &str = + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-._"; + +const DNS_LIKE_IDENTITY_REGEX: &str = + r"^(\*|[a-z0-9]([-a-z0-9]*[a-z0-9])?)(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$"; + +#[derive(Debug, Error, PartialEq, Clone)] +pub enum IdError { + /// The trust domain name of SPIFFE ID cannot be empty. + #[error("SPIFFE trust domain is missing")] + MissingTrustDomain, + + /// A trust domain name can only contain chars in a limited char set. + #[error( + "SPIFFE trust domain characters are limited to lowercase letters, numbers, dots, dashes, and \ + underscores" + )] + BadTrustDomainChar, + + /// A path segment can only contain chars in a limited char set. + #[error( + "SPIFFE path segment characters are limited to letters, numbers, dots, dashes, and underscores" + )] + BadPathSegmentChar, + + /// Path cannot contain empty segments, e.g '//' + #[error("SPIFFE path cannot contain empty segments")] + EmptySegment, + + /// Path cannot contain dot segments, e.g '/.', '/..' + #[error("SPIFFE path cannot contain dot segments")] + DotSegment, + + /// Path cannot have a trailing slash. + #[error("SPIFFE path cannot have a trailing slash")] + TrailingSlash, + + #[error( + "identity must be a valid SPIFFE id or a DNS SAN, matching the regex: {}", + DNS_LIKE_IDENTITY_REGEX + )] + Invalid, +} + +// Validates that an ID is either in DNS or SPIFFE form. SPIFFE +// validation is based on https://github.com/spiffe/spiffe/blob/27b59b81ba8c56885ac5d4be73b35b9b3305fd7a/standards/SPIFFE-ID.md. +// Implementation is based on: https://github.com/maxlambrecht/rust-spiffe/blob/3d3614f70d0d7a4b9190ab9650e224f2ac362368/spiffe/src/spiffe_id/mod.rs +pub(crate) fn validate_identity(id: &str) -> Result<(), IdError> { + if let Some(rest) = id.strip_prefix(SCHEME_PREFIX) { + let i = rest.find('/').unwrap_or(rest.len()); + + if i == 0 { + return Err(IdError::MissingTrustDomain); + } + + let td = &rest[..i]; + if td.chars().any(|c| !VALID_TRUST_DOMAIN_CHARS.contains(c)) { + return Err(IdError::BadTrustDomainChar); + } + + let path = &rest[i..]; + + if !path.is_empty() { + validate_path(path)?; + } + + Ok(()) + } else { + let regex = Regex::new(DNS_LIKE_IDENTITY_REGEX).expect("should_compile"); + if !regex.is_match(id) { + return Err(IdError::Invalid); + } + Ok(()) + } +} + +/// Validates that a path string is a conformant path for a SPIFFE ID. +/// See https://github.com/spiffe/spiffe/blob/main/standards/SPIFFE-ID.md#22-path +fn validate_path(path: &str) -> Result<(), IdError> { + let chars = path.char_indices().peekable(); + let mut segment_start = 0; + + for (idx, c) in chars { + if c == '/' { + match &path[segment_start..idx] { + "/" => return Err(IdError::EmptySegment), + "/." | "/.." => return Err(IdError::DotSegment), + _ => {} + } + segment_start = idx; + continue; + } + + if !VALID_PATH_SEGMENT_CHARS.contains(c) { + return Err(IdError::BadPathSegmentChar); + } + } + + match &path[segment_start..] { + "/" => return Err(IdError::TrailingSlash), + "/." | "/.." => return Err(IdError::DotSegment), + _ => {} + } + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn valid_dns() { + assert!(validate_identity("system.local").is_ok()) + } + + #[test] + fn valid_dns_all() { + assert!(validate_identity("**").is_ok()) + } + + #[test] + fn valid_dns_prefix() { + assert!(validate_identity("*.system.local").is_ok()) + } + + #[test] + fn invalid_dns_suffix() { + let err = validate_identity("system.local.*").unwrap_err(); + assert_eq!(err, IdError::Invalid); + } + + #[test] + fn invalid_dns_trailing_dot() { + let err = validate_identity("system.local.").unwrap_err(); + assert_eq!(err, IdError::Invalid); + } + + #[test] + fn invalid_dns_leading_dot() { + let err = validate_identity(".system.local").unwrap_err(); + assert_eq!(err, IdError::Invalid); + } + + #[test] + fn invalid_dns_double_dots() { + let err = validate_identity("system..local").unwrap_err(); + assert_eq!(err, IdError::Invalid); + } + + #[test] + fn valid_spiffe_no_path() { + assert!(validate_identity("spiffe://trustdomain").is_ok()) + } + + #[test] + fn valid_spiffe_with_path() { + assert!(validate_identity("spiffe://trustdomain/path/element").is_ok()) + } + + #[test] + fn invalid_spiffe_scheme() { + let err = validate_identity("http://domain.test/path/element").unwrap_err(); + assert_eq!(err, IdError::Invalid); + } + + #[test] + fn invalid_spiffe_wrong_scheme() { + let err = validate_identity("spiffe:/path/element").unwrap_err(); + assert_eq!(err, IdError::Invalid); + } + + #[test] + fn invalid_spiffe_empty_trust_domain() { + let err = validate_identity("spiffe:///path/element").unwrap_err(); + assert_eq!(err, IdError::MissingTrustDomain); + } + + #[test] + fn invalid_spiffe_no_slashes_in_scheme() { + let err = validate_identity("spiffe:path/element").unwrap_err(); + assert_eq!(err, IdError::Invalid); + } + + #[test] + fn invalid_spiffe_uri_with_query() { + let err = validate_identity("spiffe://domain.test/path/element?query=").unwrap_err(); + assert_eq!(err, IdError::BadPathSegmentChar); + } + + #[test] + fn invalid_spiffe_uri_with_fragment() { + let err = validate_identity("spiffe://domain.test/path/element#fragment-1").unwrap_err(); + assert_eq!(err, IdError::BadPathSegmentChar); + } + + #[test] + fn invalid_spiffe_uri_with_str_port() { + let err = validate_identity("spiffe://domain.test:8080/path/element").unwrap_err(); + assert_eq!(err, IdError::BadTrustDomainChar); + } + + #[test] + fn invalid_spiffe_uri_with_user_info() { + let err = validate_identity("spiffe://user:password@test.org/path/element").unwrap_err(); + assert_eq!(err, IdError::BadTrustDomainChar); + } + + #[test] + fn invalid_spiffe_uri_with_trailing_slash() { + let err = validate_identity("spiffe://test.org/").unwrap_err(); + assert_eq!(err, IdError::TrailingSlash); + } + + #[test] + fn invalid_spiffe_uri_with_empty_segment() { + let err = validate_identity("spiffe://test.org//").unwrap_err(); + assert_eq!(err, IdError::EmptySegment); + } + + #[test] + fn invalid_spiffe_uri_str_with_path_with_trailing_slash() { + let err = validate_identity("spiffe://test.org/path/other/").unwrap_err(); + assert_eq!(err, IdError::TrailingSlash); + } + + #[test] + fn invalid_spiffe_uri_str_with_dot_segment() { + let err = validate_identity("spiffe://test.org/./other").unwrap_err(); + assert_eq!(err, IdError::DotSegment); + } + + #[test] + fn invalid_spiffe_uri_str_with_double_dot_segment() { + let err = validate_identity("spiffe://test.org/../other").unwrap_err(); + assert_eq!(err, IdError::DotSegment); + } +}