Skip to content

Commit 214726d

Browse files
zhaohuabingzhaohuabing
committed
reject csrf additionalOrigins that carry a scheme or path
Envoy strips the scheme from the Origin header before matching, and the header never carries a path, so a value like `https://www.example.com` is accepted by the CRD but can never match, silently 403ing legitimate requests. Add a CEL rule on the CSRF type that rejects such values at admission, with cel-validation coverage for the valid host / host:port forms and the scheme and path forms. Also point the Gateway/HTTPRoute/GRPCRoute links in the CSRF task doc at /reference/api-types/, matching the other task docs; the old /api-types/ paths 404 and broke docs-check-links. Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
1 parent 8a5bc79 commit 214726d

8 files changed

Lines changed: 110 additions & 4 deletions

File tree

‎api/v1alpha1/csrf_types.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import (
1818
// (the scheme is stripped before matching). Additional origins must be specified as
1919
// host or host:port values, not full URLs. For example, use "www.example.com"
2020
// instead of "https://www.example.com".
21+
//
22+
// +kubebuilder:validation:XValidation:message="additionalOrigins must be host or host:port values without a scheme or path, for example www.example.com instead of https://www.example.com",rule="!has(self.additionalOrigins) || self.additionalOrigins.all(o, !o.value.contains('/'))"
2123
type CSRF struct {
2224
// EnforcedFraction represents the fraction of requests for which the CSRF
2325
// policy is enforced. Requests that are not selected are allowed through

‎charts/gateway-crds-helm/templates/generated/gateway.envoyproxy.io_securitypolicies.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -778,6 +778,11 @@ spec:
778778
- message: numerator must be less than or equal to denominator
779779
rule: self.numerator <= self.denominator
780780
type: object
781+
x-kubernetes-validations:
782+
- message: additionalOrigins must be host or host:port values without
783+
a scheme or path, for example www.example.com instead of https://www.example.com
784+
rule: '!has(self.additionalOrigins) || self.additionalOrigins.all(o,
785+
!o.value.contains(''/''))'
781786
extAuth:
782787
description: ExtAuth defines the configuration for External Authorization.
783788
properties:

‎charts/gateway-helm/charts/crds/crds/generated/gateway.envoyproxy.io_securitypolicies.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -777,6 +777,11 @@ spec:
777777
- message: numerator must be less than or equal to denominator
778778
rule: self.numerator <= self.denominator
779779
type: object
780+
x-kubernetes-validations:
781+
- message: additionalOrigins must be host or host:port values without
782+
a scheme or path, for example www.example.com instead of https://www.example.com
783+
rule: '!has(self.additionalOrigins) || self.additionalOrigins.all(o,
784+
!o.value.contains(''/''))'
780785
extAuth:
781786
description: ExtAuth defines the configuration for External Authorization.
782787
properties:

‎site/content/en/latest/tasks/security/csrf.md‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ Non-mutating requests (GET, HEAD, OPTIONS) are not affected.
2121

2222
Note: Envoy's CSRF filter compares against the host and port of the origin only (the scheme is stripped
2323
before matching). Additional origins must be specified as `host` or `host:port` values, not full URLs.
24-
For example, use `www.example.com` instead of `https://www.example.com`.
24+
For example, use `www.example.com` instead of `https://www.example.com`. A SecurityPolicy whose
25+
`additionalOrigins` contain a scheme or a path is rejected at admission, since such a value could never match.
2526

2627
The filter supports gradual rollout via `enforcedFraction` (the fraction of requests for which the policy is
2728
enforced, defaults to 100%) and `shadowFraction` (the fraction of requests evaluated in dry-run mode without
@@ -116,6 +117,6 @@ so you can monitor the impact before enabling enforcement.
116117

117118
[csrf]: https://owasp.org/www-community/attacks/csrf
118119
[SecurityPolicy]: ../../../api/extension_types#securitypolicy
119-
[Gateway]: https://gateway-api.sigs.k8s.io/api-types/gateway
120-
[HTTPRoute]: https://gateway-api.sigs.k8s.io/api-types/httproute
121-
[GRPCRoute]: https://gateway-api.sigs.k8s.io/api-types/grpcroute
120+
[Gateway]: https://gateway-api.sigs.k8s.io/reference/api-types/gateway/
121+
[HTTPRoute]: https://gateway-api.sigs.k8s.io/reference/api-types/httproute/
122+
[GRPCRoute]: https://gateway-api.sigs.k8s.io/reference/api-types/grpcroute/

‎test/cel-validation/securitypolicy_test.go‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -627,6 +627,84 @@ func TestSecurityPolicyTarget(t *testing.T) {
627627
},
628628
},
629629

