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

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

Open
dmelliot opened this issue Jul 14, 2020 · 3 comments

Comments

@dmelliot
Copy link

dmelliot commented Jul 14, 2020

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

  1. Create a file containing a firewall rule as per above
  2. 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

@ghost ghost added bug labels Jul 14, 2020
@venkykuberan venkykuberan self-assigned this Jul 14, 2020
@venkykuberan
Copy link
Contributor

Can you please share the plan output of your config ?

@yanc0
Copy link

yanc0 commented Dec 18, 2020

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
Copy link

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.

modular-magician added a commit to modular-magician/terraform-provider-google that referenced this issue Nov 28, 2022
…pe (beta) (hashicorp#6789)

* feat: add the iam resources for featurestore entitytype

* fix: fix the unit test errors

Signed-off-by: Modular Magician <magic-modules@google.com>
modular-magician added a commit that referenced this issue Nov 28, 2022
…pe (beta) (#6789) (#13126)

* feat: add the iam resources for featurestore entitytype

* fix: fix the unit test errors

Signed-off-by: Modular Magician <magic-modules@google.com>

Signed-off-by: Modular Magician <magic-modules@google.com>
@edwardmedia edwardmedia removed the forward/review In review; remove label to forward label Oct 6, 2023
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

6 participants