policy: Support empty `requiredAuthenticationRefs` (#8185)

Currently, the policy admission controller requires that the
`AuthorizationPolicy` resources include a non-empty
`requiredAuthenticationRefs` field. This means that all authorization
policies require at least a `NetworkAuthentication` to permit traffic.
For example:

```yaml
---
apiVersion: policy.linkerd.io/v1alpha1
kind: AuthorizationPolicy
metadata:
  name: ingress
spec:
  targetRef:
    group: policy.linkerd.io
    kind: Server
    name: ingress-http
  requiredAuthenticationRefs:
  - group: policy.linkerd.io
    kind: NetworkAuthentication
    name: all-nets
---
apiVersion: policy.linkerd.io/v1alpha1
kind: NetworkAuthentication
metadata:
  name: ingress-all-nets
spec:
  networks:
  - cidr: 0.0.0.0/0
  - cidr: ::/0
```

This is needlessly verbose and can more simply be expressed as:

```yaml
---
apiVersion: policy.linkerd.io/v1alpha1
kind: AuthorizationPolicy
metadata:
  name: ingress
spec:
  targetRef:
    group: policy.linkerd.io
    kind: Server
    name: ingress-http
  requiredAuthenticationRefs: []
```

That is: there are explicitly no required authentications for this
policy.

This change updates the admission controller to permit such a policy.
Note that the `requiredAuthenticationRefs` field is still required so
that it's harder for simple misconfigurations to result in allowing
traffic.

This change also removes `Default` implementation for resources where do
they not make sense because there are required fields.

Signed-off-by: Oliver Gould <ver@buoyant.io>
This commit is contained in:
Oliver Gould 2022-04-01 10:16:35 -07:00 committed by GitHub
parent 7df00c5b4e
commit 1a0c1c3172
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 59 additions and 44 deletions

View File

@ -1,13 +1,7 @@
use super::{LocalTargetRef, NamespacedTargetRef};
#[derive(
Clone,
Debug,
Default,
kube::CustomResource,
serde::Deserialize,
serde::Serialize,
schemars::JsonSchema,
Clone, Debug, kube::CustomResource, serde::Deserialize, serde::Serialize, schemars::JsonSchema,
)]
#[kube(
group = "policy.linkerd.io",

View File

