[openstack-dev] [os-brick][nova][cinder] os-brick/privsep change is done and awaiting your review

John Garbutt john at johngarbutt.com
Thu Feb 25 14:26:49 UTC 2016


On 25 February 2016 at 05:32, Angus Lees <gus at inodes.org> wrote:
> (Reposting my reply to your gerrit comment here as well - conversation will
> probably be easier here than in gerrit)
>
> On Thu, 25 Feb 2016 at 00:07 Duncan Thomas <duncan.thomas at gmail.com> wrote:
>>
>> My (negative) feedback is on the review - I'm really not sure that this
>> matches what I understood the vision of privsep to be at all.
>>
>> - If this is the vision / the new vision then I think it is majorly
>> flawed.
>>
>> - If it is skipping the vision in the name of expediency of
>> implementation, then I think it has gone too far in that direction and we've
>> better off holding off one more cycle and putting it in next cycle instead
>> with a touch more purity of vision.
>>
>> Apologies since you've clearly put work into it, and I should have
>> provided such feedback earlier.
>
>
> Yes, I agree with your concerns 100% and I'm actually quite glad that you
> also understand that this isn't a very satisfying use of privsep.
>
> Better uses of privsep would look more like
> https://review.openstack.org/#/c/258252/ - but they're slow to write since
> they typically involve quite a bit of refactoring of code in order to move
> the trust boundary to a useful point up the call stack.  For os-brick in
> particular, I found it quite difficult/risky performing such code refactors
> without an easy way to actually test the affected low-level device
> manipulation.
>
> At the nova mid-cycle (and again in the nova-cinder VC conversation you were
> part of), it was decided that the difficulties in merging the os-brick
> rootwrap filters into nova (and I presume cinder) were too urgent to wait
> for such a slow os-brick refactoring process.  The conclusion we reached was
> that we would do a quick+dirty rootwrap drop-in replacement that effectively
> just ran commands as root for Mitaka, and then we would come back and
> refactor various codepaths away from that mechanism over time.  This is that
> first quick+dirty change.
> I tried to capture that in the commit description, but evidently did a poor
> job - if the above makes it any clearer to you, I'd welcome any suggested
> rewording for the commit description.
>
> TL;DR: what this buys us is the ability to use new/different command lines
> in os-brick without having to go through a rootwrap filters merge in
> downstream projects (and it is also that first baby step towards a
> technology that will allow something better in the future).

Agreed with the concerns here, but I thought these are the same we
raised at the midcycle.

My understanding of what came out of the midcycle was:
* current rootwrap system horribly breaks upgrade
* adopting privsep in this "sudo" like form fixes upgrade
* this approach is much lower risk than a full conversion at this
point in the release
* security wise its terrible, but then the current rules don't buy us
much anyhow
* makes it easier to slowly transition to better privsep integration
* all seems better than reverting os-brick integration to fix upgrade issues

Now at this point, we are way closer to release, but I want to check
we are making the correct tradeoff here.

Maybe the upgrade problem is not too bad this release, as the hard bit
was done with the last upgrade? Or is that total nonsense?

Thanks,
johnthetubaguy

>> On 24 February 2016 at 14:59, Michał Dulko <michal.dulko at intel.com> wrote:
>>>
>>> On 02/24/2016 04:51 AM, Angus Lees wrote:
>>> > Re: https://review.openstack.org/#/c/277224
>>> >
>>> > Most of the various required changes have flushed out by now, and this
>>> > change now passes the dsvm-full integration tests(*).
>>> >
>>> > (*) well, the experimental job anyway.  It still relies on a
>>> > merged-but-not-yet-released change in oslo.privsep so gate + 3rd party
>>> > won't pass until that happens.
>>> >
>>> > What?
>>> > This change replaces os-brick's use of rootwrap with a quick+dirty
>>> > privsep-based drop-in replacement.  Privsep doesn't actually provide
>>> > much security isolation when used in this way, but it *does* now run
>>> > commands with CAP_SYS_ADMIN (still uid=0/gid=0) rather than full root
>>> > superpowers.  The big win from a practical point of view is that it
>>> > also means os-brick's rootwrap filters file is essentially deleted and
>>> > no longer has to be manually merged with downstream projects.
>>> >
>>> > Code changes required in nova/cinder:
>>> > There is one change each to nova+cinder to add the relevant
>>> > privsep-helper command to rootwrap filters, and a devstack change to
>>> > add a nova.conf/cinder.conf setting.  That's it - this is otherwise a
>>> > backwards/forwards compatible change for nova+cinder.
>>> >
>>> > Deployment changes required in nova/cinder:
>>> > A new "privsep_rootwrap.helper_command" needs to be defined in
>>> > nova/cinder.conf (default is something sensible using sudo), and
>>> > rootwrap filters or sudoers updated depending on the exact command
>>> > chosen.  Be aware that any commands will now be run with CAP_SYS_ADMIN
>>> > (only), and if that's insufficient for your hardware/drivers it can be
>>> > tweaked with other oslo_config options.
>>> >
>>> > Risks:
>>> > The end-result is still just running the same commands as before, via
>>> > a different path - so there's not a lot of adventurousness here.  The
>>> > big behavioural change is CAP_SYS_ADMIN, and (as highlighted above)
>>> > it's conceivable that the driver for some exotic os-brick/cinder
>>> > hardware out there wants something more than that.
>>> >
>>> > Work remaining:
>>> > - global-requirements change needed (for os-brick) once the latest
>>> > oslo.privsep release is made
>>> > - cinder/nova/devstack changes need to be merged
>>> > - after the above, the os-brick gate integration jobs will be able to
>>> > pass, and it can be merged
>>> > - If we want to *force* the new version of os-brick, we then need an
>>> > appropriate global-requirements os-brick bump
>>> > - Documentation, release notes, etc
>>> >
>>> > I'll continue chewing through those remaining work items, but
>>> > essentially this is now in your combined hands to prioritise for
>>> > mitaka as you deem appropriate.
>>> >
>>> >  - Gus
>>> >
>>>
>>> It seems too me that risks are higher than advantages. Moreover final
>>> release for libraries like os.brick should happen in just 2 days and I
>>> don't believe we have time to get every part of the job merged given how
>>> long TODO list is.
>>>
>>> Just my $0.02.
>>>
>>>
>>> __________________________________________________________________________
>>> OpenStack Development Mailing List (not for usage questions)
>>> Unsubscribe:
>>> OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
>>
>>
>>
>> --
>> --
>> Duncan Thomas
>>
>> Message protected by MailGuard: e-mail anti-virus, anti-spam and content
>> filtering.
>> http://www.mailguard.com.au/mg
>>
>>
>> Report this message as spam
>>
>> __________________________________________________________________________
>> OpenStack Development Mailing List (not for usage questions)
>> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
>
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>



More information about the OpenStack-dev mailing list