298b337a16
The server fault "message" is always shown in the API server response, regardless of policy or user role. The fault "details" are only shown to users with the admin role when the fault code is 500. The problem with this is for non-nova exceptions, the fault message is a string-ified version of the exception (see nova.compute.utils.exception_to_dict) which can contain sensitive information which the non-admin owner of the server can see. This change adds a functional test to recreate the issue and a change to exception_to_dict which for the non-nova case changes the fault message by simply storing the exception type class name. Admins can still see the fault traceback in the "details" key of the fault dict in the server API response. Note that _get_fault_details is changed so that the details also includes the exception value which is what used to be in the fault message for non-nova exceptions. This is necessary so admins can still get the exception message with the traceback details. Note that nova exceptions with a %(reason)s replacement variable could potentially be leaking sensitive details as well but those would need to be cleaned up on a case-by-case basis since we don't want to change the behavior of all fault messages otherwise users might not see information like NoValidHost when their server goes to ERROR status during scheduling. SecurityImpact: This change contains a fix for CVE-2019-14433. Change-Id: I5e0a43ec59341c9ac62f89105ddf82c4a014df81 Closes-Bug: #1837877
115 lines
5.4 KiB
Python
115 lines
5.4 KiB
Python
# 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 nova import test
|
|
from nova.tests import fixtures as nova_fixtures
|
|
from nova.tests.functional import fixtures as func_fixtures
|
|
from nova.tests.functional import integrated_helpers
|
|
from nova.tests.unit.image import fake as fake_image
|
|
from nova.tests.unit import policy_fixture
|
|
|
|
|
|
class HypervisorError(Exception):
|
|
"""This is just used to make sure the exception type is in the fault."""
|
|
pass
|
|
|
|
|
|
class ServerFaultTestCase(test.TestCase,
|
|
integrated_helpers.InstanceHelperMixin):
|
|
"""Tests for the server faults reporting from the API."""
|
|
|
|
def setUp(self):
|
|
super(ServerFaultTestCase, self).setUp()
|
|
# Setup the standard fixtures.
|
|
fake_image.stub_out_image_service(self)
|
|
self.addCleanup(fake_image.FakeImageService_reset)
|
|
self.useFixture(nova_fixtures.NeutronFixture(self))
|
|
self.useFixture(func_fixtures.PlacementFixture())
|
|
self.useFixture(policy_fixture.RealPolicyFixture())
|
|
|
|
# Start the compute services.
|
|
self.start_service('conductor')
|
|
self.start_service('scheduler')
|
|
self.compute = self.start_service('compute')
|
|
api_fixture = self.useFixture(nova_fixtures.OSAPIFixture(
|
|
api_version='v2.1'))
|
|
self.api = api_fixture.api
|
|
self.admin_api = api_fixture.admin_api
|
|
|
|
def test_server_fault_non_nova_exception(self):
|
|
"""Creates a server using the non-admin user, then reboots it which
|
|
will generate a non-NovaException fault and put the instance into
|
|
ERROR status. Then checks that fault details are only visible to the
|
|
admin user.
|
|
"""
|
|
# Create the server with the non-admin user.
|
|
server = self._build_minimal_create_server_request(
|
|
self.api, 'test_server_fault_non_nova_exception',
|
|
image_uuid=fake_image.get_valid_image_id(),
|
|
networks=[{'port': nova_fixtures.NeutronFixture.port_1['id']}])
|
|
server = self.api.post_server({'server': server})
|
|
server = self._wait_for_state_change(self.admin_api, server, 'ACTIVE')
|
|
|
|
# Stop the server before rebooting it so that after the driver.reboot
|
|
# method raises an exception, the fake driver does not report the
|
|
# instance power state as running - that will make the compute manager
|
|
# set the instance vm_state to error.
|
|
self.api.post_server_action(server['id'], {'os-stop': None})
|
|
server = self._wait_for_state_change(self.admin_api, server, 'SHUTOFF')
|
|
|
|
# Stub out the compute driver reboot method to raise a non-nova
|
|
# exception to simulate some error from the underlying hypervisor
|
|
# which in this case we are going to say has sensitive content.
|
|
error_msg = 'sensitive info'
|
|
with mock.patch.object(
|
|
self.compute.manager.driver, 'reboot',
|
|
side_effect=HypervisorError(error_msg)) as mock_reboot:
|
|
reboot_request = {'reboot': {'type': 'HARD'}}
|
|
self.api.post_server_action(server['id'], reboot_request)
|
|
# In this case we wait for the status to change to ERROR using
|
|
# the non-admin user so we can assert the fault details. We also
|
|
# wait for the task_state to be None since the wrap_instance_fault
|
|
# decorator runs before the reverts_task_state decorator so we will
|
|
# be sure the fault is set on the server.
|
|
server = self._wait_for_server_parameter(
|
|
self.api, server, {'status': 'ERROR',
|
|
'OS-EXT-STS:task_state': None})
|
|
mock_reboot.assert_called_once()
|
|
# The server fault from the non-admin user API response should not
|
|
# have details in it.
|
|
self.assertIn('fault', server)
|
|
fault = server['fault']
|
|
self.assertNotIn('details', fault)
|
|
# And the sensitive details from the non-nova exception should not be
|
|
# in the message.
|
|
self.assertIn('message', fault)
|
|
self.assertNotIn(error_msg, fault['message'])
|
|
# The exception type class name should be in the message.
|
|
self.assertIn('HypervisorError', fault['message'])
|
|
|
|
# Get the server fault details for the admin user.
|
|
server = self.admin_api.get_server(server['id'])
|
|
fault = server['fault']
|
|
# The admin can see the fault details which includes the traceback.
|
|
self.assertIn('details', fault)
|
|
# The details also contain the exception message (which is not in the
|
|
# fault message).
|
|
self.assertIn(error_msg, fault['details'])
|
|
# Make sure the traceback is there by looking for part of it.
|
|
self.assertIn('in reboot_instance', fault['details'])
|
|
# The exception type class name should be in the message for the admin
|
|
# user as well since the fault handling code cannot distinguish who
|
|
# is going to see the message so it only sets class name.
|
|
self.assertIn('HypervisorError', fault['message'])
|