diff options
| -rw-r--r-- | ironic/drivers/modules/agent_base_vendor.py | 10 | ||||
| -rw-r--r-- | ironic/tests/db/utils.py | 1 | ||||
| -rw-r--r-- | ironic/tests/drivers/test_agent_base_vendor.py | 24 | ||||
| -rw-r--r-- | releasenotes/notes/fix-cve-2016-4985-b62abae577025365.yaml | 11 |
4 files changed, 39 insertions, 7 deletions
diff --git a/ironic/drivers/modules/agent_base_vendor.py b/ironic/drivers/modules/agent_base_vendor.py index 12b3e7060..24973761e 100644 --- a/ironic/drivers/modules/agent_base_vendor.py +++ b/ironic/drivers/modules/agent_base_vendor.py @@ -16,12 +16,13 @@ # License for the specific language governing permissions and limitations # under the License. - +import ast import time from oslo_config import cfg from oslo_log import log from oslo_utils import excutils +from oslo_utils import strutils import retrying from ironic.common import boot_devices @@ -436,9 +437,14 @@ class BaseAgentVendor(base.VendorInterface): LOG.info(_LI('Initial lookup for node %s succeeded, agent is running ' 'and waiting for commands'), node.uuid) + ndict = node.as_dict() + if not context.show_password: + ndict['driver_info'] = ast.literal_eval( + strutils.mask_password(ndict['driver_info'], "******")) + return { 'heartbeat_timeout': CONF.agent.heartbeat_timeout, - 'node': node.as_dict() + 'node': ndict, } def _get_completed_cleaning_command(self, task): diff --git a/ironic/tests/db/utils.py b/ironic/tests/db/utils.py index 7ea0c5811..4bfdb5f51 100644 --- a/ironic/tests/db/utils.py +++ b/ironic/tests/db/utils.py @@ -152,6 +152,7 @@ def get_test_agent_driver_info(): return { 'deploy_kernel': 'glance://deploy_kernel_uuid', 'deploy_ramdisk': 'glance://deploy_ramdisk_uuid', + 'ipmi_password': 'foo', } diff --git a/ironic/tests/drivers/test_agent_base_vendor.py b/ironic/tests/drivers/test_agent_base_vendor.py index d89dc1bfc..fdd7f341b 100644 --- a/ironic/tests/drivers/test_agent_base_vendor.py +++ b/ironic/tests/drivers/test_agent_base_vendor.py @@ -15,6 +15,7 @@ # License for the specific language governing permissions and limitations # under the License. +import copy import time import types @@ -91,7 +92,8 @@ class TestBaseAgentVendor(db_base.DbTestCase): @mock.patch('ironic.drivers.modules.agent_base_vendor.BaseAgentVendor' '._find_node_by_macs', autospec=True) - def test_lookup_v2(self, find_mock): + def _test_lookup_v2(self, find_mock, show_password=True): + self.context.show_password = show_password kwargs = { 'version': '2', 'inventory': { @@ -108,10 +110,20 @@ class TestBaseAgentVendor(db_base.DbTestCase): ] } } + # NOTE(jroll) apparently as_dict() returns a dict full of references + expected = copy.deepcopy(self.node.as_dict()) + if not show_password: + expected['driver_info']['ipmi_password'] = '******' find_mock.return_value = self.node with task_manager.acquire(self.context, self.node.uuid) as task: node = self.passthru.lookup(task.context, **kwargs) - self.assertEqual(self.node.as_dict(), node['node']) + self.assertEqual(expected, node['node']) + + def test_lookup_v2_show_password(self): + self._test_lookup_v2(show_password=True) + + def test_lookup_v2_hide_password(self): + self._test_lookup_v2(show_password=False) def test_lookup_v2_missing_inventory(self): with task_manager.acquire(self.context, self.node.uuid) as task: @@ -136,9 +148,11 @@ class TestBaseAgentVendor(db_base.DbTestCase): @mock.patch.object(objects.Node, 'get_by_uuid') def test_lookup_v2_with_node_uuid(self, mock_get_node): + self.context.show_password = True + expected = copy.deepcopy(self.node.as_dict()) kwargs = { 'version': '2', - 'node_uuid': 'fake uuid', + 'node_uuid': 'fake-uuid', 'inventory': { 'interfaces': [ { @@ -156,8 +170,8 @@ class TestBaseAgentVendor(db_base.DbTestCase): mock_get_node.return_value = self.node with task_manager.acquire(self.context, self.node.uuid) as task: node = self.passthru.lookup(task.context, **kwargs) - self.assertEqual(self.node.as_dict(), node['node']) - mock_get_node.assert_called_once_with(mock.ANY, 'fake uuid') + self.assertEqual(expected, node['node']) + mock_get_node.assert_called_once_with(mock.ANY, 'fake-uuid') @mock.patch.object(objects.port.Port, 'get_by_address', spec_set=types.FunctionType) diff --git a/releasenotes/notes/fix-cve-2016-4985-b62abae577025365.yaml b/releasenotes/notes/fix-cve-2016-4985-b62abae577025365.yaml new file mode 100644 index 000000000..6127f61e1 --- /dev/null +++ b/releasenotes/notes/fix-cve-2016-4985-b62abae577025365.yaml @@ -0,0 +1,11 @@ +--- +security: + - A critical security vulnerability (CVE-2016-4985) was fixed in this + release. Previously, a client with network access to the ironic-api service + was able to bypass Keystone authentication and retrieve all information + about any Node registered with Ironic, if they knew (or were able to guess) + the MAC address of a network card belonging to that Node, by sending a + crafted POST request to the /v1/drivers/$DRIVER_NAME/vendor_passthru + resource. Ironic's policy.json configuration is now respected when + responding to this request such that, if passwords should be masked for + other requests, they are also masked for this request. |