From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <t.lamprecht@proxmox.com> 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) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 37D5374D7B for <pve-devel@lists.proxmox.com>; Wed, 23 Jun 2021 06:44:18 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 21C1F2D69F for <pve-devel@lists.proxmox.com>; Wed, 23 Jun 2021 06:43:48 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id E8EBD2D68E for <pve-devel@lists.proxmox.com>; Wed, 23 Jun 2021 06:43:46 +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 A8F45425B9 for <pve-devel@lists.proxmox.com>; Wed, 23 Jun 2021 06:43:46 +0200 (CEST) Message-ID: <dade164d-005f-8a14-c879-2e97a3a7b02f@proxmox.com> Date: Wed, 23 Jun 2021 06:43:36 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:90.0) Gecko/20100101 Thunderbird/90.0 Content-Language: en-US To: Stoiko Ivanov <s.ivanov@proxmox.com> Cc: Proxmox VE development discussion <pve-devel@lists.proxmox.com> References: <20210622142824.18773-1-s.ivanov@proxmox.com> <5f4ba4d0-8092-0096-5b10-7f02d50fc865@proxmox.com> <20210622171022.23690ff6@rosa.proxmox.com> <80dd1c9f-75c8-9010-9267-8b3191f09e0a@proxmox.com> <20210622185204.37a259bf@rosa.proxmox.com> From: Thomas Lamprecht <t.lamprecht@proxmox.com> In-Reply-To: <20210622185204.37a259bf@rosa.proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.687 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.001 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH common] run_command: untaint end of buffer X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com> List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe> List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/> List-Post: <mailto:pve-devel@lists.proxmox.com> List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help> List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe> X-List-Received-Date: Wed, 23 Jun 2021 04:44:18 -0000 On 22.06.21 18:52, Stoiko Ivanov wrote: > On Tue, 22 Jun 2021 17:15:08 +0200 > Thomas Lamprecht <t.lamprecht@proxmox.com> wrote: > >> On 22.06.21 17:10, Stoiko Ivanov wrote: >>> I had a patch for untainting the individual values in >>> PVE::Storage::Plugin::volume_size_info but then went with this patch, >> >> I'd rather have that patch, especially for back-porting to stable. > Makes sense - sent the patch for pve-storage > >> I mean, else we can probably just turn of the taint mode completely, what's the >> point then. > I'm always a bit (too) cautious when it comes to turning of 'security' > related 'features' (even if mostly doubting that taint-mode fits either of > those 2 categories) - so not sure about disabling it in general > > the taint of the some of the run_command output on the other hand was > introduced as a side-effect with the changes last year afaict, and has it really wasn't, it gave no guarantees and some callers did not checked for it, some floated up then, if we just blindly untainted anything it just has no benefit to run under taint mode, especially as we want to move over as much as possible to run_command anyway. Rather than just band-aiding it somewhere in the middle with a catch all regex that *completely* defeats the purpose of the concept of tainting, it can be better to either just disable or fix the few places where it's actual wrong with a local decision about how closely we can restrict the untainting, sometimes a match-all is all it can realistically be there, but not always. > caused at least 2 glitches since then... > which is really not much, and the whole "fool me once, ..." should make it easier to spot any remaining one ;-P