public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH http-server/manager/pmg-api/docs 0/10] expose more TLS knobs
@ 2021-12-17 12:57 Fabian Grünbichler
  2021-12-17 12:57 ` [pve-devel] [PATCH http-server 1/3] fix #3790: allow setting TLS 1.3 cipher suites Fabian Grünbichler
                   ` (8 more replies)
  0 siblings, 9 replies; 13+ messages in thread
From: Fabian Grünbichler @ 2021-12-17 12:57 UTC (permalink / raw)
  To: pve-devel

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.




^ permalink raw reply	[flat|nested] 13+ messages in thread

* [pve-devel] [PATCH http-server 1/3] fix #3790: allow setting TLS 1.3 cipher suites
  2021-12-17 12:57 [pve-devel] [PATCH http-server/manager/pmg-api/docs 0/10] expose more TLS knobs Fabian Grünbichler
@ 2021-12-17 12:57 ` Fabian Grünbichler
  2021-12-20 17:57   ` Stoiko Ivanov
  2021-12-17 12:57 ` [pve-devel] [PATCH http-server 2/3] fix #3745: allow overriding TLS key location Fabian Grünbichler
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 13+ messages in thread
From: Fabian Grünbichler @ 2021-12-17 12:57 UTC (permalink / raw)
  To: pve-devel

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





^ permalink raw reply	[flat|nested] 13+ messages in thread

* [pve-devel] [PATCH http-server 2/3] fix #3745: allow overriding TLS key location
  2021-12-17 12:57 [pve-devel] [PATCH http-server/manager/pmg-api/docs 0/10] expose more TLS knobs Fabian Grünbichler
  2021-12-17 12:57 ` [pve-devel] [PATCH http-server 1/3] fix #3790: allow setting TLS 1.3 cipher suites Fabian Grünbichler
@ 2021-12-17 12:57 ` Fabian Grünbichler
  2021-12-17 12:57 ` [pve-devel] [PATCH http-server 3/3] fix #3789: allow disabling TLS v1.2/v1.3 Fabian Grünbichler
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Fabian Grünbichler @ 2021-12-17 12:57 UTC (permalink / raw)
  To: pve-devel

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





^ permalink raw reply	[flat|nested] 13+ messages in thread

* [pve-devel] [PATCH http-server 3/3] fix #3789: allow disabling TLS v1.2/v1.3
  2021-12-17 12:57 [pve-devel] [PATCH http-server/manager/pmg-api/docs 0/10] expose more TLS knobs Fabian Grünbichler
  2021-12-17 12:57 ` [pve-devel] [PATCH http-server 1/3] fix #3790: allow setting TLS 1.3 cipher suites Fabian Grünbichler
  2021-12-17 12:57 ` [pve-devel] [PATCH http-server 2/3] fix #3745: allow overriding TLS key location Fabian Grünbichler
@ 2021-12-17 12:57 ` Fabian Grünbichler
  2021-12-17 12:57 ` [pve-devel] [PATCH manager 1/3] fix #3790: pass TLS 1.3 ciphersuites if set Fabian Grünbichler
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Fabian Grünbichler @ 2021-12-17 12:57 UTC (permalink / raw)
  To: pve-devel

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





^ permalink raw reply	[flat|nested] 13+ messages in thread

* [pve-devel] [PATCH manager 1/3] fix #3790: pass TLS 1.3 ciphersuites if set
  2021-12-17 12:57 [pve-devel] [PATCH http-server/manager/pmg-api/docs 0/10] expose more TLS knobs Fabian Grünbichler
                   ` (2 preceding siblings ...)
  2021-12-17 12:57 ` [pve-devel] [PATCH http-server 3/3] fix #3789: allow disabling TLS v1.2/v1.3 Fabian Grünbichler
@ 2021-12-17 12:57 ` Fabian Grünbichler
  2021-12-17 12:57 ` [pve-devel] [PATCH manager 2/3] fix #3745: handle overridden TLS key location Fabian Grünbichler
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Fabian Grünbichler @ 2021-12-17 12:57 UTC (permalink / raw)
  To: pve-devel

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





