[pve-devel] [PATCH firewall 5/5] Add hostname resolving to corosync firewall rule generation

Fabian Grünbichler f.gruenbichler at proxmox.com
Mon Jul 1 14:05:16 CEST 2019


On Mon, Jul 01, 2019 at 10:49:24AM +0200, Stefan Reiter wrote:
> All ringX_addr entries that cannot be parsed as an IP address will be
> best-effort resolved as hostnames. This has to happen in the exact same
> way as corosync does internally, to ensure consistency with firewall
> rules.
> 
> Includes changes to the testing corosync.conf to trigger resolving
> codepaths during test runs.
> 
> Source for corosync behaviour:
> http://people.redhat.com/ccaulfie/docs/KnetCorosync.pdf
> 
> Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
> ---
>  src/PVE/Firewall.pm | 68 +++++++++++++++++++++++++++++++++++++++++++--
>  test/corosync.conf  |  4 +++
>  2 files changed, 69 insertions(+), 3 deletions(-)
> 
> diff --git a/src/PVE/Firewall.pm b/src/PVE/Firewall.pm
> index ea714db..a4cd846 100644
> --- a/src/PVE/Firewall.pm
> +++ b/src/PVE/Firewall.pm
> @@ -10,7 +10,7 @@ use File::Path;
>  use IO::File;
>  use Net::IP;
>  use POSIX;
> -use Socket qw(AF_INET6 inet_ntop inet_pton);
> +use Socket qw(AF_INET AF_INET6 inet_ntop inet_pton);
>  use Storable qw(dclone);
>  
>  use PVE::Cluster;
> @@ -23,6 +23,7 @@ use PVE::ProcFSTools;
>  use PVE::SafeSyslog;
>  use PVE::Tools qw($IPV4RE $IPV6RE);
>  use PVE::Tools qw(run_command lock_file dir_glob_foreach);
> +use PVE::Tools qw(unpack_sockaddr_in46 getaddrinfo_all);
>  
>  my $pvefw_conf_dir = "/etc/pve/firewall";
>  my $clusterfw_conf_filename = "$pvefw_conf_dir/cluster.fw";
> @@ -2564,13 +2565,74 @@ sub for_all_corosync_addresses {
>  		my $node_ip = $node_config->{$node_key};
>  		my $testip = Net::IP->new($node_ip);
>  
> -		$lambda->($node_name, $node_ip, $testip->version, $node_key)
> -		    if defined($testip);
> +		if (defined($testip)) {
> +		    $lambda->($node_name, $node_ip, $testip->version, $node_key);
> +		} else {
> +		    # invalid ip, assume hostname and try resolving (best-effort)
> +		    my $resolver_result = resolve_hostname_like_corosync($node_ip, $corosync_conf);
> +
> +		    $testip = Net::IP->new($resolver_result)
> +			if defined($resolver_result);
> +		    $lambda->($node_name, $testip->ip, $testip->version, $node_key)
> +			if defined($testip);
> +		}

if you resolve first, and early return there in case the result is an IP
address already, the whole block after 

my $node_ip = ..

could become:

my ($ip, $version) = resolve_hostname_like_corosync($node_ip, $corosync_conf);
$lambda->($node_name, $ip, $version, $node_key) if defined($ip);

or

my ($ip, $version) = resolve_hostname_like_corosync($node_ip, $corosync_conf);
next if !defined($ip);
next if $ipversion && $ipversion != $version;
$lambda->($node_name, $ip, $version, $node_key);

if the filter according to IPv4/6 gets moved here.

resolving-as-parsing is also what Corosync does btw:

https://github.com/corosync/corosync/blob/master/exec/totemip.c#L314

>  	    }
>  	}
>      }
>  }
>  
> +# NOTE: Corosync actually only resolves on startup or config change, but we
> +# currently do not have an easy way to synchronize our behaviour to that.

I think this is okay - we recommend and configure resolved addresses
only for quite a while, and users upgrading from 5 to 6.x also get
another nudge via pve5to6 to put IPs into corosync.conf

> +sub resolve_hostname_like_corosync {
> +    my ($hostname, $corosync_conf) = @_;
> +
> +    my $corosync_strategy = $corosync_conf->{main}->{totem}->{ip_version};
> +    if (defined($corosync_strategy)) {
> +	$corosync_strategy = lc $corosync_strategy;
> +    } else {
> +	$corosync_strategy = "ipv6-4"; # corosync default
> +    }
> +
> +    my $resolved_ip4;
> +    my $resolved_ip6;
> +
> +    my @resolved_raw;
> +    eval { @resolved_raw = getaddrinfo_all($hostname); };
> +    return undef if $@ || !@resolved_raw; # ignore errors in resolving
> +
> +    foreach my $socket_info (@resolved_raw) {
> +	next if !$socket_info->{addr};
> +
> +	my ($family, undef, $host) = unpack_sockaddr_in46($socket_info->{addr});
> +
> +	if ($family == AF_INET && !defined($resolved_ip4)) {
> +	    $resolved_ip4 = inet_ntop(AF_INET, $host);
> +	} elsif ($family == AF_INET6 && !defined($resolved_ip6)) {
> +	    $resolved_ip6 = inet_ntop(AF_INET6, $host);
> +	}
> +
> +	last if defined($resolved_ip4) && defined($resolved_ip6);
> +    }
> +
> +    # corosync_strategy specifies the order in which IP addresses are resolved
> +    # by corosync. We need to match that order, to ensure we create firewall
> +    # rules for the correct address family.
> +    my $resolved_ip;
> +    if ($corosync_strategy eq "ipv4") {
> +	$resolved_ip = $resolved_ip4;
> +    } elsif ($corosync_strategy eq "ipv6") {
> +	$resolved_ip = $resolved_ip6;
> +    } elsif ($corosync_strategy eq "ipv6-4") {
> +	$resolved_ip = $resolved_ip6;
> +	$resolved_ip = $resolved_ip4 if !defined($resolved_ip);

nit could be shortened to:

$resolved_ip = $resolved_ip6 // $resolved_ip4;

> +    } elsif ($corosync_strategy eq "ipv4-6") {
> +	$resolved_ip = $resolved_ip4;
> +	$resolved_ip = $resolved_ip6 if !defined($resolved_ip);

same

> +    }
> +
> +    return $resolved_ip;
> +}
> +
>  sub generate_group_rules {
>      my ($ruleset, $cluster_conf, $group, $ipversion) = @_;
>  
> diff --git a/test/corosync.conf b/test/corosync.conf
> index 3e8cb10..75385ec 100644
> --- a/test/corosync.conf
> +++ b/test/corosync.conf
> @@ -10,6 +10,7 @@ nodelist {
>      quorum_votes: 1
>      ring0_addr: 172.16.1.11
>      ring1_addr: 172.16.2.11
> +    ring2_addr: hostname1
>    }
>    node {
>      name: prox2
> @@ -17,6 +18,7 @@ nodelist {
>      quorum_votes: 1
>      ring0_addr: 172.16.1.12
>      ring1_addr: 172.16.2.12
> +    ring2_addr: hostname2
>    }
>    node {
>      name: prox3
> @@ -24,6 +26,7 @@ nodelist {
>      quorum_votes: 1
>      ring0_addr: 172.16.1.3
>      ring1_addr: 172.16.2.3
> +    ring2_addr: hostname3
>    }
>    node {
>      name: proxself
> @@ -31,6 +34,7 @@ nodelist {
>      quorum_votes: 1
>      ring0_addr: 172.16.1.2
>      ring1_addr: 172.16.2.2
> +    ring2_addr: proxself
>    }
>  }
>  
> -- 
> 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