[pve-devel] [PATCH http-server/manager/pmg-api/docs 0/10] expose more TLS knobs

[pve-devel] [PATCH http-server/manager/pmg-api/docs 0/10] expose more TLS knobs

[Fabian Grünbichler]

this series adds the following options to /etc/default/$proxy, and
corresponding handling in pveproxy/pmgproxy/api-server:

- TLS 1.3 ciphersuites (these are different to < 1.3 cipher lists)
- disable TLS 1.2 / disable TLS 1.3 option (rest are disabled by default
  anyway)
- alternative location for pveproxy-ssl.key outside of /etc/pve (PVE
  only)

while not strictly required, it probably makes sense to add a/bump the
versioned dep from pve-manager/pmg-api to patched
libpve-http-server-perl - nothing should break, but the new options are
only handled if both packages are updated.

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

[Fabian Grünbichler]

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') {
-- 
2.30.2

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

[Stoiko Ivanov]

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') {

[pve-devel] [PATCH http-server 2/3] fix #3745: allow overriding TLS key location

[Fabian Grünbichler]

when using a custom pveproxy certificate. actual handling is done in
pve-manager.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/PVE/APIServer/Utils.pm | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/PVE/APIServer/Utils.pm b/src/PVE/APIServer/Utils.pm
index 0124f44..2ec2dad 100644
--- a/src/PVE/APIServer/Utils.pm
+++ b/src/PVE/APIServer/Utils.pm
@@ -21,6 +21,7 @@ sub read_proxy_config {
     $shcmd .= 'echo \"CIPHERS:\$CIPHERS\";';
     $shcmd .= 'echo \"CIPHERSUITES:\$CIPHERSUITES\";';
     $shcmd .= 'echo \"DHPARAMS:\$DHPARAMS\";';
+    $shcmd .= 'echo \"TLS_KEY_FILE:\$TLS_KEY_FILE\";';
     $shcmd .= 'echo \"HONOR_CIPHER_ORDER:\$HONOR_CIPHER_ORDER\";';
     $shcmd .= 'echo \"COMPRESSION:\$COMPRESSION\";';
 
@@ -53,6 +54,8 @@ sub read_proxy_config {
 	    $res->{$key} = $value;
 	} elsif ($key eq 'DHPARAMS') {
 	    $res->{$key} = $value;
+	} elsif ($key eq 'TLS_KEY_FILE') {
+	    $res->{$key} = $value;
 	} elsif ($key eq 'HONOR_CIPHER_ORDER' || $key eq 'COMPRESSION') {
 	    die "unknown value '$value' - use 0 or 1\n" if $value !~ m/^(0|1)$/;
 	    $res->{$key} = $value;
-- 
2.30.2

[pve-devel] [PATCH http-server 3/3] fix #3789: allow disabling TLS v1.2/v1.3

[Fabian Grünbichler]

SSL 2 and 3 are already disabled by default by us, and TLS 1.1 and below
are disabled by default on Debian systems.

requires corresponding patch in pve-manager to have an effect.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/PVE/APIServer/AnyEvent.pm |  5 +++++
 src/PVE/APIServer/Utils.pm    | 11 ++++++++++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/src/PVE/APIServer/AnyEvent.pm b/src/PVE/APIServer/AnyEvent.pm
index e31cf7d..e0c3eee 100644
--- a/src/PVE/APIServer/AnyEvent.pm
+++ b/src/PVE/APIServer/AnyEvent.pm
@@ -1904,6 +1904,11 @@ sub new {
 	if (delete $self->{ssl}->{honor_cipher_order}) {
 	    $tls_ctx_flags |= &Net::SSLeay::OP_CIPHER_SERVER_PREFERENCE;
 	}
+	# workaround until anyevent supports disabling TLS 1.3 directly
+	if (exists($self->{ssl}->{tlsv1_3}) && !$self->{ssl}->{tlsv1_3}) {
+	    $tls_ctx_flags |= &Net::SSLeay::OP_NO_TLSv1_3;
+	}
+
 
 	$self->{tls_ctx} = AnyEvent::TLS->new(%{$self->{ssl}});
 	Net::SSLeay::CTX_set_options($self->{tls_ctx}->{ctx}, $tls_ctx_flags);
diff --git a/src/PVE/APIServer/Utils.pm b/src/PVE/APIServer/Utils.pm
index 2ec2dad..5728d97 100644
--- a/src/PVE/APIServer/Utils.pm
+++ b/src/PVE/APIServer/Utils.pm
@@ -24,11 +24,20 @@ sub read_proxy_config {
     $shcmd .= 'echo \"TLS_KEY_FILE:\$TLS_KEY_FILE\";';
     $shcmd .= 'echo \"HONOR_CIPHER_ORDER:\$HONOR_CIPHER_ORDER\";';
     $shcmd .= 'echo \"COMPRESSION:\$COMPRESSION\";';
+    $shcmd .= 'echo \"DISABLE_TLS_1_2:\$DISABLE_TLS_1_2\";';
+    $shcmd .= 'echo \"DISABLE_TLS_1_3:\$DISABLE_TLS_1_3\";';
 
     my $data = -f $conffile ? `bash -c "$shcmd"` : '';
 
     my $res = {};
 
+    my $boolean_options = [
+	'HONOR_CIPHER_ORDER',
+	'COMPRESSION',
+	'DISABLE_TLS_1_2',
+	'DISABLE_TLS_1_3',
+    ];
+
     while ($data =~ s/^(.*)\n//) {
 	my ($key, $value) = split(/:/, $1, 2);
 	next if !defined($value) || $value eq '';
@@ -56,7 +65,7 @@ sub read_proxy_config {
 	    $res->{$key} = $value;
 	} elsif ($key eq 'TLS_KEY_FILE') {
 	    $res->{$key} = $value;
-	} elsif ($key eq 'HONOR_CIPHER_ORDER' || $key eq 'COMPRESSION') {
+	} elsif (grep { $key eq $_ } @$boolean_options) {
 	    die "unknown value '$value' - use 0 or 1\n" if $value !~ m/^(0|1)$/;
 	    $res->{$key} = $value;
 	} else {
-- 
2.30.2

[pve-devel] [PATCH manager 1/3] fix #3790: pass TLS 1.3 ciphersuites if set

[Fabian Grünbichler]

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 PVE/Service/pveproxy.pm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/PVE/Service/pveproxy.pm b/PVE/Service/pveproxy.pm
index 31493dbf..61424d77 100755
--- a/PVE/Service/pveproxy.pm
+++ b/PVE/Service/pveproxy.pm
@@ -105,6 +105,7 @@ sub init {
 	policy => $proxyconf->{POLICY},
 	ssl => {
 	    cipher_list => $proxyconf->{CIPHERS},
+	    ciphersuites => $proxyconf->{CIPHERSUITES},
 	    key_file => '/etc/pve/local/pve-ssl.key',
 	    cert_file => '/etc/pve/local/pve-ssl.pem',
 	    honor_cipher_order => $proxyconf->{HONOR_CIPHER_ORDER},
-- 
2.30.2

[pve-devel] [PATCH manager 2/3] fix #3745: handle overridden TLS key location

[Fabian Grünbichler]

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 PVE/Service/pveproxy.pm | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/PVE/Service/pveproxy.pm b/PVE/Service/pveproxy.pm
index 61424d77..b746ebf1 100755
--- a/PVE/Service/pveproxy.pm
+++ b/PVE/Service/pveproxy.pm
@@ -131,9 +131,13 @@ sub init {
     if (defined($proxyconf->{DHPARAMS})) {
 	$self->{server_config}->{ssl}->{dh_file} = $proxyconf->{DHPARAMS};
     }
-    if (-f '/etc/pve/local/pveproxy-ssl.pem' && -f '/etc/pve/local/pveproxy-ssl.key') {
+    my $custom_key_path = '/etc/pve/local/pveproxy-ssl.key';
+    if (defined($proxyconf->{TLS_KEY_FILE})) {
+	$custom_key_path = $proxyconf->{TLS_KEY_FILE};
+    }
+    if (-f '/etc/pve/local/pveproxy-ssl.pem' && -f $custom_key_path) {
 	$self->{server_config}->{ssl}->{cert_file} = '/etc/pve/local/pveproxy-ssl.pem';
-	$self->{server_config}->{ssl}->{key_file} = '/etc/pve/local/pveproxy-ssl.key';
+	$self->{server_config}->{ssl}->{key_file} = $custom_key_path;
 	syslog('info', 'Using \'/etc/pve/local/pveproxy-ssl.pem\' as certificate for the web interface.');
     }
 }
-- 
2.30.2

[pve-devel] [PATCH manager 3/3] fix #3789: pass disable TLS 1.2/1.3 options

[Fabian Grünbichler]

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 PVE/Service/pveproxy.pm | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/PVE/Service/pveproxy.pm b/PVE/Service/pveproxy.pm
index b746ebf1..f73fdd6f 100755
--- a/PVE/Service/pveproxy.pm
+++ b/PVE/Service/pveproxy.pm
@@ -131,6 +131,12 @@ sub init {
     if (defined($proxyconf->{DHPARAMS})) {
 	$self->{server_config}->{ssl}->{dh_file} = $proxyconf->{DHPARAMS};
     }
+    if (defined($proxyconf->{DISABLE_TLS_1_2})) {
+	$self->{server_config}->{ssl}->{tlsv1_2} = !$proxyconf->{DISABLE_TLS_1_2};
+    }
+    if (defined($proxyconf->{DISABLE_TLS_1_3})) {
+	$self->{server_config}->{ssl}->{tlsv1_3} = !$proxyconf->{DISABLE_TLS_1_3};
+    }
     my $custom_key_path = '/etc/pve/local/pveproxy-ssl.key';
     if (defined($proxyconf->{TLS_KEY_FILE})) {
 	$custom_key_path = $proxyconf->{TLS_KEY_FILE};
-- 
2.30.2

[pve-devel] [PATCH docs] pveproxy: document newly added options

[Fabian Grünbichler]

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 pveproxy.adoc | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/pveproxy.adoc b/pveproxy.adoc
index 4696d66..8fc6195 100644
--- a/pveproxy.adoc
+++ b/pveproxy.adoc
@@ -117,9 +117,11 @@ effect.
 SSL Cipher Suite
 ----------------
 
-You can define the cipher list in `/etc/default/pveproxy`, for example
+You can define the cipher list in `/etc/default/pveproxy` via the `CIPHERS`
+(TLS <= 1.2) and `CIPHERSUITES` (TLS >= 1.3) keys. For example
 
  CIPHERS="ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256"
+ CIPHERSUITES="TLS_AES_256_GCM_SHA384:TLS_CHACHA20_POLY1305_SHA256:TLS_AES_128_GCM_SHA256"
 
 Above is the default. See the ciphers(1) man page from the openssl
 package for a list of all available options.
@@ -131,6 +133,25 @@ both client and `pveproxy`):
  HONOR_CIPHER_ORDER=0
 
 
+Supported TLS versions
+----------------------
+
+The insecure SSL versions 2 and 3 are unconditionally disabled for pveproxy.
+TLS versions below 1.1 are disabled by default on recent OpenSSL versions,
+which is honored by `pveproxy` (see `/etc/ssl/openssl.cnf`).
+
+To disable TLS version 1.2 or 1.3, set the following in `/etc/default/pveproxy`:
+
+ DISABLE_TLS_1_2=1
+
+or, respectively:
+
+ DISABLE_TLS_1_3=1
+
+NOTE: Unless there is a specific reason to do so, it is not recommended to
+manually adjust the supported TLS versions.
+
+
 Diffie-Hellman Parameters
 -------------------------
 
@@ -157,6 +178,13 @@ pveproxy uses `/etc/pve/local/pveproxy-ssl.pem` and
 `/etc/pve/local/pve-ssl.pem` and `/etc/pve/local/pve-ssl.key`.
 The private key may not use a passphrase.
 
+It is possible to override the location of the certificate private key by
+setting `TLS_KEY_FILE` in `/etc/default/pveproxy`, for example:
+
+ TLS_KEY_FILE="/secrets/pveproxy.key"
+
+NOTE: The included ACME integration does not honor this setting.
+
 See the Host System Administration chapter of the documentation for details.
 
 COMPRESSION
-- 
2.30.2

Re: [pve-devel] [PATCH docs] pveproxy: document newly added options

[Stoiko Ivanov]

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

> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
>  pveproxy.adoc | 30 +++++++++++++++++++++++++++++-
>  1 file changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/pveproxy.adoc b/pveproxy.adoc
> index 4696d66..8fc6195 100644
> --- a/pveproxy.adoc
> +++ b/pveproxy.adoc
> @@ -117,9 +117,11 @@ effect.
>  SSL Cipher Suite
>  ----------------
>  
> -You can define the cipher list in `/etc/default/pveproxy`, for example
> +You can define the cipher list in `/etc/default/pveproxy` via the `CIPHERS`
> +(TLS <= 1.2) and `CIPHERSUITES` (TLS >= 1.3) keys. For example
>  
>   CIPHERS="ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256"
> + CIPHERSUITES="TLS_AES_256_GCM_SHA384:TLS_CHACHA20_POLY1305_SHA256:TLS_AES_128_GCM_SHA256"
>  
>  Above is the default. See the ciphers(1) man page from the openssl
>  package for a list of all available options.
> @@ -131,6 +133,25 @@ both client and `pveproxy`):
>   HONOR_CIPHER_ORDER=0
>  
>  
> +Supported TLS versions
> +----------------------
> +
> +The insecure SSL versions 2 and 3 are unconditionally disabled for pveproxy.
> +TLS versions below 1.1 are disabled by default on recent OpenSSL versions,
> +which is honored by `pveproxy` (see `/etc/ssl/openssl.cnf`).
> +
> +To disable TLS version 1.2 or 1.3, set the following in `/etc/default/pveproxy`:
> +
> + DISABLE_TLS_1_2=1
> +
> +or, respectively:
> +
> + DISABLE_TLS_1_3=1
> +
> +NOTE: Unless there is a specific reason to do so, it is not recommended to
> +manually adjust the supported TLS versions.
> +
> +
>  Diffie-Hellman Parameters
>  -------------------------
>  
> @@ -157,6 +178,13 @@ pveproxy uses `/etc/pve/local/pveproxy-ssl.pem` and
>  `/etc/pve/local/pve-ssl.pem` and `/etc/pve/local/pve-ssl.key`.
>  The private key may not use a passphrase.
>  
> +It is possible to override the location of the certificate private key by
maybe add a `(/etc/pve/local/pveproxy-ssl.key)`  here to avoid confusion
afaicu - overriding works only for pveproxy-ssl.pem

> +setting `TLS_KEY_FILE` in `/etc/default/pveproxy`, for example:
> +
> + TLS_KEY_FILE="/secrets/pveproxy.key"
> +
> +NOTE: The included ACME integration does not honor this setting.
> +
>  See the Host System Administration chapter of the documentation for details.
>  
>  COMPRESSION

[pve-devel] applied: [PATCH docs] pveproxy: document newly added options

[Thomas Lamprecht]

On 17.12.21 13:57, Fabian Grünbichler wrote:
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
>  pveproxy.adoc | 30 +++++++++++++++++++++++++++++-
>  1 file changed, 29 insertions(+), 1 deletion(-)
> 
>

applied this and the manager part, thanks! I did not address any of the nits mentioned by
stoiko (thx for the review), so we'd need follow ups for those.

[pmg-devel] [PATCH pmg-api 1/2] pass TLS 1.3 ciphersuites if set

[Fabian Grünbichler]

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
requires http-server patches
https://lists.proxmox.com/pipermail/pve-devel/2021-December/051227.html

 src/PMG/Service/pmgproxy.pm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/PMG/Service/pmgproxy.pm b/src/PMG/Service/pmgproxy.pm
index 0efde23..cde6d3c 100755
--- a/src/PMG/Service/pmgproxy.pm
+++ b/src/PMG/Service/pmgproxy.pm
@@ -104,6 +104,7 @@ sub init {
 	    cert_file => '/etc/pmg/pmg-api.pem',
 	    dh => 'skip2048',
 	    cipher_list => $proxyconf->{CIPHERS},
+	    ciphersuites => $proxyconf->{CIPHERSUITES},
 	    honor_cipher_order => $proxyconf->{HONOR_CIPHER_ORDER},
 	},
 	compression => $proxyconf->{COMPRESSION},
-- 
2.30.2

[pmg-devel] [PATCH pmg-api 2/2] pass disable TLS 1.2/1.3 options

[Fabian Grünbichler]

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
requires http-server patches
https://lists.proxmox.com/pipermail/pve-devel/2021-December/051227.html

 src/PMG/Service/pmgproxy.pm | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/PMG/Service/pmgproxy.pm b/src/PMG/Service/pmgproxy.pm
index cde6d3c..5334e6f 100755
--- a/src/PMG/Service/pmgproxy.pm
+++ b/src/PMG/Service/pmgproxy.pm
@@ -129,6 +129,12 @@ sub init {
     if (defined($proxyconf->{DHPARAMS})) {
 	$self->{server_config}->{ssl}->{dh_file} = $proxyconf->{DHPARAMS};
     }
+    if (defined($proxyconf->{DISABLE_TLS_1_2})) {
+	$self->{server_config}->{ssl}->{tlsv1_2} = !$proxyconf->{DISABLE_TLS_1_2};
+    }
+    if (defined($proxyconf->{DISABLE_TLS_1_3})) {
+	$self->{server_config}->{ssl}->{tlsv1_3} = !$proxyconf->{DISABLE_TLS_1_3};
+    }
 }
 
 sub run {
-- 
2.30.2

[pmg-devel] applied: [PATCH pmg-api 2/2] pass disable TLS 1.2/1.3 options

[Thomas Lamprecht]

On 17.12.21 14:00, Fabian Grünbichler wrote:
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> requires http-server patches
> https://lists.proxmox.com/pipermail/pve-devel/2021-December/051227.html
> 
>  src/PMG/Service/pmgproxy.pm | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
>

applied, thanks!

[pmg-devel] [PATCH pmg-docs] pmgproxy: document newly added options

[Fabian Grünbichler]

adapted from pve-docs -> pveproxy

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 pmgproxy.adoc | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/pmgproxy.adoc b/pmgproxy.adoc
index 6e48fba..101f269 100644
--- a/pmgproxy.adoc
+++ b/pmgproxy.adoc
@@ -125,7 +125,10 @@ should set a maintenance window to bring this change into effect.
 SSL Cipher Suite
 ----------------
 
-You can define the cipher list in `/etc/default/pmgproxy`, for example:
+You can define the cipher list in `/etc/default/pmgproxy`, via the `CIPHERS`
+(TLS <= 1.2) and `CIPHERSUITES` (TLS >= 1.3) keys.
+
+For example:
 
  CIPHERS="ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256"
 
@@ -141,6 +144,25 @@ by disabling the HONOR_CIPHER_ORDER option in `/etc/default/pmgproxy`:
  HONOR_CIPHER_ORDER=0
 
 
+Supported TLS versions
+----------------------
+
+The insecure SSL versions 2 and 3 are unconditionally disabled for `pmgproxy`.
+TLS versions below 1.1 are disabled by default on recent OpenSSL versions,
+which is honored by `pmgproxy` (see `/etc/ssl/openssl.cnf`).
+
+To disable TLS version 1.2, set the following in `/etc/default/pmgproxy`:
+
+ DISABLE_TLS_1_2=1
+
+or, respectively, to disable TLS version 1.3:
+
+ DISABLE_TLS_1_3=1
+
+NOTE: Unless there is a specific reason to do so, it is not recommended to
+manually adjust the supported TLS versions.
+
+
 Diffie-Hellman Parameters
 -------------------------
 
-- 
2.30.2

[pmg-devel] applied: [PATCH pmg-docs] pmgproxy: document newly added options

[Thomas Lamprecht]

On 17.12.21 14:00, Fabian Grünbichler wrote:
> adapted from pve-docs -> pveproxy
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
>  pmgproxy.adoc | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
>

applied, thanks!

[pmg-devel] applied: [PATCH pmg-api 1/2] pass TLS 1.3 ciphersuites if set

[Thomas Lamprecht]

On 17.12.21 14:00, Fabian Grünbichler wrote:
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
> requires http-server patches
> https://lists.proxmox.com/pipermail/pve-devel/2021-December/051227.html
> 
>  src/PMG/Service/pmgproxy.pm | 1 +
>  1 file changed, 1 insertion(+)
> 
>

applied, thanks!

Re: [pve-devel] [PATCH http-server/manager/pmg-api/docs 0/10] expose more TLS knobs

[Stoiko Ivanov]

Thanks for the series!

tried each of the option (and verified with `sslscan localhost:8006`)

2 minor cosmetic nits (mentioned as replies to the individual patches)

with and without them LGTM:
Tested-by: Stoiko Ivanov <s.ivanov@proxmox.com>
Reviewed-by: Stoiko Ivanov <s.ivanov@proxmox.com>

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

> this series adds the following options to /etc/default/$proxy, and
> corresponding handling in pveproxy/pmgproxy/api-server:
> 
> - TLS 1.3 ciphersuites (these are different to < 1.3 cipher lists)
> - disable TLS 1.2 / disable TLS 1.3 option (rest are disabled by default
>   anyway)
> - alternative location for pveproxy-ssl.key outside of /etc/pve (PVE
>   only)
> 
> while not strictly required, it probably makes sense to add a/bump the
> versioned dep from pve-manager/pmg-api to patched
> libpve-http-server-perl - nothing should break, but the new options are
> only handled if both packages are updated.
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 

[pve-devel] partially-applied-series: [PATCH http-server/manager/pmg-api/docs 0/10] expose more TLS knobs

[Thomas Lamprecht]

On 17.12.21 13:57, Fabian Grünbichler wrote:
> this series adds the following options to /etc/default/$proxy, and
> corresponding handling in pveproxy/pmgproxy/api-server:
> 
> - TLS 1.3 ciphersuites (these are different to < 1.3 cipher lists)
> - disable TLS 1.2 / disable TLS 1.3 option (rest are disabled by default
>   anyway)
> - alternative location for pveproxy-ssl.key outside of /etc/pve (PVE
>   only)
> 
> while not strictly required, it probably makes sense to add a/bump the
> versioned dep from pve-manager/pmg-api to patched
> libpve-http-server-perl - nothing should break, but the new options are
> only handled if both packages are updated.
> 

applied the http-server part for now.