[pve-devel] [PATCH-SERIES qemu-server/storage/common 0/4] fix UTF-8 handling for PBS_PASSWORD

[pve-devel] [PATCH-SERIES qemu-server/storage/common 0/4] fix UTF-8 handling for PBS_PASSWORD

[Fiona Ebner]

The PBS password is saved as UTF-8 and decoded to Perl's internal
string representation upon reading from the password file. When the
password contains multi-byte UTF-8 characters, backing up a diskless
VM would fail with:
> Error: error building client for repository XXX -
> PBS_PASSWORD contains bad characters

The same error would occur for uploading the log file after backup, as
well as extracting the configuration file from backup.

Restoring would fail with:
> restore failed: invalid utf-8 sequence of 1 bytes from index 0

Fix these issues by properly encoding the value for the PBS_PASSWORD
environment variable value again as UTF-8.

For PMG, using passwords that would be auto-encoded by Perl as either
ASCII or UTF-8 already worked, but other encodings would not, for
example ISO-8859 would result in:
> proxmox-backup-client failed: Error: error building client for
> repository latin@pbs@10.10.100.180:8007:bigone - PBS_PASSWORD
> contains bad characters (500)

Follow pve-storage commit 5245e04 ("fix #5181: pbs: store and read
passwords as unicode") and align the behavior of the storage plugin
and pbs client module.

qemu-server:

Fiona Ebner (2):
  qmp client: encode JSON as UTF-8 to fix PBS backup when password
    contains multi-byte UTF-8
  pbs: properly encode PBS password as UTF-8 when setting the
    environment variable

 src/PVE/QMPClient.pm         | 14 ++++++++------
 src/PVE/QemuServer.pm        |  4 ++++
 src/PVE/VZDump/QemuServer.pm |  4 ++++
 3 files changed, 16 insertions(+), 6 deletions(-)


storage:

Fiona Ebner (1):
  pbs plugin: raw client command: properly encode PBS password as UTF-8
    when setting the environment variable

 src/PVE/Storage/PBSPlugin.pm | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)


common:

Fiona Ebner (1):
  pbs client: allow using password that would be auto-encoded as neither
    ASCII nor UTF-8

 src/PVE/PBSClient.pm | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)


Summary over all repositories:
  5 files changed, 32 insertions(+), 11 deletions(-)

-- 
Generated by git-murpp 0.5.0


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

[pve-devel] [PATCH qemu-server 1/4] qmp client: encode JSON as UTF-8 to fix PBS backup when password contains multi-byte UTF-8

[Fiona Ebner]

As reported in the community forum, PBS backup of VMs would not work
when the password contained a multi-byte UTF-8 character [0].

The reason is that when writing a string in Perl's internal
representation with character values >= 128 to the QMP socket, QEMU
seems to have a bug where it reads too few characters (one less for
each char value >= 128). In such a case, the QMP client will just time
out, as the command is never completely read on the QEMU side.
Stuffing with additional characters would actually lead to QEMU
reading enough characters and executing the command, but Perl's
internal representation should not be submitted to QMP in the first
place. Encode the JSON properly as UTF-8 to avoid the issue.

In particular, this makes backing up a VM to PBS possible when the PBS
password contains multi-byte UTF-8 characters.

This also fixes future similar issues, for example when a QMP command
is passed a filesystem path with multi-byte UTF-8 characters.

[0]: https://forum.proxmox.com/threads/172871/post-804921

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Unfortunately, doing the same for QGA is not yet enough to fix issue
#6609.

 src/PVE/QMPClient.pm | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/PVE/QMPClient.pm b/src/PVE/QMPClient.pm
