]> git.proxmox.com Git - pve-firewall.git/blobdiff - src/PVE/Firewall.pm
fix #2721: remove reject tcp 43 from default drop and reject actions
[pve-firewall.git] / src / PVE / Firewall.pm
index a6157e343b3992cec41279b806f9a0a6d36e2a19..edc5336c726ce7805025dfcda86d9a0050304aa8 100644 (file)
@@ -213,7 +213,7 @@ my $pve_fw_macros = {
        { action => 'PARAM', proto => 'udp', dport => '6881' },
     ],
     'Ceph' => [
-        "Ceph Storage Cluster traffic (Ceph Monitors, OSD & MDS Deamons)",
+        "Ceph Storage Cluster traffic (Ceph Monitors, OSD & MDS Daemons)",
        # Legacy port for protocol v1
         { action => 'PARAM', proto => 'tcp', dport => '6789' },
        # New port for protocol v2
@@ -276,7 +276,7 @@ my $pve_fw_macros = {
        { action => 'PARAM', proto => 'tcp', dport => '9418' },
     ],
     'HKP' => [
-       "OpenPGP HTTP keyserver protocol traffic",
+       "OpenPGP HTTP key server protocol traffic",
        { action => 'PARAM', proto => 'tcp', dport => '11371' },
     ],
     'HTTP' => [
@@ -592,7 +592,6 @@ $pve_std_chains_conf->{4} = {
        # same as shorewall 'Drop', which is equal to DROP,
        # but REJECT/DROP some packages to reduce logging,
        # and ACCEPT critical ICMP types
-       { action => 'PVEFW-reject',  proto => 'tcp', dport => '43' }, # REJECT 'auth'
        # we are not interested in BROADCAST/MULTICAST/ANYCAST
        { action => 'PVEFW-DropBroadcast' },
        # ACCEPT critical ICMP types
@@ -615,7 +614,6 @@ $pve_std_chains_conf->{4} = {
        # same as shorewall 'Reject', which is equal to Reject,
        # but REJECT/DROP some packages to reduce logging,
        # and ACCEPT critical ICMP types
-       { action => 'PVEFW-reject',  proto => 'tcp', dport => '43' }, # REJECT 'auth'
        # we are not interested in BROADCAST/MULTICAST/ANYCAST
        { action => 'PVEFW-DropBroadcast' },
        # ACCEPT critical ICMP types
@@ -636,7 +634,7 @@ $pve_std_chains_conf->{4} = {
     ],
     'PVEFW-tcpflags' => [
        # same as shorewall tcpflags action.
-       # Packets arriving on this interface are checked for som illegal combinations of TCP flags
+       # Packets arriving on this interface are checked for some illegal combinations of TCP flags
        { match => '-p tcp -m tcp --tcp-flags FIN,SYN,RST,PSH,ACK,URG FIN,PSH,URG', target => '-g PVEFW-logflags' },
        { match => '-p tcp -m tcp --tcp-flags FIN,SYN,RST,PSH,ACK,URG NONE', target => '-g PVEFW-logflags' },
        { match => '-p tcp -m tcp --tcp-flags SYN,RST SYN,RST', target => '-g PVEFW-logflags' },
@@ -729,7 +727,7 @@ $pve_std_chains_conf->{6} = {
     ],
     'PVEFW-tcpflags' => [
        # same as shorewall tcpflags action.
-       # Packets arriving on this interface are checked for som illegal combinations of TCP flags
+       # Packets arriving on this interface are checked for some illegal combinations of TCP flags
        { match => '-p tcp -m tcp --tcp-flags FIN,SYN,RST,PSH,ACK,URG FIN,PSH,URG', target => '-g PVEFW-logflags' },
        { match => '-p tcp -m tcp --tcp-flags FIN,SYN,RST,PSH,ACK,URG NONE', target => '-g PVEFW-logflags' },
        { match => '-p tcp -m tcp --tcp-flags SYN,RST SYN,RST', target => '-g PVEFW-logflags' },
@@ -812,6 +810,17 @@ my $icmpv6_type_names = {
     'redirect' => 1,
 };
 
+my $is_valid_icmp_type = sub {
+    my ($type, $valid_types) = @_;
+
+    if ($type =~ m/^\d+$/) {
+       # values for icmp-type range between 0 and 255 (8 bit field)
+       die "invalid icmp-type '$type'\n" if $type > 255;
+    } else {
+       die "unknown icmp-type '$type'\n" if !defined($valid_types->{$type});
+    }
+};
+
 sub init_firewall_macros {
 
     $pve_fw_parsed_macros = {};
@@ -1133,6 +1142,19 @@ sub pve_fw_verify_protocol_spec {
    return $proto;
 }
 
+PVE::JSONSchema::register_format('pve-fw-icmp-type-spec', \&pve_fw_verify_icmp_type_spec);
+sub pve_fw_verify_icmp_type_spec {
+    my ($icmp_type) = @_;
+
+    if ($icmp_type_names->{$icmp_type} ||  $icmpv6_type_names->{$icmp_type}) {
+       return $icmp_type;
+    }
+
+    die "invalid icmp-type value '$icmp_type'\n" if $icmp_type ne '';
+
+    return $icmp_type;
+}
+
 
 # helper function for API
 
@@ -1232,7 +1254,7 @@ our $cluster_option_properties = {
                type => 'integer',
                minimum => 0,
                optional => 1,
-               description => 'Inital burst of packages which will get logged',
+               description => 'Initial burst of packages which will always get logged before the rate is applied',
                default => 5,
            },
        },
@@ -1425,11 +1447,13 @@ my $rule_properties = {
        description => "Restrict packet source address. $addr_list_descr",
        type => 'string', format => 'pve-fw-addr-spec',
        optional => 1,
+       maxLength => 512,
     },
     dest => {
        description => "Restrict packet destination address. $addr_list_descr",
        type => 'string', format => 'pve-fw-addr-spec',
        optional => 1,
+       maxLength => 512,
     },
     proto => {
        description => "IP protocol. You can use protocol names ('tcp'/'udp') or simple numbers, as defined in '/etc/protocols'.",
@@ -1460,6 +1484,11 @@ my $rule_properties = {
        type => 'string',
        optional => 1,
     },
+    'icmp-type' => {
+       description => "Specify icmp-type. Only valid if proto equals 'icmp'.",
+       type => 'string', format => 'pve-fw-icmp-type-spec',
+       optional => 1,
+    },
 };
 
 sub add_rule_properties {
@@ -1580,7 +1609,7 @@ sub verify_rule {
     my $set_ip_version = sub {
        my $vers = shift;
        if ($vers) {
-           die "detected mixed ipv4/ipv6 adresses in rule\n"
+           die "detected mixed ipv4/ipv6 addresses in rule\n"
                if $ipversion && ($vers != $ipversion);
            $ipversion = $vers;
        }
@@ -1625,8 +1654,6 @@ sub verify_rule {
                if !$allow_groups;
            &$add_error('action', "invalid characters in security group name")
                if $action && ($action !~ m/^${security_group_name_pattern}$/);
-           &$add_error('action', "security group '$action' does not exist")
-               if $action && !defined($cluster_conf->{groups}->{$action});
        } else {
            &$add_error('type', "unknown rule type '$type'");
        }
@@ -1655,7 +1682,8 @@ sub verify_rule {
        eval { pve_fw_verify_protocol_spec($rule->{proto}); };
        &$add_error('proto', $@) if $@;
        &$set_ip_version(4) if $rule->{proto} eq 'icmp';
-       &$set_ip_version(6) if $rule->{proto} eq 'icmpv6';
+       &$set_ip_version(6) if $rule->{proto} eq 'icmpv6';
+       &$set_ip_version(6) if $rule->{proto} eq 'ipv6-icmp';
     }
 
     if ($rule->{dport}) {
@@ -1669,6 +1697,19 @@ sub verify_rule {
                $proto ne 'icmp' && $proto ne 'icmpv6'; # special cases
     }
 
+    if (my $icmp_type = $rule ->{'icmp-type'}) {
+       my $proto = $rule->{proto};
+       &$add_error('proto', "missing property - 'icmp-type' requires this property")
+           if $proto ne 'icmp' && $proto ne 'icmpv6' && $proto ne 'ipv6-icmp';
+       &$add_error('icmp-type', "'icmp-type' cannot be specified together with 'dport'")
+           if $rule->{dport};
+       if ($proto eq 'icmp' && !$icmp_type_names->{$icmp_type}) {
+           &$add_error('icmp-type', "invalid icmp-type '$icmp_type' for proto 'icmp'");
+       } elsif (($proto eq 'icmpv6' || $proto eq 'ipv6-icmp') && !$icmpv6_type_names->{$icmp_type}) {
+           &$add_error('icmp-type', "invalid icmp-type '$icmp_type' for proto '$proto'");
+       }
+    }
+
     if ($rule->{sport}) {
        eval { parse_port_name_number_or_range($rule->{sport}, 0); };
        &$add_error('sport', $@) if $@;
@@ -1775,11 +1816,9 @@ sub rules_audit_permissions {
 }
 
 # core functions
-my $bridge_firewall_enabled = 0;
 
 sub enable_bridge_firewall {
 
-    return if $bridge_firewall_enabled; # only once
 
     PVE::ProcFSTools::write_proc_entry("/proc/sys/net/bridge/bridge-nf-call-iptables", "1");
     PVE::ProcFSTools::write_proc_entry("/proc/sys/net/bridge/bridge-nf-call-ip6tables", "1");
@@ -1787,7 +1826,6 @@ sub enable_bridge_firewall {
     # make sure syncookies are enabled (which is default on newer 3.X kernels anyways)
     PVE::ProcFSTools::write_proc_entry("/proc/sys/net/ipv4/tcp_syncookies", "1");
 
-    $bridge_firewall_enabled = 1;
 }
 
 sub iptables_restore_cmdlist {
@@ -1935,9 +1973,10 @@ sub ebtables_get_chains {
        my $line = shift;
        return if $line =~ m/^#/;
        return if $line =~ m/^\s*$/;
-       if ($line =~ m/^:(\S+)\s\S+$/) {
+       if ($line =~ m/^:(\S+)\s(ACCEPT|DROP|RETURN)$/) {
            # Make sure we know chains exist even if they're empty.
            $chains->{$1} //= [];
+           $res->{$1}->{policy} = $2;
        } elsif ($line =~ m/^(?:\S+)\s(\S+)\s(?:\S+).*/) {
            my $chain = $1;
            $line =~ s/\s+$//;
@@ -1957,7 +1996,7 @@ sub ebtables_get_chains {
     return $res;
 }
 
-# substitude action of rule according to action hash
+# substitute action of rule according to action hash
 sub rule_substitude_action {
     my ($rule, $actions) = @_;
 
@@ -2041,21 +2080,12 @@ sub ipt_rule_to_cmds {
            my $add_dport = sub {
                return if !defined($rule->{dport});
 
+               # NOTE: we re-use dport to store --icmp-type for icmp* protocol
                if ($proto eq 'icmp') {
-                   # Note: we use dport to store --icmp-type
-                   die "unknown icmp-type '$rule->{dport}'\n"
-                       if $rule->{dport} !~ /^\d+$/ && !defined($icmp_type_names->{$rule->{dport}});
-                   # values for icmp-type range between 0 and 255
-                   # higher values and iptables-restore fails
-                   die "invalid icmp-type '$rule->{dport}'\n" if ($rule->{dport} =~ m/^(\d+)$/) && ($1 > 255);
+                   $is_valid_icmp_type->($rule->{dport}, $icmp_type_names);
                    push @match, "-m icmp --icmp-type $rule->{dport}";
                } elsif ($proto eq 'icmpv6') {
-                   # Note: we use dport to store --icmpv6-type
-                   die "unknown icmpv6-type '$rule->{dport}'\n"
-                       if $rule->{dport} !~ /^\d+$/ && !defined($icmpv6_type_names->{$rule->{dport}});
-                   # values for icmpv6-type range between 0 and 255
-                   # higher values and iptables-restore fails
-                   die "invalid icmpv6-type '$rule->{dport}'\n" if ($rule->{dport} =~ m/^(\d+)$/) && ($1 > 255);
+                   $is_valid_icmp_type->($rule->{dport}, $icmpv6_type_names);
                    push @match, "-m icmpv6 --icmpv6-type $rule->{dport}";
                } elsif (!$PROTOCOLS_WITH_PORTS->{$proto}) {
                    die "protocol $proto does not have ports\n";
@@ -2079,7 +2109,18 @@ sub ipt_rule_to_cmds {
                }
            };
 
+           my $add_icmp_type = sub {
+               return if !defined($rule->{'icmp-type'}) || $rule->{'icmp-type'} eq '';
+
+               die "'icmp-type' can only be set if 'icmp', 'icmpv6' or 'ipv6-icmp' is specified\n"
+                   if ($proto ne 'icmp') && ($proto ne 'icmpv6') && ($proto ne 'ipv6-icmp');
+               my $type = $proto eq 'icmp' ? 'icmp-type' : 'icmpv6-type';
+
+               push @match, "-m $proto --$type $rule->{'icmp-type'}";
+           };
+
            # order matters - single port before multiport!
+           $add_icmp_type->();
            $add_dport->() if $multisport;
            $add_sport->();
            $add_dport->() if !$multisport;
@@ -2703,32 +2744,36 @@ sub parse_fw_rule {
 
        last if $rule->{type} eq 'group';
 
-       if ($line =~ s/^-p (\S+)\s*//) {
+       if ($line =~ s/^(?:-p|--?proto) (\S+)\s*//) {
            $rule->{proto} = $1;
            next;
        }
 
-       if ($line =~ s/^-dport (\S+)\s*//) {
+       if ($line =~ s/^--?dport (\S+)\s*//) {
            $rule->{dport} = $1;
            next;
        }
 
-       if ($line =~ s/^-sport (\S+)\s*//) {
+       if ($line =~ s/^--?sport (\S+)\s*//) {
            $rule->{sport} = $1;
            next;
        }
-       if ($line =~ s/^-source (\S+)\s*//) {
+       if ($line =~ s/^--?source (\S+)\s*//) {
            $rule->{source} = $1;
            next;
        }
-       if ($line =~ s/^-dest (\S+)\s*//) {
+       if ($line =~ s/^--?dest (\S+)\s*//) {
            $rule->{dest} = $1;
            next;
        }
-       if ($line =~ s/^-log (emerg|alert|crit|err|warning|notice|info|debug|nolog)\s*//) {
+       if ($line =~ s/^--?log (emerg|alert|crit|err|warning|notice|info|debug|nolog)\s*//) {
            $rule->{log} = $1;
            next;
        }
+       if ($line =~ s/^--?icmp-type (\S+)\s*//) {
+           $rule->{'icmp-type'} = $1;
+           next;
+       }
 
        last;
     }
@@ -3178,6 +3223,7 @@ my $format_rules = sub {
                $raw .= " -dport $rule->{dport}" if $rule->{dport};
                $raw .= " -sport $rule->{sport}" if $rule->{sport};
                $raw .= " -log $rule->{log}" if $rule->{log};
+               $raw .= " -icmp-type $rule->{'icmp-type'}" if defined($rule->{'icmp-type'}) && $rule->{'icmp-type'} ne '';
            }
 
            $raw .= " # " . encode('utf8', $rule->{comment})
@@ -3808,7 +3854,7 @@ sub compile_ipsets {
            return if !$vmfw_conf;
 
            # When the 'ipfilter' option is enabled every device for which there
-           # is no 'ipfilter-netX' ipset defiend gets an implicit empty default
+           # is no 'ipfilter-netX' ipset defined gets an implicit empty default
            # ipset.
            # The reason is that ipfilter ipsets are always filled with standard
            # IPv6 link-local filters.
@@ -3847,7 +3893,7 @@ sub compile_ipsets {
            return if !$vmfw_conf;
 
            # When the 'ipfilter' option is enabled every device for which there
-           # is no 'ipfilter-netX' ipset defiend gets an implicit empty default
+           # is no 'ipfilter-netX' ipset defined gets an implicit empty default
            # ipset.
            # The reason is that ipfilter ipsets are always filled with standard
            # IPv6 link-local filters, as well as the IP addresses configured
@@ -4063,6 +4109,7 @@ sub get_ruleset_status {
        if (defined($change_only_regex)) {
            $action = 'ignore' if ($chain !~ m/$change_only_regex/);
            $statushash->{$chain}->{rules} = $active_chains->{$chain}->{rules};
+           $statushash->{$chain}->{policy} = $active_chains->{$chain}->{policy};
            $sig = $sig->{sig};
        }
        $statushash->{$chain}->{action} = $action;
@@ -4163,7 +4210,8 @@ sub get_ebtables_cmdlist {
     my $pve_include = 0;
     foreach my $chain (sort keys %$statushash) {
        next if ($statushash->{$chain}->{action} eq 'delete');
-       $cmdlist .= ":$chain ACCEPT\n";
+       my $policy = $statushash->{$chain}->{policy} // 'ACCEPT';
+       $cmdlist .= ":$chain $policy\n";
        $pve_include = 1 if ($chain eq 'PVEFW-FORWARD');
     }