Firewall source_ranges defaults to 0.0.0.0/0 when source_tags not provided (dangerous!)

dmelliot commented 4 years ago

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 the modular-magician user, it is either in the process of being autogenerated, or is planned to be autogenerated soon. If an issue is assigned to a user, that user is claiming responsibility for the issue. If an issue is assigned to hashibot, a community member has claimed the issue already.

Terraform Version

Terraform v0.12.28

provider.google v3.30.0

Affected Resource(s)

google_compute_firewall

Terraform Configuration Files

locals {
        firewall_allow_rules = {
                mgmt-ssh = {
                        tcp_ports = ["22"]
                        target_tags = ["bastion"]
                }
        }
} 
resource "google_compute_firewall" "fw_allow_access" {

        for_each = local.firewall_allow_rules

        name    = "allow-${each.key}"
        network = google_compute_network.network01.self_link
        source_tags = lookup(each.value,"source_tags",null)
        source_ranges = lookup(each.value,"source_ranges",null)
        target_tags = each.value["target_tags"]
        disabled = lookup(each.value,"disabled","false")

        allow {
                protocol = "tcp"
                ports = each.value.tcp_ports
        }
}

Expected Behavior

As neither source_tags nor source_ranges were provided, Terraform should report an error when applying. This would match the behavior of the Google Cloud Console UI which requires at least one of these to be defined to create a firewall rule.

Actual Behavior

Terraform happily applies the rule and sets the source_ranges to 0.0.0.0/0 - giving the whole internet access to the resource

Steps to Reproduce

Create a file containing a firewall rule as per above
terraform apply

Important Factoids

The goal here is to reduce repeatitive code by defining firewall rules in a map and using for_each. As is stands, with this issue we need to define 2 - 3 maps. One map for source_ranges based rules, one for source_tags based rules and potentially another for rules that specify both.

Please also note I've tried using [] in place of the null value and the behavior is the same.

References

The following issues seems similar/related

b/304967966

venkykuberan commented 4 years ago

Can you please share the plan output of your config ?

yanc0 commented 3 years ago

I can easily reproduce the problem when I use tfvars:

main.tf

provider "google" {
  version = "~> 3.14"
  region  = "europe-west1"
  project = "redacted"
}

variable "authorized_ip" {
  type = list
  default = [""]
}

resource "google_compute_firewall" "powned-fw-rule" {
  name          = "you-have-been-powned"
  network       = "default"
  source_ranges = var.authorized_ip

  allow {
    protocol = "udp"
    ports    = ["666"]
  }
}

test.tfvars

authorized_ip = [] # previously ["1.1.1.1/32"]

Execution

$ terraform apply -var-file=tfvars/test.tfvars
  # google_compute_firewall.powned-fw-rule will be updated in-place
  ~ resource "google_compute_firewall" "powned-fw-rule" {
        creation_timestamp      = "2020-12-17T23:52:28.117-08:00"
        destination_ranges      = []
        direction               = "INGRESS"
        disabled                = false
        id                      = "projects/redacted/global/firewalls/you-have-been-powned"
        name                    = "you-have-been-powned"
        network                 = "https://www.googleapis.com/compute/v1/projects/redacted/global/networks/default"
        priority                = 1000
        project                 = "redacted"
        self_link               = "https://www.googleapis.com/compute/v1/projects/redacted/global/firewalls/you-have-been-powned"
      ~ source_ranges           = [
          - "1.1.1.1/32",
        ]
        source_service_accounts = []
        source_tags             = []
        target_service_accounts = []
        target_tags             = []

        allow {
            ports    = [
                "666",
            ]
            protocol = "udp"
        }
    }

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

$ gcloud compute firewall-rules list --format=json --filter="name=you-have-been-powned"
[
  {
    "allowed": [
      {
        "IPProtocol": "udp",
        "ports": [
          "666"
        ]
      }
    ],
    "creationTimestamp": "2020-12-17T23:52:28.117-08:00",
    "description": "",
    "direction": "INGRESS",
    "disabled": false,
    "id": "4451896396621144403",
    "kind": "compute#firewall",
    "logConfig": {
      "enable": false
    },
    "name": "you-have-been-powned",
    "network": "https://www.googleapis.com/compute/v1/projects/redacted/global/networks/default",
    "priority": 1000,
    "selfLink": "https://www.googleapis.com/compute/v1/projects/redacted/global/firewalls/you-have-been-powned",
    "sourceRanges": [
      "0.0.0.0/0"
    ]
  }
]

As you can see, the 0.0.0.0/0 source range have been set and I think we cannot be implicit on that rule. This is a dangerous behaviour.

gygitlab commented 2 years ago

Recently got stung by this.

Big security issue this, especially the fact it's undocumented. In fact the documentation is wrong:

For INGRESS traffic, one of source_ranges, source_tags or source_service_accounts is required.

This should really be fixed - It should at least require one of the acceptable values as described to make the user specify a range. At a minimum the default behaviour of 0.0.0.0/0 should be added to the docs as a priority.

hashicorp / terraform-provider-google