Merge pull request #371 from communitiesuk/remove-waf-login-limit

Remove WAF login rate limit
communitiesuk · Jan 4, 2024 · 356add9 · 356add9
2 parents 39ccd75 + 9fa4a41
commit 356add9
Show file tree

Hide file tree

Showing 6 changed files with 7 additions and 134 deletions.
diff --git a/terraform/modules/cloudfront_distributions/main.tf b/terraform/modules/cloudfront_distributions/main.tf
@@ -26,8 +26,6 @@ module "delta_website_waf" {
   excluded_rules                 = ["CrossSiteScripting_BODY", "GenericLFI_BODY"]
   ip_allowlist                   = var.delta.ip_allowlist
   cloudwatch_log_expiration_days = var.waf_cloudwatch_log_expiration_days
-  login_ip_rate_limit_enabled    = true
-  login_ip_rate_limit            = var.login_ip_rate_limit
   alarms_sns_topic_global_arn    = var.alarms_sns_topic_global_arn
   security_sns_topic_global_arn  = var.security_sns_topic_global_arn
 }

diff --git a/terraform/modules/cloudfront_distributions/variables.tf b/terraform/modules/cloudfront_distributions/variables.tf
@@ -8,7 +8,7 @@ variable "environment" {
 
 variable "auth_waf_per_ip_rate_limit" {
   type        = number
-  default     = 500
+  default     = 200
   description = "The per-IP rate limit enforced by AWS WAF in requests per five minutes for the auth service"
 }
 
@@ -18,12 +18,6 @@ variable "waf_per_ip_rate_limit" {
   description = "The per-IP rate limit enforced by AWS WAF in requests per five minutes"
 }
 
-variable "login_ip_rate_limit" {
-  type        = number
-  default     = 100
-  description = "The per-IP rate limit enforced by AWS WAF in requests per five minutes to the login page"
-}
-
 variable "apply_aws_shield" {
   type = bool
 }

diff --git a/terraform/modules/waf/main.tf b/terraform/modules/waf/main.tf
@@ -15,13 +15,12 @@ locals {
   excluded_rules = concat(var.excluded_rules, ["SizeRestrictions_BODY"])
 
   metric_names = {
-    main                = replace("${var.prefix}cloudfront-waf-acl", "-", "")
-    rate_limit          = replace("${var.prefix}cloudfront-waf-rate-limit", "-", "")
-    login_ip_rate_limit = replace("${var.prefix}cloudfront-waf-login-rate-limit", "-", "")
-    common              = replace("${var.prefix}cloudfront-waf-common-rules", "-", "")
-    bad_inputs          = replace("${var.prefix}cloudfront-waf-bad-inputs", "-", "")
-    ip_reputation       = replace("${var.prefix}cloudfront-waf-ip-reputation", "-", "")
-    ip_allowlist        = replace("${var.prefix}cloudfront-waf-ip-allowlist", "-", "")
+    main          = replace("${var.prefix}cloudfront-waf-acl", "-", "")
+    rate_limit    = replace("${var.prefix}cloudfront-waf-rate-limit", "-", "")
+    common        = replace("${var.prefix}cloudfront-waf-common-rules", "-", "")
+    bad_inputs    = replace("${var.prefix}cloudfront-waf-bad-inputs", "-", "")
+    ip_reputation = replace("${var.prefix}cloudfront-waf-ip-reputation", "-", "")
+    ip_allowlist  = replace("${var.prefix}cloudfront-waf-ip-allowlist", "-", "")
   }
   all_routes_ip_allowlist_enabled    = var.ip_allowlist != null && var.ip_allowlist_uri_path_regex == null
   path_specific_ip_allowlist_enabled = var.ip_allowlist != null && var.ip_allowlist_uri_path_regex != null
@@ -30,7 +29,6 @@ locals {
   all_routes_ip_allowlist_foreach    = local.all_routes_ip_allowlist_enabled ? [{}] : []
   path_specific_ip_allowlist_foreach = local.path_specific_ip_allowlist_enabled ? [{}] : []
   ip_reputation_foreach              = local.ip_reputation_enabled ? [{}] : []
-  login_ip_rate_limit_foreach        = var.login_ip_rate_limit_enabled ? [{}] : []
 }
 
 output "acl_arn" {
@@ -262,42 +260,6 @@ resource "aws_wafv2_web_acl" "waf_acl" {
       }
     }
   }
-
-  dynamic "rule" {
-    for_each = local.login_ip_rate_limit_foreach
-    content {
-      name     = "login-ip-rate-limit"
-      priority = 70 + local.priority_base
-
-      action {
-        block {}
-      }
-
-      statement {
-        rate_based_statement {
-          limit              = var.login_ip_rate_limit
-          aggregate_key_type = "IP"
-          scope_down_statement {
-            regex_pattern_set_reference_statement {
-              arn = aws_wafv2_regex_pattern_set.waf_rate_limit_urls[0].arn
-              field_to_match {
-                uri_path {}
-              }
-              text_transformation {
-                priority = 0
-                type     = "URL_DECODE"
-              }
-            }
-          }
-        }
-      }
-      visibility_config {
-        cloudwatch_metrics_enabled = true
-        metric_name                = local.metric_names.login_ip_rate_limit
-        sampled_requests_enabled   = true
-      }
-    }
-  }
 }
 
 resource "aws_wafv2_regex_pattern_set" "ip_restricted_paths" {
@@ -313,22 +275,3 @@ resource "aws_wafv2_regex_pattern_set" "ip_restricted_paths" {
     }
   }
 }
