]> 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 4a534d06f9a2fc02d406403f9d109e786d598e96..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' => [
@@ -394,6 +394,10 @@ my $pve_fw_macros = {
        { action => 'PARAM', proto => 'udp', dport => '5632' },
        { action => 'PARAM', proto => 'tcp', dport => '5631' },
     ],
+    'PMG' => [
+       "Proxmox Mail Gateway web interface",
+       { action => 'PARAM', proto => 'tcp', dport => '8006' },
+    ],
     'POP3' => [
        "POP3 traffic",
        { action => 'PARAM', proto => 'tcp', dport => '110' },
@@ -588,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
@@ -611,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
@@ -632,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' },
@@ -725,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' },
@@ -781,12 +783,14 @@ my $icmp_type_names = {
 # ip6tables -p icmpv6 -h
 
 my $icmpv6_type_names = {
-    'any' => 1,
     'destination-unreachable' => 1,
     'no-route' => 1,
     'communication-prohibited' => 1,
+    'beyond-scope' => 1,
     'address-unreachable' => 1,
     'port-unreachable' => 1,
+    'failed-policy' => 1,
+    'reject-route' => 1,
     'packet-too-big' => 1,
     'time-exceeded' => 1,
     'ttl-zero-during-transit' => 1,
@@ -806,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 = {};
@@ -1076,7 +1091,7 @@ sub parse_port_name_number_or_range {
        }
     }
 
-    die "ICPM ports not allowed in port range\n" if $icmp_port && $count > 0;
+    die "ICMP ports not allowed in port range\n" if $icmp_port && $count > 0;
 
     # I really don't like to use the word number here, but it's the only thing
     # that makes sense in a literal way. The range 1:100 counts as 2, not as
@@ -1127,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
 
@@ -1226,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,
            },
        },
@@ -1419,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'.",
@@ -1454,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 {
@@ -1574,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;
        }
@@ -1647,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}) {
@@ -1661,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 $@;
@@ -1767,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");
@@ -1779,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 {
@@ -1927,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+$//;
@@ -1949,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) = @_;
 
@@ -2031,23 +2078,21 @@ sub ipt_rule_to_cmds {
            my $multisport = defined($rule->{sport}) && parse_port_name_number_or_range($rule->{sport}, 0);
 
            my $add_dport = sub {
-               return if !$rule->{dport};
+               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}});
+                   $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}});
+                   $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";
                } elsif ($multidport) {
                    push @match, "--match multiport", "--dports $rule->{dport}";
                } else {
+                   return if !$rule->{dport};
                    push @match, "--dport $rule->{dport}";
                }
            };
