[pbs-devel] [PATCH proxmox-backup v2 0/3] close #3612: allow config of SSL cipher-suites for proxy

[pbs-devel] [PATCH proxmox-backup v2 0/3] close #3612: allow config of SSL cipher-suites for proxy

[Hannes Laimer]

Cannot be configured in the WebUI, only through proxmox-backup-manager,
api or in the config file directly(not recommended). For changes to take
effect the proxy has to be restarted.

Since the string can be rather long and I assume most of the time the
defaults are used, it is not in the WebUI.

v2:
  - allow setting for TLSv1.3 and TLS <= 1.2 individually

Hannes Laimer (3):
  config: add cipher-suites to NodeConfig
  proxy: use ssl cipher-suites from config if set
  api2: make cipher-suites updatable

 src/api2/node/config.rs         |  8 ++++++++
 src/bin/proxmox-backup-proxy.rs | 10 ++++++++++
 src/config/node.rs              | 24 ++++++++++++++++++++++++
 3 files changed, 42 insertions(+)

-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v2 1/3] config: add cipher-suites to NodeConfig

[Hannes Laimer]

for TLS 1.3 and for TLS <= 1.2

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/config/node.rs | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/src/config/node.rs b/src/config/node.rs
index 4f2ab029..6a6038e6 100644
--- a/src/config/node.rs
+++ b/src/config/node.rs
@@ -1,5 +1,6 @@
 use std::collections::HashSet;
 
+use openssl::ssl::{SslAcceptor, SslMethod};
 use anyhow::{bail, Error};
 use serde::{Deserialize, Serialize};
 
@@ -91,6 +92,14 @@ pub struct AcmeConfig {
             schema: EMAIL_SCHEMA,
             optional: true,
         },
+        "cipher-suites-tls3": {
+            optional: true,
+            type: String,
+        },
+        "cipher-suites-tls2": {
+            optional: true,
+            type: String,
+        },
     },
 )]
 #[derive(Deserialize, Serialize, Updater)]
@@ -121,6 +130,14 @@ pub struct NodeConfig {
     
     #[serde(skip_serializing_if = "Option::is_none")]
     pub email_from: Option<String>,
+
+    /// List of SSL ciphers for tls 1.3 that will be used by the proxy. (Proxy has to be restarted for changes to take effect)
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub cipher_suites_tls3: Option<String>,
+    
+    /// List of SSL ciphers for tls <= 1.2 that will be used by the proxy. (Proxy has to be restarted for changes to take effect)
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub cipher_suites_tls2: Option<String>,
 }
 
 impl NodeConfig {
@@ -172,6 +189,13 @@ impl NodeConfig {
                 bail!("duplicate domain '{}' in ACME config", domain.domain);
             }
         }
+        let mut dummy_acceptor = SslAcceptor::mozilla_intermediate_v5(SslMethod::tls()).unwrap();
+        if let Some(cipher_suites) = self.cipher_suites_tls3.as_deref() {
+            dummy_acceptor.set_ciphersuites(cipher_suites)?;
+        }
+        if let Some(cipher_suites) = self.cipher_suites_tls2.as_deref() {
+            dummy_acceptor.set_cipher_list(cipher_suites)?;
+        }
 
         Ok(())
     }
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v2 2/3] proxy: use ssl cipher-suites from config if set

[Hannes Laimer]

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/bin/proxmox-backup-proxy.rs | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index 287cff0a..063430e4 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -343,7 +343,17 @@ fn make_tls_acceptor() -> Result<SslAcceptor, Error> {
     let key_path = configdir!("/proxy.key");
     let cert_path = configdir!("/proxy.pem");
 
+    let (config, _) = proxmox_backup::config::node::config()?;
+    let cipher_suites_tls3 = config.cipher_suites_tls3;
+    let cipher_suites_tls2 = config.cipher_suites_tls2;
+
     let mut acceptor = SslAcceptor::mozilla_intermediate_v5(SslMethod::tls()).unwrap();
+    if let Some(cipher_suites) = cipher_suites_tls3.as_deref() {
+        acceptor.set_ciphersuites(cipher_suites)?;
+    }
+    if let Some(cipher_suites) = cipher_suites_tls2.as_deref() {
+        acceptor.set_cipher_list(cipher_suites)?;
+    }
     acceptor.set_private_key_file(key_path, SslFiletype::PEM)
         .map_err(|err| format_err!("unable to read proxy key {} - {}", key_path, err))?;
     acceptor.set_certificate_chain_file(cert_path)
-- 
2.30.2

[pbs-devel] [PATCH proxmox-backup v2 3/3] api2: make cipher-suites updatable

[Hannes Laimer]

Signed-off-by: Hannes Laimer <h.laimer@proxmox.com>
---
 src/api2/node/config.rs | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/api2/node/config.rs b/src/api2/node/config.rs
index 3f060981..adb39ac2 100644
--- a/src/api2/node/config.rs
+++ b/src/api2/node/config.rs
@@ -56,6 +56,10 @@ pub enum DeletableProperty {
     http_proxy,
     /// Delete the email-from property.
     email_from,
+    /// Delete the cipher-suites-tls3 property.
+    cipher_suites_tls3,
+    /// Delete the cipher-suites-tls2 property.
+    cipher_suites_tls2,
 }
 
 #[api(
@@ -113,6 +117,8 @@ pub fn update_node_config(
                 DeletableProperty::acmedomain4 => { config.acmedomain4 = None; },
                 DeletableProperty::http_proxy => { config.http_proxy = None; },
                 DeletableProperty::email_from => { config.email_from = None; },
+                DeletableProperty::cipher_suites_tls3 => { config.cipher_suites_tls3 = None; },
+                DeletableProperty::cipher_suites_tls2 => { config.cipher_suites_tls2 = None; },
             }
         }
     }