@ -1,24 +1,18 @@
#[derive(
Clone, Debug, Default, PartialEq, serde::Deserialize, serde::Serialize, schemars::JsonSchema,
)]
#[derive(Clone, Debug, PartialEq, serde::Deserialize, serde::Serialize, schemars::JsonSchema)]
pub struct ClusterTargetRef {
pub group: Option<String>,
pub kind: String,
pub name: String,
}
#[derive(
Clone, Debug, Default, PartialEq, serde::Deserialize, serde::Serialize, schemars::JsonSchema,
)]
#[derive(Clone, Debug, PartialEq, serde::Deserialize, serde::Serialize, schemars::JsonSchema)]
pub struct LocalTargetRef {
pub group: Option<String>,
pub kind: String,
pub name: String,
}
#[derive(
Clone, Debug, Default, PartialEq, serde::Deserialize, serde::Serialize, schemars::JsonSchema,
)]
#[derive(Clone, Debug, PartialEq, serde::Deserialize, serde::Serialize, schemars::JsonSchema)]
pub struct NamespacedTargetRef {
pub group: Option<String>,
pub kind: String,
@ -242,9 +236,9 @@ mod tests {
#[test]
fn cluster_targets_namespace() {
let t = ClusterTargetRef {
group: None,
kind: "Namespace".to_string(),
name: "appns".to_string(),
..Default::default()
};
assert!(t.targets_kind::<Namespace>());
assert!(t.targets(&Namespace {
@ -260,10 +254,10 @@ mod tests {
fn namespaced_targets_service_account() {
for tgt in &[
NamespacedTargetRef {
group: None,
kind: "ServiceAccount".to_string(),
name: "default".to_string(),
namespace: Some("appns".to_string()),
..Default::default()
},
NamespacedTargetRef {
group: Some("core".to_string()),
@ -278,9 +272,10 @@ mod tests {
namespace: Some("APPNS".to_string()),
},
NamespacedTargetRef {
group: None,
kind: "ServiceAccount".to_string(),
name: "default".to_string(),
..Default::default()
namespace: None,
},
] {
assert!(tgt.targets_kind::<ServiceAccount>());
@ -317,9 +312,10 @@ mod tests {
}
let tgt = NamespacedTargetRef {
group: None,
kind: "ServiceAccount".to_string(),
name: "default".to_string(),
..Default::default()
namespace: None,
};
assert!(
{

View File

@ -49,7 +49,7 @@ fn links_authorization_policy_with_mtls_name() {
group: Some("policy.linkerd.io".to_string()),
kind: "NetworkAuthentication".to_string(),
name: "net-foo".to_string(),
..Default::default()
namespace: None,
},
NamespacedTargetRef {
group: Some("policy.linkerd.io".to_string()),

View File

@ -229,10 +229,6 @@ impl Validate<AuthorizationPolicySpec> for Admission {
bail!("unsupported authentication kind(s): {}", kinds.join(", "));
}
if mtls_authns_count + net_authns_count == 0 {
bail!("at least one authentication reference must be set");
}
Ok(())
}
}

View File

@ -23,7 +23,7 @@ async fn accepts_valid() {
group: Some("policy.linkerd.io".to_string()),
kind: "MeshTLSAuthentication".to_string(),
name: "mtls-clients".to_string(),
..Default::default()
namespace: None,
},
NamespacedTargetRef {
group: Some("policy.linkerd.io".to_string()),
@ -55,7 +55,7 @@ async fn accepts_valid_with_only_meshtls() {
group: Some("policy.linkerd.io".to_string()),
kind: "MeshTLSAuthentication".to_string(),
name: "mtls-clients".to_string(),
..Default::default()
namespace: None,
}],
},
})
@ -88,21 +88,8 @@ async fn accepts_valid_with_only_network() {
}
#[tokio::test(flavor = "current_thread")]
async fn rejects_empty() {
admission::rejects(|ns| AuthorizationPolicy {
metadata: api::ObjectMeta {
namespace: Some(ns),
name: Some("test".to_string()),
..Default::default()
},
spec: AuthorizationPolicySpec::default(),
})
.await;
}
#[tokio::test(flavor = "current_thread")]
async fn rejects_empty_required_authentications() {
admission::rejects(|ns| AuthorizationPolicy {
async fn accepts_empty_required_authentications() {
admission::accepts(|ns| AuthorizationPolicy {
metadata: api::ObjectMeta {
namespace: Some(ns),
name: Some("test".to_string()),
@ -120,6 +107,46 @@ async fn rejects_empty_required_authentications() {
.await;
}
#[tokio::test(flavor = "current_thread")]
async fn rejects_missing_required_authentications() {
#[derive(
Clone,
Debug,
kube::CustomResource,
serde::Deserialize,
serde::Serialize,
schemars::JsonSchema,
)]
#[kube(
group = "policy.linkerd.io",
version = "v1alpha1",
kind = "AuthorizationPolicy",
namespaced
)]
#[serde(rename_all = "camelCase")]
pub struct FakeAuthorizationPolicySpec {
pub target_ref: LocalTargetRef,
pub required_authentication_refs: Option<Vec<NamespacedTargetRef>>,
}
admission::rejects(|ns| AuthorizationPolicy {
metadata: api::ObjectMeta {
namespace: Some(ns),
name: Some("test".to_string()),
..Default::default()
},
spec: FakeAuthorizationPolicySpec {
target_ref: LocalTargetRef {
group: Some("policy.linkerd.io".to_string()),
kind: "Server".to_string(),
name: "deny".to_string(),
},
required_authentication_refs: None,
},
})
.await;
}
#[tokio::test(flavor = "current_thread")]
async fn rejects_target_ref_deployment() {
admission::rejects(|ns| AuthorizationPolicy {

View File

@ -14,9 +14,10 @@ async fn accepts_valid_ref() {
},
spec: MeshTLSAuthenticationSpec {
identity_refs: Some(vec![NamespacedTargetRef {
group: None,
kind: "ServiceAccount".to_string(),
name: "default".to_string(),
..Default::default()
namespace: None,
}]),
..Default::default()
},
@ -64,9 +65,10 @@ async fn rejects_both_refs_and_strings() {
spec: MeshTLSAuthenticationSpec {
identities: Some(vec!["example.id".to_string()]),
identity_refs: Some(vec![NamespacedTargetRef {
group: None,
kind: "ServiceAccount".to_string(),
name: "default".to_string(),
..Default::default()
namespace: None,
}]),
},
})