From a523e0578dfac2575074cf765994a055e0ae0fb6 Mon Sep 17 00:00:00 2001 From: Dietmar Maurer Date: Mon, 26 May 2014 12:45:41 +0200 Subject: [PATCH] improve rule verification Also verify ipset/aliases. --- src/PVE/API2/Firewall/Rules.pm | 35 +++++++++-------- src/PVE/Firewall.pm | 68 ++++++++++++++++++++++------------ 2 files changed, 64 insertions(+), 39 deletions(-) diff --git a/src/PVE/API2/Firewall/Rules.pm b/src/PVE/API2/Firewall/Rules.pm index 56b5331..fba5c10 100644 --- a/src/PVE/API2/Firewall/Rules.pm +++ b/src/PVE/API2/Firewall/Rules.pm @@ -22,7 +22,7 @@ sub load_config { die "implement this in subclass"; - #return ($fw_conf, $rules); + #return ($cluster_conf, $fw_conf, $rules); } sub save_rules { @@ -82,7 +82,7 @@ sub register_get_rules { code => sub { my ($param) = @_; - my ($fw_conf, $rules) = $class->load_config($param); + my ($cluster_conf, $fw_conf, $rules) = $class->load_config($param); my ($list, $digest) = PVE::Firewall::copy_list_with_digest($rules); @@ -122,7 +122,7 @@ sub register_get_rule { code => sub { my ($param) = @_; - my ($fw_conf, $rules) = $class->load_config($param); + my ($cluster_conf, $fw_conf, $rules) = $class->load_config($param); my ($list, $digest) = PVE::Firewall::copy_list_with_digest($rules); @@ -158,12 +158,12 @@ sub register_create_rule { code => sub { my ($param) = @_; - my ($fw_conf, $rules) = $class->load_config($param); + my ($cluster_conf, $fw_conf, $rules) = $class->load_config($param); my $rule = {}; PVE::Firewall::copy_rule_data($rule, $param); - PVE::Firewall::verify_rule($rule, $class->rule_env()); + PVE::Firewall::verify_rule($rule, $cluster_conf, $fw_conf, $class->rule_env()); $rule->{enable} = 0 if !defined($param->{enable}); @@ -211,7 +211,7 @@ sub register_update_rule { code => sub { my ($param) = @_; - my ($fw_conf, $rules) = $class->load_config($param); + my ($cluster_conf, $fw_conf, $rules) = $class->load_config($param); my (undef, $digest) = PVE::Firewall::copy_list_with_digest($rules); PVE::Tools::assert_if_modified($digest, $param->{digest}); @@ -237,7 +237,7 @@ sub register_update_rule { PVE::Firewall::delete_rule_properties($rule, $param->{'delete'}) if $param->{'delete'}; - PVE::Firewall::verify_rule($rule, $class->rule_env()); + PVE::Firewall::verify_rule($rule, $cluster_conf, $fw_conf, $class->rule_env()); } $class->save_rules($param, $fw_conf, $rules); @@ -269,7 +269,7 @@ sub register_delete_rule { code => sub { my ($param) = @_; - my ($fw_conf, $rules) = $class->load_config($param); + my ($cluster_conf, $fw_conf, $rules) = $class->load_config($param); my (undef, $digest) = PVE::Firewall::copy_list_with_digest($rules); PVE::Tools::assert_if_modified($digest, $param->{digest}); @@ -317,7 +317,7 @@ sub load_config { my $rules = $fw_conf->{groups}->{$param->{group}}; die "no such security group '$param->{group}'\n" if !defined($rules); - return ($fw_conf, $rules); + return (undef, $fw_conf, $rules); } sub save_rules { @@ -348,7 +348,7 @@ sub load_config { my $fw_conf = PVE::Firewall::load_clusterfw_conf(); my $rules = $fw_conf->{rules}; - return ($fw_conf, $rules); + return (undef, $fw_conf, $rules); } sub save_rules { @@ -379,10 +379,11 @@ sub rule_env { sub load_config { my ($class, $param) = @_; - my $fw_conf = PVE::Firewall::load_hostfw_conf(); + my $cluster_conf = PVE::Firewall::load_clusterfw_conf(); + my $fw_conf = PVE::Firewall::load_hostfw_conf($cluster_conf); my $rules = $fw_conf->{rules}; - return ($fw_conf, $rules); + return ($cluster_conf, $fw_conf, $rules); } sub save_rules { @@ -416,10 +417,11 @@ sub rule_env { sub load_config { my ($class, $param) = @_; - my $fw_conf = PVE::Firewall::load_vmfw_conf('vm', $param->{vmid}); + my $cluster_conf = PVE::Firewall::load_clusterfw_conf(); + my $fw_conf = PVE::Firewall::load_vmfw_conf($cluster_conf, 'vm', $param->{vmid}); my $rules = $fw_conf->{rules}; - return ($fw_conf, $rules); + return ($cluster_conf, $fw_conf, $rules); } sub save_rules { @@ -453,10 +455,11 @@ sub rule_env { sub load_config { my ($class, $param) = @_; - my $fw_conf = PVE::Firewall::load_vmfw_conf('ct', $param->{vmid}); + my $cluster_conf = PVE::Firewall::load_clusterfw_conf(); + my $fw_conf = PVE::Firewall::load_vmfw_conf($cluster_conf, 'ct', $param->{vmid}); my $rules = $fw_conf->{rules}; - return ($fw_conf, $rules); + return ($cluster_conf, $fw_conf, $rules); } sub save_rules { diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm index deef1ae..5f96c8a 100644 --- a/src/PVE/Firewall.pm +++ b/src/PVE/Firewall.pm @@ -1013,14 +1013,15 @@ my $rule_env_iface_lookup = { }; sub verify_rule { - my ($rule, $rule_env, $noerr) = @_; + my ($rule, $cluster_conf, $fw_conf, $rule_env, $noerr) = @_; my $allow_groups = $rule_env eq 'group' ? 0 : 1; my $allow_iface = $rule_env_iface_lookup->{$rule_env}; die "unknown rule_env '$rule_env'\n" if !defined($allow_iface); # should not happen - my $errors = {}; + my $errors = $rule->{errors} || {}; + my $error_count = 0; my $add_error = sub { @@ -1031,6 +1032,26 @@ sub verify_rule { $errors->{$param} = $msg if !$errors->{$param}; }; + my $check_ipset_or_alias_property = sub { + my ($name) = @_; + + if (my $value = $rule->{$name}) { + if ($value =~ m/^\+/) { + if ($value =~ m/^\+(${security_group_name_pattern})$/) { + &$add_error($name, "no such ipset '$1'") + if !($cluster_conf->{ipset}->{$1} || ($fw_conf && $fw_conf->{ipset}->{$1})); + + } else { + &$add_error($name, "invalid security group name '$value'"); + } + } elsif ($value =~ m/^${ip_alias_pattern}$/){ + my $alias = lc($value); + &$add_error($name, "no such alias '$value'") + if !($cluster_conf->{aliases}->{$alias} || ($fw_conf && $fw_conf->{aliases}->{$alias})) + } + } + }; + my $type = $rule->{type}; my $action = $rule->{action}; @@ -1095,14 +1116,16 @@ sub verify_rule { if ($rule->{source}) { eval { parse_address_list($rule->{source}); }; &$add_error('source', $@) if $@; + &$check_ipset_or_alias_property('source'); } if ($rule->{dest}) { eval { parse_address_list($rule->{dest}); }; &$add_error('dest', $@) if $@; + &$check_ipset_or_alias_property('dest'); } - if ($rule->{macro}) { + if ($rule->{macro} && !$error_count) { eval { &$apply_macro($rule->{macro}, $rule, 1); }; if (my $err = $@) { if (ref($err) eq "PVE::Exception" && $err->{errors}) { @@ -1875,7 +1898,7 @@ for (my $i = 0; $i < $MAX_NETS; $i++) { } sub parse_fw_rule { - my ($prefix, $line, $rule_env, $verbose) = @_; + my ($prefix, $line, $cluster_conf, $fw_conf, $rule_env, $verbose) = @_; chomp $line; @@ -1941,7 +1964,7 @@ sub parse_fw_rule { die "unable to parse rule parameters: $line\n" if length($line); - $rule = verify_rule($rule, $rule_env, 1); + $rule = verify_rule($rule, $cluster_conf, $fw_conf, $rule_env, 1); if ($verbose && $rule->{errors}) { warn "$prefix - errors in rule parameters: $orig_line\n"; foreach my $p (keys %{$rule->{errors}}) { @@ -2044,7 +2067,7 @@ sub parse_alias { } sub parse_vm_fw_rules { - my ($filename, $fh, $rule_env, $verbose) = @_; + my ($filename, $fh, $cluster_conf, $rule_env, $verbose) = @_; my $res = { rules => [], @@ -2092,7 +2115,7 @@ sub parse_vm_fw_rules { } my $rule; - eval { $rule = parse_fw_rule($prefix, $line, $rule_env, $verbose); }; + eval { $rule = parse_fw_rule($prefix, $line, $cluster_conf, $res, $rule_env, $verbose); }; if (my $err = $@) { warn "$prefix: $err"; next; @@ -2105,7 +2128,7 @@ sub parse_vm_fw_rules { } sub parse_host_fw_rules { - my ($filename, $fh, $verbose) = @_; + my ($filename, $fh, $cluster_conf, $verbose) = @_; my $res = { rules => [], options => {}}; @@ -2140,7 +2163,7 @@ sub parse_host_fw_rules { } my $rule; - eval { $rule = parse_fw_rule($prefix, $line, 'host', $verbose); }; + eval { $rule = parse_fw_rule($prefix, $line, $cluster_conf, $res, 'host', $verbose); }; if (my $err = $@) { warn "$prefix: $err"; next; @@ -2229,7 +2252,7 @@ sub parse_cluster_fw_rules { warn "$prefix: $@" if $@; } elsif ($section eq 'rules') { my $rule; - eval { $rule = parse_fw_rule($prefix, $line, 'cluster', $verbose); }; + eval { $rule = parse_fw_rule($prefix, $line, $res, undef, 'cluster', $verbose); }; if (my $err = $@) { warn "$prefix: $err"; next; @@ -2237,7 +2260,7 @@ sub parse_cluster_fw_rules { push @{$res->{$section}}, $rule; } elsif ($section eq 'groups') { my $rule; - eval { $rule = parse_fw_rule($prefix, $line, 'group', $verbose); }; + eval { $rule = parse_fw_rule($prefix, $line, $res, undef, 'group', $verbose); }; if (my $err = $@) { warn "$prefix: $err"; next; @@ -2321,7 +2344,7 @@ sub read_local_vm_config { }; sub load_vmfw_conf { - my ($rule_env, $vmid, $dir, $verbose) = @_; + my ($cluster_conf, $rule_env, $vmid, $dir, $verbose) = @_; my $vmfw_conf = {}; @@ -2329,7 +2352,7 @@ sub load_vmfw_conf { my $filename = "$dir/$vmid.fw"; if (my $fh = IO::File->new($filename, O_RDONLY)) { - $vmfw_conf = parse_vm_fw_rules($filename, $fh, $rule_env, $verbose); + $vmfw_conf = parse_vm_fw_rules($filename, $fh, $cluster_conf, $rule_env, $verbose); } return $vmfw_conf; @@ -2449,17 +2472,17 @@ sub save_vmfw_conf { } sub read_vm_firewall_configs { - my ($vmdata, $dir, $verbose) = @_; + my ($cluster_conf, $vmdata, $dir, $verbose) = @_; my $vmfw_configs = {}; foreach my $vmid (keys %{$vmdata->{qemu}}) { - my $vmfw_conf = load_vmfw_conf('vm', $vmid, $dir, $verbose); + my $vmfw_conf = load_vmfw_conf($cluster_conf, 'vm', $vmid, $dir, $verbose); next if !$vmfw_conf->{options}; # skip if file does not exists $vmfw_configs->{$vmid} = $vmfw_conf; } foreach my $vmid (keys %{$vmdata->{openvz}}) { - my $vmfw_conf = load_vmfw_conf('ct', $vmid, $dir, $verbose); + my $vmfw_conf = load_vmfw_conf($cluster_conf, 'ct', $vmid, $dir, $verbose); next if !$vmfw_conf->{options}; # skip if file does not exists $vmfw_configs->{$vmid} = $vmfw_conf; } @@ -2638,13 +2661,13 @@ sub save_clusterfw_conf { } sub load_hostfw_conf { - my ($filename, $verbose) = @_; + my ($cluster_conf, $filename, $verbose) = @_; $filename = $hostfw_conf_filename if !defined($filename); my $hostfw_conf = {}; if (my $fh = IO::File->new($filename, O_RDONLY)) { - $hostfw_conf = parse_host_fw_rules($filename, $fh, $verbose); + $hostfw_conf = parse_host_fw_rules($filename, $fh, $cluster_conf, $verbose); } return $hostfw_conf; } @@ -2678,19 +2701,18 @@ sub compile { $cluster_conf = load_clusterfw_conf($filename, $verbose); $filename = "$testdir/host.fw"; - $hostfw_conf = load_hostfw_conf($filename, $verbose); + $hostfw_conf = load_hostfw_conf($cluster_conf, $filename, $verbose); - $vmfw_configs = read_vm_firewall_configs($vmdata, $testdir, $verbose); + $vmfw_configs = read_vm_firewall_configs($cluster_conf, $vmdata, $testdir, $verbose); } else { # normal operation $cluster_conf = load_clusterfw_conf(undef, $verbose) if !$cluster_conf; - $hostfw_conf = load_hostfw_conf(undef, $verbose) if !$hostfw_conf; + $hostfw_conf = load_hostfw_conf($cluster_conf, undef, $verbose) if !$hostfw_conf; $vmdata = read_local_vm_config(); - $vmfw_configs = read_vm_firewall_configs($vmdata, undef, $verbose); + $vmfw_configs = read_vm_firewall_configs($cluster_conf, $vmdata, undef, $verbose); } - $cluster_conf->{ipset}->{venet0} = []; my $localnet; -- 2.39.2