<div dir="ltr"><div class="gmail_quote"><div>(Reposting my reply to your gerrit comment here as well - conversation will probably be easier here than in gerrit)</div><div><br></div><div dir="ltr">On Thu, 25 Feb 2016 at 00:07 Duncan Thomas <<a href="mailto:duncan.thomas@gmail.com">duncan.thomas@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>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.<br><br>- If this is the vision / the new vision then I think it is majorly flawed. <br><br>- 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.<br><br></div>Apologies since you've clearly put work into it, and I should have provided such feedback earlier.</div></blockquote><div> </div><div><div>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.</div><div><br></div><div>Better uses of privsep would look more like <a href="https://review.openstack.org/#/c/258252/">https://review.openstack.org/#/c/258252/</a> - 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.</div><div><br></div><div>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.</div><div>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.</div><div><br></div><div>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).</div></div><div><br></div><div><br></div><div>Please continue to ask questions, since I know very few people have actually looked at any of privsep nor the os-brick change until now.</div><div><br></div><div> - Gus</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_extra"><div class="gmail_quote">On 24 February 2016 at 14:59, Michał Dulko <span dir="ltr"><<a href="mailto:michal.dulko@intel.com" target="_blank">michal.dulko@intel.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div>On 02/24/2016 04:51 AM, Angus Lees wrote:<br>
> Re: <a href="https://review.openstack.org/#/c/277224" rel="noreferrer" target="_blank">https://review.openstack.org/#/c/277224</a><br>
><br>
> Most of the various required changes have flushed out by now, and this<br>
> change now passes the dsvm-full integration tests(*).<br>
><br>
> (*) well, the experimental job anyway.  It still relies on a<br>
> merged-but-not-yet-released change in oslo.privsep so gate + 3rd party<br>
> won't pass until that happens.<br>
><br>
> What?<br>
> This change replaces os-brick's use of rootwrap with a quick+dirty<br>
> privsep-based drop-in replacement.  Privsep doesn't actually provide<br>
> much security isolation when used in this way, but it *does* now run<br>
> commands with CAP_SYS_ADMIN (still uid=0/gid=0) rather than full root<br>
> superpowers.  The big win from a practical point of view is that it<br>
> also means os-brick's rootwrap filters file is essentially deleted and<br>
> no longer has to be manually merged with downstream projects.<br>
><br>
> Code changes required in nova/cinder:<br>
> There is one change each to nova+cinder to add the relevant<br>
> privsep-helper command to rootwrap filters, and a devstack change to<br>
> add a nova.conf/cinder.conf setting.  That's it - this is otherwise a<br>
> backwards/forwards compatible change for nova+cinder.<br>
><br>
> Deployment changes required in nova/cinder:<br>
> A new "privsep_rootwrap.helper_command" needs to be defined in<br>
> nova/cinder.conf (default is something sensible using sudo), and<br>
> rootwrap filters or sudoers updated depending on the exact command<br>
> chosen.  Be aware that any commands will now be run with CAP_SYS_ADMIN<br>
> (only), and if that's insufficient for your hardware/drivers it can be<br>
> tweaked with other oslo_config options.<br>
><br>
> Risks:<br>
> The end-result is still just running the same commands as before, via<br>
> a different path - so there's not a lot of adventurousness here.  The<br>
> big behavioural change is CAP_SYS_ADMIN, and (as highlighted above)<br>
> it's conceivable that the driver for some exotic os-brick/cinder<br>
> hardware out there wants something more than that.<br>
><br>
> Work remaining:<br>
> - global-requirements change needed (for os-brick) once the latest<br>
> oslo.privsep release is made<br>
> - cinder/nova/devstack changes need to be merged<br>
> - after the above, the os-brick gate integration jobs will be able to<br>
> pass, and it can be merged<br>
> - If we want to *force* the new version of os-brick, we then need an<br>
> appropriate global-requirements os-brick bump<br>
> - Documentation, release notes, etc<br>
><br>
> I'll continue chewing through those remaining work items, but<br>
> essentially this is now in your combined hands to prioritise for<br>
> mitaka as you deem appropriate.<br>
><br>
>  - Gus<br>
><br>
<br>
</div></div>It seems too me that risks are higher than advantages. Moreover final<br>
release for libraries like os.brick should happen in just 2 days and I<br>
don't believe we have time to get every part of the job merged given how<br>
long TODO list is.<br>
<br>
Just my $0.02.<br>
<br>
__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
</blockquote></div><br><br clear="all"><br>-- <br></div><div class="gmail_extra"><div><div dir="ltr"><div>-- <br>Duncan Thomas</div></div></div>
</div>
<br><p>Message  protected by MailGuard: e-mail anti-virus, anti-spam and content filtering.<br><a href="http://www.mailguard.com.au/mg" target="_blank">http://www.mailguard.com.au/mg</a></p>
<br><a href="https://console.mailguard.com.au/ras/1NU0lK0F0b/3bhu1b0Gm5cuxeirJNa32w/2.626" target="_blank">Report this message as spam</a>

 <br> <br>__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
</blockquote></div></div>