Firewall Improvements

[Sebbo94BY]

I've just tested the beta firewall feature using Terraform and also in the console and I really like it. This helps a lot to avoid eg. iptables firewall configurations on all the hosts. While testing and setting up a few firewalls and rules, I've noted some things, which would be cool to have:

• Adding a description field to each single rule would be very helpful to see and know for which purpose this rule for example is, when you view it in the console (in Terraform, you can simply add # comments, but this would not help someone, who is checking something through the Hetzner Cloud Console only)
• Instead of only having port, I would implemt from_port and to_port. In the console, you're also able to fill out two fields. This would also allow you to change the value type from string to number to better check the given values. :)
• Would be cool to have a simple option to define a rule, which simply allows everything (eg. for outgoing traffic to be explicit; AWS does this for example with protocol = "-1", from_port = 0 and to_port = 0)
• Outgoing firewall rules are currently not possible to manage as direction out is not available in Terraform (#348)

Below some examples for these improvements:

resource "hcloud_firewall" "default" {
  name = "default"

  # ICMP
  rule {
    description = "ICMP"
    direction   = "in"
    protocol    = "icmp"
    source_ips = [
      "0.0.0.0/0",
      "::/0",
    ]
  }

  # SSH (default port)
  rule {
    description = "SSH (default port)"
    direction   = "in"
    protocol    = "tcp"
    from_port   = 22
    to_port     = 22
    source_ips = [
      "0.0.0.0/0",
      "::/0",
    ]
  }

  # SSH (custom port)
  rule {
    description = "SSH (custom port)"
    direction   = "in"
    protocol    = "tcp"
    from_port   = 2223
    to_port     = 2223
    source_ips = [
      "0.0.0.0/0",
      "::/0",
    ]
  }

  # Allow any outgoing TCP traffic
  rule {
    description = "Any TCP traffic"
    direction   = "out"
    protocol    = "tcp"
    from_port   = 0
    to_port     = 0
    source_ips = [
      "0.0.0.0/0",
      "::/0",
    ]
  }

  # Or eg. allow any outgoing traffic (doesn't matter which protocol and port)
  rule {
    description = "Any TCP traffic"
    direction   = "out"
    protocol    = "-1"
    from_port   = 0
    to_port     = 0
    source_ips = [
      "0.0.0.0/0",
      "::/0",
    ]
  }

  labels = merge(
    local.default_labels,
    var.additional_labels,
    {
      Description = "Allows-necessary-minimum-traffic"
    },
  )
}

[LKaemmerling]

Hey @Sebi94nbg,

thank you for your feedback! I passed the things to our team.

We decided to not implement two different fields for the ports because it doesn't felt right to have something like this:

   from_port   = 2223
   to_port     = 2223

We want to be simple and easy to use, therefore we decided to only have a port. Within this field, you can use single ports or even port ranges (Sample: 1-65535).

Would be cool to have a simple option to define a rule, which simply allows everything (eg. for outgoing traffic to be explicit; AWS does this for example with protocol = "-1", from_port = 0 and to_port = 0)

Personally, I think this is quite dangerous. When you define a firewall rule you should be explicit, having such indirect rules (one rule that allows everything) is fine as long as there are only a few supported protocols. With this "wildcard" you would directly allow new protocols we might add to the firewalls without knowing about it.

[Sebbo94BY]

You're welcome! Thank you!

When you define a firewall rule you should be explicit

Currently, you allow any outgoing traffic, when there is no rule configured in any assigned firewall. With my approach this would be explicit then. You are explicitly allowing anything. :)

With this "wildcard" you would directly allow new protocols we might add to the firewalls without knowing about it.

When you add / support new things, then this rule shouldn't be your problem. That's something, about what the respective DevOps needs to think about. They also could be more explicit and only allow the protocols and ports, which they want to allow instead of allowing anything. I wouldn't see this as a problem from Hetzner side. A DevOps has the job to check the changes of new releases, Cloud features etc. and if required, the DevOps needs to react. But that's only my opinion. ;)

[github-actions[bot]]

This issue has been marked as stale because it has not had recent activity. The bot will close the issue if no further action occurs.

[Sebbo94BY]

There are still some open improvement requests.

[apricote]

Hello,

thanks for you suggestions. We do not plan on implementing anything in addition to what is already supported right now. I will close the issue. Feel free to open a new issue for any new suggestions :)

Descriptions

Rules Descriptions are supported since v1.28.0 of the provider.

Port

We have decided to keep the single field. This is consistent with the current API. If we would add new fields from_port and to_port it would complicate the handling. On reading the resources we would need to parse the API response and decide if we need to fill port or from_port+to_port.

Allow-all Rule

I have added a +1 to a related request in our internal feature tracker. If this gets added to the API, we will also add this in the provider.