some comments inline:
> diff --git a/PVE/HTTPServer.pm b/PVE/HTTPServer.pm
> index 1c7d033..5ddbd07 100755
> --- a/PVE/HTTPServer.pm
> +++ b/PVE/HTTPServer.pm
> @@ -105,6 +105,53 @@ sub get_login_formatter {
> return $info->{func};
> }
>
> +my $cert_cache_nodes = {};
> +my $cert_cache_fingerprints = {};
> +sub update_cert_cache {
> + my ($node) = @_;
> +
> + if (!defined($node)) {
> + for $node (keys %$cert_cache_nodes) {
> + update_cert_cache($node);
please can we avoid the recursive call here?
my $list = defined($node) ? [ $node ] : [ keys %$cert_cache_nodes ];
foreach my $node (@$list) {
# do your work here
}
> + }
> + return;
> + }
> +
> + if (my $old_fp = $cert_cache_nodes->{$node}) {
> + delete $cert_cache_fingerprints->{$old_fp};
I would do that later, after we successfully read the new data.
> + }
> +
> + 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 $bio = Net::SSLeay::BIO_new_file($cert_path, 'r');
> + my $cert = Net::SSLeay::PEM_read_bio_X509($bio);
> + Net::SSLeay::BIO_free($bio);
What if any of these methods raise exceptions?
> +
> + my $fp = Net::SSLeay::X509_get_fingerprint($cert, 'sha256');
> + return if !defined($fp);
> +
> + $cert_cache_fingerprints->{$fp} = 1;
> + $cert_cache_nodes->{$node} = $fp;
> +}
> +
> +sub check_cert_fp {
Please use a better name: check_cert_fingerprints()
> + my ($fp) = @_;
> +
> + my $check = sub {
> + for my $expected (keys %$cert_cache_fingerprints) {
> + return 1 if $fp eq $expected;
> + }
> + return 0;
> + };
> +
> + return 1 if &$check();
> +
> + # refresh cache and retry once
> + update_cert_cache();
> + return &$check();
> +}
> +
> # server implementation
>
> sub log_request {
> @@ -601,6 +648,28 @@ sub proxy_request {
> }
> }
>
> + my $tls = {
> + # TLS 1.x only, with certificate pinning
> + method => 'any',
> + sslv2 => 0,
> + sslv3 => 0,
> + verify => 1,
> + verify_cb => sub {
> + my (undef, undef, undef, $depth, undef, undef, $cert) = @_;
> + # we don't care about intermediate or root certificates
> + return 0 if $depth != 0;
> +
> + # get fingerprint of server certificate
> + my $fp = Net::SSLeay::X509_get_fingerprint($cert, 'sha256');
> + return 0 if !defined($fp); # error
> + return check_cert_fp($fp); # check against cache of pinned FPs
I would merge this code with check_cert_fp, so that we have a
single method (maybe call it 'tls_verify_callback'?
> + },
> + };
> +
> + # load and cache cert fingerprint if first time we proxy to this node
> + update_cert_cache($node)
> + if defined($node) && !defined($cert_cache_nodes->{$node});
> +
> my $w; $w = http_request(
> $method => $target,
> headers => $headers,
> @@ -609,7 +678,7 @@ sub proxy_request {
> proxy => undef, # avoid use of $ENV{HTTP_PROXY}
> keepalive => $keep_alive,
> body => $content,
> - tls_ctx => $self->{tls_ctx},
> + tls_ctx => AnyEvent::TLS->new(%{$tls}),
> sub {
> my ($body, $hdr) = @_;
>
> --
> 2.1.4
>
>
> _______________________________________________
> pve-devel mailing list
> pve-devel@pve.proxmox.com
> http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
