Skip to content

Commit

Permalink
Only allow safe context fields in notifications
Browse files Browse the repository at this point in the history 
Publishing a fully hydrated context object in a notification would give
someone with access to that notification the ability to impersonate the
original actor through inclusion of sensitive fields.

Now, instead, we pare down the context object to the bare minimum before
passing it for serialization in notification workflows.

Related-bug: 2030976
Change-Id: Ic94323658c89df1c1ff32f511ca23502317d0f00
  • Loading branch information
@jayofdoom
jayofdoom committed Aug 11, 2023
1 parent ec1c99d commit 1b31561
Show file tree
Hide file tree
Showing 3 changed files with 223 additions and 25 deletions.
48 changes: 47 additions & 1 deletion oslo_messaging/notify/notifier.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -296,7 +337,12 @@ def prepare(self, publisher_id=_marker, retry=_marker):
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,
Expand Down
34 changes: 17 additions & 17 deletions oslo_messaging/tests/notify/test_listener.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,18 +286,18 @@ def test_two_topics(self):
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)
Expand Down Expand Up @@ -326,23 +326,23 @@ def side_effect(target, ctxt, message, version, retry):
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)
Expand Down Expand Up @@ -414,16 +414,16 @@ def test_two_pools(self):
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")
self.assertFalse(listener2_thread.stop())
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})

Expand Down Expand Up @@ -452,22 +452,22 @@ def test_two_pools_three_listener(self):
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))

self.wait_for_messages(25, 'pool2')
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))

Expand All @@ -476,15 +476,15 @@ def mocked_endpoint_call(i):
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))

self.wait_for_messages(75, 'pool2')
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))

Expand Down
166 changes: 159 additions & 7 deletions oslo_messaging/tests/notify/test_notifier.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ class TestMessagingNotifier(test_utils.BaseTestCase):
]

_context = [
('ctxt', dict(ctxt={'user': 'bob'})),
('ctxt', dict(ctxt={'user_name': 'bob'})),
]

_retry = [
Expand Down Expand Up @@ -229,6 +229,157 @@ def test_notifier(self, mock_utcnow):
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):
Expand Down Expand Up @@ -328,12 +479,12 @@ def test_serializer(self, mock_utcnow):
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),
Expand All @@ -344,13 +495,14 @@ def test_serializer(self, mock_utcnow):
'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):
Expand Down

0 comments on commit 1b31561

Please sign in to comment.