-
Notifications
You must be signed in to change notification settings - Fork 6
Add Cloud Armor rules for HTTP, gRPC and GCS backends #87
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
Conversation
955942e
to
9a16fe8
Compare
Plan looks good: https://github.com/sigstore/public-good-instance/actions/runs/17448557776/job/49548568574?pr=3217 I think I've ironed out any issues. The main difference from what we've used in the past is that the compute bucket backend uses an "edge" armor policy. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, no notes (I'm not an expert though)
|
||
match { | ||
expr { | ||
expression = "int(request.headers['content-length']) > 8388608" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should be sure to document this somewhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack, will document this
} | ||
|
||
advanced_options_config { | ||
json_parsing = "STANDARD" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm. will this cover both HTTP and gRPC traffic? I don't think we want the firewall parsing gRPC traffic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add separate security policies for HTTP and gRPC to remove this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah actually i want one security policy for all write traffic. i'll dig into this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can include specific content types for when to parse the body. This should prevent gRPC traffic from being parsed.
} | ||
|
||
advanced_options_config { | ||
json_parsing = "STANDARD" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this needed for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Armor will reject traffic that isn't valid JSON. It's an early check to keep CPU load down for requests that will never be valid. I'm creating separate security policies for http and grpc, since grpc won't be valid json.
9a16fe8
to
264ef8d
Compare
This adds rate limiting for read and write requests for tile-backed logs. This also restricts request sizes. The defaults are reasonable, and variables have been provided to tweak rate limiting to meet deployment needs. We've chosen to add rate limiting for the read path to prevent unlimited reads, which come with egress costs. The threshold is much higher than writes to support monitoring, and will be increased over time for the public deployment as write QPS increases. Ref sigstore/rekor-tiles#354 Ref sigstore/rekor-tiles#355 Signed-off-by: Hayden <[email protected]>
Signed-off-by: Hayden <[email protected]>
264ef8d
to
ef7590d
Compare
Co-authored-by: Bob Callaway <[email protected]> Signed-off-by: Hayden <[email protected]>
This adds rate limiting for read and write requests for tile-backed logs. This also restricts request sizes.
The defaults are reasonable, and variables have been provided to tweak rate limiting to meet deployment needs.
We've chosen to add rate limiting for the read path to prevent unlimited reads, which come with egress costs. The threshold is much higher than writes to support monitoring, and will be increased over time for the public deployment as write QPS increases.
Ref sigstore/rekor-tiles#354
Ref sigstore/rekor-tiles#355
#vc4a
Summary
Release Note
Documentation