From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id AA5E061628 for ; Thu, 9 Jul 2020 10:26:17 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 5859710694 for ; Thu, 9 Jul 2020 10:25:47 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id E905810658 for ; Thu, 9 Jul 2020 10:25:45 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 99D6C43103 for ; Thu, 9 Jul 2020 10:25:45 +0200 (CEST) From: Wolfgang Bumiller To: pve-devel@pve.proxmox.com Date: Thu, 9 Jul 2020 10:25:43 +0200 Message-Id: <20200709082544.14550-2-w.bumiller@proxmox.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200709082544.14550-1-w.bumiller@proxmox.com> References: <20200709082544.14550-1-w.bumiller@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pve-devel] [PATCH storage 2/3] refactor sensitive parameter handling X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Jul 2020 08:26:17 -0000 Signed-off-by: Wolfgang Bumiller --- PVE/API2/Storage/Config.pm | 61 ++++++++++++++++++++------------------ PVE/Storage/CIFSPlugin.pm | 6 ++++ PVE/Storage/PBSPlugin.pm | 6 ++-- 3 files changed, 41 insertions(+), 32 deletions(-) diff --git a/PVE/API2/Storage/Config.pm b/PVE/API2/Storage/Config.pm index 09c5d59..251b4af 100755 --- 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 0eb1722..8de234b 100644 --- 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 31af450..10e4fbc 100644 --- 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); -- 2.20.1