-
-resource "aws_wafv2_regex_pattern_set" "waf_rate_limit_urls" {
-  provider = aws.us-east-1
-  count    = var.login_ip_rate_limit_enabled ? 1 : 0
-  name     = "${var.prefix}cloudfront-waf-regex-patterns"
-  scope    = "CLOUDFRONT"
-
-  regular_expression {
-    regex_string = "/login"
-  }
-
-  regular_expression {
-    regex_string = "/forgot-password"
-  }
-
-  regular_expression {
-    regex_string = "/reset-password"
-  }
-}
diff --git a/terraform/modules/waf/monitoring.tf b/terraform/modules/waf/monitoring.tf
@@ -44,10 +44,6 @@ resource "aws_cloudwatch_dashboard" "waf_dashboard" {
               ["AWS/WAFV2", "CountedRequests", "Rule", local.metric_names.bad_inputs, "WebACL", aws_wafv2_web_acl.waf_acl.name],
               ["AWS/WAFV2", "BlockedRequests", "Rule", local.metric_names.ip_reputation, "WebACL", aws_wafv2_web_acl.waf_acl.name],
               ["AWS/WAFV2", "CountedRequests", "Rule", local.metric_names.ip_reputation, "WebACL", aws_wafv2_web_acl.waf_acl.name],
-              ],
-              !var.login_ip_rate_limit_enabled ? [] : [
-                ["AWS/WAFV2", "BlockedRequests", "Rule", local.metric_names.login_ip_rate_limit, "WebACL", aws_wafv2_web_acl.waf_acl.name],
-                ["AWS/WAFV2", "CountedRequests", "Rule", local.metric_names.login_ip_rate_limit, "WebACL", aws_wafv2_web_acl.waf_acl.name],
             ]),
             "region" : "us-east-1",
             "title" : "Blocked and counted requests by rule group",
@@ -83,27 +79,6 @@ resource "aws_cloudwatch_dashboard" "waf_dashboard" {
           x      = 12
           y      = 0
         },
-        ],
-        !var.login_ip_rate_limit_enabled ? [] : [
-          {
-            type = "metric",
-            properties = {
-              "title" : "Blocked login requests alarm",
-              "annotations" : {
-                "alarms" : [aws_cloudwatch_metric_alarm.blocked_login_requests[0].arn]
-              },
-              "liveData" : false,
-              "start" : "-PT3H",
-              "end" : "PT0H",
-              "region" : "us-east-1",
-              "view" : "timeSeries",
-              "stacked" : false
-            }
-            height = 8
-            width  = 8
-            x      = 12
-            y      = 0
-          }
       ])
     }
   )
@@ -135,28 +110,3 @@ look for any suspicious activity (e.g. lots of login attempts) and escalate if u
   alarm_actions = [var.security_sns_topic_global_arn]
   ok_actions    = [var.security_sns_topic_global_arn]
 }
-
-resource "aws_cloudwatch_metric_alarm" "blocked_login_requests" {
-  provider = aws.us-east-1
-  count    = var.login_ip_rate_limit_enabled ? 1 : 0
-
-  alarm_name          = "${var.prefix}cloudfront-waf-blocked-login-requests"
-  comparison_operator = "GreaterThanOrEqualToThreshold"
-  evaluation_periods  = "1"
-  metric_name         = "BlockedRequests"
-  namespace           = "AWS/WAFV2"
-  period              = "300"
-  statistic           = "Sum"
-  threshold           = "1"
-  alarm_description   = <<EOF
-TODO: Replaced by application-level rate limiting in the auth service and should be removed.
-  EOF
-  treat_missing_data  = "notBreaching"
-  dimensions = {
-    Rule   = local.metric_names.login_ip_rate_limit
-    WebACL = aws_wafv2_web_acl.waf_acl.name
-  }
-
-  alarm_actions = [var.security_sns_topic_global_arn]
-  ok_actions    = [var.security_sns_topic_global_arn]
-}
diff --git a/terraform/modules/waf/variables.tf b/terraform/modules/waf/variables.tf
@@ -27,17 +27,6 @@ variable "cloudwatch_log_expiration_days" {
   type = number
 }
 
-variable "login_ip_rate_limit" {
-  type        = number
-  default     = 100
-  description = "The per-IP rate limit enforced by AWS WAF in requests per five minutes to the login page"
-}
-
-variable "login_ip_rate_limit_enabled" {
-  type    = bool
-  default = false
-}
-
 variable "alarms_sns_topic_global_arn" {
   description = "SNS topic ARN to send alarm notifications to"
   type        = string

diff --git a/terraform/test/main.tf b/terraform/test/main.tf
@@ -190,7 +190,6 @@ module "cloudfront_distributions" {
   base_domains                             = [var.primary_domain]
   waf_per_ip_rate_limit                    = 100000
   auth_waf_per_ip_rate_limit               = 1000
-  login_ip_rate_limit                      = 500
   apply_aws_shield                         = local.apply_aws_shield
   waf_cloudwatch_log_expiration_days       = local.cloudwatch_log_expiration_days
   cloudfront_access_s3_log_expiration_days = local.s3_log_expiration_days