diff --git a/oslo_messaging/notify/notifier.py b/oslo_messaging/notify/notifier.py index 02ff643ed..5fa0ebeeb 100644 --- a/oslo_messaging/notify/notifier.py +++ b/oslo_messaging/notify/notifier.py @@ -171,6 +171,47 @@ def get_notification_transport(conf, url=None, allowed_remote_exmods=None): transport_cls=msg_transport.NotificationTransport) +def _sanitize_context(ctxt): + # NOTE(JayF): The below values are in the same order they are in + # oslo_context.context.RequestContext.__init__() + safe_keys = ( + 'user_id', + 'project_id', + 'domain_id', + 'user_domain_id', + 'project_domain_id', + 'request_id', + 'roles', + 'user_name', + 'project_name', + 'domain_name', + 'user_domain_name', + 'project_domain_name', + 'service_user_id', + 'service_user_domain_id', + 'service_user_domain_name', + 'service_project_id', + 'service_project_name', + 'service_project_domain_id', + 'service_project_domain_name', + 'service_roles', + 'global_request_id', + 'system_scope', + # NOTE(JayF) These have been renamed but may show up in notifications + 'user', + 'domain', + 'user_domain', + 'project_domain', + ) + ctxt_dict = ctxt if isinstance(ctxt, dict) else ctxt.to_dict() + safe_dict = {k: v for k, v in ctxt_dict.items() + if k in safe_keys} + if ctxt_dict is ctxt: + return safe_dict + else: + return ctxt.__class__.from_dict(safe_dict) + + class Notifier(object): """Send notification messages. @@ -296,7 +337,12 @@ class Notifier(object): def _notify(self, ctxt, event_type, payload, priority, publisher_id=None, retry=None): payload = self._serializer.serialize_entity(ctxt, payload) - ctxt = self._serializer.serialize_context(ctxt) + + # NOTE(JayF): We must remove secure information from notification + # payloads, otherwise we risk sending sensitive creds + # to a notification bus. + safe_ctxt = _sanitize_context(ctxt) + ctxt = self._serializer.serialize_context(safe_ctxt) msg = dict(message_id=str(uuid.uuid4()), publisher_id=publisher_id or self.publisher_id, diff --git a/oslo_messaging/tests/notify/test_listener.py b/oslo_messaging/tests/notify/test_listener.py index f0cabaec7..60bbf042d 100644 --- a/oslo_messaging/tests/notify/test_listener.py +++ b/oslo_messaging/tests/notify/test_listener.py @@ -286,18 +286,18 @@ class TestNotifyListener(test_utils.BaseTestCase, ListenerSetupMixin): listener_thread = self._setup_listener(transport, [endpoint], targets=targets) notifier = self._setup_notifier(transport, topics=['topic1']) - notifier.info({'ctxt': '1'}, 'an_event.start1', 'test') + notifier.info({'user_name': 'bob'}, 'an_event.start1', 'test') notifier = self._setup_notifier(transport, topics=['topic2']) - notifier.info({'ctxt': '2'}, 'an_event.start2', 'test') + notifier.info({'user_name': 'bob2'}, 'an_event.start2', 'test') self.wait_for_messages(2) self.assertFalse(listener_thread.stop()) endpoint.info.assert_has_calls([ - mock.call({'ctxt': '1'}, 'testpublisher', + mock.call({'user_name': 'bob'}, 'testpublisher', 'an_event.start1', 'test', {'timestamp': mock.ANY, 'message_id': mock.ANY}), - mock.call({'ctxt': '2'}, 'testpublisher', + mock.call({'user_name': 'bob2'}, 'testpublisher', 'an_event.start2', 'test', {'timestamp': mock.ANY, 'message_id': mock.ANY})], any_order=True) @@ -326,23 +326,23 @@ class TestNotifyListener(test_utils.BaseTestCase, ListenerSetupMixin): transport._send_notification = mock.MagicMock( side_effect=side_effect) - notifier.info({'ctxt': '0'}, + notifier.info({'user_name': 'bob0'}, 'an_event.start', 'test message default exchange') mock_notifier_exchange('exchange1') - notifier.info({'ctxt': '1'}, + notifier.info({'user_name': 'bob1'}, 'an_event.start', 'test message exchange1') mock_notifier_exchange('exchange2') - notifier.info({'ctxt': '2'}, + notifier.info({'user_name': 'bob2'}, 'an_event.start', 'test message exchange2') self.wait_for_messages(2) self.assertFalse(listener_thread.stop()) endpoint.info.assert_has_calls([ - mock.call({'ctxt': '1'}, 'testpublisher', 'an_event.start', + mock.call({'user_name': 'bob1'}, 'testpublisher', 'an_event.start', 'test message exchange1', {'timestamp': mock.ANY, 'message_id': mock.ANY}), - mock.call({'ctxt': '2'}, 'testpublisher', 'an_event.start', + mock.call({'user_name': 'bob2'}, 'testpublisher', 'an_event.start', 'test message exchange2', {'timestamp': mock.ANY, 'message_id': mock.ANY})], any_order=True) @@ -414,8 +414,8 @@ class TestNotifyListener(test_utils.BaseTestCase, ListenerSetupMixin): targets=targets, pool="pool2") notifier = self._setup_notifier(transport, topics=["topic"]) - notifier.info({'ctxt': '0'}, 'an_event.start', 'test message0') - notifier.info({'ctxt': '1'}, 'an_event.start', 'test message1') + notifier.info({'user_name': 'bob0'}, 'an_event.start', 'test message0') + notifier.info({'user_name': 'bob1'}, 'an_event.start', 'test message1') self.wait_for_messages(2, "pool1") self.wait_for_messages(2, "pool2") @@ -423,7 +423,7 @@ class TestNotifyListener(test_utils.BaseTestCase, ListenerSetupMixin): self.assertFalse(listener1_thread.stop()) def mocked_endpoint_call(i): - return mock.call({'ctxt': '%d' % i}, 'testpublisher', + return mock.call({'user_name': 'bob%d' % i}, 'testpublisher', 'an_event.start', 'test message%d' % i, {'timestamp': mock.ANY, 'message_id': mock.ANY}) @@ -452,14 +452,14 @@ class TestNotifyListener(test_utils.BaseTestCase, ListenerSetupMixin): targets=targets, pool="pool2") def mocked_endpoint_call(i): - return mock.call({'ctxt': '%d' % i}, 'testpublisher', + return mock.call({'user_name': 'bob%d' % i}, 'testpublisher', 'an_event.start', 'test message%d' % i, {'timestamp': mock.ANY, 'message_id': mock.ANY}) notifier = self._setup_notifier(transport, topics=["topic"]) mocked_endpoint1_calls = [] for i in range(0, 25): - notifier.info({'ctxt': '%d' % i}, 'an_event.start', + notifier.info({'user_name': 'bob%d' % i}, 'an_event.start', 'test message%d' % i) mocked_endpoint1_calls.append(mocked_endpoint_call(i)) @@ -467,7 +467,7 @@ class TestNotifyListener(test_utils.BaseTestCase, ListenerSetupMixin): listener2_thread.stop() for i in range(0, 25): - notifier.info({'ctxt': '%d' % i}, 'an_event.start', + notifier.info({'user_name': 'bob%d' % i}, 'an_event.start', 'test message%d' % i) mocked_endpoint1_calls.append(mocked_endpoint_call(i)) @@ -476,7 +476,7 @@ class TestNotifyListener(test_utils.BaseTestCase, ListenerSetupMixin): listener3_thread.stop() for i in range(0, 25): - notifier.info({'ctxt': '%d' % i}, 'an_event.start', + notifier.info({'user_name': 'bob%d' % i}, 'an_event.start', 'test message%d' % i) mocked_endpoint1_calls.append(mocked_endpoint_call(i)) @@ -484,7 +484,7 @@ class TestNotifyListener(test_utils.BaseTestCase, ListenerSetupMixin): listener3_thread.start() for i in range(0, 25): - notifier.info({'ctxt': '%d' % i}, 'an_event.start', + notifier.info({'user_name': 'bob%d' % i}, 'an_event.start', 'test message%d' % i) mocked_endpoint1_calls.append(mocked_endpoint_call(i)) diff --git a/oslo_messaging/tests/notify/test_notifier.py b/oslo_messaging/tests/notify/test_notifier.py index 330bdaba5..8a918fdcf 100644 --- a/oslo_messaging/tests/notify/test_notifier.py +++ b/oslo_messaging/tests/notify/test_notifier.py @@ -122,7 +122,7 @@ class TestMessagingNotifier(test_utils.BaseTestCase): ] _context = [ - ('ctxt', dict(ctxt={'user': 'bob'})), + ('ctxt', dict(ctxt={'user_name': 'bob'})), ] _retry = [ @@ -229,6 +229,157 @@ class TestMessagingNotifier(test_utils.BaseTestCase): TestMessagingNotifier.generate_scenarios() +class TestMessagingNotifierContextFiltering(test_utils.BaseTestCase): + + _v1 = [ + ('v1', dict(v1=True)), + ('not_v1', dict(v1=False)), + ] + + _v2 = [ + ('v2', dict(v2=True)), + ('not_v2', dict(v2=False)), + ] + + _publisher_id = [ + ('ctor_pub_id', dict(ctor_pub_id='test', + expected_pub_id='test')), + ('prep_pub_id', dict(prep_pub_id='test.localhost', + expected_pub_id='test.localhost')), + ('override', dict(ctor_pub_id='test', + prep_pub_id='test.localhost', + expected_pub_id='test.localhost')), + ] + + _topics = [ + ('no_topics', dict(topics=[])), + ('single_topic', dict(topics=['notifications'])), + ('multiple_topic2', dict(topics=['foo', 'bar'])), + ] + + _priority = [ + ('audit', dict(priority='audit')), + ('debug', dict(priority='debug')), + ('info', dict(priority='info')), + ('warn', dict(priority='warn')), + ('error', dict(priority='error')), + ('sample', dict(priority='sample')), + ('critical', dict(priority='critical')), + ] + + _payload = [ + ('payload', dict(payload={'foo': 'bar'})), + ] + + _context = [ + ('ctxt', dict(ctxt={'user_name': 'bob'})), + ] + + _retry = [ + ('unconfigured', dict()), + ('None', dict(retry=None)), + ('0', dict(retry=0)), + ('5', dict(retry=5)), + ] + + @classmethod + def generate_scenarios(cls): + cls.scenarios = testscenarios.multiply_scenarios(cls._v1, + cls._v2, + cls._publisher_id, + cls._topics, + cls._priority, + cls._payload, + cls._retry) + + def setUp(self): + super(TestMessagingNotifierContextFiltering, self).setUp() + + self.logger = self.useFixture(_ReRaiseLoggedExceptionsFixture()).logger + self.useFixture(fixtures.MockPatchObject( + messaging, 'LOG', self.logger)) + self.useFixture(fixtures.MockPatchObject( + msg_notifier, '_LOG', self.logger)) + + @mock.patch('oslo_utils.timeutils.utcnow') + def test_notifier(self, mock_utcnow): + ctxt = {'user_name': 'bob', 'secret_data': 'redact_me'} + safe_ctxt = {'user_name': 'bob'} + drivers = [] + if self.v1: + drivers.append('messaging') + if self.v2: + drivers.append('messagingv2') + + self.config(driver=drivers, + topics=self.topics, + group='oslo_messaging_notifications') + + transport = oslo_messaging.get_notification_transport(self.conf, + url='fake:') + + if hasattr(self, 'ctor_pub_id'): + notifier = oslo_messaging.Notifier(transport, + publisher_id=self.ctor_pub_id) + else: + notifier = oslo_messaging.Notifier(transport) + + prepare_kwds = {} + if hasattr(self, 'retry'): + prepare_kwds['retry'] = self.retry + if hasattr(self, 'prep_pub_id'): + prepare_kwds['publisher_id'] = self.prep_pub_id + if prepare_kwds: + notifier = notifier.prepare(**prepare_kwds) + + transport._send_notification = mock.Mock() + + message_id = uuid.uuid4() + uuid.uuid4 = mock.Mock(return_value=message_id) + + mock_utcnow.return_value = datetime.datetime.utcnow() + + message = { + 'message_id': str(message_id), + 'publisher_id': self.expected_pub_id, + 'event_type': 'test.notify', + 'priority': self.priority.upper(), + 'payload': self.payload, + 'timestamp': str(timeutils.utcnow()), + } + + sends = [] + if self.v1: + sends.append(dict(version=1.0)) + if self.v2: + sends.append(dict(version=2.0)) + + calls = [] + for send_kwargs in sends: + for topic in self.topics: + if hasattr(self, 'retry'): + send_kwargs['retry'] = self.retry + else: + send_kwargs['retry'] = -1 + target = oslo_messaging.Target(topic='%s.%s' % (topic, + self.priority)) + calls.append(mock.call(target, + safe_ctxt, + message, + **send_kwargs)) + + method = getattr(notifier, self.priority) + method(ctxt, 'test.notify', self.payload) + + uuid.uuid4.assert_called_once_with() + transport._send_notification.assert_has_calls(calls, any_order=True) + + self.assertTrue(notifier.is_enabled()) + + +TestMessagingNotifierContextFiltering.generate_scenarios() + + class TestMessagingNotifierRetry(test_utils.BaseTestCase): class TestingException(BaseException): @@ -328,12 +479,12 @@ class TestSerializer(test_utils.BaseTestCase): mock_utcnow.return_value = datetime.datetime.utcnow() serializer.serialize_context = mock.Mock() - serializer.serialize_context.return_value = dict(user='alice') + serializer.serialize_context.return_value = dict(user_name='alice') serializer.serialize_entity = mock.Mock() serializer.serialize_entity.return_value = 'sbar' - notifier.info(dict(user='bob'), 'test.notify', 'bar') + notifier.info(dict(user_name='bob'), 'test.notify', 'bar') message = { 'message_id': str(message_id), @@ -344,13 +495,14 @@ class TestSerializer(test_utils.BaseTestCase): 'timestamp': str(timeutils.utcnow()), } - self.assertEqual([(dict(user='alice'), message, 'INFO', -1)], + self.assertEqual([(dict(user_name='alice'), message, 'INFO', -1)], _impl_test.NOTIFICATIONS) uuid.uuid4.assert_called_once_with() - serializer.serialize_context.assert_called_once_with(dict(user='bob')) - serializer.serialize_entity.assert_called_once_with(dict(user='bob'), - 'bar') + serializer.serialize_context.assert_called_once_with( + dict(user_name='bob')) + serializer.serialize_entity.assert_called_once_with( + dict(user_name='bob'), 'bar') class TestNotifierTopics(test_utils.BaseTestCase):