refactor sensitive parameter handling

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>

diff --git a/PVE/API2/Storage/Config.pm b/PVE/API2/Storage/Config.pm
index 09c5d5928727785a248e14be7f833002ffc6b631..251b4afdb91935dd10c92f4f271f53b1672d011a 100755 (executable)
--- a/PVE/API2/Storage/Config.pm
+++ b/PVE/API2/Storage/Config.pm
@@ -112,6 +112,29 @@ __PACKAGE__->register_method ({
        return &$api_storage_config($cfg, $param->{storage});
     }});
 
+my sub extract_sensitive_params :prototype($$) {
+    my ($param, $delete_list) = @_;
+
+    my $sensitive;
+
+    my %delete = map { $_ => 1 } ($delete_list || [])->@*;
+
+    # always extract pw and keys, so they don't get written to the www-data readable scfg
+    for my $opt (qw(password encryption_key)) {
+       # First handle deletions as explicitly setting `undef`, afterwards new values may override
+       # it.
+       if (exists($delete{$opt})) {
+           $sensitive->{$opt} = undef;
+       }
+
+       if (defined(my $value = extract_param($param, $opt))) {
+           $sensitive->{$opt} = $value;
+       }
+    }
+
+    return $sensitive;
+}
+
 __PACKAGE__->register_method ({
     name => 'create',
     protected => 1,
@@ -133,15 +156,7 @@ __PACKAGE__->register_method ({
        # fix me in section config create never need an empty entity.
        delete $param->{nodes} if !$param->{nodes};
 
-       my $password;
-       # always extract pw, else it gets written to the www-data readable scfg
-       if (my $tmp_pw = extract_param($param, 'password')) {
-           if (($type eq 'pbs') || ($type eq 'cifs' && $param->{username})) {
-               $password = $tmp_pw;
-           } else {
-               warn "ignore password parameter\n";
-           }
-       }
+       my $sensitive = extract_sensitive_params($param, []);
 
        my $plugin = PVE::Storage::Plugin->lookup($type);
        my $opts = $plugin->check_config($storeid, $param, 1, 1);
@@ -157,7 +172,7 @@ __PACKAGE__->register_method ({
 
                $cfg->{ids}->{$storeid} = $opts;
 
-               $plugin->on_add_hook($storeid, $opts, password => $password);
+               $plugin->on_add_hook($storeid, $opts, %$sensitive);
 
                eval {
                    # try to activate if enabled on local node,
@@ -197,6 +212,10 @@ __PACKAGE__->register_method ({
        my $digest = extract_param($param, 'digest');
        my $delete = extract_param($param, 'delete');
 
+       if ($delete) {
+           $delete = [ PVE::Tools::split_list($delete) ];
+       }
+
         PVE::Storage::lock_storage_config(sub {
 
            my $cfg = PVE::Storage::config();
@@ -206,24 +225,14 @@ __PACKAGE__->register_method ({
            my $scfg = PVE::Storage::storage_config($cfg, $storeid);
            my $type = $scfg->{type};
 
-           my $password;
-           # always extract pw, else it gets written to the www-data readable scfg
-           if (my $tmp_pw = extract_param($param, 'password')) {
-               if (($type eq 'pbs') || ($type eq 'cifs' && $param->{username})) {
-                   $password = $tmp_pw;
-               } else {
-                   warn "ignore password parameter\n";
-               }
-           }
+           my $sensitive = extract_sensitive_params($param, $delete);
 
            my $plugin = PVE::Storage::Plugin->lookup($type);
            my $opts = $plugin->check_config($storeid, $param, 0, 1);
 
-           my $delete_password = 0;
-
            if ($delete) {
                my $options = $plugin->private()->{options}->{$type};
-               foreach my $k (PVE::Tools::split_list($delete)) {
+               foreach my $k (@$delete) {
                    my $d = $options->{$k} || die "no such option '$k'\n";
                    die "unable to delete required option '$k'\n" if !$d->{optional};
                    die "unable to delete fixed option '$k'\n" if $d->{fixed};
@@ -231,16 +240,10 @@ __PACKAGE__->register_method ({
                        if defined($opts->{$k});
 
                    delete $scfg->{$k};
-
-                   $delete_password = 1 if $k eq 'password';
                }
            }
 
-           if ($delete_password || defined($password)) {
-               $plugin->on_update_hook($storeid, $opts, password => $password);
-           } else {
-               $plugin->on_update_hook($storeid, $opts);
-           }
+           $plugin->on_update_hook($storeid, $opts, %$sensitive);
 
            for my $k (keys %$opts) {
                $scfg->{$k} = $opts->{$k};

diff --git a/PVE/Storage/CIFSPlugin.pm b/PVE/Storage/CIFSPlugin.pm
index 5249a62ab85837b3a5620aafbf728ef84624b290..72ec757b0beb7885e43f9ff5623947c3ccc474ab 100644 (file)
--- a/PVE/Storage/CIFSPlugin.pm
+++ b/PVE/Storage/CIFSPlugin.pm
@@ -161,6 +161,9 @@ sub on_add_hook {
 
     if (defined($param{password})) {
        cifs_set_credentials($param{password}, $storeid);
+       if (!exists($scfg->{username})) {
+           warn "ignoring password parameter\n";
+       }
     } else {
        cifs_delete_credentials($storeid);
     }
@@ -173,6 +176,9 @@ sub on_update_hook {
 
     if (defined($param{password})) {
        cifs_set_credentials($param{password}, $storeid);
+       if (!exists($scfg->{username})) {
+           warn "ignoring password parameter\n";
+       }
     } else {
        cifs_delete_credentials($storeid);
     }

diff --git a/PVE/Storage/PBSPlugin.pm b/PVE/Storage/PBSPlugin.pm
index 31af4505c9e55e815c252ef0f36e9d26aa4d5101..10e4fbc861fa3bcd7239a24b760480ed5d2a9c08 100644 (file)
--- a/PVE/Storage/PBSPlugin.pm
+++ b/PVE/Storage/PBSPlugin.pm
@@ -266,7 +266,7 @@ sub on_add_hook {
        pbs_delete_password($scfg, $storeid);
     }
 
-    if (defined(my $encryption_key = delete($scfg->{encryption_key}))) {
+    if (defined(my $encryption_key = $param{encryption_key})) {
        pbs_set_encryption_key($scfg, $storeid, $encryption_key);
     } else {
        pbs_delete_encryption_key($scfg, $storeid);
@@ -284,8 +284,8 @@ sub on_update_hook {
        }
     }
 
-    if (exists($scfg->{encryption_key})) {
-       if (defined(my $encryption_key = delete($scfg->{encryption_key}))) {
+    if (exists($param{encryption_key})) {
+       if (defined(my $encryption_key = delete($param{encryption_key}))) {
            pbs_set_encryption_key($scfg, $storeid, $encryption_key);
        } else {
            pbs_delete_encryption_key($scfg, $storeid);