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

OWASP ruleset override fails because of missing "enabled" attribute. #1403

Closed
2 tasks done
brandonstrohmeyer opened this issue Jan 21, 2022 · 5 comments · Fixed by #1405
Closed
2 tasks done

OWASP ruleset override fails because of missing "enabled" attribute. #1403

brandonstrohmeyer opened this issue Jan 21, 2022 · 5 comments · Fixed by #1405
Labels
triage/needs-information Indicates an issue needs more information in order to work on it.

Comments

@brandonstrohmeyer
Copy link

brandonstrohmeyer commented Jan 21, 2022

Confirmation

  • My issue isn't already found on the issue tracker.
  • I have replicated my issue using the latest version of the provider and it is still present.

Terraform and Cloudflare provider version

❯ terraform -v
Terraform v1.1.2
on darwin_amd64

  • provider registry.terraform.io/cloudflare/cloudflare v3.7.0
  • provider registry.terraform.io/hashicorp/aws v3.73.0
  • provider registry.terraform.io/hashicorp/http v2.1.0
  • provider registry.terraform.io/hashicorp/time v0.7.2
  • provider registry.terraform.io/ns1-terraform/ns1 v1.12.2

Affected resource(s)

cloudflare_ruleset

Terraform configuration files

resource "cloudflare_ruleset" "zone_managed" {
  zone_id     = cloudflare_zone.this.id
  name        = "zone_managed"
  description = "Zone-level Cloudflare Managed Rulesets"
  kind        = "zone"
  phase       = "http_request_firewall_managed"

  # Configure Cloudflare OWASP Core Ruleset with defaults
  rules {
    action = "execute"
    action_parameters {
      id = "4814384a9e5d4991b9815dcfc25d2f1f" # Cloudflare OWASP Core Ruleset

      overrides {

        # Cloudflare API expects the paranoia levels above the desired level to be disabled in order to set the desired level. 
        # Example: If PL2 is desired, then PL3 and PL4 must be disabled. 
        # The 'for' expression below only returns the levels above the desired user provided level, which then are disabled.
        dynamic "categories" {
          for_each = [
            for i, v in local.owasp.paranoia_levels :
            v if i > index(local.owasp.paranoia_levels, local.owasp.paranoia[var.cloudflare_owasp_ruleset.paranoia_level])
          ]
          content {
            category = categories.value
            enabled  = false
          }
        }

        rules {
          id              = "6179ae15870a4bb7b2d480d4843b323c" # 949110: Inbound Anomaly Score Exceeded
          score_threshold = try(local.owasp.threshold[var.cloudflare_owasp_ruleset.anomaly_score_threshold], 40)
          action          = try(var.cloudflare_owasp_ruleset.action, "block")
        }

        rules {
          id      = "010cc6b1d9ed4cdc82b2dc8dce6f319a"
          enabled = false
        }
      }
    }

    expression  = "true"
    description = "OWASP Managed Ruleset"
    enabled     = try(var.cloudflare_owasp_ruleset.enabled, true)
  }
}

Debug output

---[ REQUEST ]---------------------------------------
PUT /client/v4/zones/dabde61ec0596fb6fc897523bf916d96/rulesets/c6ab70fca4634bcb8f1917ed06ffe634 HTTP/1.1
Host: api.cloudflare.com
User-Agent: terraform/1.1.2 terraform-plugin-sdk/2.10.1 terraform-provider-cloudflare/3.7.0
Content-Length: 25682
Authorization: Bearer xxx
Content-Type: application/json
Accept-Encoding: gzip

{
 "description": "Zone-level Cloudflare Managed Rulesets",
 "rules": [
  {
   "action": "execute",
   "action_parameters": {
    "id": "4814384a9e5d4991b9815dcfc25d2f1f",
    "overrides": {
     "enabled": false,
     "categories": [
      {
       "category": "paranoia-level-3",
       "enabled": false
      },
      {
       "category": "paranoia-level-4",
       "enabled": false
      }
     ],
     "rules": [
      {
       "id": "6179ae15870a4bb7b2d480d4843b323c",
       "action": "block",
       "score_threshold": 60
      },
      {
       "id": "010cc6b1d9ed4cdc82b2dc8dce6f319a" **<----- Missing "enabled" attribute -----**
      },
      {
       "id": "011b5927ccbc479087a35b5cfe899e02",
       "enabled": true
      },
      {
       "id": "0126f1c6245f400399d82d39bcfd6659",
       "enabled": true
      },
      {
       "id": "02a11d6fc5c74dbc911455294b629ea8",
       "enabled": true
      },
      ...snip...
      ]
    },
    "version": "latest"
   },
   "expression": "true",
   "description": "OWASP Managed Ruleset",
   "enabled": true
  }
]

  2022-01-21T15:35:55.731-0500 [INFO]  provider.terraform-provider-cloudflare_v3.7.0: 2022/01/21 15:35:55 [DEBUG] Cloudflare API Response Details:
---[ RESPONSE ]--------------------------------------
HTTP/2.0 400 Bad Request
Cf-Cache-Status: DYNAMIC
Cf-Ray: 6d1354e95c34f321-ATL
Content-Type: application/json; charset=UTF-8
Date: Fri, 21 Jan 2022 20:35:55 GMT
Expect-Ct: max-age=604800, report-uri="https://report-uri.cloudflare.com/cdn-cgi/beacon/expect-ct"
Server: cloudflare
Set-Cookie: __cflb=0H28vgHxwvgAQtjUGU4vq74ZFe3sNVUZYixTxpzQHWm; SameSite=Lax; path=/; expires=Fri, 21-Jan-22 23:05:56 GMT; HttpOnly
Set-Cookie: __cfruid=7581a612f086ee64c96e4d6373446a4f5a4cc9d6-1642797355; path=/; domain=.api.cloudflare.com; HttpOnly; Secure; SameSite=None
Vary: Accept-Encoding
X-Envoy-Upstream-Service-Time: 784
X-Version: 4864-dbff164f7a45

{
  "result": null,
  "success": false,
  "errors": [
    {
      "message": "Rule overrides requires either the action, the enabled flag or the score threshold set"
    }
  ],
  "messages": null
}

Panic output

No response

Expected output

In the module call that contains the above resource call, I am configuring a rule override to disable a specific OWASP rule:

      {
        id      = "010cc6b1d9ed4cdc82b2dc8dce6f319a"
        enabled = false
      }
    ]

I am expecting this rule to be disabled.

Actual output

As shown in the above debug output, the enabled attribute is not being set which in turn causes the following error:

╷
│ Error: error updating ruleset with ID "c6ab70fca4634bcb8f1917ed06ffe634": HTTP status 400: Rule overrides requires either the action, the enabled flag or the score threshold set
│ 
│   with module.cloudflare_stro_debug.module.cloudflare_website.cloudflare_ruleset.zone_managed,
│   on .terraform/modules/cloudflare_stro_debug.cloudflare_website/rulesets.tf line 29, in resource "cloudflare_ruleset" "zone_managed":
│   29: resource "cloudflare_ruleset" "zone_managed" {
│ 
╵

Steps to reproduce

  1. Attach OWASP managed ruleset to zone
  2. Attempt to "disable" a rule via override using the Terraform resource.

Additional factoids

  1. To work around issues described in [Ruleset] Single rule override in a managed ruleset disables all other rules #1397 and Ruleset - enabled doesn't work as intended #1273 I am creating a rule override for every individual rule for both OWASP and Cloudflare Managed ruleset. This behavior was not observed (or at least did not throw any error and is working as expected) with the Cloudflare Managed Ruleset using the same pattern.

References

No response

@brandonstrohmeyer brandonstrohmeyer added kind/bug Categorizes issue or PR as related to a bug. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jan 21, 2022
@jacobbednarz
Copy link
Member

have you tried this without the dynamics? I think we have a test case covering this so it should work.

@jacobbednarz jacobbednarz added triage/needs-information Indicates an issue needs more information in order to work on it. and removed kind/bug Categorizes issue or PR as related to a bug. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jan 21, 2022
@brandonstrohmeyer
Copy link
Author

@jacobbednarz I tested hard coding the rule override without dynamics and experience the same behavior with the missing "enabled" attribute.

@jacobbednarz
Copy link
Member

can you please update your reproduction test case to not use dynamics? I generally don’t use them for debugging as they have a class of their own issues.

@brandonstrohmeyer
Copy link
Author

I have updated the test case above with just a simple rule override. Deleting the OWASP ruleset to clean out any leftover configuration and re-adding it to the zone with this one override is still failing.

jacobbednarz added a commit that referenced this issue Jan 24, 2022
Updates the check for whether or not to populate the `enabled` flag on
category/rule based overrides to use the `GetOkExists` method (not
recommended due to undocumented edge cases) to check the existence
before converting the pointer to a value.

Closes #1403
jacobbednarz added a commit that referenced this issue Jan 24, 2022
Updates the check for whether or not to populate the `enabled` flag on
category/rule based overrides to use the `GetOkExists` method (not
recommended due to undocumented edge cases) to check the existence
before converting the pointer to a value.

Closes #1403
@jacobbednarz
Copy link
Member

jacobbednarz commented Jan 24, 2022

arrgh, thanks for this @brandonstrohmeyer. it's Yet Another tristate bool issue 😢

i've updated this in #1405 which i've confirmed is fixed with a test case covering it. btw, the API throws an error with "010cc6b1d9ed4cdc82b2dc8dce6f319a" disabled 🤷

jacobbednarz added a commit that referenced this issue Jan 24, 2022
Updates the check for whether or not to populate the `enabled` flag on
category/rule based overrides to use the `GetOkExists` method (not
recommended due to undocumented edge cases) to check the existence
before converting the pointer to a value.

Closes #1403
jacobbednarz added a commit that referenced this issue Jan 24, 2022
Updates the check for whether or not to populate the `enabled` flag on
category/rule based overrides to use the `GetOkExists` method (not
recommended due to undocumented edge cases) to check the existence
before converting the pointer to a value.

Closes #1403
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage/needs-information Indicates an issue needs more information in order to work on it.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants