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

[PATCH backup 0/2] 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.

Maximiliano Sandoval (2):
  fix #7054: client: remove trailing newlines from credentials
  docs: client: document further password constrains

 docs/backup-client.rst      |  7 ++++---
 pbs-client/src/tools/mod.rs | 11 +++++++++++
 2 files changed, 15 insertions(+), 3 deletions(-)

-- 
2.47.3

[PATCH backup 1/2] 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.

In order to prevent an allocation for credentials that do not need to be
stripped we perform a preemptive check with str::ends_with before
allocating.

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

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

[PATCH backup 2/2] 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

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

[Christian Ebner]

no in depth review, but just a quick comment

On 2/20/26 12:19 PM, Maximiliano Sandoval wrote:
> 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.
> 
> In order to prevent an allocation for credentials that do not need to be
> stripped we perform a preemptive check with str::ends_with before
> allocating.
> 
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> ---
>   pbs-client/src/tools/mod.rs | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
> index 7a496d14c..8dc7c2cd5 100644
> --- a/pbs-client/src/tools/mod.rs
> +++ b/pbs-client/src/tools/mod.rs
> @@ -169,6 +169,17 @@ fn get_secret_impl(env_variable: &str, credential_name: &str) -> Result<Option<S
>           Ok(Some(password))
>       } else if let Some(password) = get_credential(credential_name)? {
>           String::from_utf8(password)
> +            .map(|s| {
> +                if matches!(credential_name, CRED_PBS_REPOSITORY | CRED_PBS_FINGERPRINT)
> +                    && s.ends_with(char::is_whitespace)

instead of doing the string allocation above and add additional checks 
to avoid allocation here and below (1), why not strip before string 
allocation of `password` directly?

You can use str::from_utf8 [0] and only do the full string allocation 
once trailing characters are stripped.

> +                {
> +                    s.trim_end().to_string()
> +                } else if credential_name == CRED_PBS_PASSWORD && s.ends_with(char::is_control) {

(1)

> +                    s.trim_end_matches(char::is_control).to_string()
> +                } else {
> +                    s
> +                }
> +            })
>               .map(Option::Some)
>               .map_err(|_err| format_err!("credential {credential_name} is not utf8 encoded"))
>       } else {

[0] https://doc.rust-lang.org/std/primitive.str.html#method.from_utf8

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

[Maximiliano Sandoval]

Christian Ebner <c.ebner@proxmox.com> writes:

> no in depth review, but just a quick comment
>
> On 2/20/26 12:19 PM, Maximiliano Sandoval wrote:
>> 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.
>> In order to prevent an allocation for credentials that do not need to be
>> stripped we perform a preemptive check with str::ends_with before
>> allocating.
>> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
>> ---
>>   pbs-client/src/tools/mod.rs | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>> diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
>> index 7a496d14c..8dc7c2cd5 100644
>> --- a/pbs-client/src/tools/mod.rs
>> +++ b/pbs-client/src/tools/mod.rs
>> @@ -169,6 +169,17 @@ fn get_secret_impl(env_variable: &str, credential_name: &str) -> Result<Option<S
>>           Ok(Some(password))
>>       } else if let Some(password) = get_credential(credential_name)? {
>>           String::from_utf8(password)
>> +            .map(|s| {
>> +                if matches!(credential_name, CRED_PBS_REPOSITORY | CRED_PBS_FINGERPRINT)
>> +                    && s.ends_with(char::is_whitespace)
>
> instead of doing the string allocation above and add additional checks to avoid
> allocation here and below (1), why not strip before string allocation of
> `password` directly?
>
> You can use str::from_utf8 [0] and only do the full string allocation once
> trailing characters are stripped.

When the password does not need any trimming, the current method does
not allocate since String::from_utf8 does not allocate. Using
str::from_utf8 would result in 1 allocation when doing such "final
allocation". For reference String::from_utf8 does:

pub fn from_utf8(vec: Vec<u8>) -> Result<String, FromUtf8Error> {
    match str::from_utf8(&vec) {
        Ok(..) => Ok(String { vec }),
        Err(e) => Err(FromUtf8Error { bytes: vec, error: e }),
    }
}

It should be possible to do the trimming before the allocation, but I
don't want to touch the get_credential bits at the moment.

>> +                {
>> +                    s.trim_end().to_string()
>> +                } else if credential_name == CRED_PBS_PASSWORD && s.ends_with(char::is_control) {
>
> (1)
>
>> +                    s.trim_end_matches(char::is_control).to_string()
>> +                } else {
>> +                    s
>> +                }
>> +            })
>>               .map(Option::Some)
>>               .map_err(|_err| format_err!("credential {credential_name} is not utf8 encoded"))
>>       } else {
>
> [0] https://doc.rust-lang.org/std/primitive.str.html#method.from_utf8

-- 
Maximiliano

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

[Fabian Grünbichler]

On February 20, 2026 12:17 pm, Maximiliano Sandoval wrote:
> 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.
> 
> In order to prevent an allocation for credentials that do not need to be
> stripped we perform a preemptive check with str::ends_with before
> allocating.
> 
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> ---
>  pbs-client/src/tools/mod.rs | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/pbs-client/src/tools/mod.rs b/pbs-client/src/tools/mod.rs
> index 7a496d14c..8dc7c2cd5 100644
> --- a/pbs-client/src/tools/mod.rs
> +++ b/pbs-client/src/tools/mod.rs
> @@ -169,6 +169,17 @@ fn get_secret_impl(env_variable: &str, credential_name: &str) -> Result<Option<S
>          Ok(Some(password))
>      } else if let Some(password) = get_credential(credential_name)? {
>          String::from_utf8(password)

nit: while we are here, could we maybe rename this variable? ;)

> +            .map(|s| {
> +                if matches!(credential_name, CRED_PBS_REPOSITORY | CRED_PBS_FINGERPRINT)
> +                    && s.ends_with(char::is_whitespace)

the s.ends_with here, and the trim below are redundant, just trim
below.. this is not a hot path where the slight performance penalty of
trim_end matters, and having two potentially deviating conditions is a
source of bugs.

> +                {
> +                    s.trim_end().to_string()
> +                } else if credential_name == CRED_PBS_PASSWORD && s.ends_with(char::is_control) {
> +                    s.trim_end_matches(char::is_control).to_string()

same here, but this is even more wrong - we should just trim a single
newline, not silently drop control character bytes just because they are
not allowed in passwords! the restriction of the password schema makes
it safe to drop that newline (since it can never be part of the
password).

> +                } else {
> +                    s
> +                }
> +            })
>              .map(Option::Some)
>              .map_err(|_err| format_err!("credential {credential_name} is not utf8 encoded"))
>      } else {
> -- 
> 2.47.3
> 
> 
> 
> 
> 
>