public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH storage v2] fix #5181: pbs: store and read passwords as unicode
@ 2025-06-13 13:11 Maximiliano Sandoval
  2025-06-13 13:11 ` Maximiliano Sandoval
  0 siblings, 1 reply; 2+ messages in thread
From: Maximiliano Sandoval @ 2025-06-13 13:11 UTC (permalink / raw)
  To: pve-devel

At the moment calling
```
pvesm add pbs test --password="bär12345" --datastore='test' # ..other params
```

Will result in the API handler getting the param->{passowrd} as a utf-8
encoded string. When dumped with Debug::Peek's Dump() one can see:

```
SV = PV(0x5a02c1a3ff10) at 0x5a02bd713670
  REFCNT = 1
  FLAGS = (POK,IsCOW,pPOK,UTF8)
  PV = 0x5a02c1a409b0 "b\xC3\xA4r12345"\0 [UTF8 "b\x{e4}r12345"]
  CUR = 9
  LEN = 11
  COW_REFCNT = 0
```

Then when writing the file via file_set_contents (using syswrite
internally) will result in perl encoding the password as latin1 and a
file with contents:

```
$ hexdump -C /etc/pve/priv/storage/test.pw
00000000  62 e4 72 31 32 33 34 35                           |b.r12345|
00000008
```

when the correct contents should have been:
```
00000000  62 c3 a4 72 31 32 33 34  35                       |b..r12345|
00000009
```

Later when the file is read via file_read_firstline it will result in

```
SV = PV(0x5e8baa411090) at 0x5e8baa5a96b8
  REFCNT = 1
  FLAGS = (POK,pPOK)
  PV = 0x5e8baa43ee20 "b\xE4r12345"\0
  CUR = 8
  LEN = 81
```

which is a different string than the original.

At the moment, adding the storage will work as the utf8 password is
still in memory, however, however subsequent uses (e.g. pvestatd) will
fail.

This patch fixes the issue by encoding the string as utf8 both when
reading and storing it to disk. The user was able in the past to go
around the issue by writing the right password in
/etc/pve/priv/{storage}.pw and this fix is compatible with that.

It is documented at
https://pbs.proxmox.com/docs/backup-client.html#environment-variables
that the Backup Server password must be valid utf-8.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 src/PVE/Storage/PBSPlugin.pm | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/PVE/Storage/PBSPlugin.pm b/src/PVE/Storage/PBSPlugin.pm
index 00170f5a..59f5fcc5 100644
--- a/src/PVE/Storage/PBSPlugin.pm
+++ b/src/PVE/Storage/PBSPlugin.pm
@@ -5,6 +5,7 @@ package PVE::Storage::PBSPlugin;
 use strict;
 use warnings;
 
+use Encode qw(decode);
 use Fcntl qw(F_GETFD F_SETFD FD_CLOEXEC);
 use IO::File;
 use JSON;
@@ -93,7 +94,7 @@ sub pbs_set_password {
     my $pwfile = pbs_password_file_name($scfg, $storeid);
     mkdir "/etc/pve/priv/storage";
 
-    PVE::Tools::file_set_contents($pwfile, "$password\n");
+    PVE::Tools::file_set_contents($pwfile, "$password\n", undef, 1);
 }
 
 sub pbs_delete_password {
@@ -109,7 +110,9 @@ sub pbs_get_password {
 
     my $pwfile = pbs_password_file_name($scfg, $storeid);
 
-    return PVE::Tools::file_read_firstline($pwfile);
+    my $contents = PVE::Tools::file_read_firstline($pwfile);
+
+    return eval { decode('UTF-8', $contents, 1) } // $contents;
 }
 
 sub pbs_encryption_key_file_name {
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

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

* Re: [pve-devel] [PATCH storage v2] fix #5181: pbs: store and read passwords as unicode
  2025-06-13 13:11 [pve-devel] [PATCH storage v2] fix #5181: pbs: store and read passwords as unicode Maximiliano Sandoval
@ 2025-06-13 13:11 ` Maximiliano Sandoval
  0 siblings, 0 replies; 2+ messages in thread
From: Maximiliano Sandoval @ 2025-06-13 13:11 UTC (permalink / raw)
  To: Maximiliano Sandoval; +Cc: pve-devel


Maximiliano Sandoval <m.sandoval@proxmox.com> writes:

> At the moment calling
> ```
> pvesm add pbs test --password="bär12345" --datastore='test' # ..other params
> ```
>
> Will result in the API handler getting the param->{passowrd} as a utf-8
> encoded string. When dumped with Debug::Peek's Dump() one can see:
>
> ```
> SV = PV(0x5a02c1a3ff10) at 0x5a02bd713670
>   REFCNT = 1
>   FLAGS = (POK,IsCOW,pPOK,UTF8)
>   PV = 0x5a02c1a409b0 "b\xC3\xA4r12345"\0 [UTF8 "b\x{e4}r12345"]
>   CUR = 9
>   LEN = 11
>   COW_REFCNT = 0
> ```
>
> Then when writing the file via file_set_contents (using syswrite
> internally) will result in perl encoding the password as latin1 and a
> file with contents:
>
> ```
> $ hexdump -C /etc/pve/priv/storage/test.pw
> 00000000  62 e4 72 31 32 33 34 35                           |b.r12345|
> 00000008
> ```
>
> when the correct contents should have been:
> ```
> 00000000  62 c3 a4 72 31 32 33 34  35                       |b..r12345|
> 00000009
> ```
>
> Later when the file is read via file_read_firstline it will result in
>
> ```
> SV = PV(0x5e8baa411090) at 0x5e8baa5a96b8
>   REFCNT = 1
>   FLAGS = (POK,pPOK)
>   PV = 0x5e8baa43ee20 "b\xE4r12345"\0
>   CUR = 8
>   LEN = 81
> ```
>
> which is a different string than the original.
>
> At the moment, adding the storage will work as the utf8 password is
> still in memory, however, however subsequent uses (e.g. pvestatd) will
> fail.
>
> This patch fixes the issue by encoding the string as utf8 both when
> reading and storing it to disk. The user was able in the past to go
> around the issue by writing the right password in
> /etc/pve/priv/{storage}.pw and this fix is compatible with that.
>
> It is documented at
> https://pbs.proxmox.com/docs/backup-client.html#environment-variables
> that the Backup Server password must be valid utf-8.
>
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>

Differences from v1:
 - Just the commit message


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

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

end of thread, other threads:[~2025-06-13 13:12 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-06-13 13:11 [pve-devel] [PATCH storage v2] fix #5181: pbs: store and read passwords as unicode Maximiliano Sandoval
2025-06-13 13:11 ` Maximiliano Sandoval

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