[PATCH backup v2 0/3] fix #7054: client: remove trailing newlines from credentials

[PATCH backup v2 0/3] fix #7054: client: remove trailing newlines from credentials

[Maximiliano Sandoval]

See the first commit for more details.

This was tested with proxmox-backup-client making a login/backup using different
credentials with and without newlines. The commands were similar to the
systemd-run commands at the Backup Client Usage docs.

I did not find a way to create the keyfile with newlines in its password using
proxmox-backup-client since it reads from the tty stdin, but surely it could be
created manually. Perhaps it is safe-ish to also remove trailing control
characters from the encryption password but this seems a safer approach for now.

Diferences from v1:
 - Always do an extra allocation to keep the code clean
 - Rename password to blob
 - Only strip newlines on passwords

Maximiliano Sandoval (3):
  fix #7054: client: remove trailing newlines from credentials
  docs: client: document further password constrains
  client: rename password to blob

 docs/backup-client.rst      |  7 ++++---
 pbs-client/src/tools/mod.rs | 14 ++++++++++++--
 2 files changed, 16 insertions(+), 5 deletions(-)

-- 
2.47.3

[PATCH backup v2 1/3] fix #7054: client: remove trailing newlines from credentials

[Maximiliano Sandoval]

For repositories and fingerprints we simply strip trailing whitespaces.
For passwords, we refer to the password regex at proxmox-schema:
`^[[:^cntrl:]]*$`, we can only strip trailing control characters without
potentially breaking existing passwords.

The encryption password is just a blob of bytes handled locally by the
client, we cannot remove trailing whitespace here without potential
breakage. Creation of such passwords (via
proxmox_sys::tty::read_and_verify_password) only verifies valid utf-8
and len >= 5.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 pbs-client/src/tools/mod.rs | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
index 7a496d14c..f28d9f32f 100644
--- a/pbs-client/src/tools/mod.rs
+++ b/pbs-client/src/tools/mod.rs
@@ -168,7 +168,17 @@ fn get_secret_impl(env_variable: &str, credential_name: &str) -> Result<Option<S
     if let Some(password) = get_secret_from_env(env_variable)? {
         Ok(Some(password))
     } else if let Some(password) = get_credential(credential_name)? {
-        String::from_utf8(password)
+        str::from_utf8(&password)
+            .map(|s| {
+                if matches!(credential_name, CRED_PBS_REPOSITORY | CRED_PBS_FINGERPRINT) {
+                    s.trim_end()
+                } else if credential_name == CRED_PBS_PASSWORD {
+                    s.trim_end_matches('\n')
+                } else {
+                    s
+                }
+            })
+            .map(ToOwned::to_owned)
             .map(Option::Some)
             .map_err(|_err| format_err!("credential {credential_name} is not utf8 encoded"))
     } else {
-- 
2.47.3

[PATCH backup v2 2/3] docs: client: document further password constrains

[Maximiliano Sandoval]

We leave the explicit newlines as "control character" might not mean
much anything to some readers.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 docs/backup-client.rst | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/docs/backup-client.rst b/docs/backup-client.rst
index 40962f0e2..03a383d9c 100644
--- a/docs/backup-client.rst
+++ b/docs/backup-client.rst
@@ -104,9 +104,10 @@ Environment Variables
    wireguard, instead of using an HTTP proxy.
 
 
-.. Note:: Passwords must be valid UTF-8 and may not contain newlines. For your
-   convenience, Proxmox Backup Server only uses the first line as password, so
-   you can add arbitrary comments after the first newline.
+.. Note:: Passwords must be valid UTF-8 and may not contain newlines or any
+   control characters in general. For your convenience, Proxmox Backup Server
+   only uses the first line as password, so you can add arbitrary comments after
+   the first newline.
 
 
 System and Service Credentials
-- 
2.47.3

[PATCH backup v2 3/3] client: rename password to blob

[Maximiliano Sandoval]

This is a Vec<u8> and not yet the password in its final form.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 pbs-client/src/tools/mod.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
index f28d9f32f..aa3ae94f2 100644
--- a/pbs-client/src/tools/mod.rs
+++ b/pbs-client/src/tools/mod.rs
@@ -167,8 +167,8 @@ fn get_secret_from_env(base_name: &str) -> Result<Option<String>, Error> {
 fn get_secret_impl(env_variable: &str, credential_name: &str) -> Result<Option<String>, Error> {
     if let Some(password) = get_secret_from_env(env_variable)? {
         Ok(Some(password))
-    } else if let Some(password) = get_credential(credential_name)? {
-        str::from_utf8(&password)
+    } else if let Some(blob) = get_credential(credential_name)? {
+        str::from_utf8(&blob)
             .map(|s| {
                 if matches!(credential_name, CRED_PBS_REPOSITORY | CRED_PBS_FINGERPRINT) {
                     s.trim_end()
-- 
2.47.3