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

policy: allow spiffe ids in MeshTLSAuthentication #11882

Merged
merged 2 commits into from
Jan 5, 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 Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1181,6 +1181,7 @@ dependencies = [
"linkerd-policy-controller-k8s-status",
"openssl",
"parking_lot",
"regex",
"serde",
"serde_json",
"thiserror",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion cli/cmd/testdata/install_crds.golden

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

1 change: 0 additions & 1 deletion cli/cmd/testdata/install_helm_crds_output.golden

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

1 change: 0 additions & 1 deletion cli/cmd/testdata/install_helm_crds_output_ha.golden

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

1 change: 1 addition & 0 deletions policy-controller/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
8 changes: 7 additions & 1 deletion policy-controller/src/admission.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use super::validation;
use crate::k8s::{
labels,
policy::{
Expand Down Expand Up @@ -296,7 +297,12 @@ fn validate_identity_ref(id: &NamespacedTargetRef) -> Result<()> {
#[async_trait::async_trait]
impl Validate<MeshTLSAuthenticationSpec> 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)?;
}
Expand Down
1 change: 1 addition & 0 deletions policy-controller/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down
245 changes: 245 additions & 0 deletions policy-controller/src/validation.rs
Original file line number Diff line number Diff line change
@@ -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);
}
}
Loading