Make serialize_args handle exception messages safely

We have recently move towards handling exceptions that may be viewed
by API users in a way that results in either the exc.format_message()
text, or just the exception class name (without details) in order to
avoid leaking sensitive information out of the API. This makes the
serialize_args decorator behave that way. Note that because for a
remotable method, serialize_args gets called twice (once on the remote
side before the call and again on the local side at replay), which
means we need to gate that message-or-class-name behavior on whether
or not we are seeing a true Exception object.

Change-Id: I22fb253616443766ef21692735b80faa284fa26d

nova/objects/base.py

@@ -332,7 +332,20 @@ def serialize_args(fn):
                 else arg for arg in args]
         for k, v in kwargs.items():
             if k == 'exc_val' and v:
-                kwargs[k] = six.text_type(v)
+                try:
+                    # NOTE(danms): When we run this for a remotable method,
+                    # we need to attempt to format_message() the exception to
+                    # get the sanitized message, and if it's not a
+                    # NovaException, fall back to just the exception class
+                    # name. However, a remotable will end up calling this again
+                    # on the other side of the RPC call, so we must not try
+                    # to do that again, otherwise we will always end up with
+                    # just str. So, only do that if exc_val is an Exception
+                    # class.
+                    kwargs[k] = (v.format_message() if isinstance(v, Exception)
+                                 else v)
+                except Exception:
+                    kwargs[k] = v.__class__.__name__
             elif k == 'exc_tb' and v and not isinstance(v, six.string_types):
                 kwargs[k] = ''.join(traceback.format_tb(v))
             elif isinstance(v, datetime.datetime):

nova/tests/unit/objects/test_instance_action.py

@@ -366,15 +366,16 @@ class _TestInstanceActionEventObject(object):
                                                   mock_pack):
         mock_format.return_value = 'traceback'
         mock_pack.side_effect = test.TestingException
+        exc = exception.NotFound()
         self.assertRaises(
             test.TestingException,
             instance_action.InstanceActionEvent.event_finish_with_failure,
             self.context, 'fake-uuid', 'fake-event',
-            exc_val=mock.sentinel.exc_val,
+            exc_val=exc,
             exc_tb=mock.sentinel.exc_tb)
         mock_pack.assert_called_once_with(self.context, 'fake-uuid',
                                           'fake-event',
-                                          exc_val=str(mock.sentinel.exc_val),
+                                          exc_val=exc.format_message(),
                                           exc_tb='traceback')
         mock_format.assert_called_once_with(mock.sentinel.exc_tb)
 

nova/tests/unit/objects/test_objects.py

@@ -28,6 +28,7 @@ from oslo_versionedobjects import fixture
 import six
 
 from nova import context
+from nova import exception
 from nova import objects
 from nova.objects import base
 from nova.objects import fields
@@ -975,7 +976,7 @@ class TestArgsSerializer(test.NoDBTestCase):
         super(TestArgsSerializer, self).setUp()
         self.now = timeutils.utcnow()
         self.str_now = utils.strtime(self.now)
-        self.unicode_str = u'\xF0\x9F\x92\xA9'
+        self.exc = exception.NotFound()
 
     @base.serialize_args
     def _test_serialize_args(self, *args, **kwargs):
@@ -984,14 +985,26 @@ class TestArgsSerializer(test.NoDBTestCase):
             self.assertEqual(expected_args[index], val)
 
         expected_kwargs = {'a': 'untouched', 'b': self.str_now,
-                           'c': self.str_now, 'exc_val': self.unicode_str}
+                           'c': self.str_now}
+
+        nonnova = kwargs.pop('nonnova', None)
+        if nonnova:
+            expected_kwargs['exc_val'] = 'TestingException'
+        else:
+            expected_kwargs['exc_val'] = self.exc.format_message()
         for key, val in kwargs.items():
             self.assertEqual(expected_kwargs[key], val)
 
     def test_serialize_args(self):
         self._test_serialize_args('untouched', self.now, self.now,
                                   a='untouched', b=self.now, c=self.now,
-                                  exc_val=self.unicode_str)
+                                  exc_val=self.exc)
+
+    def test_serialize_args_non_nova_exception(self):
+        self._test_serialize_args('untouched', self.now, self.now,
+                                  a='untouched', b=self.now, c=self.now,
+                                  exc_val=test.TestingException('foo'),
+                                  nonnova=True)
 
 
 class TestRegistry(test.NoDBTestCase):