@@ -125,6 +131,8 @@ pub fn update_node_config(
     if update.acmedomain4.is_some() { config.acmedomain4 = update.acmedomain4; }
     if update.http_proxy.is_some() { config.http_proxy = update.http_proxy; }
     if update.email_from.is_some() { config.email_from = update.email_from; }
+    if update.cipher_suites_tls3.is_some() { config.cipher_suites_tls3 = update.cipher_suites_tls3; }
+    if update.cipher_suites_tls2.is_some() { config.cipher_suites_tls2 = update.cipher_suites_tls2; }
 
     crate::config::node::save_config(&config)?;
 
-- 
2.30.2

Re: [pbs-devel] [PATCH proxmox-backup v2 0/3] close #3612: allow config of SSL cipher-suites for proxy

[Dietmar Maurer]

> Yes, but just hardcoding the list probably wont be enough since the 
> string is allowed to contain !,+,- and some other things[1]. This check 
> was mostly thought to check if the proxy would still start with the 
> given chiphers, not if the given string was valid. Also I'm not sure if 
> we should be more strict than openssl[2].

Please test what happens when you pass a string including a newline. I am quite sure we do not want or need that.

Re: [pbs-devel] [PATCH proxmox-backup v2 0/3] close #3612: allow config of SSL cipher-suites for proxy

[Hannes Laimer]

Am 05.01.22 um 10:27 schrieb Dietmar Maurer:
> 
>> But this does not throw an error:
>>
>> # proxmox-backup-manager node update --cipher-suites-tls2 asdasd,BBB,BBB.XZY,ECDHE-RSA-AES256-SHA
>>
>> Seems ssl simply ignores all unknown ciphers. The only error is when the list contains no known cipher.
> 
> I wonder if we can hardcode the list of available values and parse it correctly? Allowed values would be:
> 
> # openssl ciphers -tls1_2
> # openssl ciphers -tls1_3

Yes, but just hardcoding the list probably wont be enough since the 
string is allowed to contain !,+,- and some other things[1]. This check 
was mostly thought to check if the proxy would still start with the 
given chiphers, not if the given string was valid. Also I'm not sure if 
we should be more strict than openssl[2].

[1] https://www.openssl.org/docs/man1.1.1/man1/ciphers.html
[2] 
https://github.com/openssl/openssl/blob/master/doc/man3/SSL_CTX_set_cipher_list.pod#notes

Re: [pbs-devel] [PATCH proxmox-backup v2 0/3] close #3612: allow config of SSL cipher-suites for proxy

[Dietmar Maurer]

> But this does not throw an error:
> 
> # proxmox-backup-manager node update --cipher-suites-tls2 asdasd,BBB,BBB.XZY,ECDHE-RSA-AES256-SHA
> 
> Seems ssl simply ignores all unknown ciphers. The only error is when the list contains no known cipher.

I wonder if we can hardcode the list of available values and parse it correctly? Allowed values would be:

# openssl ciphers -tls1_2
# openssl ciphers -tls1_3

Re: [pbs-devel] [PATCH proxmox-backup v2 0/3] close #3612: allow config of SSL cipher-suites for proxy

[Dietmar Maurer]

> I can do the following without getting an error:
> 
> # proxmox-backup-manager node update --cipher-suites-tls2 asdasd,BBB,BBB.XZY
> 
> This makes no sense to me!

Need to correct myself, I get the following error:

Error: error:1410D0B9:SSL routines:SSL_CTX_set_cipher_list:no cipher match:../ssl/ssl_lib.c:2566:

But this does not throw an error:

# proxmox-backup-manager node update --cipher-suites-tls2 asdasd,BBB,BBB.XZY,ECDHE-RSA-AES256-SHA

Seems ssl simply ignores all unknown ciphers. The only error is when the list contains no known cipher.

Re: [pbs-devel] [PATCH proxmox-backup v2 0/3] close #3612: allow config of SSL cipher-suites for proxy

[Dietmar Maurer]

I can do the following without getting an error:

# proxmox-backup-manager node update --cipher-suites-tls2 asdasd,BBB,BBB.XZY

This makes no sense to me!


> On 01/04/2022 12:48 PM Hannes Laimer <h.laimer@proxmox.com> wrote:
> 
>  
> Cannot be configured in the WebUI, only through proxmox-backup-manager,
> api or in the config file directly(not recommended). For changes to take
> effect the proxy has to be restarted.
> 
> Since the string can be rather long and I assume most of the time the
> defaults are used, it is not in the WebUI.
> 
> v2:
>   - allow setting for TLSv1.3 and TLS <= 1.2 individually
> 
> Hannes Laimer (3):
>   config: add cipher-suites to NodeConfig
>   proxy: use ssl cipher-suites from config if set
>   api2: make cipher-suites updatable
> 
>  src/api2/node/config.rs         |  8 ++++++++
>  src/bin/proxmox-backup-proxy.rs | 10 ++++++++++
>  src/config/node.rs              | 24 ++++++++++++++++++++++++
>  3 files changed, 42 insertions(+)
> 
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel