Message from discussion
bdev: Add verification for file storage paths
Received: by 10.204.4.211 with SMTP id 19mr2149459bks.5.1350993748847;
Tue, 23 Oct 2012 05:02:28 -0700 (PDT)
X-BeenThere: ganeti-devel@googlegroups.com
Received: by 10.204.131.72 with SMTP id w8ls259840bks.3.gmail; Tue, 23 Oct
2012 05:02:27 -0700 (PDT)
Received: by 10.204.149.65 with SMTP id s1mr2147117bkv.3.1350993747722;
Tue, 23 Oct 2012 05:02:27 -0700 (PDT)
Received: by 10.204.149.65 with SMTP id s1mr2147116bkv.3.1350993747704;
Tue, 23 Oct 2012 05:02:27 -0700 (PDT)
Return-Path: <ius...@google.com>
Received: from mail-bk0-f43.google.com (mail-bk0-f43.google.com [209.85.214.43])
by gmr-mx.google.com with ESMTPS id o9si1309183bko.2.2012.10.23.05.02.27
(version=TLSv1/SSLv3 cipher=OTHER);
Tue, 23 Oct 2012 05:02:27 -0700 (PDT)
Received-SPF: pass (google.com: domain of ius...@google.com designates 209.85.214.43 as permitted sender) client-ip=209.85.214.43;
Authentication-Results: gmr-mx.google.com; spf=pass (google.com: domain of ius...@google.com designates 209.85.214.43 as permitted sender) smtp.mail=ius...@google.com; dkim=pass header...@google.com
Received: by mail-bk0-f43.google.com with SMTP id w5so1359749bku.2
for <ganeti-devel@googlegroups.com>; Tue, 23 Oct 2012 05:02:27 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=google.com; s=20120113;
h=date:from:to:cc:subject:message-id:references:mime-version
:content-type:content-disposition:in-reply-to:user-agent;
bh=CRU6E3JPBGCuJ576tamtfbfeMa4521pyCTsZ9fhBoL4=;
b=IAvogmMtWxiaGvdQwzw8JZEJrMDdwgiAS3lpO7WDZXGraEReabIqfKfcw+92OBXQfg
4dLenAY7mGjPdsa2wqoJCZMRKNSiqg7cY5hK48gtP9zKT+25WSTP6MSl7/qeEn923JZ0
B07W5K6oKu/ly9hhQ8mJNKfe/Re+8zPfoXydNobGkwPxRGJKMdHtaZJsfspejl3RpyVu
oHVlQWXXbrU9QrMQeExg8uGgcBL+fHzS/MNBc1jo7FSUi7xoUkcJhuIrem0Haky6xyhR
QALaCzspxBFGhZ7Grhf2eZJQSwygYc2rlYBe4bU4tN5XgTCHgQuBgLOFBliz8a4pHgAa
2KAA==
d=google.com; s=20120113;
h=date:from:to:cc:subject:message-id:references:mime-version
:content-type:content-disposition:in-reply-to:user-agent
:x-gm-message-state;
bh=CRU6E3JPBGCuJ576tamtfbfeMa4521pyCTsZ9fhBoL4=;
b=E1e8L1IrrhConMiYUDyUE8X6JzagYLaF2rcBIwwRXrzFo22Kg9DHYYXSpzf3/Z3G+R
8g77JUxB11cQ2myhyjCcv3Y1sKblp8AUJwzsjV5ArHVsYQDXTf7d9ti8dU1YSof+t39+
QheBH0Mp7yJeLLwyZ5GSiHLSfx0i7XFzwzm6pohxfG/LAy0gEINZi5LjL7hfQALbMoQZ
jGDqLNFsny9dW1ijSdf+DshvKhsJtHtRYx0I1/hh0QLIEkLSOZahjzS2und3gZrIjP9n
1icvU0OoYLXjTlQZoGHHRSYXXGs3hDOr2jCORhPh8KDnXfYkq/ZCPB+Uo6SlqpeGnG7f
Bg1w==
Received: by 10.204.11.79 with SMTP id s15mr1283451bks.136.1350993747421;
Tue, 23 Oct 2012 05:02:27 -0700 (PDT)
Return-Path: <ius...@google.com>
Received: from google.com ([2620:0:105f:5:226:b9ff:fe84:a92e])
by mx.google.com with ESMTPS id j24sm5577360bkv.0.2012.10.23.05.02.25
(version=SSLv3 cipher=OTHER);
Tue, 23 Oct 2012 05:02:26 -0700 (PDT)
Date: Tue, 23 Oct 2012 14:02:24 +0200
From: Iustin Pop <ius...@google.com>
To: Michael Hanselmann <han...@google.com>
Cc: ganeti-devel@googlegroups.com
Subject: Re: [PATCH master =?iso-8859-1?Q?4=BD=2F8?= =?iso-8859-1?Q?=5D?=
bdev: Add verification for file storage paths
Message-ID: <20121023120224.GX19...@google.com>
References: <027abd011fb48a032fc28ab567d8ec219a0cde64.1350483271.git.han...@google.com>
<20121023120002.GW19...@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20121023120002.GW19...@google.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
X-Gm-Message-State: ALoCoQl7UkiDIGAbN03VUFq8ZRFCpLzvF8TupViaQwUVq1vLxP9e554FYit5I+3m8qb0GOTR7gXQICsgG8SAp8ex+8jo/ThLrIer1LRWX44hEbaHnBCE6xCKjLwhhWLcHNhWgGxYEcq4JFeijq7FLAfJQ1rg2TGuoGOubZov4N//vCRtymgA8M0asZAvGh5uHnuHq/czeDQ/nPbBe2URpUiA0Z2pv3qZXQ==
On Tue, Oct 23, 2012 at 02:00:02PM +0200, Iustin Pop wrote:
> On Wed, Oct 17, 2012 at 04:18:00PM +0200, Michael Hanselmann wrote:
> > An earlier version of this patch series verified all paths in cmdlib in
> > the master daemon. With this change all that verification code is moved
> > to bdev to run inside the node daemon. The checks are much stricter
> > now--it is no longer possible to use forbidden paths (e.g. /bin) to
> > manipulate file storage devices (once these checks are being used).
>
> > +def _VerifyFileStoragePaths(paths, _forbidden=_GetForbiddenFileStoragePaths()):
> > + """Cross-checks a list of paths for prefixes considered bad.
> > +
> > + Some paths, e.g. "/bin", should not be used for file storage.
> > +
> > + @type paths: list
> > + @param paths: List of paths to be checked
> > + @rtype: list
> > + @return: Sorted list of paths for which the user should be warned
> > +
> > + """
> > + def _Check(path):
> > + return (not os.path.isabs(path) or
> > + path in _forbidden or
> > + filter(lambda p: utils.IsBelowDir(p, path), _forbidden))
>
> This filter is not very clear here, because you're using it in the
> bool(list) context (i.e. not empty list), rather than for the results. I
> was tempted to ask to replace this with a compat.any, but that function
> doesn't take a predicate (bad Python!), so LGTM as is.
Hmm, actually, looking at new patch 5/8, the functions in bdev and
cmdlib are named exactly the same.
Could we name the ones in bdev ComputeWrongFileStoragePaths? Because
this is what they do, instead of verifying them.
thanks,
iustin