Skip to content
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

Fix Scope Down Statement Rule #88

Merged
merged 1 commit into from
Jul 10, 2024
Merged

Fix Scope Down Statement Rule #88

merged 1 commit into from
Jul 10, 2024

Conversation

RoseSecurity
Copy link
Member

@RoseSecurity RoseSecurity commented Jul 10, 2024

what

  • Corrects byte_match_statement handling within scope-down rules to accurately query scope_down_statement parameters instead of the statement parameters

why

  • The current configuration produces the following error when correct variables are passed into the module:
│ Error: Insufficient text_transformation blocks

│   on .terraform/dev/modules/aws_waf/rules.tf line 756, in resource "aws_wafv2_web_acl" "default":
│  756:                   content {

│ At least 1 "text_transformation" blocks are required.


│ Error: Unsupported attribute

│   on .terraform/dev/modules/aws_waf/rules.tf line 757, in resource "aws_wafv2_web_acl" "default":
│  757:                     positional_constraint = byte_match_statement.value.positional_constraint
│     ├────────────────
│     │ byte_match_statement.value is object with 4 attributes

│ This object does not have an attribute named "positional_constraint".


│ Error: Unsupported attribute

│   on .terraform/dev/modules/aws_waf/rules.tf line 758, in resource "aws_wafv2_web_acl" "default":
│  758:                     search_string         = byte_match_statement.value.search_string
│     ├────────────────
│     │ byte_match_statement.value is object with 4 attributes

│ This object does not have an attribute named "search_string".

Releasing state lock. This may take a few moments...
exit status 1
  • To correct this, this change proposes to use the proper parameters, allowing for the following values to be passed to the module:
      scope_down_statement = optional(object({
        byte_match_statement = object({
          positional_constraint = string
          search_string         = string
          field_to_match = object({
            all_query_arguments   = optional(bool)
            body                  = optional(bool)
            method                = optional(bool)
            query_string          = optional(bool)
            single_header         = optional(object({ name = string }))
            single_query_argument = optional(object({ name = string }))
            uri_path              = optional(bool)
          })
          text_transformation = list(object({
            priority = number
            type     = string
          }))
        })
      }))

@RoseSecurity RoseSecurity requested review from a team as code owners July 10, 2024 02:30
@mergify mergify bot added the triage Needs triage label Jul 10, 2024
@RoseSecurity
Copy link
Member Author

/terratest

@aknysh aknysh added the patch A minor, backward compatible change label Jul 10, 2024
Copy link
Member

@aknysh aknysh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @RoseSecurity

@aknysh aknysh merged commit d69dd89 into cloudposse:main Jul 10, 2024
32 checks passed
@mergify mergify bot removed the triage Needs triage label Jul 10, 2024
Copy link

These changes were released in v1.7.1.

@likhitha-ch
Copy link

still the error exists inspite using 1.7.1
Error: Insufficient text_transformation blocks

│ on .terraform/modules/backend_waf/rules.tf line 756, in resource "aws_wafv2_web_acl" "default":
│ 756: content {

│ At least 1 "text_transformation" blocks are required.

@RoseSecurity
Copy link
Member Author

still the error exists inspite using 1.7.1 Error: Insufficient text_transformation blocks │ │ on .terraform/modules/backend_waf/rules.tf line 756, in resource "aws_wafv2_web_acl" "default": │ 756: content { │ │ At least 1 "text_transformation" blocks are required.

Thank you for trying it out @likhitha-ch. I have an updated pull request here to address this if you are interested in reviewing.

@likhitha-ch
Copy link

likhitha-ch commented Jul 17, 2024

Hi @RoseSecurity,
Thanks for updating but I am still facing the issue, could you please check the below code which I am trying.

`rate_based_statement_rules = [
  {
    name          = "guest-access-block"
    priority      = 10
    action        = local.is_production ? "block" : "allow"

    statement = {
      limit                     = 1000
      aggregate_key_type        = "IP"
      evaluation_window_sec     = 600

      scope_down_statement = {
        byte_match_statement = {
          positional_constraint = "STARTS_WITH"
          search_string         = "/test"

          field_to_match = {
            uri_path = {}
          }

          text_transformation = {
            priority = 0
            type     = "NONE"
          }
        }
      }
    }

    visibility_config = {
      cloudwatch_metrics_enabled = false
      sampled_requests_enabled   = false
      metric_name                = "rule-40-metric"
    }
  }
]

`

@RoseSecurity
Copy link
Member Author

Hi @RoseSecurity, Thanks for updating but I am still facing the issue, could you please check the below code which I am trying.

`rate_based_statement_rules = [
  {
    name          = "guest-access-block"
    priority      = 10
    action        = local.is_production ? "block" : "allow"

    statement = {
      limit                     = 1000
      aggregate_key_type        = "IP"
      evaluation_window_sec     = 600

      scope_down_statement = {
        byte_match_statement = {
          positional_constraint = "STARTS_WITH"
          search_string         = "/test"

          field_to_match = {
            uri_path = {}
          }

          text_transformation = {
            priority = 0
            type     = "NONE"
          }
        }
      }
    }

    visibility_config = {
      cloudwatch_metrics_enabled = false
      sampled_requests_enabled   = false
      metric_name                = "rule-40-metric"
    }
  }
]

`

@likhitha-ch Have you tried setting uri_path = true? Here is an example:

  rate_based_statement_rules = [
    {
      name     = "rule-40"
      action   = "block"
      priority = 40

      statement = {
        limit                 = 100
        aggregate_key_type    = "IP"
        evaluation_window_sec = 300
        scope_down_statement = {
          byte_match_statement = {
            positional_constraint = "STARTS_WITH"
            search_string         = "example-scope-down-statement"
            field_to_match = {
              uri_path = true
            }
            text_transformation = [
              {
                priority = 40
                type     = "NONE"
              }
            ]
          }
        }
      }

@likhitha-ch
Copy link

@RoseSecurity yes this is working but evaluation window of 600sec is not accepting again facing the below error
Error: Unsupported argument

│ on .terraform/modules/backend_waf/rules.tf line 739, in resource "aws_wafv2_web_acl" "default":
│ 739: evaluation_window_sec = lookup(rate_based_statement.value, "evaluation_window_sec", 300)

│ An argument named "evaluation_window_sec" is not expected here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch A minor, backward compatible change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants