hashicorp / terraform-provider-aws

The AWS Provider enables Terraform to manage AWS resources.
https://registry.terraform.io/providers/hashicorp/aws
Mozilla Public License 2.0
9.74k stars 9.1k forks source link

[Bug]: Multiple rule_option entries in aws_networkfirewall_rule_group not preserving order #35147

Open adaminato42 opened 8 months ago

adaminato42 commented 8 months ago

Terraform Core Version

1.6.6

AWS Provider Version

5.31.0

Affected Resource(s)

aws_networkfirewall_rule_group

Expected Behavior

Network firewall group created

Actual Behavior

Error (due to re-ordering of options) "reason: endswith needs a preceding content option"

Relevant Error/Panic Output Snippet

│ Error: creating NetworkFirewall Rule Group (Allow-DNS-01-Validation): InvalidRequestException: stateful rule is invalid, rule: pass dns ANY ANY -> [8.8.4.4, 8.8.8.8] 53 (dns.query; endswith; sid:1000000; pcre:/^_acme-challenge/; content:example.com; gid:1234567890;), reason: endswith needs a preceding content option

Terraform Configuration Files

main.tf

  required_version = "~> 1.6.6"
  required_providers {
    aws = {
      source  = "registry.terraform.io/hashicorp/aws"
      version = "~> 5.0"
    }
  }
  backend "s3" {
    bucket   = "example.aws.tfstates"
    key      = "state"
    region   = "us-east-2"
    role_arn = "arn:aws:iam::123456789012:role/AWSAdministrator"
    encrypt  = true
  }
}

provider "aws" {
  region = "us-east-2"
  assume_role {
    role_arn = "arn:aws:iam::123456789012:role/AWSAdministrator"
  }
}

network_firewall_group.tf

resource "aws_networkfirewall_rule_group" "allow_dns01_challenge" {
  capacity = 1
  name     = "Allow-DNS-01-Validation"
  type     = "STATEFUL"
  rule_group {

    rule_variables {
      # Google's public DNS servers, used by cert-manager for DNS-01 validation
      ip_sets {
        key = "GOOGLE_DNS_IPS"
        ip_set {
          definition = ["8.8.8.8", "8.8.4.4"]
        }
      }
    }

    # creates this Suricata ruleset
    # pass dns any any -> $GOOGLE_DNS_IPS 53 (dns.query; content: "example.com"; endswith; pcre:"/^_acme-challenge/"; sid:"1000000";)
    rules_source {
      stateful_rule {
        action = "PASS" # pass
        header {
          protocol         = "DNS"
          source           = "ANY"
          source_port      = "ANY"
          direction        = "FORWARD"
          destination      = "$GOOGLE_DNS_IPS"
          destination_port = "53"
        }
        rule_option {
          keyword = "dns.query"
        }
        rule_option {
          keyword  = "content"
          settings = ["example.com"]
        }
        rule_option {
          keyword = "endswith"
        }
        rule_option {
          keyword  = "pcre"
          settings = ["/^_acme-challenge/"]
        }
        rule_option {
          keyword  = "sid"
          settings = ["1000000"]
        }
      }
    }
  }
}

Steps to Reproduce

A terraform plan will be created without issue, but you can see the rule_option sections are now ordered alphabetically by keyword:

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # aws_networkfirewall_rule_group.allow_dns01_challenge will be created
  + resource "aws_networkfirewall_rule_group" "allow_dns01_challenge" {
      + arn          = (known after apply)
terraform {
      + capacity     = 1
      + id           = (known after apply)
      + name         = "Allow-DNS-01-Validation"
      + tags_all     = (known after apply)
      + type         = "STATEFUL"
      + update_token = (known after apply)

      + rule_group {
          + rule_variables {
              + ip_sets {
                  + key = "GOOGLE_DNS_IPS"

                  + ip_set {
                      + definition = [
                          + "8.8.4.4",
                          + "8.8.8.8",
                        ]
                    }
                }
            }
          + rules_source {
              + stateful_rule {
                  + action = "PASS"

                  + header {
                      + destination      = "$GOOGLE_DNS_IPS"
                      + destination_port = "53"
                      + direction        = "FORWARD"
                      + protocol         = "DNS"
                      + source           = "ANY"
                      + source_port      = "ANY"
                    }

                  + rule_option {
                      + keyword  = "content"
                      + settings = [
                          + "example.com",
                        ]
                    }
                  + rule_option {
                      + keyword  = "dns.query"
                      + settings = []
                    }
                  + rule_option {
                      + keyword  = "endswith"
                      + settings = []
                    }
                  + rule_option {
                      + keyword  = "pcre"
                      + settings = [
                          + "/^_acme-challenge/",
                        ]
                    }
                  + rule_option {
                      + keyword  = "sid"
                      + settings = [
                          + "1000000",
                        ]
                    }
                }
            }
        }
    }

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

A terraform apply will fail, as the ordering appears random

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

aws_networkfirewall_rule_group.allow_dns01_challenge: Creating...
╷
│ Error: creating NetworkFirewall Rule Group (Allow-DNS-01-Validation): InvalidRequestException: stateful rule is invalid, rule: pass dns ANY ANY -> [8.8.4.4, 8.8.8.8] 53 (dns.query; endswith; sid:1000000; pcre:/^_acme-challenge/; content:example.com; gid:1234567890;), reason: endswith needs a preceding content option
│
│   with aws_networkfirewall_rule_group.allow_dns01_challenge,
│   on example.tf line 1, in resource "aws_networkfirewall_rule_group" "allow_dns01_challenge":
│    1: resource "aws_networkfirewall_rule_group" "allow_dns01_challenge" {
│
╵

Debug Output

aws_networkfirewall_rule_group.tf_log.debug.txt

Panic Output

No response

Important Factoids

No response

References

We are able to create these rules via the AWS Console. The API documentation https://docs.aws.amazon.com/network-firewall/latest/developerguide/stateful-rule-groups-basic.html https://docs.aws.amazon.com/network-firewall/latest/APIReference/API_StatefulRule.html https://docs.aws.amazon.com/network-firewall/latest/APIReference/API_RuleOption.html

Would you like to implement a fix?

None

github-actions[bot] commented 8 months ago

Community Note

Voting for Prioritization

Volunteering to Work on This Issue

Michagogo commented 8 months ago

Do multiple stateful_rule blocks get ordered properly? I see that’s also described in documentation as a set, which is unordered (and thus inappropriate in this context, if the documentation is accurate).

adaminato42 commented 8 months ago

Do multiple stateful_rule blocks get ordered properly? I see that’s also described in documentation as a set, which is unordered (and thus inappropriate in this context, if the documentation is accurate).

When declared in Terraform, only a single stateful_rule can exist per rules_source section as per the docs.

The ordering of the rule groups is handled in aws_networkfirewall_firewall_policy, using multiple stateful_rule_group_reference entries which have a priority.

The important part here is the ordering of the rule_options has to be maintained to create properly constructed Suricata rule string

Michagogo commented 8 months ago

Do multiple stateful_rule blocks get ordered properly? I see that’s also described in documentation as a set, which is unordered (and thus inappropriate in this context, if the documentation is accurate).

When declared in Terraform, only a single stateful_rule can exist per rules_source section as per the docs.

That’s not how I’m reading it — the docs there describe it as “(Optional) Set of configuration blocks containing stateful inspection criteria for 5-tuple rules to be used together in a rule group.”, which is the exact same way that rule_option is described (as a set of configuration blocks) and talks about rules, plural. And the nature of a rule group is to have a collection of rules within it, some of which may well interact with each other, so it seems to me that it couldn’t possibly be the case that Terraform-created rule groups are only allowed to contain a single rule, any more so than it would make senses for it to only allow setting a single rule option. If that is the case then that’s a much bigger problem in the provider.

adaminato42 commented 8 months ago

That’s not how I’m reading it — the docs there describe it as “(Optional) Set of configuration blocks containing stateful inspection criteria for 5-tuple rules to be used together in a rule group.”, which is the exact same way that rule_option is described (as a set of configuration blocks) and talks about rules, plural.

It is a set of blocks, but they are specific blocks:

rule_group {
  rules_source {            # Only one of rules_source_list, rules_string, stateful_rule, or stateless_rules_and_custom_actions must be specified
    stateful_rule {         # Required
      action                # Required
      header { block }      # Required, one only
      rule_option { block } # Optional, zero to many
      rule_option { block } ...
    }
  }
}

While the AWS Console UI allows to add multiple stateful rules to the same rule group, Terraform does not. From my perspective, that's a feature parity issue.

The concern for us is that multiple rule_option entries do not maintain their ordering, which breaks the Suricata language syntax when generated.

justinretzolk commented 8 months ago

Similar to #27102, which addressed ordering of the stateful_rule block