Page MenuHomePhabricator

Refactor P:base::firewall to pull host directly from puppetdb
Open, LowPublic

Description

Currently profile::base::firewall pulls in a bunch of static IPv4/6 lists for various global hosts so that we can set up firewall rules e.g.

hieradata/common.yaml
bastion_hosts:
- 208.80.155.110                # bast1003.wikimedia.org
- 2620:0:861:4:208:80:155:110   # bast1003.wikimedia.org
- 208.80.153.54                 # bast2002.wikimedia.org
- 2620:0:860:2:208:80:153:54    # bast2002.wikimedia.org
- 91.198.174.6                  # bast3005.wikimedia.org
- 2620:0:862:1:91:198:174:6     # bast3005.wikimedia.org
- 198.35.26.13                  # bast4003.wikimedia.org
- 2620:0:863:1:198:35:26:13     # bast4003.wikimedia.org
- 103.102.166.6                 # bast5002.wikimedia.org
- 2001:df2:e500:1:103:102:166:6 # bast5002.wikimedia.org
- 185.15.58.6                   # bast6001.wikimedia.org
- 2a02:ec80:600:1:185:15:58:6   # bast6001.wikimedia.org
modules/profile/manifests/base/firewall.pp
class profile::base::firewall (
  Array[Stdlib::IP::Address] $bastion_hosts = lookup('bastion_hosts', {default_value => []}),
) {
  class { 'base::firewall':
    bastion_hosts => $bastion_hosts
  }
}
modules/base/manifests/firewall.pp
class base::firewall (
  Array[Stdlib::IP::Address] $bastion_hosts = [],
) {
      $bastion_hosts_str = join($bastion_hosts, ' ')
      ferm::rule { 'bastion-ssh':
          rule   => "proto tcp dport ssh saddr (${bastion_hosts_str}) ACCEPT;",
      }  
}

Proposal

Drop the access lists in ferm all together and instead rely on puppetdb (wmflib::role_ips) to inform us of the host

modules/profile/manifests/base/firewall.pp
class profile::base::firewall (
   class { 'base::firewall':
    bastion_hosts => wmflib::role_ips('bastionhost')
  }
}
modules/base/manifests/firewall.pp
class base::firewall (
  Array[Stdlib::Fqdn] $bastion_hosts = [],
) {
      ferm::rule { 'bastion-ssh':
          proto    => 'tcp',
          port      => 22,
          srange => $bastion_hosts,
      }  
}
Advantage

The main advantage of this is that we no longer need to maintain manual lists of these global hosts and we can rely on puppet adding them and removing them as our infrastructre changes

Disadvantage

The main disadvantage here is also the fact that we pull the information from puppetdb. Using puppetdb add some additional complexity and moving parts to adding firewall rules. specificity if we where to add a new bastion_host then we would first need to ensure the new host has the 'bastionhost' role (i.e. not system::spare) and run puppet on the new bastion host before its ip address is known to pupetdb and other hosts can add them to there rules.

Another minor disadvantage is this would not work with cloud as they do not have puppetdb however it would be fairly trivial to support both use cases in the profile with e.g.

modules/profile/manifests/base/firewall.pp
class profile::base::firewall (
  Optional[Array[Stdlib::Fqdn]] $bastion_hosts = lookup('bastion_hosts', {default_value => undef}),
) {
  class { 'base::firewall':
    # im not the biggest fan of lest so we could use some other syntax but you get the idea
    bastion_hosts => $bastion_hosts.lest || { wmflib::role_hosts('bastionhost') }
  }
}

Event Timeline

jbond added a parent task: Restricted Task.Feb 4 2022, 10:23 AM
jbond triaged this task as Low priority.Feb 16 2022, 4:57 PM

I think we can close this; the new profile:: ferm::service and ferm::service offer opt-in name resolution on the Puppet server side if an array of hosts is passed to srange() and drange(). This will slowly trickle in as I move over firewall defitions to the new API. @jbond Unless you think anything else is missing to resolve this?

@MoritzMuehlenhoff I have updated the description a bit. This task is more about removing the lists in hiera and instead relying on puppetdb and have the lists become dynamic or possibly moving the lists to netbox if we want to continue relying on a static list