[pve-devel] [PATCH firewall 3/5] Create corosync firewall rules independant of localnet
Fabian Grünbichler
f.gruenbichler at proxmox.com
Mon Jul 1 14:05:05 CEST 2019
s/independant/independently
On Mon, Jul 01, 2019 at 10:49:22AM +0200, Stefan Reiter wrote:
> "localnet" does not necessarily correspond to the correct network for
> corosync (e.g. corosync rings/link can be run independently from other PVE
> cluster service networks).
>
> This change uses the previously introduced sub 'for_all_corosync_addresses'
> to iterate through all nodes in a corosync cluster and create rules for
> all nodes and all their respective ringX_addr's it finds.
>
> The rules are created as strict as possible, there is a specific rule
> for every remote node and every ring/link. Also, communication "between"
> different links/rings is not allowed, e.g. a remote ring1_addr cannot
> contact a local ring0_addr, and vice versa.
>
> Multicast is always allowed, for backwards compatibility (as was the
> case before).
>
> Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
maybe s/create/generate/ ? creating a rule is what the admin does when
configuring the firewall, generating is what the firewall code does
given certain input/config ;)
> ---
> src/PVE/Firewall.pm | 69 +++++++++++++++++++++++++++++++++++----------
> 1 file changed, 54 insertions(+), 15 deletions(-)
>
> diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
> index 3a3bd11..ea714db 100644
> --- a/src/PVE/Firewall.pm
> +++ b/src/PVE/Firewall.pm
> @@ -14,6 +14,7 @@ use Socket qw(AF_INET6 inet_ntop inet_pton);
> use Storable qw(dclone);
>
> use PVE::Cluster;
> +use PVE::Corosync;
> use PVE::Exception qw(raise raise_param_exc);
> use PVE::INotify;
> use PVE::JSONSchema qw(register_standard_option get_standard_option);
> @@ -2395,13 +2396,27 @@ sub generate_tap_rules_direction {
> }
>
> sub enable_host_firewall {
> - my ($ruleset, $hostfw_conf, $cluster_conf, $ipversion) = @_;
> + my ($ruleset, $hostfw_conf, $cluster_conf, $ipversion, $corosync_conf) = @_;
>
> my $options = $hostfw_conf->{options};
> my $cluster_options = $cluster_conf->{options};
> my $rules = $hostfw_conf->{rules};
> my $cluster_rules = $cluster_conf->{rules};
>
> + # corosync preparation
> + my $corosync_rule = "-p udp --dport 5404:5405";
> + my $corosync_local_addresses = {};
> + my $local_hostname = PVE::INotify::nodename();
> + if (%$corosync_conf) {
nit: this is rather uncommon style, we'd usually use either
if ($corosync_conf)
or
if (defined($corosync_conf))
(depending on whether we want to check truthiness or definedness)
> + for_all_corosync_addresses($corosync_conf, sub {
> + my ($node_name, $node_ip, $node_ipversion, $key) = @_;
> +
> + if ($node_ipversion == $ipversion && $node_name eq $local_hostname) {
you check for matching $node_ipversion in all three calls - maybe it
would make sense to pass that responsibility to the iterator method?
e.g., something like this:
for_all_corosync_addresses($conf, $ipversion, sub {
}
with 0/undef meaning both versions.
> + $corosync_local_addresses->{$key} = $node_ip;
> + }
> + });
> + }
> +
> # host inbound firewall
> my $chain = "PVEFW-HOST-IN";
> ruleset_create_chain($ruleset, $chain);
> @@ -2446,14 +2461,20 @@ sub enable_host_firewall {
> ruleset_addrule($ruleset, $chain, "$mngmntsrc -p tcp --dport 3128", "-j $accept_action"); # SPICE Proxy
> ruleset_addrule($ruleset, $chain, "$mngmntsrc -p tcp --dport 22", "-j $accept_action"); # SSH
>
> - my $localnet = $cluster_conf->{aliases}->{local_network}->{cidr};
> - my $localnet_ver = $cluster_conf->{aliases}->{local_network}->{ipversion};
> + # corosync inbound rules
> + if (%$corosync_conf) {
> + # always allow multicast
> + ruleset_addrule($ruleset, $chain, "-m addrtype --dst-type MULTICAST $corosync_rule", "-j $accept_action");
this is now less strict then before, as it is missing '-s $localnet'
(for inbound), which is a bit at odds with the commit message ('same as
before')
>
> - # corosync
> - if ($localnet && ($ipversion == $localnet_ver)) {
> - my $corosync_rule = "-p udp --dport 5404:5405";
> - ruleset_addrule($ruleset, $chain, "-s $localnet -d $localnet $corosync_rule", "-j $accept_action");
> - ruleset_addrule($ruleset, $chain, "-s $localnet -m addrtype --dst-type MULTICAST $corosync_rule", "-j $accept_action");
> + for_all_corosync_addresses($corosync_conf, sub {
> + my ($node_name, $node_ip, $node_ipversion, $key) = @_;
> +
> + if ($node_ipversion == $ipversion && $node_name ne $local_hostname) {
> + my $destination = $corosync_local_addresses->{$key};
> + ruleset_addrule($ruleset, $chain, "-d $destination -s $node_ip $corosync_rule", "-j $accept_action")
> + if defined($destination); # accept only traffic on same ring
> + }
nit: I'd move this comment to its own line above the appropriate code,
and move the allocation into the condition, e.g. like this:
# accept only traffic on same ring
if (defined(my $destination = $corosync_local_addresses->{$key})) {
ruleset_addrule($ruleset, $chain, "-d $destination -s $node_ip $corosync_rule", "-j $accept_action")
}
> + });
> }
>
> # implement input policy
> @@ -2496,15 +2517,30 @@ sub enable_host_firewall {
> }
>
> # allow standard traffic on cluster network
> + my $localnet = $cluster_conf->{aliases}->{local_network}->{cidr};
> + my $localnet_ver = $cluster_conf->{aliases}->{local_network}->{ipversion};
> +
> if ($localnet && ($ipversion == $localnet_ver)) {
> ruleset_addrule($ruleset, $chain, "-d $localnet -p tcp --dport 8006", "-j $accept_action"); # PVE API
> ruleset_addrule($ruleset, $chain, "-d $localnet -p tcp --dport 22", "-j $accept_action"); # SSH
> ruleset_addrule($ruleset, $chain, "-d $localnet -p tcp --dport 5900:5999", "-j $accept_action"); # PVE VNC Console
> ruleset_addrule($ruleset, $chain, "-d $localnet -p tcp --dport 3128", "-j $accept_action"); # SPICE Proxy
> + }
>
> - my $corosync_rule = "-p udp --dport 5404:5405";
> - ruleset_addrule($ruleset, $chain, "-d $localnet $corosync_rule", "-j $accept_action");
> + # corosync outbound rules
> + if (%$corosync_conf) {
same nit as above
> + # always allow multicast
> ruleset_addrule($ruleset, $chain, "-m addrtype --dst-type MULTICAST $corosync_rule", "-j $accept_action");
> +
> + for_all_corosync_addresses($corosync_conf, sub {
> + my ($node_name, $node_ip, $node_ipversion, $key) = @_;
> +
> + if ($node_ipversion == $ipversion && $node_name ne $local_hostname) {
> + my $source = $corosync_local_addresses->{$key};
> + ruleset_addrule($ruleset, $chain, "-s $source -d $node_ip $corosync_rule", "-j $accept_action")
> + if defined($source); # accept only traffic on same ring
same nit as above
> + }
> + });
> }
>
> # implement output policy
> @@ -3476,7 +3512,7 @@ sub save_hostfw_conf {
> }
>
> sub compile {
> - my ($cluster_conf, $hostfw_conf, $vmdata) = @_;
> + my ($cluster_conf, $hostfw_conf, $vmdata, $corosync_conf) = @_;
>
> my $vmfw_configs;
>
> @@ -3497,6 +3533,9 @@ sub compile {
>
> $hostfw_conf = load_hostfw_conf($cluster_conf, undef) if !$hostfw_conf;
>
> + # cfs_update is handled by daemon or API
> + $corosync_conf = PVE::Cluster::cfs_read_file("corosync.conf") if !$corosync_conf;
> +
> $vmdata = read_local_vm_config();
> $vmfw_configs = read_vm_firewall_configs($cluster_conf, $vmdata, undef);
> }
> @@ -3516,8 +3555,8 @@ sub compile {
>
> push @{$cluster_conf->{ipset}->{management}}, { cidr => $localnet };
>
> - my $ruleset = compile_iptables_filter($cluster_conf, $hostfw_conf, $vmfw_configs, $vmdata, 4);
> - my $rulesetv6 = compile_iptables_filter($cluster_conf, $hostfw_conf, $vmfw_configs, $vmdata, 6);
> + my $ruleset = compile_iptables_filter($cluster_conf, $hostfw_conf, $vmfw_configs, $vmdata, $corosync_conf, 4);
> + my $rulesetv6 = compile_iptables_filter($cluster_conf, $hostfw_conf, $vmfw_configs, $vmdata, $corosync_conf, 6);
> my $ebtables_ruleset = compile_ebtables_filter($cluster_conf, $hostfw_conf, $vmfw_configs, $vmdata);
> my $ipset_ruleset = compile_ipsets($cluster_conf, $vmfw_configs, $vmdata);
>
> @@ -3525,7 +3564,7 @@ sub compile {
> }
>
> sub compile_iptables_filter {
> - my ($cluster_conf, $hostfw_conf, $vmfw_configs, $vmdata, $ipversion) = @_;
> + my ($cluster_conf, $hostfw_conf, $vmfw_configs, $vmdata, $corosync_conf, $ipversion) = @_;
>
> my $ruleset = {};
>
> @@ -3555,7 +3594,7 @@ sub compile_iptables_filter {
> my $hostfw_enable = !(defined($hostfw_options->{enable}) && ($hostfw_options->{enable} == 0));
>
> if ($hostfw_enable) {
> - eval { enable_host_firewall($ruleset, $hostfw_conf, $cluster_conf, $ipversion); };
> + eval { enable_host_firewall($ruleset, $hostfw_conf, $cluster_conf, $ipversion, $corosync_conf); };
> warn $@ if $@; # just to be sure - should not happen
> }
>
> --
> 2.20.1
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
More information about the pve-devel
mailing list