630+
// csrf
631+
{
632+
desc: "csrf additionalOrigins valid with host and host:port",
633+
mutate: func(sp *egv1a1.SecurityPolicy) {
634+
sp.Spec = egv1a1.SecurityPolicySpec{
635+
CSRF: &egv1a1.CSRF{
636+
AdditionalOrigins: []egv1a1.StringMatch{
637+
{Type: new(egv1a1.StringMatchExact), Value: "www.example.com"},
638+
{Type: new(egv1a1.StringMatchExact), Value: "www.example.com:8080"},
639+
{Type: new(egv1a1.StringMatchSuffix), Value: ".trusted.com"},
640+
{Type: new(egv1a1.StringMatchRegularExpression), Value: `.*\.partner\.com$`},
641+
},
642+
},
643+
PolicyTargetReferences: egv1a1.PolicyTargetReferences{
644+
TargetRef: &gwapiv1.LocalPolicyTargetReferenceWithSectionName{
645+
LocalPolicyTargetReference: gwapiv1.LocalPolicyTargetReference{
646+
Group: gwapiv1.Group("gateway.networking.k8s.io"),
647+
Kind: gwapiv1.Kind("Gateway"),
648+
Name: gwapiv1.ObjectName("eg"),
649+
},
650+
},
651+
},
652+
}
653+
},
654+
wantErrors: []string{},
655+
},
656+
{
657+
desc: "csrf additionalOrigins invalid with scheme",
658+
mutate: func(sp *egv1a1.SecurityPolicy) {
659+
sp.Spec = egv1a1.SecurityPolicySpec{
660+
CSRF: &egv1a1.CSRF{
661+
AdditionalOrigins: []egv1a1.StringMatch{
662+
// invalid, Envoy strips the scheme from the Origin header
663+
// before matching, so this could never match.
664+
{Type: new(egv1a1.StringMatchExact), Value: "https://www.example.com"},
665+
},
666+
},
667+
PolicyTargetReferences: egv1a1.PolicyTargetReferences{
668+
TargetRef: &gwapiv1.LocalPolicyTargetReferenceWithSectionName{
669+
LocalPolicyTargetReference: gwapiv1.LocalPolicyTargetReference{
670+
Group: gwapiv1.Group("gateway.networking.k8s.io"),
671+
Kind: gwapiv1.Kind("Gateway"),
672+
Name: gwapiv1.ObjectName("eg"),
673+
},
674+
},
675+
},
676+
}
677+
},
678+
wantErrors: []string{
679+
"additionalOrigins must be host or host:port values without a scheme or path, for example www.example.com instead of https://www.example.com",
680+
},
681+
},
682+
{
683+
desc: "csrf additionalOrigins invalid with path",
684+
mutate: func(sp *egv1a1.SecurityPolicy) {
685+
sp.Spec = egv1a1.SecurityPolicySpec{
686+
CSRF: &egv1a1.CSRF{
687+
AdditionalOrigins: []egv1a1.StringMatch{
688+
// invalid, the Origin header never carries a path.
689+
{Type: new(egv1a1.StringMatchPrefix), Value: "www.example.com/app"},
690+
},
691+
},
692+
PolicyTargetReferences: egv1a1.PolicyTargetReferences{
693+
TargetRef: &gwapiv1.LocalPolicyTargetReferenceWithSectionName{
694+
LocalPolicyTargetReference: gwapiv1.LocalPolicyTargetReference{
695+
Group: gwapiv1.Group("gateway.networking.k8s.io"),
696+
Kind: gwapiv1.Kind("Gateway"),
697+
Name: gwapiv1.ObjectName("eg"),
698+
},
699+
},
700+
},
701+
}
702+
},
703+
wantErrors: []string{
704+
"additionalOrigins must be host or host:port values without a scheme or path, for example www.example.com instead of https://www.example.com",
705+
},
706+
},
707+
630708
// ExtAuth
631709
{
632710
desc: "GRPC external auth service",

‎test/helm/gateway-crds-helm/all.out.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53374,6 +53374,11 @@ spec:
5337453374
- message: numerator must be less than or equal to denominator
5337553375
rule: self.numerator <= self.denominator
5337653376
type: object
53377+
x-kubernetes-validations:
53378+
- message: additionalOrigins must be host or host:port values without
53379+
a scheme or path, for example www.example.com instead of https://www.example.com
53380+
rule: '!has(self.additionalOrigins) || self.additionalOrigins.all(o,
53381+
!o.value.contains(''/''))'
5337753382
extAuth:
5337853383
description: ExtAuth defines the configuration for External Authorization.
5337953384
properties:

‎test/helm/gateway-crds-helm/e2e.out.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29312,6 +29312,11 @@ spec:
2931229312
- message: numerator must be less than or equal to denominator
2931329313
rule: self.numerator <= self.denominator
2931429314
type: object
29315+
x-kubernetes-validations:
29316+
- message: additionalOrigins must be host or host:port values without
29317+
a scheme or path, for example www.example.com instead of https://www.example.com
29318+
rule: '!has(self.additionalOrigins) || self.additionalOrigins.all(o,
29319+
!o.value.contains(''/''))'
2931529320
extAuth:
2931629321
description: ExtAuth defines the configuration for External Authorization.
2931729322
properties:

‎test/helm/gateway-crds-helm/envoy-gateway-crds.out.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29312,6 +29312,11 @@ spec:
2931229312
- message: numerator must be less than or equal to denominator
2931329313
rule: self.numerator <= self.denominator
2931429314
type: object
29315+
x-kubernetes-validations:
29316+
- message: additionalOrigins must be host or host:port values without
29317+
a scheme or path, for example www.example.com instead of https://www.example.com
29318+
rule: '!has(self.additionalOrigins) || self.additionalOrigins.all(o,
29319+
!o.value.contains(''/''))'
2931529320
extAuth:
2931629321
description: ExtAuth defines the configuration for External Authorization.
2931729322
properties:

0 commit comments

Comments
 (0)