Re: [openstack-dev] [tripleo] Limiting sudo coverage of heat-admin / stack and other users.

2018-05-22 Thread Luke Hinds 
On Tue, May 22, 2018 at 8:24 AM, Cédric Jeanneret 
wrote:

>
>
> On 05/22/2018 09:08 AM, Luke Hinds wrote:
> >
> >
> > On Tue, May 22, 2018 at 5:27 AM, Cédric Jeanneret  > > wrote:
> >
> >
> >
> > On 05/21/2018 03:49 PM, Luke Hinds wrote:
> > > A few operators have requested if its possible to limit sudo's
> coverage
> > > on both the under / overcloud. There is concern over `ALL=(ALL)
> > > NOPASSWD:ALL` , which allows someone to  `sudo su`.
> > >
> > > This task has come under the care of the tripleo security squad.
> > >
> > > The work is being tracked and discussed here [0].
> > >
> > > So far it looks like the approach will be to use regexp within
> > > /etc/sudoers.d/*., to narrow down as close as possible to the
> specific
> > > commands called. Some services already do this with rootwrap:
> > >
> > > ironic ALL = (root) NOPASSWD: /usr/bin/ironic-rootwrap
> > > /etc/ironic/rootwrap.conf *
> > >
> > > It's fairly easy to pick up a list of all sudo calls using a simple
> > > script [1]
> > >
> > > The other prolific user of sudo is ansible / stack, for example:
> > >
> > > /bin/sh -c echo BECOME-SUCCESS-kldpbeueyodisjajjqthpafzadrncdff;
> > > /usr/bin/python
> > > /home/stack/.ansible/tmp/ansible-tmp-1526579105.0-
> 109863952786117/systemd.py;
> > > rm -rf
> > > "/home/stack/.ansible/tmp/ansible-tmp-1526579105.0-109863952786117/"
> >
> > > /dev/null 2>&1
> > >
> > > My feelings here are to again use regexp around the immutable non
> random
> > > parts of the command.  cjeanner also made some suggestions in the
> > > etherpad [0].
> >
> > Might be a temporary way to limit the surface indeed, but an upstream
> > change in Ansible would still be really nice. Predictable names is
> the
> > only "right" way, although this will create a long sudo ruleset. A
> > really long one to be honnest. Maintainability is also to be
> discussed
> > in either way (maintain a couple of regexp vs 200+ rules.. hmmm).
> >
> >
> > As I understand it, the problem with predicable names is they also
> > become predictable to attackers (this would be the reason ansible adds
> > in the random string). It helps prevent someone creating a race
> > condition to replace the python script with something more nefarious.
> > Its the same reason commands such as mktemp exists.
>
> Fair enough indeed. Both solution have their pros and cons. In order to
> move on, I think the regexp in sudoers is acceptable for the following
> reasons:
> - limits accesses outside of ansible generated code
> - allows others to still push new content without having to change sudo
> listing (thanks to regexp)
> - still hard to inject bad things in the executed script/code
> - quick to implement (well, fastest than requiring an upstream change
> that will most probably break some internal things before working
> properly, and without adding more security as you explained it)
>
>
Thanks for chiming in Cédric , value your contributions here.

I was thinking about this earlier on my way to work..

Perhaps we could have a script in CI that fails on sudo calls being blocked
(as no regexp exists for them)? This way it will prevent people going on a
wild goose chase trying to work out why a patch they are working on has
failed. As an example, someone might change a single argument in an
iptables command (a few of those run under sudo) that desyncs the command
to the sudo regexp?


@Juan do you agree with that statement? As we had some quick chat about it.
>
> Note: I'm not part of the security squad ;). But I like secured things.
>
> >
> > >
> > > However aside to the approach, we need to consider the impact
> locking
> > > down might have should someone create a develop a new bit of code
> that
> > > leverages commands wrapped in sudo and assumes ALL with be in
> place.
> > > This of course will be blocked.
> >
> > This will indeed require some doc, as this is a "major" change.
> However,
> > the use of regexp should somewhat limit the impact, especially since
> > Ansible pushes its exec script in the same location.
> > Even new parts should be allowed (that might be a bit of concern if
> we
> > want to really dig in the consequences of a bad template being
> injected
> > in some way [looking config-download ;)]).
> > But at some point, we might also decide to let the OPs ensure their
> > infra isn't compromised.
> > Always the same thread-of with Security vs The World - convenience vs
> > cumbersome management, and so on.
> >
> > >
> > > Now my guess is that our CI would capture this as the deploy would
> > > fail(?) and the developer should work out an entry is needed when
> > > testing their patch, but wanted to open this up to others who know
> > > testing at gate better

Re: [openstack-dev] [tripleo] Limiting sudo coverage of heat-admin / stack and other users.

2018-05-22 Thread Cédric Jeanneret 


On 05/22/2018 09:24 AM, Cédric Jeanneret wrote:
> 
> 
> On 05/22/2018 09:08 AM, Luke Hinds wrote:
>>
>>
>> On Tue, May 22, 2018 at 5:27 AM, Cédric Jeanneret > > wrote:
>>
>>
>>
>> On 05/21/2018 03:49 PM, Luke Hinds wrote:
>> > A few operators have requested if its possible to limit sudo's coverage
>> > on both the under / overcloud. There is concern over `ALL=(ALL)
>> > NOPASSWD:ALL` , which allows someone to  `sudo su`.
>> > 
>> > This task has come under the care of the tripleo security squad.
>> > 
>> > The work is being tracked and discussed here [0].
>> > 
>> > So far it looks like the approach will be to use regexp within
>> > /etc/sudoers.d/*., to narrow down as close as possible to the specific
>> > commands called. Some services already do this with rootwrap:
>> > 
>> > ironic ALL = (root) NOPASSWD: /usr/bin/ironic-rootwrap
>> > /etc/ironic/rootwrap.conf *   
>> > 
>> > It's fairly easy to pick up a list of all sudo calls using a simple
>> > script [1]
>> > 
>> > The other prolific user of sudo is ansible / stack, for example:
>> > 
>> > /bin/sh -c echo BECOME-SUCCESS-kldpbeueyodisjajjqthpafzadrncdff;
>> > /usr/bin/python
>> > 
>> /home/stack/.ansible/tmp/ansible-tmp-1526579105.0-109863952786117/systemd.py;
>> > rm -rf
>> > "/home/stack/.ansible/tmp/ansible-tmp-1526579105.0-109863952786117/" >
>> > /dev/null 2>&1
>> > 
>> > My feelings here are to again use regexp around the immutable non 
>> random
>> > parts of the command.  cjeanner also made some suggestions in the
>> > etherpad [0].
>>
>> Might be a temporary way to limit the surface indeed, but an upstream
>> change in Ansible would still be really nice. Predictable names is the
>> only "right" way, although this will create a long sudo ruleset. A
>> really long one to be honnest. Maintainability is also to be discussed
>> in either way (maintain a couple of regexp vs 200+ rules.. hmmm).
>>
>>
>> As I understand it, the problem with predicable names is they also
>> become predictable to attackers (this would be the reason ansible adds
>> in the random string). It helps prevent someone creating a race
>> condition to replace the python script with something more nefarious.
>> Its the same reason commands such as mktemp exists.
> 
> Fair enough indeed. Both solution have their pros and cons. In order to
> move on, I think the regexp in sudoers is acceptable for the following
> reasons:
> - limits accesses outside of ansible generated code
> - allows others to still push new content without having to change sudo
> listing (thanks to regexp)
> - still hard to inject bad things in the executed script/code
> - quick to implement (well, fastest than requiring an upstream change
> that will most probably break some internal things before working
> properly, and without adding more security as you explained it)

Small idea: it might be interesting to check if SELinux can't be a ally
for that issue in fact: dedicated context, separation, that's a SELinux
kind of thing isn't it?
I'm no SELinux poweruser¹, but that kind of usage is, to my small
knowledge of this product, a perfect fit.

Would be good to dig in that direction, don't you think?



###
¹ I'm more the poor guy at the end head-banging his desk when SELinux
comes in the way ;). That hurts.

> 
> @Juan do you agree with that statement? As we had some quick chat about it.
> 
> Note: I'm not part of the security squad ;). But I like secured things.
> 
>>
>> > 
>> > However aside to the approach, we need to consider the impact locking
>> > down might have should someone create a develop a new bit of code that
>> > leverages commands wrapped in sudo and assumes ALL with be in place.
>> > This of course will be blocked.
>>
>> This will indeed require some doc, as this is a "major" change. However,
>> the use of regexp should somewhat limit the impact, especially since
>> Ansible pushes its exec script in the same location.
>> Even new parts should be allowed (that might be a bit of concern if we
>> want to really dig in the consequences of a bad template being injected
>> in some way [looking config-download ;)]).
>> But at some point, we might also decide to let the OPs ensure their
>> infra isn't compromised.
>> Always the same thread-of with Security vs The World - convenience vs
>> cumbersome management, and so on.
>>
>> >
>> > Now my guess is that our CI would capture this as the deploy would
>> > fail(?) and the developer should work out an entry is needed when
>> > testing their patch, but wanted to open this up to others who know
>> > testing at gate better much better than myself.  Also encourage any
>> > thoughts on the topic to be introduced to the etherpad [0]
>> >
>> > [0]

Re: [openstack-dev] [tripleo] Limiting sudo coverage of heat-admin / stack and other users.

2018-05-22 Thread Cédric Jeanneret 


On 05/22/2018 09:08 AM, Luke Hinds wrote:
> 
> 
> On Tue, May 22, 2018 at 5:27 AM, Cédric Jeanneret  > wrote:
> 
> 
> 
> On 05/21/2018 03:49 PM, Luke Hinds wrote:
> > A few operators have requested if its possible to limit sudo's coverage
> > on both the under / overcloud. There is concern over `ALL=(ALL)
> > NOPASSWD:ALL` , which allows someone to  `sudo su`.
> > 
> > This task has come under the care of the tripleo security squad.
> > 
> > The work is being tracked and discussed here [0].
> > 
> > So far it looks like the approach will be to use regexp within
> > /etc/sudoers.d/*., to narrow down as close as possible to the specific
> > commands called. Some services already do this with rootwrap:
> > 
> > ironic ALL = (root) NOPASSWD: /usr/bin/ironic-rootwrap
> > /etc/ironic/rootwrap.conf *   
> > 
> > It's fairly easy to pick up a list of all sudo calls using a simple
> > script [1]
> > 
> > The other prolific user of sudo is ansible / stack, for example:
> > 
> > /bin/sh -c echo BECOME-SUCCESS-kldpbeueyodisjajjqthpafzadrncdff;
> > /usr/bin/python
> > 
> /home/stack/.ansible/tmp/ansible-tmp-1526579105.0-109863952786117/systemd.py;
> > rm -rf
> > "/home/stack/.ansible/tmp/ansible-tmp-1526579105.0-109863952786117/" >
> > /dev/null 2>&1
> > 
> > My feelings here are to again use regexp around the immutable non random
> > parts of the command.  cjeanner also made some suggestions in the
> > etherpad [0].
> 
> Might be a temporary way to limit the surface indeed, but an upstream
> change in Ansible would still be really nice. Predictable names is the
> only "right" way, although this will create a long sudo ruleset. A
> really long one to be honnest. Maintainability is also to be discussed
> in either way (maintain a couple of regexp vs 200+ rules.. hmmm).
> 
> 
> As I understand it, the problem with predicable names is they also
> become predictable to attackers (this would be the reason ansible adds
> in the random string). It helps prevent someone creating a race
> condition to replace the python script with something more nefarious.
> Its the same reason commands such as mktemp exists.

Fair enough indeed. Both solution have their pros and cons. In order to
move on, I think the regexp in sudoers is acceptable for the following
reasons:
- limits accesses outside of ansible generated code
- allows others to still push new content without having to change sudo
listing (thanks to regexp)
- still hard to inject bad things in the executed script/code
- quick to implement (well, fastest than requiring an upstream change
that will most probably break some internal things before working
properly, and without adding more security as you explained it)

@Juan do you agree with that statement? As we had some quick chat about it.

Note: I'm not part of the security squad ;). But I like secured things.

> 
> > 
> > However aside to the approach, we need to consider the impact locking
> > down might have should someone create a develop a new bit of code that
> > leverages commands wrapped in sudo and assumes ALL with be in place.
> > This of course will be blocked.
> 
> This will indeed require some doc, as this is a "major" change. However,
> the use of regexp should somewhat limit the impact, especially since
> Ansible pushes its exec script in the same location.
> Even new parts should be allowed (that might be a bit of concern if we
> want to really dig in the consequences of a bad template being injected
> in some way [looking config-download ;)]).
> But at some point, we might also decide to let the OPs ensure their
> infra isn't compromised.
> Always the same thread-of with Security vs The World - convenience vs
> cumbersome management, and so on.
> 
> >
> > Now my guess is that our CI would capture this as the deploy would
> > fail(?) and the developer should work out an entry is needed when
> > testing their patch, but wanted to open this up to others who know
> > testing at gate better much better than myself.  Also encourage any
> > thoughts on the topic to be introduced to the etherpad [0]
> >
> > [0] https://etherpad.openstack.org/p/tripleo-heat-admin-security
> 
> > [1]
> https://gist.github.com/lukehinds/4cdb1bf4de526a049c51f05698b8b04f
> 
> >
> > --
> > Luke Hinds
> 
> -- 
> Cédric Jeanneret
> Software Engineer
> DFG:DF
> 
> 
> __
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe:
>

Re: [openstack-dev] [tripleo] Limiting sudo coverage of heat-admin / stack and other users.

2018-05-22 Thread Luke Hinds 
On Tue, May 22, 2018 at 5:27 AM, Cédric Jeanneret 
wrote:

>
>
> On 05/21/2018 03:49 PM, Luke Hinds wrote:
> > A few operators have requested if its possible to limit sudo's coverage
> > on both the under / overcloud. There is concern over `ALL=(ALL)
> > NOPASSWD:ALL` , which allows someone to  `sudo su`.
> >
> > This task has come under the care of the tripleo security squad.
> >
> > The work is being tracked and discussed here [0].
> >
> > So far it looks like the approach will be to use regexp within
> > /etc/sudoers.d/*., to narrow down as close as possible to the specific
> > commands called. Some services already do this with rootwrap:
> >
> > ironic ALL = (root) NOPASSWD: /usr/bin/ironic-rootwrap
> > /etc/ironic/rootwrap.conf *
> >
> > It's fairly easy to pick up a list of all sudo calls using a simple
> > script [1]
> >
> > The other prolific user of sudo is ansible / stack, for example:
> >
> > /bin/sh -c echo BECOME-SUCCESS-kldpbeueyodisjajjqthpafzadrncdff;
> > /usr/bin/python
> > /home/stack/.ansible/tmp/ansible-tmp-1526579105.0-
> 109863952786117/systemd.py;
> > rm -rf
> > "/home/stack/.ansible/tmp/ansible-tmp-1526579105.0-109863952786117/" >
> > /dev/null 2>&1
> >
> > My feelings here are to again use regexp around the immutable non random
> > parts of the command.  cjeanner also made some suggestions in the
> > etherpad [0].
>
> Might be a temporary way to limit the surface indeed, but an upstream
> change in Ansible would still be really nice. Predictable names is the
> only "right" way, although this will create a long sudo ruleset. A
> really long one to be honnest. Maintainability is also to be discussed
> in either way (maintain a couple of regexp vs 200+ rules.. hmmm).
>
>
As I understand it, the problem with predicable names is they also become
predictable to attackers (this would be the reason ansible adds in the
random string). It helps prevent someone creating a race condition to
replace the python script with something more nefarious. Its the same
reason commands such as mktemp exists.

>
> > However aside to the approach, we need to consider the impact locking
> > down might have should someone create a develop a new bit of code that
> > leverages commands wrapped in sudo and assumes ALL with be in place.
> > This of course will be blocked.
>
> This will indeed require some doc, as this is a "major" change. However,
> the use of regexp should somewhat limit the impact, especially since
> Ansible pushes its exec script in the same location.
> Even new parts should be allowed (that might be a bit of concern if we
> want to really dig in the consequences of a bad template being injected
> in some way [looking config-download ;)]).
> But at some point, we might also decide to let the OPs ensure their
> infra isn't compromised.
> Always the same thread-of with Security vs The World - convenience vs
> cumbersome management, and so on.
>
> >
> > Now my guess is that our CI would capture this as the deploy would
> > fail(?) and the developer should work out an entry is needed when
> > testing their patch, but wanted to open this up to others who know
> > testing at gate better much better than myself.  Also encourage any
> > thoughts on the topic to be introduced to the etherpad [0]
> >
> > [0] https://etherpad.openstack.org/p/tripleo-heat-admin-security
> > [1] https://gist.github.com/lukehinds/4cdb1bf4de526a049c51f05698b8b04f
> >
> > --
> > Luke Hinds
>
> --
> Cédric Jeanneret
> Software Engineer
> DFG:DF
>
>
> __
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
>


-- 
Luke Hinds | NFV Partner Engineering | CTO Office | Red Hat
e: lhi...@redhat.com | irc: lhinds @freenode | t: +44 12 52 36 2483
__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [tripleo] Limiting sudo coverage of heat-admin / stack and other users.

2018-05-21 Thread Cédric Jeanneret 


On 05/21/2018 03:49 PM, Luke Hinds wrote:
> A few operators have requested if its possible to limit sudo's coverage
> on both the under / overcloud. There is concern over `ALL=(ALL)
> NOPASSWD:ALL` , which allows someone to  `sudo su`.
> 
> This task has come under the care of the tripleo security squad.
> 
> The work is being tracked and discussed here [0].
> 
> So far it looks like the approach will be to use regexp within
> /etc/sudoers.d/*., to narrow down as close as possible to the specific
> commands called. Some services already do this with rootwrap:
> 
> ironic ALL = (root) NOPASSWD: /usr/bin/ironic-rootwrap
> /etc/ironic/rootwrap.conf *   
> 
> It's fairly easy to pick up a list of all sudo calls using a simple
> script [1]
> 
> The other prolific user of sudo is ansible / stack, for example:
> 
> /bin/sh -c echo BECOME-SUCCESS-kldpbeueyodisjajjqthpafzadrncdff;
> /usr/bin/python
> /home/stack/.ansible/tmp/ansible-tmp-1526579105.0-109863952786117/systemd.py;
> rm -rf
> "/home/stack/.ansible/tmp/ansible-tmp-1526579105.0-109863952786117/" >
> /dev/null 2>&1
> 
> My feelings here are to again use regexp around the immutable non random
> parts of the command.  cjeanner also made some suggestions in the
> etherpad [0].

Might be a temporary way to limit the surface indeed, but an upstream
change in Ansible would still be really nice. Predictable names is the
only "right" way, although this will create a long sudo ruleset. A
really long one to be honnest. Maintainability is also to be discussed
in either way (maintain a couple of regexp vs 200+ rules.. hmmm).

> 
> However aside to the approach, we need to consider the impact locking
> down might have should someone create a develop a new bit of code that
> leverages commands wrapped in sudo and assumes ALL with be in place.
> This of course will be blocked.

This will indeed require some doc, as this is a "major" change. However,
the use of regexp should somewhat limit the impact, especially since
Ansible pushes its exec script in the same location.
Even new parts should be allowed (that might be a bit of concern if we
want to really dig in the consequences of a bad template being injected
in some way [looking config-download ;)]).
But at some point, we might also decide to let the OPs ensure their
infra isn't compromised.
Always the same thread-of with Security vs The World - convenience vs
cumbersome management, and so on.

> 
> Now my guess is that our CI would capture this as the deploy would
> fail(?) and the developer should work out an entry is needed when
> testing their patch, but wanted to open this up to others who know
> testing at gate better much better than myself.  Also encourage any
> thoughts on the topic to be introduced to the etherpad [0]
> 
> [0] https://etherpad.openstack.org/p/tripleo-heat-admin-security
> [1] https://gist.github.com/lukehinds/4cdb1bf4de526a049c51f05698b8b04f
> 
> -- 
> Luke Hinds

-- 
Cédric Jeanneret
Software Engineer
DFG:DF



signature.asc
Description: OpenPGP digital signature
__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

[openstack-dev] [tripleo] Limiting sudo coverage of heat-admin / stack and other users.

2018-05-21 Thread Luke Hinds 
A few operators have requested if its possible to limit sudo's coverage on
both the under / overcloud. There is concern over `ALL=(ALL) NOPASSWD:ALL`
, which allows someone to  `sudo su`.

This task has come under the care of the tripleo security squad.

The work is being tracked and discussed here [0].

So far it looks like the approach will be to use regexp within
/etc/sudoers.d/*., to narrow down as close as possible to the specific
commands called. Some services already do this with rootwrap:

ironic ALL = (root) NOPASSWD: /usr/bin/ironic-rootwrap
/etc/ironic/rootwrap.conf *

It's fairly easy to pick up a list of all sudo calls using a simple script
[1]

The other prolific user of sudo is ansible / stack, for example:

/bin/sh -c echo BECOME-SUCCESS-kldpbeueyodisjajjqthpafzadrncdff;
/usr/bin/python
/home/stack/.ansible/tmp/ansible-tmp-1526579105.0-109863952786117/systemd.py;
rm -rf "/home/stack/.ansible/tmp/ansible-tmp-1526579105.0-109863952786117/"
> /dev/null 2>&1

My feelings here are to again use regexp around the immutable non random
parts of the command.  cjeanner also made some suggestions in the etherpad
[0].

However aside to the approach, we need to consider the impact locking down
might have should someone create a develop a new bit of code that leverages
commands wrapped in sudo and assumes ALL with be in place. This of course
will be blocked.

Now my guess is that our CI would capture this as the deploy would fail(?)
and the developer should work out an entry is needed when testing their
patch, but wanted to open this up to others who know testing at gate better
much better than myself.  Also encourage any thoughts on the topic to be
introduced to the etherpad [0]

[0] https://etherpad.openstack.org/p/tripleo-heat-admin-security
[1] https://gist.github.com/lukehinds/4cdb1bf4de526a049c51f05698b8b04f

-- 
Luke Hinds
__
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev