Open Stack

On 4/1/19 2:59 PM, Michael Still wrote:
> On Tue, Apr 2, 2019 at 5:06 AM Ben Nemec <openstack at nemebean.com 
> <mailto:openstack at nemebean.com>> wrote:
> 
> [snip]
> 
>     This matches my understanding of the intent behind privsep. Instead of
>     trying to figure out how to filter out dangerous paths or whatever, you
>     have the privileged code build the path out of sanitized data passed
>     in.
>     You still have to be careful about preventing injection attacks (say I
>     pass in [uuid]/../../..), but at least you don't have to try to write a
>     rootwrap filter for every possible path in every possible driver. The
>     privileged driver code itself knows the correct path and won't allow
>     anything else.
> 
> 
> I don't think anyone disputes that theory.
> 
> The issue here is that the calling code in nova makes that very hard in 
> some cases. For example, nova.privsep.path.readfile exists because it is 
> called by nova.virt.disk.localfs.read_file [1], which takes a raw path 
> to read. That is in turn used by calling code to read at least two paths 
> that are generated in calling code. Now all of that is fixable, and 
> that's a relatively simple example, but it requires a call by call 
> analysis that is quite expensive to do and will cause a lot of code 
> churn to fix.
> 
> No one is saying we shouldn't do it. However, the current privsep code 
> took the approach of an incremental improvement on top of what we 
> already had, instead of attempting to boil the ocean in a single patch 
> series. The code for nova.virt.disk.localfs.read_file used to call a 
> method called read_file_as_root that was just as bad as what we have 
> now, but used rootwrap to cat arbitrary files in the filesystem (git 
> sha f535e8bb9905b5632416135af5789704db6d2867 is the original transition).
> 
> My point is that we need to start somewhere, and proposing that we 
> abandon privsep entirely because its not immediately perfect isn't very 
> helpful.

For the record, that was not at all my intent. It doesn't sound to me 
like the current privsep implementation is meaningfully worse than using 
rootwrap, and it seems to me that it should be much easier (note: 
easier, not easy) to move forward now that everything is using privsep. 
I was just +1'ing the desired end state of privsep calls not taking 
paths as input anymore.

> 
> Michael
> 
> 1: Well, there is one other caller, but you get the idea.