^ permalink raw reply	[flat|nested] 13+ messages in thread

* [pve-devel] [PATCH manager 2/3] fix #3745: handle overridden TLS key location
  2021-12-17 12:57 [pve-devel] [PATCH http-server/manager/pmg-api/docs 0/10] expose more TLS knobs Fabian Grünbichler
                   ` (3 preceding siblings ...)
  2021-12-17 12:57 ` [pve-devel] [PATCH manager 1/3] fix #3790: pass TLS 1.3 ciphersuites if set Fabian Grünbichler
@ 2021-12-17 12:57 ` Fabian Grünbichler
  2021-12-17 12:57 ` [pve-devel] [PATCH manager 3/3] fix #3789: pass disable TLS 1.2/1.3 options Fabian Grünbichler
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Fabian Grünbichler @ 2021-12-17 12:57 UTC (permalink / raw)
  To: pve-devel

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





^ permalink raw reply	[flat|nested] 13+ messages in thread

* [pve-devel] [PATCH manager 3/3] fix #3789: pass disable TLS 1.2/1.3 options
  2021-12-17 12:57 [pve-devel] [PATCH http-server/manager/pmg-api/docs 0/10] expose more TLS knobs Fabian Grünbichler
                   ` (4 preceding siblings ...)
  2021-12-17 12:57 ` [pve-devel] [PATCH manager 2/3] fix #3745: handle overridden TLS key location Fabian Grünbichler
@ 2021-12-17 12:57 ` Fabian Grünbichler
  2021-12-17 12:57 ` [pve-devel] [PATCH docs] pveproxy: document newly added options Fabian Grünbichler
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 13+ messages in thread
From: Fabian Grünbichler @ 2021-12-17 12:57 UTC (permalink / raw)
  To: pve-devel

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





^ permalink raw reply	[flat|nested] 13+ messages in thread

* [pve-devel] [PATCH docs] pveproxy: document newly added options
  2021-12-17 12:57 [pve-devel] [PATCH http-server/manager/pmg-api/docs 0/10] expose more TLS knobs Fabian Grünbichler
                   ` (5 preceding siblings ...)
  2021-12-17 12:57 ` [pve-devel] [PATCH manager 3/3] fix #3789: pass disable TLS 1.2/1.3 options Fabian Grünbichler
@ 2021-12-17 12:57 ` Fabian Grünbichler
  2021-12-20 18:00   ` Stoiko Ivanov
  2022-01-13 16:22   ` [pve-devel] applied: " Thomas Lamprecht
  2021-12-20 18:01 ` [pve-devel] [PATCH http-server/manager/pmg-api/docs 0/10] expose more TLS knobs Stoiko Ivanov
  2022-01-13 12:36 ` [pve-devel] partially-applied-series: " Thomas Lamprecht
  8 siblings, 2 replies; 13+ messages in thread
From: Fabian Grünbichler @ 2021-12-17 12:57 UTC (permalink / raw)
  To: pve-devel

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





^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [pve-devel] [PATCH http-server 1/3] fix #3790: allow setting TLS 1.3 cipher suites
  2021-12-17 12:57 ` [pve-devel] [PATCH http-server 1/3] fix #3790: allow setting TLS 1.3 cipher suites Fabian Grünbichler
@ 2021-12-20 17:57   ` Stoiko Ivanov
  0 siblings, 0 replies; 13+ messages in thread
From: Stoiko Ivanov @ 2021-12-20 17:57 UTC (permalink / raw)
  To: Fabian Grünbichler; +Cc: Proxmox VE development discussion

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





^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [pve-devel] [PATCH docs] pveproxy: document newly added options
  2021-12-17 12:57 ` [pve-devel] [PATCH docs] pveproxy: document newly added options Fabian Grünbichler
@ 2021-12-20 18:00   ` Stoiko Ivanov
  2022-01-13 16:22   ` [pve-devel] applied: " Thomas Lamprecht
  1 sibling, 0 replies; 13+ messages in thread
From: Stoiko Ivanov @ 2021-12-20 18:00 UTC (permalink / raw)
  To: Fabian Grünbichler; +Cc: Proxmox VE development discussion

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





