On 31/03/2021 20:37, Ben Nemec wrote:
On 3/31/21 2:18 PM, Sean Mooney wrote:
the other anti patter to avoid is centralising all privsep cunciton in a set of common modules that are called into form differnt moduels. nova and neutorn are unfrotunetly both guilty of that too but eventully i would hope that that will be fixed.
I thought this was intentional. It's even mentioned in the docs as a way to make it clear that privileged calls will run in a different context: https://docs.openstack.org/oslo.privsep/victoria/user/index.html#using-a-pri...
yep we have talked about this before and said we shoudl fix that before. i dont have my old mailing list post but what we have found at least in nova centralising the calls encurages overly broad functions with far to much privaldge to be written. case in point novas mount functjion https://github.com/openstack/nova/blob/master/nova/privsep/fs.py#L30-L38 which take andy files system devnec enad mount point with any options you care to pass. our our chown , chmod and writefile commands https://github.com/openstack/nova/blob/master/nova/privsep/path.py#L28-L63 we even had to write a hacking check to stope people form importing modules form privesep using from nova.privsep import path since at the call path.writefile(...) would give no indication that its actuly privaldaged. we have a hacking check to enfore no aliasing of privsep import in nova https://github.com/openstack/nova/blob/master/nova/hacking/checks.py#L832-L8... privsep contntexts should be defiled at the top of project module namespace e.g. nova and ideally targeted privladge fucntion shoud be written in the module where they are used, using a context that provides only the permissions they need. ideally you should have privaldged in the name of the function too. os-vif does not quite get the nameing right but we do at least use a seperate prive spec context for the plugins vs common code and use inline privesep function instead of centralisting them https://github.com/openstack/os-vif/blob/master/vif_plug_linux_bridge/privse... https://github.com/openstack/os-vif/blob/master/vif_plug_ovs/privsep.py https://github.com/openstack/os-vif/blob/master/vif_plug_ovs/linux_net.py#L7... one of the imporant thing to not for example in the ovs plugin is we restrict the context to only be used by submodules of the plugin vif_plug = priv_context.PrivContext( "vif_plug_ovs", <----- this is what resticts what module the decorator can be used in. cfg_section="vif_plug_ovs_privileged", pypath=__name__ + ".vif_plug", capabilities=[c.CAP_NET_ADMIN], ) the enforcement is done by https://github.com/openstack/oslo.privsep/blob/83870bd2655f3250bb5d5aed7c986... it also only has cap_net_admin since that is the only capablity the ovs plugin needed. if the linuxbridge plugin need CAP_SYS_ADMIN for some reason we could add it to its context without affacting ovs's althoguht the correct thing to do would be to define a new context instead of extending the exsiging one. novas privsep context has far to many permission and shoudl be broken into at least 3 contexts https://github.com/openstack/nova/blob/master/nova/privsep/__init__.py sys_admin_pctxt = priv_context.PrivContext( 'nova', cfg_section='nova_sys_admin', pypath=__name__ + '.sys_admin_pctxt', capabilities=[capabilities.CAP_CHOWN, capabilities.CAP_DAC_OVERRIDE, capabilities.CAP_DAC_READ_SEARCH, capabilities.CAP_FOWNER, capabilities.CAP_NET_ADMIN, capabilities.CAP_SYS_ADMIN], ) should be sys_admin_pctxt = priv_context.PrivContext( 'nova', cfg_section='nova_sys_admin', pypath=__name__ + '.sys_admin_pctxt', capabilities=[capabilities.CAP_SYS_ADMIN], ) file_admin_pctxt = priv_context.PrivContext( 'nova', cfg_section='nova_sys_admin', pypath=__name__ + '.sys_admin_pctxt', capabilities=[capabilities.CAP_CHOWN, capabilities.CAP_DAC_OVERRIDE, capabilities.CAP_DAC_READ_SEARCH, capabilities.CAP_FOWNER, ], ) net_admin_pctxt = priv_context.PrivContext( 'nova', cfg_section='nova_sys_admin', pypath=__name__ + '.sys_admin_pctxt', capabilities=[capabilities.CAP_NET_ADMIN], ) neutron is in a slightly better shape since it has 3 contexts but its not really devied up correctly IMO https://github.com/openstack/neutron/blob/master/neutron/privileged/__init__... the default context still has far to many caps as noted in the to do. privsep as used today in nova and neutron provides far less security then the libary is capably of as the current usage is very primatie. unfortunetlly i dont think https://docs.openstack.org/oslo.privsep/victoria/user/index.html actully models the best practicies for defining, contexts or function or using them. its based on what nova did before we had learned how to actully use it. we should rewrite all of nova privsep useage but we have never had the time to do that. neutron is in better shape but it can still delete any file on disk or spawn any process with caps.CAP_SYS_ADMIN, caps.CAP_NET_ADMIN, caps.CAP_DAC_OVERRIDE, caps.CAP_DAC_READ_SEARCH, caps.CAP_SYS_PTRACE https://github.com/openstack/neutron/blob/master/neutron/privileged/agent/li... that process would not be fulll root but it would be close enough. having a centralised privsep module and 1 or few context tends to make people lazy and either not think what is the smalles set of permission i need or what is the smallest amount of function inputs i need. so neutron and nova might have almost got rid fo rootwap but they are not secure by any means in there current usage. i also dont know of any active explitis as a result of this, its greate that neutron is almost done moveing but i just wanted to highlight htat once all the calls are moved the real work or actully hardinging neutron and using privesep properly need to be done. if you just use privsep blinindly its really easy to end up with a less secure system then when you had rootwarp.