[pve-devel] [PATCH cluster 1/2] Refactor host fingerprint calculation in two extra subs

[Fabian Grünbichler]

On Wed, Nov 22, 2017 at 09:49:10AM +0100, Emmanuel Kasper wrote:
> This will be used later for feeding an extra API call
> 
> Signed-off-by: Emmanuel Kasper <e.kasper at proxmox.com>
> ---
>  data/PVE/Cluster.pm | 51 ++++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 40 insertions(+), 11 deletions(-)
> 
> diff --git a/data/PVE/Cluster.pm b/data/PVE/Cluster.pm
> index fef5842..5d69f0c 100644
> --- a/data/PVE/Cluster.pm
> +++ b/data/PVE/Cluster.pm
> @@ -1443,6 +1443,42 @@ cfs_register_file('datacenter.cfg',
>  		  \&parse_datacenter_config,
>  		  \&write_datacenter_config);
>  
> +sub get_node_ssl_cert {
> +    my ($node) = @_;
> +
> +    my $cert_path = "/etc/pve/nodes/$node/pve-ssl.pem";
> +    my $custom_cert_path = "/etc/pve/nodes/$node/pveproxy-ssl.pem";

this is missing the line overriding $cert_path with $custom_cert_path if
the latter exists, so this would break custom SSL certificates..

> +
> +    my $cert;
> +
> +    eval {
> +	my $bio = Net::SSLeay::BIO_new_file($cert_path, 'r');
> +	$cert = Net::SSLeay::PEM_read_bio_X509($bio);
> +	Net::SSLeay::BIO_free($bio);
> +    };
> +
> +    my $err = $@;
> +
> +    if ($err || !defined($cert)) {
> +	die "unable to read host SSL cert at $cert_path $err\n";

potentially undefined $err

> +    }
> +    return $cert;
> +}
> +
> +sub get_cert_fingerprint {
> +    my ($cert) = @_;
> +    my $fingerprint;
> +    eval {
> +	$fingerprint = Net::SSLeay::X509_get_fingerprint($cert, 'sha256');
> +    };
> +
> +    my $err = $@;
> +    if ($err || !defined($fingerprint) || $fingerprint eq '') {
> +	die "unable to fingerprint the SSL cert $err\n";

potentially undefined $err, and we don't know which certificate the die
refers to..

maybe we could combine get_node_ssl_cert and get_cert_fingerprint and
their error handling in update_cert_cache (the latter does the same no
matter which operation fails)?

that would make your second patch obsolete as well ;)

> +    }
> +    return $fingerprint;
> +}
> +
>  # X509 Certificate cache helper
>  
>  my $cert_cache_nodes = {};
> @@ -1470,29 +1506,22 @@ sub update_cert_cache {
>  	    }
>  	};
>  
> -	my $cert_path = "/etc/pve/nodes/$node/pve-ssl.pem";
> -	my $custom_cert_path = "/etc/pve/nodes/$node/pveproxy-ssl.pem";
> -
> -	$cert_path = $custom_cert_path if -f $custom_cert_path;
> -
>  	my $cert;
>  	eval {
> -	    my $bio = Net::SSLeay::BIO_new_file($cert_path, 'r');
> -	    $cert = Net::SSLeay::PEM_read_bio_X509($bio);
> -	    Net::SSLeay::BIO_free($bio);
> +	    $cert = get_node_ssl_cert($node);
>  	};
>  	my $err = $@;
> -	if ($err || !defined($cert)) {
> +	if ($err) {
>  	    &$clear_old() if $clear;
>  	    next;
>  	}
>  
>  	my $fp;
>  	eval {
> -	    $fp = Net::SSLeay::X509_get_fingerprint($cert, 'sha256');
> +	    $fp = get_cert_fingerprint($cert);
>  	};
>  	$err = $@;
> -	if ($err || !defined($fp) || $fp eq '') {
> +	if ($err) {
>  	    &$clear_old() if $clear;
>  	    next;
>  	}
> -- 
> 2.11.0
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel