Skip to content

Commit 9e6dd18

Browse files
committed
api: drop csrf enforcedFraction in favour of shadowFraction
Partially enforcing an origin check is not a useful security posture: with enforcedFraction below 100% a fraction of cross-site mutating requests is let through unvalidated and unobserved. The legitimate use case behind the knob is dry running the filter before turning it on, which shadowFraction already covers, so drop enforcedFraction and make shadowFraction the single rollout control. Envoy only lets an invalid request through in shadow mode if that request was not also selected by filter_enabled, so filter_enabled is now derived as the complement of the shadow fraction. Every request is therefore either enforced or observed, never neither, and a shadow fraction of 100% is a complete dry run instead of a no-op. Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
1 parent 4c89dc4 commit 9e6dd18

21 files changed

Lines changed: 165 additions & 204 deletions

File tree

api/v1alpha1/csrf_types.go

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -21,21 +21,16 @@ import (
2121
//
2222
// +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('/'))"
2323
type CSRF struct {
24-
// EnforcedFraction represents the fraction of requests for which the CSRF
25-
// policy is enforced. Requests that are not selected are allowed through
26-
// without any origin validation.
27-
// Defaults to 100% (all requests are enforced) if not specified.
24+
// ShadowFraction represents the fraction of requests for which the CSRF policy is
25+
// evaluated in shadow (dry-run) mode. For these requests, the filter records whether
26+
// the request would have been allowed or rejected in the `csrf.request_valid` and
27+
// `csrf.request_invalid` stats, but always lets the request through. The remaining
28+
// requests are enforced, i.e. a mutating request with a missing or non-matching
29+
// Origin header is rejected with a 403.
2830
//
29-
// +optional
30-
EnforcedFraction *gwapiv1.Fraction `json:"enforcedFraction,omitempty"`
31-
32-
// ShadowFraction represents the fraction of requests for which the CSRF
33-
// policy is evaluated in shadow (dry-run) mode. In this mode, the filter
34-
// evaluates requests and tracks whether they would be allowed or rejected in
35-
// the `csrf.request_invalid` and `csrf.request_valid` stats, but does not
36-
// enforce the policy. This is useful for rolling out CSRF protection
37-
// gradually while monitoring the impact.
38-
// Only takes effect for requests that are not selected by EnforcedFraction.
31+
// Defaults to 0% (all requests are enforced) if not specified. Set it to 100% to
32+
// dry run the filter, watch the stats to find origins that would be rejected, then
33+
// lower it to roll enforcement out gradually.
3934
//
4035
// +optional
4136
ShadowFraction *gwapiv1.Fraction `json:"shadowFraction,omitempty"`

api/v1alpha1/zz_generated.deepcopy.go

Lines changed: 0 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 10 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -730,37 +730,18 @@ spec:
730730
type: object
731731
maxItems: 16
732732
type: array
733-
enforcedFraction:
734-
description: |-
735-
EnforcedFraction represents the fraction of requests for which the CSRF
736-
policy is enforced. Requests that are not selected are allowed through
737-
without any origin validation.
738-
Defaults to 100% (all requests are enforced) if not specified.
739-
properties:
740-
denominator:
741-
default: 100
742-
format: int32
743-
minimum: 1
744-
type: integer
745-
numerator:
746-
format: int32
747-
minimum: 0
748-
type: integer
749-
required:
750-
- numerator
751-
type: object
752-
x-kubernetes-validations:
753-
- message: numerator must be less than or equal to denominator
754-
rule: self.numerator <= self.denominator
755733
shadowFraction:
756734
description: |-
757-
ShadowFraction represents the fraction of requests for which the CSRF
758-
policy is evaluated in shadow (dry-run) mode. In this mode, the filter
759-
evaluates requests and tracks whether they would be allowed or rejected in
760-
the `csrf.request_invalid` and `csrf.request_valid` stats, but does not
761-
enforce the policy. This is useful for rolling out CSRF protection
762-
gradually while monitoring the impact.
763-
Only takes effect for requests that are not selected by EnforcedFraction.
735+
ShadowFraction represents the fraction of requests for which the CSRF policy is
736+
evaluated in shadow (dry-run) mode. For these requests, the filter records whether
737+
the request would have been allowed or rejected in the `csrf.request_valid` and
738+
`csrf.request_invalid` stats, but always lets the request through. The remaining
739+
requests are enforced, i.e. a mutating request with a missing or non-matching
740+
Origin header is rejected with a 403.
741+
742+
Defaults to 0% (all requests are enforced) if not specified. Set it to 100% to
743+
dry run the filter, watch the stats to find origins that would be rejected, then
744+
lower it to roll enforcement out gradually.
764745
properties:
765746
denominator:
766747
default: 100

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

Lines changed: 10 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -729,37 +729,18 @@ spec:
729729
type: object
730730
maxItems: 16
731731
type: array
732-
enforcedFraction:
733-
description: |-
734-
EnforcedFraction represents the fraction of requests for which the CSRF
735-
policy is enforced. Requests that are not selected are allowed through
736-
without any origin validation.
737-
Defaults to 100% (all requests are enforced) if not specified.
738-
properties:
739-
denominator:
740-
default: 100
741-
format: int32
742-
minimum: 1
743-
type: integer
744-
numerator:
745-
format: int32
746-
minimum: 0
747-
type: integer
748-
required:
749-
- numerator
750-
type: object
751-
x-kubernetes-validations:
752-
- message: numerator must be less than or equal to denominator
753-
rule: self.numerator <= self.denominator
754732
shadowFraction:
755733
description: |-
756-
ShadowFraction represents the fraction of requests for which the CSRF
757-
policy is evaluated in shadow (dry-run) mode. In this mode, the filter
758-
evaluates requests and tracks whether they would be allowed or rejected in
759-
the `csrf.request_invalid` and `csrf.request_valid` stats, but does not
760-
enforce the policy. This is useful for rolling out CSRF protection
761-
gradually while monitoring the impact.
762-
Only takes effect for requests that are not selected by EnforcedFraction.
734+
ShadowFraction represents the fraction of requests for which the CSRF policy is
735+
evaluated in shadow (dry-run) mode. For these requests, the filter records whether
736+
the request would have been allowed or rejected in the `csrf.request_valid` and
737+
`csrf.request_invalid` stats, but always lets the request through. The remaining
738+
requests are enforced, i.e. a mutating request with a missing or non-matching
739+
Origin header is rejected with a 403.
740+
741+
Defaults to 0% (all requests are enforced) if not specified. Set it to 100% to
742+
dry run the filter, watch the stats to find origins that would be rejected, then
743+
lower it to roll enforcement out gradually.
763744
properties:
764745
denominator:
765746
default: 100

internal/gatewayapi/securitypolicy.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1758,7 +1758,6 @@ func (t *Translator) buildCSRF(csrf *egv1a1.CSRF) *ir.CSRF {
17581758
additionalOrigins = append(additionalOrigins, irStringMatch("csrf", origin))
17591759
}
17601760
return &ir.CSRF{
1761-
EnforcedFraction: csrf.EnforcedFraction,
17621761
ShadowFraction: csrf.ShadowFraction,
17631762
AdditionalOrigins: additionalOrigins,
17641763
}

internal/gatewayapi/testdata/securitypolicy-with-csrf.in.yaml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@ securityPolicies:
1010
kind: HTTPRoute
1111
name: httproute-1
1212
csrf:
13-
enforcedFraction:
14-
numerator: 80
1513
shadowFraction:
1614
numerator: 20
1715
additionalOrigins:

internal/gatewayapi/testdata/securitypolicy-with-csrf.out.yaml

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,6 @@ securityPolicies:
112112
value: .trusted.com
113113
- type: RegularExpression
114114
value: .*\.partner\.com$
115-
enforcedFraction:
116-
numerator: 80
117115
shadowFraction:
118116
numerator: 20
119117
targetRef:
@@ -227,8 +225,6 @@ xdsIR:
227225
- distinct: false
228226
name: csrf
229227
safeRegex: .*\.partner\.com$
230-
enforcedFraction:
231-
numerator: 80
232228
shadowFraction:
233229
numerator: 20
234230
readyListener:

internal/ir/xds.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1232,10 +1232,8 @@ type CORS struct {
12321232
//
12331233
// +k8s:deepcopy-gen=true
12341234
type CSRF struct {
1235-
// EnforcedFraction is the fraction of requests for which CSRF is enforced.
1236-
// nil means 100%.
1237-
EnforcedFraction *gwapiv1.Fraction `json:"enforcedFraction,omitempty" yaml:"enforcedFraction,omitempty"`
12381235
// ShadowFraction is the fraction of requests evaluated in shadow/dry-run mode.
1236+
// The remaining requests are enforced. nil means 0%, i.e. everything is enforced.
12391237
ShadowFraction *gwapiv1.Fraction `json:"shadowFraction,omitempty" yaml:"shadowFraction,omitempty"`
12401238
// AdditionalOrigins specifies additional origins that are allowed.
12411239
AdditionalOrigins []*StringMatch `json:"additionalOrigins,omitempty" yaml:"additionalOrigins,omitempty"`

internal/ir/zz_generated.deepcopy.go

Lines changed: 0 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/xds/translator/csrf.go

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ import (
1414
csrfv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/csrf/v3"
1515
hcmv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3"
1616
"google.golang.org/protobuf/types/known/anypb"
17+
"k8s.io/utils/ptr"
18+
gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"
1719

1820
"github.com/envoyproxy/gateway/internal/ir"
1921
"github.com/envoyproxy/gateway/internal/xds/types"
@@ -142,22 +144,24 @@ func (*csrf) patchResources(*types.ResourceVersionTable, []*ir.HTTPRoute) error
142144

143145
// buildXdsCSRFPolicy builds the full Envoy CSRF policy from the IR.
144146
func buildXdsCSRFPolicy(csrf *ir.CSRF) *csrfv3.CsrfPolicy {
145-
// Default to 100% enforced.
147+
// Envoy only lets an invalid request through in shadow mode if that request wasn't
148+
// also selected by FilterEnabled, so enforcement is set to the complement of the
149+
// shadow fraction: every request is either enforced or observed, never neither.
150+
// With no shadow fraction, all requests are enforced.
146151
enforcedFraction := fractionalpercent.FromIn32(100)
147-
if csrf.EnforcedFraction != nil {
148-
enforcedFraction = fractionalpercent.FromFraction(csrf.EnforcedFraction)
152+
var shadowEnabled *corev3.RuntimeFractionalPercent
153+
if csrf.ShadowFraction != nil {
154+
enforcedFraction = fractionalpercent.FromFraction(complementFraction(csrf.ShadowFraction))
155+
shadowEnabled = &corev3.RuntimeFractionalPercent{
156+
DefaultValue: fractionalpercent.FromFraction(csrf.ShadowFraction),
157+
}
149158
}
150159

151160
policy := &csrfv3.CsrfPolicy{
152161
FilterEnabled: &corev3.RuntimeFractionalPercent{
153162
DefaultValue: enforcedFraction,
154163
},
155-
}
156-
157-
if csrf.ShadowFraction != nil {
158-
policy.ShadowEnabled = &corev3.RuntimeFractionalPercent{
159-
DefaultValue: fractionalpercent.FromFraction(csrf.ShadowFraction),
160-
}
164+
ShadowEnabled: shadowEnabled,
161165
}
162166

163167
// Values are passed directly to Envoy without transformation. Users must provide
@@ -169,3 +173,13 @@ func buildXdsCSRFPolicy(csrf *ir.CSRF) *csrfv3.CsrfPolicy {
169173

170174
return policy
171175
}
176+
177+
// complementFraction returns the remainder of the given fraction, i.e. 1 - fraction.
178+
// The numerator is never greater than the denominator, so the result is never negative.
179+
func complementFraction(fraction *gwapiv1.Fraction) *gwapiv1.Fraction {
180+
denominator := ptr.Deref(fraction.Denominator, fractionalpercent.Hundred)
181+
return &gwapiv1.Fraction{
182+
Numerator: denominator - fraction.Numerator,
183+
Denominator: ptr.To(denominator),
184+
}
185+
}

0 commit comments

Comments
 (0)