Re: [pve-devel] [PATCH http-server 1/3] fix #3790: allow setting TLS 1.3 cipher suites

[Stoiko Ivanov <s.ivanov@proxmox.com>]

Thanks for tackling this!
gave it a spin - works as advertised

one thing I think could be improved - is that currently nothing is logged
when CIPHERSUITES is set to an invalid setting.
(tested with "garbage TLS_CHACHA20_POLY1305_SHA256")

with TLS1.2 CIPHERS the log gets filled with:
'garbage' was not accepted as a valid cipher list by AnyEvent::TLS at /usr/share/perl5/PVE/APIServer/AnyEvent.pm line 1913.

The handling of openssl seems kinda sane[0] (fallback to the default list
if the provided one is not recognized) - so I think simply a 
syslog('warn', - might help as feedback to users
(quickly tested it - Net::SSLeay::CTX_set_ciphersuites returns '0' if it
did not set the list).



[0]
https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_set_ciphersuites.html

On Fri, 17 Dec 2021 13:57:27 +0100
Fabian Grünbichler <f.gruenbichler@proxmox.com> wrote:

> like the TLS <= 1.2 cipher list, but needs a different option since the
> format and values are incompatible. AnyEvent doesn't yet handle this
> directly like the cipher list, so set it directly on the context.
> 
> requires corresponding patch in pve-manager (which reads the config, and
> passes relevant parts back to the API server).
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
>  src/PVE/APIServer/AnyEvent.pm | 4 ++++
>  src/PVE/APIServer/Utils.pm    | 3 +++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/src/PVE/APIServer/AnyEvent.pm b/src/PVE/APIServer/AnyEvent.pm
> index f0305b3..e31cf7d 100644
> --- a/src/PVE/APIServer/AnyEvent.pm
> +++ b/src/PVE/APIServer/AnyEvent.pm
> @@ -1885,6 +1885,9 @@ sub new {
>  	    honor_cipher_order => 1,
>  	};
>  
> +	# workaround until anyevent supports TLS 1.3 ciphersuites directly
> +	my $ciphersuites = delete $self->{ssl}->{ciphersuites};
> +
>  	foreach my $k (keys %$ssl_defaults) {
>  	    $self->{ssl}->{$k} //= $ssl_defaults->{$k};
>  	}
> @@ -1904,6 +1907,7 @@ sub new {
>  
>  	$self->{tls_ctx} = AnyEvent::TLS->new(%{$self->{ssl}});
>  	Net::SSLeay::CTX_set_options($self->{tls_ctx}->{ctx}, $tls_ctx_flags);
> +	Net::SSLeay::CTX_set_ciphersuites($self->{tls_ctx}->{ctx}, $ciphersuites) if defined($ciphersuites);
>      }
>  
>      if ($self->{spiceproxy}) {
> diff --git a/src/PVE/APIServer/Utils.pm b/src/PVE/APIServer/Utils.pm
> index 449d764..0124f44 100644
> --- a/src/PVE/APIServer/Utils.pm
> +++ b/src/PVE/APIServer/Utils.pm
> @@ -19,6 +19,7 @@ sub read_proxy_config {
>      $shcmd .= 'echo \"DENY_FROM:\$DENY_FROM\";';
>      $shcmd .= 'echo \"POLICY:\$POLICY\";';
>      $shcmd .= 'echo \"CIPHERS:\$CIPHERS\";';
> +    $shcmd .= 'echo \"CIPHERSUITES:\$CIPHERSUITES\";';
>      $shcmd .= 'echo \"DHPARAMS:\$DHPARAMS\";';
>      $shcmd .= 'echo \"HONOR_CIPHER_ORDER:\$HONOR_CIPHER_ORDER\";';
>      $shcmd .= 'echo \"COMPRESSION:\$COMPRESSION\";';
> @@ -48,6 +49,8 @@ sub read_proxy_config {
>  	    $res->{$key} = $value;
>  	} elsif ($key eq 'CIPHERS') {
>  	    $res->{$key} = $value;
> +	} elsif ($key eq 'CIPHERSUITES') {
> +	    $res->{$key} = $value;
>  	} elsif ($key eq 'DHPARAMS') {
>  	    $res->{$key} = $value;
>  	} elsif ($key eq 'HONOR_CIPHER_ORDER' || $key eq 'COMPRESSION') {