@@ -2064,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;
@@ -2487,6 +2543,7 @@ sub enable_host_firewall {
        $rule->{iface_in} = $rule->{iface} if $rule->{iface};
 
        eval {
+           $rule->{logmsg} = "$rule->{action}: ";
            if ($rule->{type} eq 'group') {
                ruleset_add_group_rule($ruleset, $cluster_conf, $chain, $rule, 'IN', $accept_action, $ipversion);
            } elsif ($rule->{type} eq 'in') {
@@ -2687,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;
     }
@@ -2892,6 +2953,8 @@ sub generic_fw_config_parser {
     }
     return {} if !$raw;
 
+    my $curr_group_keys = {};
+
     my $linenr = 0;
     while ($raw =~ /^\h*(.*?)\h*$/gm) {
        my $line = $1;
@@ -2952,6 +3015,8 @@ sub generic_fw_config_parser {
            }
 
            $res->{$section}->{$group} = [];
+           $curr_group_keys = {};
+
            $res->{ipset_comments}->{$group} = decode('utf8', $comment)
                if $comment;
            next;
@@ -3016,6 +3081,8 @@ sub generic_fw_config_parser {
                } else {
                    $cidr = parse_ip_or_cidr($cidr);
                }
+               die "duplicate ipset entry for '$cidr'\n"
+                   if defined($curr_group_keys->{$cidr});
            };
            if (my $err = $@) {
                chomp $err;
@@ -3039,6 +3106,7 @@ sub generic_fw_config_parser {
            }
 
            push @{$res->{$section}->{$group}}, $entry;
+           $curr_group_keys->{$cidr} = 1;
        } else {
            warn "$prefix: skip line - unknown section\n";
            next;
@@ -3048,6 +3116,8 @@ sub generic_fw_config_parser {
     return $res;
 }
 
+# this is only used to prevent concurrent runs of rule compilation/application
+# see lock_*_conf for cfs locks protectiong config modification
 sub run_locked {
     my ($code, @param) = @_;
 
@@ -3096,6 +3166,18 @@ sub read_local_vm_config {
     return $vmdata;
 };
 
+sub lock_vmfw_conf {
+    my ($vmid, $timeout, $code, @param) = @_;
+
+    die "can't lock VM firewall config for undefined VMID\n"
+       if !defined($vmid);
+
+    my $res = PVE::Cluster::cfs_lock_firewall("vm-$vmid", $timeout, $code, @param);
+    die $@ if $@;
+
+    return $res;
+}
+
 sub load_vmfw_conf {
     my ($cluster_conf, $rule_env, $vmid, $dir) = @_;
 
@@ -3141,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})
@@ -3202,7 +3285,13 @@ my $format_ipsets = sub {
 
        my $nethash = {};
        foreach my $entry (@$options) {
-           $nethash->{$entry->{cidr}} = $entry;
+           my $cidr = $entry->{cidr};
+           if (defined($nethash->{$cidr})) {
+               warn "ignoring duplicate ipset entry '$cidr'\n";
+               next;
+           }
+
+           $nethash->{$cidr} = $entry;
        }
 
        foreach my $cidr (sort keys %$nethash) {
@@ -3263,13 +3352,15 @@ sub clone_vmfw_conf {
     my $sourcevm_conffile = "$pvefw_conf_dir/$vmid.fw";
     my $clonevm_conffile = "$pvefw_conf_dir/$newid.fw";
 
-    if (-f $clonevm_conffile) {
-       unlink $clonevm_conffile;
-    }
-    if (-f $sourcevm_conffile) {
-       my $data = PVE::Tools::file_get_contents($sourcevm_conffile);
-       PVE::Tools::file_set_contents($clonevm_conffile, $data);
-    }
+    lock_vmfw_conf($newid, 10, sub {
+       if (-f $clonevm_conffile) {
+           unlink $clonevm_conffile;
+       }
+       if (-f $sourcevm_conffile) {
+           my $data = PVE::Tools::file_get_contents($sourcevm_conffile);
+           PVE::Tools::file_set_contents($clonevm_conffile, $data);
+       }
+    });
 }
 
 sub read_vm_firewall_configs {
@@ -3443,6 +3534,15 @@ my $set_global_log_ratelimit = sub {
     }
 };
 
+sub lock_clusterfw_conf {
+    my ($timeout, $code, @param) = @_;
+
+    my $res = PVE::Cluster::cfs_lock_firewall("cluster", $timeout, $code, @param);
+    die $@ if $@;
+
+    return $res;
+}
+
 sub load_clusterfw_conf {
     my ($filename) = @_;
 
@@ -3506,6 +3606,15 @@ sub save_clusterfw_conf {
     }
 }
 
+sub lock_hostfw_conf {
+    my ($timeout, $code, @param) = @_;
+
+    my $res = PVE::Cluster::cfs_lock_firewall("host-$nodename", $timeout, $code, @param);
+    die $@ if $@;
+
+    return $res;
+}
+
 sub load_hostfw_conf {
     my ($cluster_conf, $filename) = @_;
 
@@ -3745,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.
@@ -3784,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
@@ -3899,7 +4008,9 @@ sub compile_ebtables_filter {
                    # ebtables changes this to a .0/MASK network but we just
                    # want the address here, no network - see #2193
                    $ip =~ s|/(\d+)$||;
-                   push @$arpfilter, $ip;
+                   if ($ip ne 'dhcp') {
+                       push @$arpfilter, $ip;
+                   }
                }
                generate_tap_layer2filter($ruleset, $iface, $macaddr, $vmfw_conf, $vmid, $arpfilter);
            }
@@ -3998,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;
@@ -4098,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');
     }