Repository navigation
feat(core): apiWaf puts AWS WAF in front of the backend API - #1027
osama-rizk wants to merge 3 commits into
Conversation
A REGIONAL web ACL on the API Gateway stage under apiFrontDoor 'apigateway', a CLOUDFRONT one on the managed distribution under 'cloudfront' (us-east-1, or bring your own ARN), and nothing plus a synth warning when a CloudFront-backed Hosting fronts the API. The managed ACL counts the 8 KB body-size rule so RPC payloads aren't blocked.
🦋 Changeset detectedLatest commit: 4d839a8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 26 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Simone319
left a comment
There was a problem hiding this comment.
Following the front door rather than always the stage is the right model, and it fits the shared gateway: one stage ACL or one distribution ACL covers every compute's namespaces, because routing between computes happens inside the gateway. Validating before the backend import, and keeping the us-east-1 rule consistent with Hosting's WAF, both look good. I also checked the stage ordering: the association's ResourceArn references the Stage, so CloudFormation creates it after the stage.
I synthesized a few topologies with this branch, and two of them don't get the protection apiWaf promises. Details are inline:
- Blocking. The default
apigatewaytier with a CloudFrontHosting: no web ACL at all, while native clients keep callingexecute-api. - Should fix. #510's non-claiming Hosting doors (ALB, API Gateway, bring-your-own): the per-IP rate rule counts the proxy's IPs, not the users'.
Question: what client IP does the stage ACL see on the default tier? shared-gateway.ts:165 leaves endpointTypes unset, so the default endpoint is edge-optimized, and requests reach the regional stage through API Gateway's own CloudFront. That's the situation the PR cites to justify CLOUDFRONT scope on the cloudfront tier. The single-client rate test can't distinguish the two, since one client always hits one edge. aws wafv2 get-sampled-requests on the us-west-2 run would show ClientIP. If it's a CloudFront address, the default tier's rate limit is per edge too.
Docs: please say what apiWaf doesn't cover. First, bb-realtime's WebSocket API, since AWS WAF can't attach to API Gateway WebSocket APIs. Second, the execute-api endpoint in every topology where something else fronts the API, not only under 'cloudfront'.
| if (this.owner._apiFrontDoorClaimedByHosting) return; | ||
| // would double the hops and split the domain. The API's WAF then belongs on | ||
| // Hosting's front door too. | ||
| if (this.owner._apiFrontDoorClaimedByHosting) { |
There was a problem hiding this comment.
Blocking: under the default apigateway tier, a CloudFront Hosting claim makes apiWaf protect nothing for native and direct callers.
publicApiUrl follows the claim only under 'cloudfront'. Under 'apigateway', ApiUrl stays the gateway URL. I synthesized BlocksStack.create(app, 'S', { …, apiWaf: { enabled: true } }) plus new Hosting(stack, 'Hosting', { root, api: stack }):
- 0
AWS::WAFv2::WebACLand 0WebACLAssociation; - the
ApiUrloutput ishttps://{RestApi}.execute-api.{region}…, which native clients call directly; - the warning still says to set the WAF on Hosting instead, which only covers browser traffic.
The Hosting test in hosting.test.ts only exercises scheduleApiFrontDoor(stack, true, …), the cloudfront tier, so this path is untested.
Suggested fix: defer to Hosting only under 'cloudfront', where the claim really moves ApiUrl. Under 'apigateway', still attach the stage ACL. Browser requests arriving through Hosting's CloudFront would then be rate-limited per edge IP, so either scope the rate rule away from Hosting-proxied requests, or at minimum warn that the gateway endpoint native clients use stays unprotected. Please add a test for the default tier with Hosting.
| name: 'RateLimitRule', | ||
| priority: 3, | ||
| action: { block: {} }, | ||
| statement: { rateBasedStatement: { limit: rateLimit, aggregateKeyType: 'IP' } }, |
There was a problem hiding this comment.
Should fix: behind the non-claiming Hosting doors from #510 (ALB, API Gateway, bring-your-own), this rule counts the proxy, not the user.
Synth with env: { region: 'us-west-2' }, apiWaf: { enabled: true } and new Hosting(stack, 'Hosting', { root, api: stack, frontDoor: { kind: 'alb' } }):
- the REGIONAL ACL, with this rule, goes on the REST API stage, and nothing warns;
- browser API calls reach the stage from the ALB's API forwarder Lambda (
HostingAlbApiProxyDefault, not in a VPC).
So WAF sees the forwarder Lambda's IPs. The forwarder passes X-Forwarded-For through, but aggregateKeyType: 'IP' ignores it, so all browser users share a few rate buckets and get 403s at about rateLimit requests per 5 minutes in total. Under 'cloudfront' with these doors, browser traffic goes ALB → forwarder → execute-api and skips the distribution's ACL altogether.
These doors already record the browser origin (the BLOCKS_PUBLIC_ORIGIN recorder in api-front-door.ts). The aspect could read that and either warn, pointing to the door's own WAF, or leave the rate rule off the stage ACL. Switching to FORWARDED_IP on its own isn't safe: direct callers can set X-Forwarded-For and dodge the limit.
| throw new Error(`apiWaf must be an object like { enabled: true } or { webAclArn } (got ${JSON.stringify(apiWaf)}).`); | ||
| } | ||
| const { enabled, rateLimit, webAclArn } = apiWaf; | ||
| if (webAclArn !== undefined) { |
There was a problem hiding this comment.
Suggestion: check a literal webAclArn's scope against where it will attach.
Under 'cloudfront' it has to be …:global/webacl/… in us-east-1. Under 'apigateway' it has to be …:regional/webacl/… in the stack's region. A mismatch passes synth today and only fails at deploy. The tier isn't known in resolveApiWaf, but it is in the aspect, so the check could run there (skipping tokens).
| managedRuleGroupStatement: { | ||
| vendorName: 'AWS', | ||
| name: 'AWSManagedRulesCommonRuleSet', | ||
| ruleActionOverrides: [{ name: 'SizeRestrictions_BODY', actionToUse: { count: {} } }], |
There was a problem hiding this comment.
Suggestion: other Common rule set body rules will also block normal RPC traffic.
Setting SizeRestrictions_BODY to count is right. But CrossSiteScripting_BODY, GenericLFI_BODY and GenericRFI_BODY still block JSON bodies that carry user-written HTML, Markdown or file paths, such as a notes app saving <script> text. Consider setting those to count as well, or document how to tune them via webAclArn.
Problem
The backend API has no WAF option. The core README tells users to wire a
CfnWebACLAssociationtostack.gateway.deploymentStageby hand. That only covers the default tier: underapiFrontDoor: 'cloudfront'the API is served from a managed CloudFront distribution, where a REGIONAL web ACL on the stage would rate-limit per CloudFront edge location instead of per client.Changes
BlocksStackandBlocksBackendtakeapiWaf. It is off by default and accepts either:{ enabled: true, rateLimit? }, which creates a Blocks-managed web ACL, or{ webAclArn }, which attaches a web ACL you already have.The prop is validated in the constructor, before the backend module is imported. Contradictory or invalid values fail fast: both
webAclArnandenabled/rateLimittogether,rateLimitbelow the WAF minimum of 100, orrateLimitwithoutenabled.The web ACL follows the front door. The decision is made by the existing synth-time front-door aspect, because only then is it known whether
Hostingclaimed the API:apiWafdoesapiFrontDoor: 'apigateway'(default)apiFrontDoor: 'cloudfront'WebACLId, so it sees real client IPs.enabled: trueoutside us-east-1 fails synth and points towebAclArnor theapigatewaytierHostingfronts the APIblocks:api-waf:deferred-to-hosting) to setfrontDoor.features.wafon HostingThe managed web ACL uses the same rule set as Hosting's WAF:
One difference: the Common rule set's
SizeRestrictions_BODYrule (block bodies over 8 KB) is set to count, because RPC bodies routinely exceed 8 KB.The core README "Shared API gateway" section and the
apiWafJSDoc document all of this. A changeset is included.Verification
Unit tests:
core: 1705 passed, 0 failed. New tests are inapi-waf.test.ts(13): validation, placement per topology, bring-your-own ARN, the us-east-1 error, the Hosting-claim warning, and the rule set.BlocksStack/BlocksBackendintegration tests (4) and a Hosting-claim test were also added.blocks: 234 + 15.Checks:
lint:depsandcheck:apipass.test:e2e:localpasses (422 + 8 + 2).Real AWS. Both runs used
test-apps/native-bindingswithapiWaf: { enabled: true, rateLimit: 100 }, applied as a temporary local edit that is not part of this PR. Both stacks were destroyed afterwards.apigatewaytier, us-west-2.wafv2 get-web-acl-for-resourceon the REST API stage returns the REGIONALApiWaf-*web ACL with the three rules andSizeRestrictions_BODY → Count.cloudfronttier, us-east-1. The managed distribution'sWebACLIdis the CLOUDFRONTApiWaf-*web ACL, with the same rules.execute-apiURL returned 200. That matches the documented scope: under this tier the direct gateway endpoint is not covered.Not changed / follow-ups
execute-apiundercloudfront. The endpoint stays reachable, so the managed distribution's web ACL can be bypassed. Closing it needs an origin-verify secret or a resource policy on the gateway.frontDoor.features.waf) still blocks onSizeRestrictions_BODY, including on the API behaviors it proxies. Aligning it with this rule set is a separate change.