[pve-devel] [PATCH manager 3/3] pveproxy: add configurable COMPRESSION

[Thomas Lamprecht]

On 2/15/19 12:36 PM, Stoiko Ivanov wrote:
> disabling http compression is considered good practice and certain TLS-testing
> scripts/sites lower the security rating if it's enabled.
> 
> compression is still on by default for the potential speed/performance gain.
> 
> Signed-off-by: Stoiko Ivanov <s.ivanov at proxmox.com>
> ---
>  PVE/API2Tools.pm        | 7 ++++---
>  PVE/Service/pveproxy.pm | 1 +
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/PVE/API2Tools.pm b/PVE/API2Tools.pm
> index e275c558..710d36ea 100644
> --- a/PVE/API2Tools.pm
> +++ b/PVE/API2Tools.pm
> @@ -223,6 +223,7 @@ sub read_proxy_config {
>      $shcmd .= 'echo \"CIPHERS:\$CIPHERS\";';
>      $shcmd .= 'echo \"DHPARAMS:\$DHPARAMS\";';
>      $shcmd .= 'echo \"HONOR_CIPHER_ORDER:\$HONOR_CIPHER_ORDER\";';
> +    $shcmd .= 'echo \"COMPRESSION:\$COMPRESSION\";';
>  
>      my $data = -f $conffile ? `bash -c "$shcmd"` : '';
>  
> @@ -230,7 +231,7 @@ sub read_proxy_config {
>  
>      while ($data =~ s/^(.*)\n//) {
>  	my ($key, $value) = split(/:/, $1, 2);
> -	next if !$value;
> +	next if $value eq '';

$value can be undefined here, the match on $data does not enforces a colon
and split then returns undef for $value, so
next if !defined($value) || $value eq '';

(maybe additionally warn on such cases to notice the user?)

>  	if ($key eq 'ALLOW_FROM' || $key eq 'DENY_FROM') {
>  	    my $ips = [];
>  	    foreach my $ip (split(/,/, $value)) {
> @@ -245,8 +246,8 @@ sub read_proxy_config {
>  	    $res->{$key} = $value;
>  	} elsif ($key eq 'DHPARAMS') {
>  	    $res->{$key} = $value;
> -	} elsif ($key eq 'HONOR_CIPHER_ORDER') {
> -	    die "unknown value '$value'\n" if $value !~ m/^(0|1)$/;
> +	} elsif ($key =~ /^(HONOR_CIPHER_ORDER|COMPRESSION)$/) {

maybe just do an ($key eq 'foo' || $key eq 'bar') here?

> +	    die "unknown value '$value' - use 0 or 1\n" if $value !~ m/^(0|1)$/;

for above, see reply to 2/3

>  	    $res->{$key} = $value;
>  	} else {
>  	    # silently skip everythin else?
> diff --git a/PVE/Service/pveproxy.pm b/PVE/Service/pveproxy.pm
> index e984cb8c..7a4a804f 100755
> --- a/PVE/Service/pveproxy.pm
> +++ b/PVE/Service/pveproxy.pm
> @@ -111,6 +111,7 @@ sub init {
>  	    cert_file => '/etc/pve/local/pve-ssl.pem',
>  	    honor_cipher_order => $proxyconf->{HONOR_CIPHER_ORDER},
>  	},
> +	compression => $proxyconf->{COMPRESSION},
>  	# Note: there is no authentication for those pages and dirs!
>  	pages => {
>  	    '/' => sub { get_index($self->{nodename}, @_) },
>