From 645d0412c360d33e40dc5ddc1b30032c7b98648d Mon Sep 17 00:00:00 2001 From: Chris Banks Date: Thu, 14 Dec 2023 14:33:44 +0000 Subject: [PATCH 1/5] Rationalise ratelimit exemption rules. Make it clearer as to what the rule does. --- terraform/projects/infra-public-wafs/cache_public_rule.tf | 8 ++++---- terraform/projects/infra-public-wafs/variables.tf | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/terraform/projects/infra-public-wafs/cache_public_rule.tf b/terraform/projects/infra-public-wafs/cache_public_rule.tf index e44ac78a1..546e0ee2d 100644 --- a/terraform/projects/infra-public-wafs/cache_public_rule.tf +++ b/terraform/projects/infra-public-wafs/cache_public_rule.tf @@ -31,12 +31,12 @@ resource "aws_wafv2_ip_set" "govuk_requesting_ips" { addresses = concat(var.traffic_replay_ips, local.nat_gateway_ips, var.eks_egress_ips) } -resource "aws_wafv2_ip_set" "external_partner_ips" { - name = "external_partner_ips" - description = "The IP addresses are used by our partners." +resource "aws_wafv2_ip_set" "high_request_rate" { + name = "high_request_rate" + description = "Source addresses from which we allow a higher ratelimit." scope = "REGIONAL" ip_address_version = "IPV4" - addresses = var.allow_external_ips + addresses = var.allow_high_request_rate_from_cidrs } resource "aws_cloudwatch_log_group" "public_cache_waf" { diff --git a/terraform/projects/infra-public-wafs/variables.tf b/terraform/projects/infra-public-wafs/variables.tf index 5a2211596..7cfb238f6 100644 --- a/terraform/projects/infra-public-wafs/variables.tf +++ b/terraform/projects/infra-public-wafs/variables.tf @@ -105,9 +105,9 @@ variable "eks_egress_ips" { description = "An array of CIDR blocks for the corresponding EKS environment's NAT gateway IPs" } -variable "allow_external_ips" { +variable "allow_high_request_rate_from_cidrs" { type = list(string) - description = "An array of CIDR blocks that are our partners using to send traffic to us" + description = "Source IP netblocks from which we allow a higher rate of requests." } variable "waf_log_retention_days" { From 6fd4aa826735d0b45383005aee16cd207b49b2bf Mon Sep 17 00:00:00 2001 From: Chris Banks Date: Thu, 14 Dec 2023 14:50:53 +0000 Subject: [PATCH 2/5] Fix statement numbering to make edits less painful. --- .../projects/infra-public-wafs/backend_public_rule.tf | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/terraform/projects/infra-public-wafs/backend_public_rule.tf b/terraform/projects/infra-public-wafs/backend_public_rule.tf index ce3c26924..8573d2f00 100644 --- a/terraform/projects/infra-public-wafs/backend_public_rule.tf +++ b/terraform/projects/infra-public-wafs/backend_public_rule.tf @@ -11,7 +11,7 @@ resource "aws_wafv2_web_acl" "backend_public" { # the waf is enabled and processing requests rule { name = "x-always-block_web_acl_rule" - priority = 1 + priority = 10 override_action { none {} @@ -33,7 +33,7 @@ resource "aws_wafv2_web_acl" "backend_public" { # this rule matches any request from our NAT gateway IPs and allows it. rule { name = "allow-govuk-infra" - priority = 2 + priority = 20 action { allow {} @@ -57,7 +57,7 @@ resource "aws_wafv2_web_acl" "backend_public" { # this is checked every 30s rule { name = "backend-public-base-rate-warning" - priority = 3 + priority = 30 action { count {} @@ -81,7 +81,7 @@ resource "aws_wafv2_web_acl" "backend_public" { # this is checked every 30s rule { name = "backend-public-base-rate-limit" - priority = 4 + priority = 40 action { block { From d0e0127a36ab64b48d47903ccd3894961a5965cd Mon Sep 17 00:00:00 2001 From: Chris Banks Date: Thu, 14 Dec 2023 14:51:11 +0000 Subject: [PATCH 3/5] Heed ratelimit exemptions in backend WAF ruleset. --- .../infra-public-wafs/backend_public_rule.tf | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/terraform/projects/infra-public-wafs/backend_public_rule.tf b/terraform/projects/infra-public-wafs/backend_public_rule.tf index 8573d2f00..33eb05120 100644 --- a/terraform/projects/infra-public-wafs/backend_public_rule.tf +++ b/terraform/projects/infra-public-wafs/backend_public_rule.tf @@ -30,9 +30,8 @@ resource "aws_wafv2_web_acl" "backend_public" { } } - # this rule matches any request from our NAT gateway IPs and allows it. rule { - name = "allow-govuk-infra" + name = "rate-limit-exemptions" priority = 20 action { @@ -40,8 +39,17 @@ resource "aws_wafv2_web_acl" "backend_public" { } statement { - ip_set_reference_statement { - arn = aws_wafv2_ip_set.govuk_requesting_ips.arn + or_statement { + statement { + ip_set_reference_statement { + arn = aws_wafv2_ip_set.govuk_requesting_ips.arn + } + } + statement { + ip_set_reference_statement { + arn = aws_wafv2_ip_set.high_request_rate.arn + } + } } } From 004099553e01e3c7d27908b471a9accee62b7f44 Mon Sep 17 00:00:00 2001 From: Chris Banks Date: Thu, 14 Dec 2023 15:08:44 +0000 Subject: [PATCH 4/5] Remove defunct WAF association objects for CKAN. These were referring to LBs that no longer exist, and their replacements are configured via aws-lb-controller and not Terraform. --- .../projects/infra-public-wafs/standard_config.tf | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/terraform/projects/infra-public-wafs/standard_config.tf b/terraform/projects/infra-public-wafs/standard_config.tf index f258a2e8e..632c7983f 100644 --- a/terraform/projects/infra-public-wafs/standard_config.tf +++ b/terraform/projects/infra-public-wafs/standard_config.tf @@ -1,14 +1,4 @@ -resource "aws_shield_protection" "ckan_public_lb" { - name = "${var.stackname}-ckan-public-lb_shield" - resource_arn = data.terraform_remote_state.infra_public_services.outputs.ckan_public_lb_id -} - -resource "aws_wafv2_web_acl_association" "ckan_public_web_acl" { - resource_arn = data.terraform_remote_state.infra_public_services.outputs.ckan_public_lb_id - web_acl_arn = aws_wafv2_web_acl.default.arn -} - resource "aws_shield_protection" "deploy_public_lb" { name = "${var.stackname}-deploy-public-lb_shield" resource_arn = data.terraform_remote_state.infra_public_services.outputs.deploy_public_lb_id From bb05970b78f16a341346c57b727c12b910a3276d Mon Sep 17 00:00:00 2001 From: Chris Banks Date: Thu, 14 Dec 2023 14:37:27 +0000 Subject: [PATCH 5/5] Regenerate docs. --- terraform/projects/infra-public-wafs/README.md | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/terraform/projects/infra-public-wafs/README.md b/terraform/projects/infra-public-wafs/README.md index daed9a5b2..f4a1c2cd6 100644 --- a/terraform/projects/infra-public-wafs/README.md +++ b/terraform/projects/infra-public-wafs/README.md @@ -25,15 +25,14 @@ No modules. | [aws_cloudwatch_log_group.public_backend_waf](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/cloudwatch_log_group) | resource | | [aws_cloudwatch_log_group.public_bouncer_waf](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/cloudwatch_log_group) | resource | | [aws_cloudwatch_log_group.public_cache_waf](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/cloudwatch_log_group) | resource | -| [aws_shield_protection.ckan_public_lb](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/shield_protection) | resource | | [aws_shield_protection.deploy_public_lb](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/shield_protection) | resource | | [aws_shield_protection.graphite_public_lb](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/shield_protection) | resource | | [aws_shield_protection.licensify_backend_public_lb](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/shield_protection) | resource | | [aws_shield_protection.licensify_frontend_public_lb](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/shield_protection) | resource | | [aws_shield_protection.monitoring_public_lb](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/shield_protection) | resource | | [aws_shield_protection.prometheus_public_lb](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/shield_protection) | resource | -| [aws_wafv2_ip_set.external_partner_ips](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/wafv2_ip_set) | resource | | [aws_wafv2_ip_set.govuk_requesting_ips](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/wafv2_ip_set) | resource | +| [aws_wafv2_ip_set.high_request_rate](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/wafv2_ip_set) | resource | | [aws_wafv2_ip_set.nat_gateway_ips](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/wafv2_ip_set) | resource | | [aws_wafv2_regex_pattern_set.x_always_block](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/wafv2_regex_pattern_set) | resource | | [aws_wafv2_rule_group.x_always_block](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/wafv2_rule_group) | resource | @@ -43,7 +42,6 @@ No modules. | [aws_wafv2_web_acl.default](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/wafv2_web_acl) | resource | | [aws_wafv2_web_acl.licensify_backend_public](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/wafv2_web_acl) | resource | | [aws_wafv2_web_acl.licensify_frontend_public](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/wafv2_web_acl) | resource | -| [aws_wafv2_web_acl_association.ckan_public_web_acl](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/wafv2_web_acl_association) | resource | | [aws_wafv2_web_acl_association.deploy_public_web_acl](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/wafv2_web_acl_association) | resource | | [aws_wafv2_web_acl_association.graphite_public_web_acl](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/wafv2_web_acl_association) | resource | | [aws_wafv2_web_acl_association.licensify_backend_public_web_acl](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/wafv2_web_acl_association) | resource | @@ -67,7 +65,7 @@ No modules. | Name | Description | Type | Default | Required | |------|-------------|------|---------|:--------:| -| [allow\_external\_ips](#input\_allow\_external\_ips) | An array of CIDR blocks that are our partners using to send traffic to us | `list(string)` | n/a | yes | +| [allow\_high\_request\_rate\_from\_cidrs](#input\_allow\_high\_request\_rate\_from\_cidrs) | Source IP netblocks from which we allow a higher rate of requests. | `list(string)` | n/a | yes | | [aws\_environment](#input\_aws\_environment) | AWS Environment | `string` | n/a | yes | | [aws\_region](#input\_aws\_region) | AWS region | `string` | `"eu-west-1"` | no | | [backend\_public\_base\_rate\_limit](#input\_backend\_public\_base\_rate\_limit) | For the backend ALB. Number of requests to allow in a 5 minute period before rate limiting is applied. | `number` | n/a | yes |