^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [pve-devel] [PATCH http-server/manager/pmg-api/docs 0/10] expose more TLS knobs
  2021-12-17 12:57 [pve-devel] [PATCH http-server/manager/pmg-api/docs 0/10] expose more TLS knobs Fabian Grünbichler
                   ` (6 preceding siblings ...)
  2021-12-17 12:57 ` [pve-devel] [PATCH docs] pveproxy: document newly added options Fabian Grünbichler
@ 2021-12-20 18:01 ` Stoiko Ivanov
  2022-01-13 12:36 ` [pve-devel] partially-applied-series: " Thomas Lamprecht
  8 siblings, 0 replies; 13+ messages in thread
From: Stoiko Ivanov @ 2021-12-20 18:01 UTC (permalink / raw)
  To: Fabian Grünbichler; +Cc: Proxmox VE development discussion

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
> 
> 





^ permalink raw reply	[flat|nested] 13+ messages in thread

* [pve-devel] partially-applied-series: [PATCH http-server/manager/pmg-api/docs 0/10] expose more TLS knobs
  2021-12-17 12:57 [pve-devel] [PATCH http-server/manager/pmg-api/docs 0/10] expose more TLS knobs Fabian Grünbichler
                   ` (7 preceding siblings ...)
  2021-12-20 18:01 ` [pve-devel] [PATCH http-server/manager/pmg-api/docs 0/10] expose more TLS knobs Stoiko Ivanov
@ 2022-01-13 12:36 ` Thomas Lamprecht
  8 siblings, 0 replies; 13+ messages in thread
From: Thomas Lamprecht @ 2022-01-13 12:36 UTC (permalink / raw)
  To: Proxmox VE development discussion, Fabian Grünbichler

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.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* [pve-devel] applied: [PATCH docs] pveproxy: document newly added options
  2021-12-17 12:57 ` [pve-devel] [PATCH docs] pveproxy: document newly added options Fabian Grünbichler
  2021-12-20 18:00   ` Stoiko Ivanov
@ 2022-01-13 16:22   ` Thomas Lamprecht
  1 sibling, 0 replies; 13+ messages in thread
From: Thomas Lamprecht @ 2022-01-13 16:22 UTC (permalink / raw)
  To: Proxmox VE development discussion, Fabian Grünbichler

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.




^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2022-01-13 16:22 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-17 12:57 [pve-devel] [PATCH http-server/manager/pmg-api/docs 0/10] expose more TLS knobs Fabian Grünbichler
2021-12-17 12:57 ` [pve-devel] [PATCH http-server 1/3] fix #3790: allow setting TLS 1.3 cipher suites Fabian Grünbichler
2021-12-20 17:57   ` Stoiko Ivanov
2021-12-17 12:57 ` [pve-devel] [PATCH http-server 2/3] fix #3745: allow overriding TLS key location Fabian Grünbichler
2021-12-17 12:57 ` [pve-devel] [PATCH http-server 3/3] fix #3789: allow disabling TLS v1.2/v1.3 Fabian Grünbichler
2021-12-17 12:57 ` [pve-devel] [PATCH manager 1/3] fix #3790: pass TLS 1.3 ciphersuites if set Fabian Grünbichler
2021-12-17 12:57 ` [pve-devel] [PATCH manager 2/3] fix #3745: handle overridden TLS key location Fabian Grünbichler
2021-12-17 12:57 ` [pve-devel] [PATCH manager 3/3] fix #3789: pass disable TLS 1.2/1.3 options Fabian Grünbichler
2021-12-17 12:57 ` [pve-devel] [PATCH docs] pveproxy: document newly added options Fabian Grünbichler
2021-12-20 18:00   ` Stoiko Ivanov
2022-01-13 16:22   ` [pve-devel] applied: " Thomas Lamprecht
2021-12-20 18:01 ` [pve-devel] [PATCH http-server/manager/pmg-api/docs 0/10] expose more TLS knobs Stoiko Ivanov
2022-01-13 12:36 ` [pve-devel] partially-applied-series: " Thomas Lamprecht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal