From 6416777feff3a86a81d7877b553e6d9e0d3210ab Mon Sep 17 00:00:00 2001 From: Jesse Haka Date: Mon, 19 Feb 2024 08:57:46 +0200 Subject: [PATCH] api: ACL API design Signed-off-by: Jesse Haka --- api/v1alpha1/acl_types.go | 24 ++++++ api/v1alpha1/securitypolicy_types.go | 5 ++ .../validation/securitypolicy_validate.go | 30 ++++++- .../securitypolicy_validate_test.go | 82 ++++++++++++++++++- api/v1alpha1/zz_generated.deepcopy.go | 45 ++++++++++ ...ateway.envoyproxy.io_securitypolicies.yaml | 31 +++++++ site/content/en/latest/api/extension_types.md | 30 +++++++ 7 files changed, 242 insertions(+), 5 deletions(-) create mode 100644 api/v1alpha1/acl_types.go diff --git a/api/v1alpha1/acl_types.go b/api/v1alpha1/acl_types.go new file mode 100644 index 000000000000..5db69a710469 --- /dev/null +++ b/api/v1alpha1/acl_types.go @@ -0,0 +1,24 @@ +// Copyright Envoy Gateway Authors +// SPDX-License-Identifier: Apache-2.0 +// The full text of the Apache license is available in the LICENSE file at +// the root of the repo. + +package v1alpha1 + +// +kubebuilder:validation:XValidation:rule="(has(self.allow) || has(self.deny))",message="one of allow or deny must be specified" +// +// ACL defines the IP deny/allow configuration. +type ACL struct { + // Allow specifies the list of IPBlocks that are allowed to access the service. + // Other cidrs are denied. + Allow []IPBlock `json:"allow,omitempty"` + // Deny specifies the list of IPBlocks that are denied to access the service. + Deny []IPBlock `json:"deny,omitempty"` +} + +// IPBlock defines policy on a particular IPBlock. +type IPBlock struct { + // cidr is a string representing the IPBlock + // Valid examples are "192.168.1.0/24" or "2001:db8::/64" + CIDR string `json:"cidr,omitempty"` +} diff --git a/api/v1alpha1/securitypolicy_types.go b/api/v1alpha1/securitypolicy_types.go index 6d90536ae918..d14ccd92db93 100644 --- a/api/v1alpha1/securitypolicy_types.go +++ b/api/v1alpha1/securitypolicy_types.go @@ -70,6 +70,11 @@ type SecurityPolicySpec struct { // // +optional ExtAuth *ExtAuth `json:"extAuth,omitempty"` + + // ACL defines the IP deny/allow configuration. + // + // +optional + ACL *ACL `json:"acl,omitempty"` } // SecurityPolicyStatus defines the state of SecurityPolicy diff --git a/api/v1alpha1/validation/securitypolicy_validate.go b/api/v1alpha1/validation/securitypolicy_validate.go index 628d3f801730..d7ce32381903 100644 --- a/api/v1alpha1/validation/securitypolicy_validate.go +++ b/api/v1alpha1/validation/securitypolicy_validate.go @@ -8,6 +8,7 @@ package validation import ( "errors" "fmt" + "net" "net/mail" "net/url" @@ -24,7 +25,7 @@ func ValidateSecurityPolicy(policy *egv1a1.SecurityPolicy) error { return errors.New("policy is nil") } if err := validateSecurityPolicySpec(&policy.Spec); err != nil { - errs = append(errs, errors.New("policy is nil")) + errs = append(errs, err) } return utilerrors.NewAggregate(errs) @@ -42,6 +43,8 @@ func validateSecurityPolicySpec(spec *egv1a1.SecurityPolicySpec) error { sum++ case spec.JWT != nil: sum++ + case spec.ACL != nil: + sum++ } if sum == 0 { errs = append(errs, errors.New("no security policy is specified")) @@ -52,13 +55,36 @@ func validateSecurityPolicySpec(spec *egv1a1.SecurityPolicySpec) error { return utilerrors.NewAggregate(errs) } - if err := ValidateJWTProvider(spec.JWT.Providers); err != nil { + if spec.JWT != nil { + if err := ValidateJWTProvider(spec.JWT.Providers); err != nil { + errs = append(errs, err) + } + } + + if err := ValidateACL(spec.ACL); err != nil { errs = append(errs, err) } return utilerrors.NewAggregate(errs) } +// ValidateACL validates the provided ACL configuration. +func ValidateACL(acl *egv1a1.ACL) error { + var errs []error + if acl == nil { + return nil + } + + for _, ipBlock := range append(acl.Allow, acl.Deny...) { + _, _, err := net.ParseCIDR(ipBlock.CIDR) + if err != nil { + errs = append(errs, fmt.Errorf("invalid CIDR: %s", ipBlock.CIDR)) + } + } + + return utilerrors.NewAggregate(errs) +} + // ValidateJWTProvider validates the provided JWT authentication configuration. func ValidateJWTProvider(providers []egv1a1.JWTProvider) error { var errs []error diff --git a/api/v1alpha1/validation/securitypolicy_validate_test.go b/api/v1alpha1/validation/securitypolicy_validate_test.go index 489c7644f8b1..53b9da120ffc 100644 --- a/api/v1alpha1/validation/securitypolicy_validate_test.go +++ b/api/v1alpha1/validation/securitypolicy_validate_test.go @@ -8,7 +8,7 @@ package validation import ( "testing" - "github.com/stretchr/testify/require" + "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1" @@ -463,6 +463,82 @@ func TestValidateSecurityPolicy(t *testing.T) { }, expected: true, }, + { + name: "acl with valid ipv4 cidr", + policy: &egv1a1.SecurityPolicy{ + TypeMeta: metav1.TypeMeta{ + Kind: egv1a1.KindSecurityPolicy, + APIVersion: egv1a1.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "test", + }, + Spec: egv1a1.SecurityPolicySpec{ + ACL: &egv1a1.ACL{ + Allow: []egv1a1.IPBlock{{CIDR: "192.168.1.0/24"}}, + }, + }, + }, + expected: true, + }, + { + name: "acl with valid ipv6 cidr", + policy: &egv1a1.SecurityPolicy{ + TypeMeta: metav1.TypeMeta{ + Kind: egv1a1.KindSecurityPolicy, + APIVersion: egv1a1.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "test", + }, + Spec: egv1a1.SecurityPolicySpec{ + ACL: &egv1a1.ACL{ + Allow: []egv1a1.IPBlock{{CIDR: "2001:db8::/64"}}, + }, + }, + }, + expected: true, + }, + { + name: "acl with invalid ipv4 cidr", + policy: &egv1a1.SecurityPolicy{ + TypeMeta: metav1.TypeMeta{ + Kind: egv1a1.KindSecurityPolicy, + APIVersion: egv1a1.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "test", + }, + Spec: egv1a1.SecurityPolicySpec{ + ACL: &egv1a1.ACL{ + Allow: []egv1a1.IPBlock{{CIDR: "inva2.4.1.3/foo"}}, + }, + }, + }, + expected: false, + }, + { + name: "acl with invalid ipv6 cidr", + policy: &egv1a1.SecurityPolicy{ + TypeMeta: metav1.TypeMeta{ + Kind: egv1a1.KindSecurityPolicy, + APIVersion: egv1a1.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "test", + }, + Spec: egv1a1.SecurityPolicySpec{ + ACL: &egv1a1.ACL{ + Allow: []egv1a1.IPBlock{{CIDR: "20.db8::/64"}}, + }, + }, + }, + expected: false, + }, } for i := range testCases { @@ -470,9 +546,9 @@ func TestValidateSecurityPolicy(t *testing.T) { t.Run(tc.name, func(t *testing.T) { err := ValidateSecurityPolicy(tc.policy) if tc.expected { - require.NoError(t, err) + assert.NoError(t, err) } else { - require.Error(t, err) + assert.Error(t, err) } }) } diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 0d1ba2bbecce..bda524671a2a 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -19,6 +19,31 @@ import ( apisv1 "sigs.k8s.io/gateway-api/apis/v1" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ACL) DeepCopyInto(out *ACL) { + *out = *in + if in.Allow != nil { + in, out := &in.Allow, &out.Allow + *out = make([]IPBlock, len(*in)) + copy(*out, *in) + } + if in.Deny != nil { + in, out := &in.Deny, &out.Deny + *out = make([]IPBlock, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ACL. +func (in *ACL) DeepCopy() *ACL { + if in == nil { + return nil + } + out := new(ACL) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ActiveHealthCheck) DeepCopyInto(out *ActiveHealthCheck) { *out = *in @@ -1838,6 +1863,21 @@ func (in *HealthCheck) DeepCopy() *HealthCheck { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *IPBlock) DeepCopyInto(out *IPBlock) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IPBlock. +func (in *IPBlock) DeepCopy() *IPBlock { + if in == nil { + return nil + } + out := new(IPBlock) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *JSONPatchOperation) DeepCopyInto(out *JSONPatchOperation) { *out = *in @@ -3056,6 +3096,11 @@ func (in *SecurityPolicySpec) DeepCopyInto(out *SecurityPolicySpec) { *out = new(ExtAuth) (*in).DeepCopyInto(*out) } + if in.ACL != nil { + in, out := &in.ACL, &out.ACL + *out = new(ACL) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecurityPolicySpec. diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml index 7feb835b9387..67ffcb444a4f 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml @@ -44,6 +44,37 @@ spec: spec: description: Spec defines the desired state of SecurityPolicy. properties: + acl: + description: ACL defines the IP deny/allow configuration. + properties: + allow: + description: Allow specifies the list of IPBlocks that are allowed + to access the service. Other cidrs are denied. + items: + description: IPBlock defines policy on a particular IPBlock. + properties: + cidr: + description: cidr is a string representing the IPBlock Valid + examples are "192.168.1.0/24" or "2001:db8::/64" + type: string + type: object + type: array + deny: + description: Deny specifies the list of IPBlocks that are denied + to access the service. + items: + description: IPBlock defines policy on a particular IPBlock. + properties: + cidr: + description: cidr is a string representing the IPBlock Valid + examples are "192.168.1.0/24" or "2001:db8::/64" + type: string + type: object + type: array + type: object + x-kubernetes-validations: + - message: one of allow or deny must be specified + rule: (has(self.allow) || has(self.deny)) basicAuth: description: BasicAuth defines the configuration for the HTTP Basic Authentication. diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index c8b4b4fa7c7e..4caed4039b4d 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -27,6 +27,21 @@ API group. +#### ACL + + + +ACL defines the IP deny/allow configuration. + +_Appears in:_ +- [SecurityPolicySpec](#securitypolicyspec) + +| Field | Type | Required | Description | +| --- | --- | --- | --- | +| `allow` | _[IPBlock](#ipblock) array_ | true | Allow specifies the list of IPBlocks that are allowed to access the service. Other cidrs are denied. | +| `deny` | _[IPBlock](#ipblock) array_ | true | Deny specifies the list of IPBlocks that are denied to access the service. | + + #### ALPNProtocol _Underlying type:_ _string_ @@ -1229,6 +1244,20 @@ _Appears in:_ | `passive` | _[PassiveHealthCheck](#passivehealthcheck)_ | false | Passive passive check configuration | +#### IPBlock + + + +IPBlock defines policy on a particular IPBlock. + +_Appears in:_ +- [ACL](#acl) + +| Field | Type | Required | Description | +| --- | --- | --- | --- | +| `cidr` | _string_ | true | cidr is a string representing the IPBlock Valid examples are "192.168.1.0/24" or "2001:db8::/64" | + + #### InfrastructureProviderType _Underlying type:_ _string_ @@ -2154,6 +2183,7 @@ _Appears in:_ | `jwt` | _[JWT](#jwt)_ | false | JWT defines the configuration for JSON Web Token (JWT) authentication. | | `oidc` | _[OIDC](#oidc)_ | false | OIDC defines the configuration for the OpenID Connect (OIDC) authentication. | | `extAuth` | _[ExtAuth](#extauth)_ | false | ExtAuth defines the configuration for External Authorization. | +| `acl` | _[ACL](#acl)_ | false | ACL defines the IP deny/allow configuration. |