Trivial rootwrap -> privsep replacement
This change replaces all uses of rootwrap with a trivial privsep-based equivalent. This replacement simply executes commands as the privsep user *without any additional checks*. There are 2 reasons why this is a reasonable thing to do: 1. We don't have a good workflow for merging rootwrap filter changes into parent projects (nova/cinder) for a loosely-coupled library like os-brick. 2. The previous situation was also insecure. The os-brick.filters rootwrap config permitted commands like "dd" and "cp" with any arguments, as root. This would have posed only a mild inconvenience to an attacker. With privsep we can at least (in principle) limit the commands to the privsep uid/gid and Linux capabilities (CAP_SYS_ADMIN by default with this change). This change addresses the urgency of (1). Later refactors will take greater advantage of privsep to address (2). Change-Id: I0af542eba97d2f89b1c283bf1e1e985d9690f5de Depends-On: I90dc41bc77993bd83b80c92286e015e14f290b45 # nova: nova.conf: Set privsep_rootwrap.helper_command Depends-On: I4e333e73ddfd45c045b9d32dac1506fc25858c4d # nova: Add os-brick rootwrap filter for privsep Depends-On: I8a0b1728cc66c4861f69623b1b16b1f759b57b25 # cinder: cinder.conf: Set privsep_rootwrap.helper_command Depends-On: I3b2e337321875cf4abc0ab9b44fe17cf9327d88b # cinder: Add os-brick rootwrap filter for privsep Depends-On: I4299c2fc059807610f83e12a2d470e020930c64c # privsep: Switch to msgpack for serialization Depends-On: Ied1ef4fc945e18516b39d1f20d58425cb633dc74 # requirements: require oslo.privsep>=1.5.0 for msgpack fixchanges/24/277224/27
Angus Lees
7 years ago
committed by
Matt Riedemann
parent
9fc9cc4a00
commit
dbf77fba10
@ -0,0 +1,23 @@
|
||||
# Licensed under the Apache License, Version 2.0 (the "License"); you may
|
||||
# not use this file except in compliance with the License. You may obtain
|
||||
# a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
|
||||
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
from oslo_privsep import capabilities as c
|
||||
from oslo_privsep import priv_context
|
||||
|
||||
# It is expected that most (if not all) os-brick operations can be
|
||||
# executed with these privileges.
|
||||
default = priv_context.PrivContext(
|
||||
__name__,
|
||||
cfg_section='privsep_osbrick',
|
||||
pypath=__name__ + '.default',
|
||||
capabilities=[c.CAP_SYS_ADMIN],
|
||||
)
|
||||
@ -0,0 +1,82 @@
|
||||
# Licensed under the Apache License, Version 2.0 (the "License"); you may
|
||||
# not use this file except in compliance with the License. You may obtain
|
||||
# a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
|
||||
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
"""Just in case it wasn't clear, this is a massive security back-door.
|
||||
|
||||
`execute_root()` (or the same via `execute(run_as_root=True)`) allows
|
||||
any command to be run as the privileged user (default "root"). This
|
||||
is intended only as an expedient transition and should be removed
|
||||
ASAP.
|
||||
|
||||
This is not completely unreasonable because:
|
||||
|
||||
1. We have no tool/workflow for merging changes to rootwrap filter
|
||||
configs from os-brick into nova/cinder, which makes it difficult
|
||||
to evolve these loosely coupled projects.
|
||||
|
||||
2. Let's not pretend the earlier situation was any better. The
|
||||
rootwrap filters config contained several entries like "allow cp as
|
||||
root with any arguments", etc, and would have posed only a mild
|
||||
inconvenience to an attacker. At least with privsep we can (in
|
||||
principle) run the "root" commands as a non-root uid, with
|
||||
restricted Linux capabilities.
|
||||
|
||||
The plan is to switch os-brick to privsep using this module (removing
|
||||
the urgency of (1)), then work on the larger refactor that addresses
|
||||
(2) in followup changes.
|
||||
|
||||
"""
|
||||
|
||||
import six
|
||||
|
||||
from oslo_concurrency import processutils as putils
|
||||
from oslo_utils import strutils
|
||||
|
||||
from os_brick import privileged
|
||||
|
||||
|
||||
# Entrypoint used for rootwrap.py transition code. Don't use this for
|
||||
# other purposes, since it will be removed when we think the
|
||||
# transition is finished.
|
||||
def execute(*cmd, **kwargs):
|
||||
"""NB: Raises processutils.ProcessExecutionError on failure."""
|
||||
run_as_root = kwargs.pop('run_as_root', False)
|
||||
kwargs.pop('root_helper', None)
|
||||
|
||||
try:
|
||||
if run_as_root:
|
||||
return execute_root(*cmd, **kwargs)
|
||||
else:
|
||||
return putils.execute(*cmd, **kwargs)
|
||||
except OSError as e:
|
||||
# Note:
|
||||
# putils.execute('bogus', run_as_root=True)
|
||||
# raises ProcessExecutionError(exit_code=1) (because there's a
|
||||
# "sh -c bogus" involved in there somewhere, but:
|
||||
# putils.execute('bogus', run_as_root=False)
|
||||
# raises OSError(not found).
|
||||
#
|
||||
# Lots of code in os-brick catches only ProcessExecutionError
|
||||
# and never encountered the latter when using rootwrap.
|
||||
# Rather than fix all the callers, we just always raise
|
||||
# ProcessExecutionError here :(
|
||||
|
||||
sanitized_cmd = strutils.mask_password(' '.join(cmd))
|
||||
raise putils.ProcessExecutionError(
|
||||
cmd=sanitized_cmd, description=six.text_type(e))
|
||||
|
||||
|
||||
# See comment on `execute`
|
||||
@privileged.default.entrypoint
|
||||
def execute_root(*cmd, **kwargs):
|
||||
"""NB: Raises processutils.ProcessExecutionError/OSError on failure."""
|
||||
return putils.execute(*cmd, shell=False, run_as_root=False, **kwargs)
|
||||
@ -0,0 +1,59 @@
|
||||
# Licensed under the Apache License, Version 2.0 (the "License"); you may
|
||||
# not use this file except in compliance with the License. You may obtain
|
||||
# a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
|
||||
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import mock
|
||||
|
||||
from oslo_concurrency import processutils as putils
|
||||
|
||||
from os_brick import privileged
|
||||
from os_brick.privileged import rootwrap as priv_rootwrap
|
||||
from os_brick.tests import base
|
||||
|
||||
|
||||
class PrivRootwrapTestCase(base.TestCase):
|
||||
def setUp(self):
|
||||
super(PrivRootwrapTestCase, self).setUp()
|
||||
|
||||
# Bypass privsep and run these simple functions in-process
|
||||
# (allows reading back the modified state of mocks)
|
||||
privileged.default.set_client_mode(False)
|
||||
self.addCleanup(privileged.default.set_client_mode, True)
|
||||
|
||||
@mock.patch('os_brick.privileged.rootwrap.execute_root')
|
||||
@mock.patch('oslo_concurrency.processutils.execute')
|
||||
def test_execute(self, mock_putils_exec, mock_exec_root):
|
||||
priv_rootwrap.execute('echo', 'foo', run_as_root=False)
|
||||
self.assertFalse(mock_exec_root.called)
|
||||
|
||||
priv_rootwrap.execute('echo', 'foo', run_as_root=True,
|
||||
root_helper='baz', check_exit_code=0)
|
||||
mock_exec_root.assert_called_once_with(
|
||||
'echo', 'foo', check_exit_code=0)
|
||||
|
||||
@mock.patch('oslo_concurrency.processutils.execute')
|
||||
def test_execute_root(self, mock_putils_exec):
|
||||
priv_rootwrap.execute_root('echo', 'foo', check_exit_code=0)
|
||||
mock_putils_exec.assert_called_once_with(
|
||||
'echo', 'foo', check_exit_code=0, shell=False, run_as_root=False)
|
||||
|
||||
# Exact exception isn't particularly important, but these
|
||||
# should be errors:
|
||||
self.assertRaises(TypeError,
|
||||
priv_rootwrap.execute_root, 'foo', shell=True)
|
||||
self.assertRaises(TypeError,
|
||||
priv_rootwrap.execute_root, 'foo', run_as_root=True)
|
||||
|
||||
@mock.patch('oslo_concurrency.processutils.execute',
|
||||
side_effect=OSError(42, 'mock error'))
|
||||
def test_oserror_raise(self, mock_putils_exec):
|
||||
self.assertRaises(putils.ProcessExecutionError,
|
||||
priv_rootwrap.execute, 'foo')
|
||||
Loading…
Reference in New Issue