diff --git a/cyborg/accelerator/drivers/gpu/utils.py b/cyborg/accelerator/drivers/gpu/utils.py index 102de908..09ab84db 100644 --- a/cyborg/accelerator/drivers/gpu/utils.py +++ b/cyborg/accelerator/drivers/gpu/utils.py @@ -16,18 +16,19 @@ """ Utils for GPU driver. """ -import re -import subprocess - +from oslo_concurrency import processutils from oslo_log import log as logging from oslo_serialization import jsonutils +import re + from cyborg.accelerator.common import utils from cyborg.common import constants from cyborg.objects.driver_objects import driver_attach_handle from cyborg.objects.driver_objects import driver_controlpath_id from cyborg.objects.driver_objects import driver_deployable from cyborg.objects.driver_objects import driver_device +import cyborg.privsep LOG = logging.getLogger(__name__) @@ -42,15 +43,27 @@ GPU_INFO_PATTERN = re.compile("(?P[0-9a-fA-F]{4}:[0-9a-fA-F]{2}:" # GPU. +@cyborg.privsep.sys_admin_pctxt.entrypoint +def lspci_privileged(): + cmd = ['lspci', '-nnn', '-D'] + return processutils.execute(*cmd) + + +def get_pci_devices(pci_flags, vendor_id=None): + device_for_vendor_out = [] + all_device_out = [] + lspci_out = lspci_privileged()[0].split('\n') + for i in range(len(lspci_out)): + if any(x in lspci_out[i] for x in pci_flags): + all_device_out.append(lspci_out[i]) + if vendor_id and vendor_id in lspci_out[i]: + device_for_vendor_out.append(lspci_out[i]) + return device_for_vendor_out if vendor_id else all_device_out + + def discover_vendors(): - cmd = "sudo lspci -nnn -D | grep -E '%s'" - cmd = cmd % "|".join(GPU_FLAGS) - # FIXME(wangzhh): Use oslo.privsep instead of subprocess here to prevent - # shell injection attacks. - p = subprocess.Popen(cmd, stdout=subprocess.PIPE, shell=True) - p.wait() - gpus = p.stdout.readlines() vendors = set() + gpus = get_pci_devices(GPU_FLAGS) for gpu in gpus: m = GPU_INFO_PATTERN.match(gpu) if m: @@ -59,17 +72,9 @@ def discover_vendors(): return vendors -def discover_gpus(vender_id=None): - cmd = "sudo lspci -nnn -D| grep -E '%s'" - cmd = cmd % "|".join(GPU_FLAGS) - if vender_id: - cmd = cmd + "| grep " + vender_id - # FIXME(wangzhh): Use oslo.privsep instead of subprocess here to prevent - # shell injection attacks. - p = subprocess.Popen(cmd, stdout=subprocess.PIPE, shell=True) - p.wait() - gpus = p.stdout.readlines() +def discover_gpus(vendor_id=None): gpu_list = [] + gpus = get_pci_devices(GPU_FLAGS, vendor_id) for gpu in gpus: m = GPU_INFO_PATTERN.match(gpu) if m: diff --git a/cyborg/cmd/agent.py b/cyborg/cmd/agent.py index d7f06c28..2358afe5 100644 --- a/cyborg/cmd/agent.py +++ b/cyborg/cmd/agent.py @@ -13,21 +13,23 @@ """The Cyborg Agent Service.""" +import shlex import sys from oslo_config import cfg +from oslo_privsep import priv_context from oslo_service import service from cyborg.common import constants from cyborg.common import service as cyborg_service - CONF = cfg.CONF def main(): # Parse config file and command line options, then start logging cyborg_service.prepare_service(sys.argv) + priv_context.init(root_helper=shlex.split('sudo')) mgr = cyborg_service.RPCService('cyborg.agent.manager', 'AgentManager', diff --git a/cyborg/privsep/__init__.py b/cyborg/privsep/__init__.py new file mode 100644 index 00000000..7020a8eb --- /dev/null +++ b/cyborg/privsep/__init__.py @@ -0,0 +1,31 @@ +# Copyright 2019 ZTE Corporation +# 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. + +"""Setup privsep decorator.""" + +from oslo_privsep import capabilities +from oslo_privsep import priv_context + +sys_admin_pctxt = priv_context.PrivContext( + 'cyborg', + cfg_section='cyborg_sys_admin', + pypath=__name__ + '.sys_admin_pctxt', + # TODO(yumeng): + # CAP_SYS_ADMIN has a lot of scary powers, so + # consider breaking this out into a separate minimal context. + capabilities=[capabilities.CAP_CHOWN, + capabilities.CAP_DAC_OVERRIDE, + capabilities.CAP_DAC_READ_SEARCH, + capabilities.CAP_FOWNER, + capabilities.CAP_SYS_ADMIN], +) diff --git a/cyborg/tests/unit/accelerator/drivers/gpu/test_utils.py b/cyborg/tests/unit/accelerator/drivers/gpu/test_utils.py index d3b6dcf6..a3789ac9 100644 --- a/cyborg/tests/unit/accelerator/drivers/gpu/test_utils.py +++ b/cyborg/tests/unit/accelerator/drivers/gpu/test_utils.py @@ -13,7 +13,6 @@ # under the License. import mock -import subprocess from oslo_serialization import jsonutils @@ -43,17 +42,17 @@ class TestGPUDriverUtils(base.TestCase): super(TestGPUDriverUtils, self).setUp() self.p = p() - @mock.patch.object(subprocess, 'Popen', autospec=True) - def test_discover_vendors(self, mock_popen): - mock_popen.return_value = self.p - gpu_venders = utils.discover_vendors() - self.assertEqual(1, len(gpu_venders)) + @mock.patch('cyborg.accelerator.drivers.gpu.utils.lspci_privileged') + def test_discover_vendors(self, mock_devices): + mock_devices.return_value = self.p.stdout.readlines() + gpu_vendors = utils.discover_vendors() + self.assertEqual(1, len(gpu_vendors)) - @mock.patch.object(subprocess, 'Popen', autospec=True) - def test_discover_gpus(self, mock_popen): - mock_popen.return_value = self.p - vender_id = '10de' - gpu_list = utils.discover_gpus(vender_id) + @mock.patch('cyborg.accelerator.drivers.gpu.utils.lspci_privileged') + def test_discover_gpus(self, mock_devices_for_vendor): + mock_devices_for_vendor.return_value = self.p.stdout.readlines() + vendor_id = '10de' + gpu_list = utils.discover_gpus(vendor_id) self.assertEqual(1, len(gpu_list)) attach_handle_list = [ {'attach_type': 'PCI', diff --git a/releasenotes/notes/implement_oslo_privsep-4fc6e15360c92772.yaml b/releasenotes/notes/implement_oslo_privsep-4fc6e15360c92772.yaml new file mode 100644 index 00000000..180e2231 --- /dev/null +++ b/releasenotes/notes/implement_oslo_privsep-4fc6e15360c92772.yaml @@ -0,0 +1,14 @@ +--- +security: + - | + Privsep transitions. Cyborg is transitioning from using the older + style rootwrap privilege escalation path to the new style Oslo privsep + path. This should improve performance and security of Cyborg + in the long term. + - | + Privsep daemons are now started by Cyborg when required. These + daemons can be started via rootwrap if required. rootwrap configs + therefore need to be updated to include new privsep daemon invocations. + - | + Use oslo.privsep instead of subprocess to execute sudo related shell + operations can prevent shell injection attacks. diff --git a/requirements.txt b/requirements.txt index fcedc7f2..01757913 100644 --- a/requirements.txt +++ b/requirements.txt @@ -28,3 +28,4 @@ jsonpatch>=1.16,!=1.20 # BSD psutil>=3.2.2 # BSD mock>=2.0.0 # BSD python-glanceclient>=2.3.0 # Apache-2.0 +oslo.privsep>=1.32.0 # Apache-2.0