index 7b19be9d..46d8b299 100644
--- a/src/PVE/QMPClient.pm
+++ b/src/PVE/QMPClient.pm
@@ -291,12 +291,14 @@ my $check_queue = sub {
                     . "\n";
 
             } else {
-
-                $qmpcmd = to_json({
-                    execute => $cmd->{execute},
-                    arguments => $cmd->{arguments},
-                    id => $cmd->{id},
-                });
+                $qmpcmd = to_json(
+                    {
+                        execute => $cmd->{execute},
+                        arguments => $cmd->{arguments},
+                        id => $cmd->{id},
+                    },
+                    { utf8 => 1 },
+                );
             }
 
             if ($fd >= 0) {
-- 
2.47.3



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

[pve-devel] [PATCH qemu-server 2/4] pbs: properly encode PBS password as UTF-8 when setting the environment variable

[Fiona Ebner]

The PBS password is saved as UTF-8 and decoded to Perl's internal
string representation upon reading from the password file. When the
password contains multi-byte UTF-8 characters, backing up a diskless
VM would fail with:
> Error: error building client for repository XXX -
> PBS_PASSWORD contains bad characters
and restoring would fail with:
> restore failed: invalid utf-8 sequence of 1 bytes from index 0

This is fixed by properly encoding the value for the PBS_PASSWORD
environment variable value again as UTF-8.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/QemuServer.pm        | 4 ++++
 src/PVE/VZDump/QemuServer.pm | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 7d5ab718..a57eecd5 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -5,6 +5,7 @@ use warnings;
 
 use Cwd 'abs_path';
 use Digest::SHA;
+use Encode qw(encode);
 use Fcntl ':flock';
 use Fcntl;
 use File::Basename;
@@ -6971,6 +6972,9 @@ sub restore_proxmox_backup_archive {
 
     # This is only used for `pbs-restore` and the QEMU PBS driver (live-restore)
     my $password = PVE::Storage::PBSPlugin::pbs_get_password($scfg, $storeid);
+    # The password is saved as UTF-8 and is decoded upon reading. Need to re-encode when setting the
+    # environment variable.
+    $password = encode('UTF-8', $password, 1);
     local $ENV{PBS_PASSWORD} = $password;
     local $ENV{PBS_FINGERPRINT} = $fingerprint if defined($fingerprint);
 
diff --git a/src/PVE/VZDump/QemuServer.pm b/src/PVE/VZDump/QemuServer.pm
index 5b94c369..30a77ea9 100644
--- a/src/PVE/VZDump/QemuServer.pm
+++ b/src/PVE/VZDump/QemuServer.pm
@@ -3,6 +3,7 @@ package PVE::VZDump::QemuServer;
 use strict;
 use warnings;
 
+use Encode qw(encode);
 use Fcntl qw(:mode);
 use File::Basename;
 use File::Path qw(make_path remove_tree);
@@ -737,6 +738,9 @@ sub archive_pbs {
     if (!$diskcount) {
         $self->loginfo("backup contains no disks");
 
+        # The password is saved as UTF-8 and is decoded upon reading. Need to re-encode when setting
+        # the environment variable.
+        $password = encode('UTF-8', $password, 1);
         local $ENV{PBS_PASSWORD} = $password;
         local $ENV{PBS_FINGERPRINT} = $fingerprint if defined($fingerprint);
         my $cmd = [
-- 
2.47.3



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

[pve-devel] [PATCH storage 3/4] pbs plugin: raw client command: properly encode PBS password as UTF-8 when setting the environment variable

[Fiona Ebner]

The PBS password is saved as UTF-8 and decoded to Perl's internal
string representation upon reading from the password file. When the
password contains multi-byte UTF-8 characters, backing up a diskless
VM would fail with:
> Error: error building client for repository XXX -
> PBS_PASSWORD contains bad characters

This is fixed by properly encoding the value for the PBS_PASSWORD
environment variable value again as UTF-8.

For example, this fixes uploading the log file after backup, as well
as extracting the configuration file from backup.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/Storage/PBSPlugin.pm | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/PVE/Storage/PBSPlugin.pm b/src/PVE/Storage/PBSPlugin.pm
index 5842004..f0f2ba3 100644
--- a/src/PVE/Storage/PBSPlugin.pm
+++ b/src/PVE/Storage/PBSPlugin.pm
@@ -5,7 +5,7 @@ package PVE::Storage::PBSPlugin;
 use strict;
 use warnings;
 
-use Encode qw(decode);
+use Encode qw(decode encode);
 use Fcntl qw(F_GETFD F_SETFD FD_CLOEXEC);
 use IO::File;
 use JSON;
@@ -327,7 +327,11 @@ my sub do_raw_client_cmd {
         push @$cmd, '--ns', $ns;
     }
 
-    local $ENV{PBS_PASSWORD} = pbs_get_password($scfg, $storeid);
+    my $password = pbs_get_password($scfg, $storeid);
+    # The password is saved as UTF-8 and is decoded upon reading. Need to re-encode when setting the
+    # environment variable.
+    $password = encode('UTF-8', $password, 1);
+    local $ENV{PBS_PASSWORD} = $password;
 
     local $ENV{PBS_FINGERPRINT} = $scfg->{fingerprint};
 
-- 
2.47.3



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

[pve-devel] [PATCH common 4/4] pbs client: allow using password that would be auto-encoded as neither ASCII nor UTF-8

[Fiona Ebner]

Using passwords that would be auto-encoded by Perl as either ASCII or
UTF-8 already worked, but other encodings would not, for example
ISO-8859 would result in:
> proxmox-backup-client failed: Error: error building client for
> repository latin@pbs@10.10.100.180:8007:bigone - PBS_PASSWORD
> contains bad characters (500)

The issue only affected PMG, because in PVE, the PBS storage plugin
uses its own implementation of {get,set}_password() which does handle
UTF-8 already since pve-storage commit 5245e04 ("fix #5181: pbs: store
and read passwords as unicode"). Follow that commit to align the
behavior. This is also in preparation to using the PBS Client more
from the storage plugin too.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/PBSClient.pm | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/PVE/PBSClient.pm b/src/PVE/PBSClient.pm
index 6333304..16d4740 100644
--- a/src/PVE/PBSClient.pm
+++ b/src/PVE/PBSClient.pm
@@ -4,6 +4,7 @@ package PVE::PBSClient;
 use strict;
 use warnings;
 
+use Encode qw(decode encode);
 use Fcntl qw(F_GETFD F_SETFD FD_CLOEXEC);
 use File::Temp qw(tempdir);
 use IO::File;
@@ -72,7 +73,7 @@ sub set_password {
     my $pwfile = password_file_name($self);
     mkdir($self->{secret_dir});
 
-    PVE::Tools::file_set_contents($pwfile, "$password\n", 0600);
+    PVE::Tools::file_set_contents($pwfile, "$password\n", 0600, 1);
 }
 
 sub delete_password {
@@ -88,7 +89,9 @@ sub get_password {
 
     my $pwfile = password_file_name($self);
 
-    return PVE::Tools::file_read_firstline($pwfile);
+    my $contents = PVE::Tools::file_read_firstline($pwfile);
+
+    return eval { decode('UTF-8', $contents, 1) } // $contents;
 }
 
 sub encryption_key_file_name {
@@ -185,7 +188,11 @@ my sub do_raw_client_cmd {
         push(@$cmd, '--ns', $ns);
     }
 
-    local $ENV{PBS_PASSWORD} = $self->get_password();
+    my $password = $self->get_password();
+    # The password is saved as UTF-8 and is decoded upon reading. Need to re-encode when setting the
+    # environment variable.
+    $password = encode('UTF-8', $password, 1);
+    local $ENV{PBS_PASSWORD} = $password;
 
     local $ENV{PBS_FINGERPRINT} = $scfg->{fingerprint};
 
-- 
2.47.3



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