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

google_compute_security_policy preconfigured_waf_config and recaptcha_options blocks are appearing during terraform plan and apply when not used #18596

Open
a-ls-100 opened this issue Jun 28, 2024 · 11 comments

Comments

@a-ls-100
Copy link

a-ls-100 commented Jun 28, 2024

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request.
  • Please do not leave +1 or me too comments, they generate extra noise for issue followers and do not help prioritize the request.
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment.
  • If an issue is assigned to a user, that user is claiming responsibility for the issue.
  • Customers working with a Google Technical Account Manager or Customer Engineer can ask them to reach out internally to expedite investigation and resolution of this issue.

Terraform Version & Provider Version(s)

Terraform v1.4

  • provider registry.terraform.io/hashicorp/google v5.35.0

  • provider registry.terraform.io/hashicorp/google-beta v5.35.0

  • provider registry.terraform.io/hashicorp/google v4.84.0

  • provider registry.terraform.io/hashicorp/google-beta v4.84.0

Affected Resource(s)

Terraform Configuration

/* Code block from our module to setup rules */
rule {
    priority = 1
    action   = "deny(${var.deny_status_code})"
    match {
      expr {
        expression = local.modsecurity_expressions.sqli
      }
    }
    preview     = var.modsecurity_sqli.preview
    description = "ModSecurity CRS SQLi attack rules"
  }

Debug Output

No response

Expected Behavior

  + rule {
      + action      = "deny(403)"
      + description = "ModSecurity CRS SQLi attack rules"
      + preview     = false
      + priority    = 1

      + match {
          + expr {
              + expression = "evaluatePreconfiguredWaf('sqli-stable', {'sensitivity': 4})"
            }
        }
    }

Actual Behavior

- rule {
          - action      = "deny(403)" -> null
          - description = "ModSecurity CRS SQLi attack rules" -> null
          - preview     = false -> null
          - priority    = 1 -> null

          - match {
              - expr {
                  - expression = "evaluatePreconfiguredWaf('sqli-stable', {'sensitivity': 4})" -> null
                }
              - expr_options {
                  - recaptcha_options {
                      - action_token_site_keys  = [] -> null
                      - session_token_site_keys = [] -> null
                    }
                }
            }

          - preconfigured_waf_config {
            }
        }

      + rule {
          + action      = "deny(403)"
          + description = "ModSecurity CRS SQLi attack rules"
          + preview     = false
          + priority    = 11

          + match {
              + expr {
                  + expression = "evaluatePreconfiguredWaf('sqli-stable', {'sensitivity': 4})"
                }
            }
        }

Steps to reproduce

While we are not using the blocks like below in our module. Terraform is somehow adding this and this always conflicts with the state showing false positives

              expr_options {
                  recaptcha_options {
                       action_token_site_keys  = [] -> null
                       session_token_site_keys = [] -> null
                    }
                }
            }

          preconfigured_waf_config {
            }

expr_options case

While looking at the source code, the expr_options is set to be an optional value,

But recaptcha_options is set to Required,

I believe this is the reason why terraform state always displays conflicting information.

preconfigured_waf_config's case

Really not sure with this case, as we do not use this code block, but we see the preconfigured_waf_config is being set as an empty map for all our rules.

Important Factoids

No response

References

No response

b/353545374

@a-ls-100 a-ls-100 added the bug label Jun 28, 2024
@ggtisc ggtisc self-assigned this Jun 28, 2024
@ggtisc ggtisc added service/compute-security-policy forward/review In review; remove label to forward labels Jun 28, 2024
@ggtisc
Copy link
Collaborator

ggtisc commented Jun 28, 2024

Hi @a-ls-100

I tried to replicate this issue with the shared terraform version, Google provider version and the next configuration since you only provided an incomplete fraction of the code:

resource "google_compute_security_policy" "csp_18596" {
  name = "csp-18596"

  rule {
    action   = "deny(403)"
    priority = "1000"
    match {
      expr {
        expression = "request.path.matches(\"/login.html\") && token.recaptcha_session.score < 0.2"
      }
    }
    preview = false
    description = "Deny access to IPs in 9.9.9.0/24"
  }

  rule {
    action   = "allow"
    priority = "2147483647"
    match {
      versioned_expr = "SRC_IPS_V1"
      config {
        src_ip_ranges = ["*"]
      }
    }
    description = "default rule"
  }
}

The result with terraform plan didn't show anything that wasn't specified in the code, after terraform apply everything was successfully without errors or something out of the normal.

After a 2nd terraform apply with the existing created resource everything was normal again having the message No changes. Your infrastructure matches the configuration

Finally checking the documentation you can find in terraform registry for preconfigured_waf_config that If the rule does not evaluate preconfigured WAF rules, i.e., if evaluatePreconfiguredWaf() is not used, this field will have no effect. And the same in Google Cloud documentation. The same happens withrecaptcha_options_config because both are optional fields and as you can notice both values don't have any impact on your infrastructure.

If you have a different code and want to test it please share it simplified, or you could include the values of the locals, variables and so on. For sensitive information you could replace the values with examples like:

project = "my-project"
org_id = "1234567890"
user = "[email protected]"

@a-ls-100
Copy link
Author

a-ls-100 commented Jul 5, 2024

Here is the generalized code from our module.

variable "modsecurity_sqli" {
  type = object({
    preview     = bool
    exceptions  = optional(list(string), [])
    sensitivity = optional(number, 4)
  })

  default = {
    preview = false
  }

  validation {
    condition     = var.modsecurity_sqli.sensitivity >= 1 && var.modsecurity_sqli.sensitivity <= 4
    error_message = "Sensitivity level may be only integer >= 1 and <= 4."
  }

  description = "Tuning the modsecurity SQLi rule."
}

locals {
      sqli = length(var.modsecurity_sqli.exceptions) > 0 ? "evaluatePreconfiguredWaf('sqli-${var.ruleset_version}', {'sensitivity': ${var.modsecurity_sqli.sensitivity}, 'opt_out_rule_ids': [${join(",", [for e in var.modsecurity_sqli.exceptions : "'${e}'"])}]})" : "evaluatePreconfiguredWaf('sqli-${var.ruleset_version}', {'sensitivity': ${var.modsecurity_sqli.sensitivity}})"
}


resource "google_compute_security_policy" "security-policy" {
  provider = google-beta

  lifecycle {
    ignore_changes = [
      adaptive_protection_config[0].layer_7_ddos_defense_config[0].rule_visibility
    ]
  }

  project = var.project_id

  name        = var.name
  description = "ModSecurity CRSv3 rules for protection against various attacks"

  dynamic "adaptive_protection_config" {
    for_each = var.adaptive_protection ? [0] : []

    content {
      layer_7_ddos_defense_config {
        enable          = true
        rule_visibility = "PREMIUM"
      }
    }
  }

  advanced_options_config {
    json_parsing = var.enable_json_parsing ? "STANDARD" : "DISABLED"
  }

  rule {
    priority = 11
    action   = "deny(${var.deny_status_code})"
    match {
      expr {
        expression = local.modsecurity_expressions.sqli
      }
    }
    preview     = var.modsecurity_sqli.preview
    description = "ModSecurity CRS SQLi attack rules"
  }
}

@ggtisc
Copy link
Collaborator

ggtisc commented Jul 6, 2024

@a-ls-100 Once again there is data which we do not have in the code you are sharing and it is not possible to replicate the scenario without that information but there is a base example like the one we shared with you in the previous answer.

We need the value of the next variables:

  • ruleset_version
  • adaptive_protection
  • enable_json_parsing
  • deny_status_code

Or you could send us a simplified version of the code without these variables with already defined values.

The use of variables, locals and so on in general is a good practice, but for us we don't have access is necessary to send that information or a simplified code version.

@wata727
Copy link

wata727 commented Jul 16, 2024

Same issue here. In our case, the recaptcha_options remain as a perma-diff.

Although the environment is almost the same, the issue sometimes occurs and sometimes doesn't, so we investigated it in more detail. It seems that there are cases where there are empty recaptchaOptions in the API response.

// A project where perma-diff is always shown
% gcloud compute security-policies rules describe --security-policy security-policy 4000
---
action: deny(403)
description: WAF rule for XSS
headerAction: {}
kind: compute#securityPolicyRule
match:
  expr:
    expression: <REDACTED>
  exprOptions:
    recaptchaOptions: {}
preconfiguredWafConfig: {}
preview: false
priority: 4000
// A project where perma-diff is not shown
% gcloud compute security-policies rules describe --security-policy security-policy 4000
---
action: deny(403)
description: WAF rule for XSS
headerAction: {}
kind: compute#securityPolicyRule
match:
  expr:
    expression: <REDACTED>
preview: false
priority: 4000

I'm not sure under what conditions this occurs, but if this is a valid response pattern for the API, I believe that Terraform should recognize both of these as the same value.

@ggtisc
Copy link
Collaborator

ggtisc commented Jul 16, 2024

After some attempts it is not possible to reproduce this issue, users commented that this issue happens sometimes but not always. It is necessary to check what is causing it at the code level, since at the terraform configuration level it is unpredictable.

@ggtisc ggtisc removed their assignment Jul 16, 2024
@ggtisc ggtisc removed the forward/review In review; remove label to forward label Jul 16, 2024
@aweberlopes
Copy link

Hello Guys we have similar issue here. If the [expr_options] (https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/compute_security_policy#expr_options)
is empty then each run produce a inplace change for the policy because on the api the
action_token_site_keys and session_token_site_keys are empty lists but in the state they are null so its tries to overwrite it every terraform run.

@FabianFrank
Copy link

I am facing the same issue. We do not (and never have) set expr_options but are facing the same perma-diff on every terraform run:

  ~ resource "google_compute_security_policy" "policy" {
        id          = "..."
        name        = "api-security-policy"
        # (5 unchanged attributes hidden)

      - rule {
          - action      = "deny(403)" -> null
          - description = "Prevent XSS injection" -> null
          - preview     = false -> null
          - priority    = 200 -> null

          - match {
                # (1 unchanged attribute hidden)

              - expr {
                  - expression = <<-EOT
                        !request.path.matches(...) &&
                        evaluatePreconfiguredExpr('xss-v33-stable',
                        ['owasp-crs-v030301-id941101-xss',
                          'owasp-crs-v030301-id941150-xss',
                          'owasp-crs-v030301-id941320-xss',
                          'owasp-crs-v030301-id941330-xss',
                          'owasp-crs-v030301-id941340-xss',
                          'owasp-crs-v030301-id941380-xss'
                        ])
                    EOT -> null
                }

              - expr_options {
                  - recaptcha_options {
                      - action_token_site_keys  = [] -> null
                      - session_token_site_keys = [] -> null
                    }
                }
            }
        }
      + rule {
          + action      = "deny(403)"
          + description = "Prevent XSS injection"
          + preview     = (known after apply)
          + priority    = 200

          + match {
                # (1 unchanged attribute hidden)

              + expr {
                  + expression = <<-EOT
                        !request.path.matches(...) &&
                        evaluatePreconfiguredExpr('xss-v33-stable',
                        ['owasp-crs-v030301-id941101-xss',
                          'owasp-crs-v030301-id941150-xss',
                          'owasp-crs-v030301-id941320-xss',
                          'owasp-crs-v030301-id941330-xss',
                          'owasp-crs-v030301-id941340-xss',
                          'owasp-crs-v030301-id941380-xss'
                        ])
                    EOT
                }

              + expr_options {
                  + recaptcha_options {
                      + action_token_site_keys  = []
                      + session_token_site_keys = []
                    }
                }
            }
        }

        # (5 unchanged blocks hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Versions:

terraform version
Terraform v1.9.2
on darwin_arm64
+ provider registry.terraform.io/hashicorp/google v5.39.1
+ provider registry.terraform.io/hashicorp/google-beta v5.32.0
+ provider registry.terraform.io/hashicorp/random v3.6.2

@CristianCamilo98
Copy link

I have find a work around, if you want terraform to stop showing the difference on every apply at the block and assign it null values:

expr_options {
  recaptcha_options {
    action_token_site_keys  = null
    session_token_site_keys = null
  }
}

@athak
Copy link

athak commented Aug 29, 2024

Same issue here, in a security policy with 10 rules only one was having this issue. @CristianCamilo98's workaround did not work for us, simply chose to ignore changes in the lifecycle block.

@imrannayer
Copy link

imrannayer commented Nov 13, 2024

preconfigured_waf_config issue resolved. match.expr_options.recaptcha_options is still causing recreation of the rule.It is blocking this issue

@roma-wl
Copy link

roma-wl commented Dec 11, 2024

It seems that recreating the rule can fix the perma-diff issue.

In my case, the issue started when I did some manual edit on the rule object on google cloud console

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants