Merge "[CVE-2016-9599] Enforce Firewall TCP / UDP rules management"

2 files changed, 39 insertions, 9 deletions

diff --git a/manifests/firewall/rule.pp b/manifests/firewall/rule.pp
index 6801dc4..816e6fe 100644
--- a/manifests/firewall/rule.pp
+++ b/manifests/firewall/rule.pp
@@ -77,8 +77,16 @@ define tripleo::firewall::rule (
   $extras      = {},
 ) {
 
+  if $port == 'all' {
+    warning("All ${proto} traffic will be open on this host.")
+    # undef so the IPtables rule won't have any port specified.
+    $port_real = undef
+  } else {
+    $port_real = $port
+  }
+
   $basic = {
-    'port'        => $port,
+    'port'        => $port_real,
     'dport'       => $dport,
     'sport'       => $sport,
     'proto'       => $proto,
@@ -100,6 +108,15 @@ define tripleo::firewall::rule (
   $rule = merge($basic, $state_rule, $extras)
   validate_hash($rule)
 
+  # This conditional will ensure that TCP and UDP firewall rules have
+  # a port specified in the configuration when using INPUT or OUTPUT chains.
+  # If not, the Puppet catalog will fail.
+  # If we don't do this sanity check, a user could create some TCP/UDP
+  # rules without port, and the result would be an iptables rule that allow any
+  # traffic on the host.
+  if ($proto in ['tcp', 'udp']) and (! ($port or $dport or $sport) and ($chain != 'FORWARD')) {
+    fail("${title} firewall rule cannot be created. TCP or UDP rules for INPUT or OUTPUT need port or sport or dport.")
+  }
   create_resources('firewall', { "${title}" => $rule })
 
 }
diff --git a/manifests/haproxy/endpoint.pp b/manifests/haproxy/endpoint.pp
index 4311049..0bba245 100644
--- a/manifests/haproxy/endpoint.pp
+++ b/manifests/haproxy/endpoint.pp
@@ -149,14 +149,27 @@ define tripleo::haproxy::endpoint (
   }
   if hiera('manage_firewall', true) {
     include ::tripleo::firewall
-    $firewall_rules = {
-      "100 ${name}_haproxy"     => {
-        'dport' => $service_port,
-      },
-      "100 ${name}_haproxy_ssl" => {
-        'dport' => $public_ssl_port,
-      },
+    # This block will construct firewall rules only when we specify
+    # a port for the regular service and also the ssl port for the service.
+    # It makes sure we're not trying to create TCP iptables rules where no port
+    # is specified.
+    if $service_port {
+      $haproxy_firewall_rules = {
+        "100 ${name}_haproxy"     => {
+          'dport' => $service_port,
+        },
+      }
+    }
+    if $public_ssl_port {
+      $haproxy_ssl_firewall_rules = {
+        "100 ${name}_haproxy_ssl" => {
+          'dport' => $public_ssl_port,
+        },
+      }
+    }
+    $firewall_rules = merge($haproxy_firewall_rules, $haproxy_ssl_firewall_rules)
+    if $service_port or $public_ssl_port {
+      create_resources('tripleo::firewall::rule', $firewall_rules)
     }
-    create_resources('tripleo::firewall::rule', $firewall_rules)
   }
 }