openstack
/
oslo.messaging
Code Issues Proposed changes

Merge "Only allow safe context fields in notifications"

This commit is contained in:
Zuul 2023-08-17 12:43:26 +00:00 committed by Gerrit Code Review
parent fa15630041 1b315615e7
commit 3485301b18
3 changed files with 223 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

48
oslo_messaging/notify/notifier.py
View File

@ -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,

34
oslo_messaging/tests/notify/test_listener.py
View File

@ -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))

166
oslo_messaging/tests/notify/test_notifier.py
View File

@ -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):