From 22e07ff1a4f4bef7ae0e315ea8ef99e78cc05581 Mon Sep 17 00:00:00 2001 From: Volker Theile Date: Mon, 8 Oct 2018 09:28:57 +0200 Subject: [PATCH] mgr/dashboard: Audit REST API calls Fixes: https://tracker.ceph.com/issues/36193 Enable API auditing with 'ceph dashboard set-audit-api-enabled true' (default is false). If you do not want to log the request payload, then disable it via 'set-audit-api-log-payload false' (default is true). Example output: 2018-10-08 10:25:21.850994 mgr.x [INF] [DASHBOARD] from='https://[::1]:44410' path='/api/auth' method='POST' user='None' params='{"username": "admin", "password": "***", "stay_signed_in": false}' Signed-off-by: Volker Theile --- doc/mgr/dashboard.rst | 26 +++++ .../mgr/dashboard/controllers/__init__.py | 33 ++---- src/pybind/mgr/dashboard/module.py | 2 + src/pybind/mgr/dashboard/settings.py | 4 + src/pybind/mgr/dashboard/tests/helper.py | 6 +- .../mgr/dashboard/tests/test_api_auditing.py | 107 ++++++++++++++++++ src/pybind/mgr/dashboard/tests/test_tools.py | 26 ++++- src/pybind/mgr/dashboard/tools.py | 56 +++++++-- 8 files changed, 225 insertions(+), 35 deletions(-) create mode 100644 src/pybind/mgr/dashboard/tests/test_api_auditing.py diff --git a/doc/mgr/dashboard.rst b/doc/mgr/dashboard.rst index 5473a7cbdeb..ac21ab42200 100644 --- a/doc/mgr/dashboard.rst +++ b/doc/mgr/dashboard.rst @@ -514,3 +514,29 @@ to use hyperlinks that include your prefix, you can set the ceph config set mgr mgr/dashboard/url_prefix $PREFIX so you can access the dashboard at ``http://$IP:$PORT/$PREFIX/``. + + +Auditing +-------- + +The REST API is capable of logging PUT, POST and DELETE requests to the Ceph +audit log. This feature is disabled by default, but can be enabled with the +following command:: + + $ ceph dashboard set-audit-api-enabled + +If enabled, the following parameters are logged per each request: + +* from - The origin of the request, e.g. https://[::1]:44410 +* path - The REST API path, e.g. /api/auth +* method - e.g. PUT, POST or DELETE +* user - The name of the user, otherwise 'None' + +The logging of the request payload (the arguments and their values) is enabled +by default. Execute the following command to disable this behaviour:: + + $ ceph dashboard set-audit-api-log-payload + +A log entry may look like this:: + + 2018-10-22 15:27:01.302514 mgr.x [INF] [DASHBOARD] from='https://[::ffff:127.0.0.1]:37022' path='/api/rgw/user/klaus' method='PUT' user='admin' params='{"max_buckets": "1000", "display_name": "Klaus Mustermann", "uid": "klaus", "suspended": "0", "email": "klaus.mustermann@ceph.com"}' diff --git a/src/pybind/mgr/dashboard/controllers/__init__.py b/src/pybind/mgr/dashboard/controllers/__init__.py index 5426ee47067..4aaf84ea72a 100644 --- a/src/pybind/mgr/dashboard/controllers/__init__.py +++ b/src/pybind/mgr/dashboard/controllers/__init__.py @@ -21,11 +21,8 @@ import cherrypy from .. import logger from ..security import Scope, Permission -from ..settings import Settings -from ..tools import wraps, getargspec, TaskManager -from ..exceptions import ViewCacheNoDataException, DashboardException, \ - ScopeNotValid, PermissionNotValid -from ..services.exception import serialize_dashboard_exception +from ..tools import wraps, getargspec, TaskManager, get_request_body_params +from ..exceptions import ScopeNotValid, PermissionNotValid from ..services.auth import AuthManager, JwtManager @@ -520,7 +517,7 @@ class BaseController(object): return result @staticmethod - def _request_wrapper(func, method, json_response): + def _request_wrapper(func, method, json_response): # pylint: disable=unused-argument @wraps(func) def inner(*args, **kwargs): for key, value in kwargs.items(): @@ -529,27 +526,11 @@ class BaseController(object): or isinstance(value, str): kwargs[key] = unquote(value) - if method in ['GET', 'DELETE']: - ret = func(*args, **kwargs) - - elif cherrypy.request.headers.get('Content-Type', '') == \ - 'application/x-www-form-urlencoded': - ret = func(*args, **kwargs) - - else: - content_length = int(cherrypy.request.headers['Content-Length']) - body = cherrypy.request.body.read(content_length) - if not body: - ret = func(*args, **kwargs) - else: - try: - data = json.loads(body.decode('utf-8')) - except Exception as e: - raise cherrypy.HTTPError(400, 'Failed to decode JSON: {}' - .format(str(e))) - kwargs.update(data.items()) - ret = func(*args, **kwargs) + # Process method arguments. + params = get_request_body_params(cherrypy.request) + kwargs.update(params) + ret = func(*args, **kwargs) if isinstance(ret, bytes): ret = ret.decode('utf-8') if json_response: diff --git a/src/pybind/mgr/dashboard/module.py b/src/pybind/mgr/dashboard/module.py index 801725386a6..c08f2b4f05e 100644 --- a/src/pybind/mgr/dashboard/module.py +++ b/src/pybind/mgr/dashboard/module.py @@ -151,6 +151,8 @@ class CherryPyConfig(object): 'application/json', 'application/javascript', ], + 'tools.json_in.on': True, + 'tools.json_in.force': False } if ssl: diff --git a/src/pybind/mgr/dashboard/settings.py b/src/pybind/mgr/dashboard/settings.py index 953e918234b..ccde39ffd31 100644 --- a/src/pybind/mgr/dashboard/settings.py +++ b/src/pybind/mgr/dashboard/settings.py @@ -21,6 +21,10 @@ class Options(object): ENABLE_BROWSABLE_API = (True, bool) REST_REQUESTS_TIMEOUT = (45, int) + # API auditing + AUDIT_API_ENABLED = (False, bool) + AUDIT_API_LOG_PAYLOAD = (True, bool) + # RGW settings RGW_API_HOST = ('', str) RGW_API_PORT = (80, int) diff --git a/src/pybind/mgr/dashboard/tests/helper.py b/src/pybind/mgr/dashboard/tests/helper.py index 1a8ea7a381b..2efeba816bb 100644 --- a/src/pybind/mgr/dashboard/tests/helper.py +++ b/src/pybind/mgr/dashboard/tests/helper.py @@ -40,7 +40,11 @@ class ControllerTestCase(helper.CPWebCase): cherrypy.tools.authenticate = AuthManagerTool() cherrypy.tools.dashboard_exception_handler = HandlerWrapperTool(dashboard_exception_handler, priority=31) - cherrypy.config.update({'error_page.default': json_error_page}) + cherrypy.config.update({ + 'error_page.default': json_error_page, + 'tools.json_in.on': True, + 'tools.json_in.force': False + }) super(ControllerTestCase, self).__init__(*args, **kwargs) def _request(self, url, method, data=None): diff --git a/src/pybind/mgr/dashboard/tests/test_api_auditing.py b/src/pybind/mgr/dashboard/tests/test_api_auditing.py new file mode 100644 index 00000000000..d22410e847b --- /dev/null +++ b/src/pybind/mgr/dashboard/tests/test_api_auditing.py @@ -0,0 +1,107 @@ +# -*- coding: utf-8 -*- +from __future__ import absolute_import + +import re +import json +import cherrypy +import mock + +from .helper import ControllerTestCase +from ..controllers import RESTController, Controller +from ..tools import RequestLoggingTool +from .. import mgr + + +# pylint: disable=W0613 +@Controller('/foo', secure=False) +class FooResource(RESTController): + def create(self, password): + pass + + def get(self, key): + pass + + def delete(self, key): + pass + + def set(self, key, password, secret_key=None): + pass + + +class ApiAuditingTest(ControllerTestCase): + settings = {} + + def __init__(self, *args, **kwargs): + cherrypy.tools.request_logging = RequestLoggingTool() + cherrypy.config.update({'tools.request_logging.on': True}) + super(ApiAuditingTest, self).__init__(*args, **kwargs) + + @classmethod + def mock_set_config(cls, key, val): + cls.settings[key] = val + + @classmethod + def mock_get_config(cls, key, default=None): + return cls.settings.get(key, default) + + @classmethod + def setUpClass(cls): + mgr.get_config.side_effect = cls.mock_get_config + mgr.set_config.side_effect = cls.mock_set_config + + @classmethod + def setup_server(cls): + cls.setup_controllers([FooResource]) + + def setUp(self): + mgr.cluster_log = mock.Mock() + mgr.set_config('AUDIT_API_ENABLED', True) + mgr.set_config('AUDIT_API_LOG_PAYLOAD', True) + + def _validate_cluster_log_msg(self, path, method, user, params): + channel, _, msg = mgr.cluster_log.call_args_list[0][0] + self.assertEqual(channel, 'audit') + pattern = r'^\[DASHBOARD\] from=\'(.+)\' path=\'(.+)\' ' \ + 'method=\'(.+)\' user=\'(.+)\' params=\'(.+)\'$' + m = re.match(pattern, msg) + self.assertEqual(m.group(2), path) + self.assertEqual(m.group(3), method) + self.assertEqual(m.group(4), user) + self.assertDictEqual(json.loads(m.group(5)), params) + + def test_no_audit(self): + mgr.set_config('AUDIT_API_ENABLED', False) + self._delete('/foo/test1') + mgr.cluster_log.assert_not_called() + + def test_no_payload(self): + mgr.set_config('AUDIT_API_LOG_PAYLOAD', False) + self._delete('/foo/test1') + _, _, msg = mgr.cluster_log.call_args_list[0][0] + self.assertNotIn('params=', msg) + + def test_no_audit_get(self): + self._get('/foo/test1') + mgr.cluster_log.assert_not_called() + + def test_audit_put(self): + self._put('/foo/test1', {'password': 'y', 'secret_key': 1234}) + mgr.cluster_log.assert_called_once() + self._validate_cluster_log_msg('/foo/test1', 'PUT', 'None', + {'key': 'test1', + 'password': '***', + 'secret_key': '***'}) + + def test_audit_post(self): + with mock.patch('dashboard.services.auth.JwtManager.get_username', + return_value='hugo'): + self._post('/foo?password=1234') + mgr.cluster_log.assert_called_once() + self._validate_cluster_log_msg('/foo', 'POST', 'hugo', + {'password': '***'}) + + def test_audit_delete(self): + self._delete('/foo/test1') + mgr.cluster_log.assert_called_once() + self._validate_cluster_log_msg('/foo/test1', 'DELETE', + 'None', {'key': 'test1'}) diff --git a/src/pybind/mgr/dashboard/tests/test_tools.py b/src/pybind/mgr/dashboard/tests/test_tools.py index bd27822931e..7cc2b0c6aa1 100644 --- a/src/pybind/mgr/dashboard/tests/test_tools.py +++ b/src/pybind/mgr/dashboard/tests/test_tools.py @@ -11,7 +11,8 @@ from ..services.exception import handle_rados_error from .helper import ControllerTestCase from ..controllers import RESTController, ApiController, Controller, \ BaseController, Proxy -from ..tools import is_valid_ipv6_address, dict_contains_path +from ..tools import is_valid_ipv6_address, dict_contains_path, \ + RequestLoggingTool # pylint: disable=W0613 @@ -146,6 +147,29 @@ class RESTControllerTest(ControllerTestCase): GenerateControllerRoutesController +class RequestLoggingToolTest(ControllerTestCase): + + def __init__(self, *args, **kwargs): + cherrypy.tools.request_logging = RequestLoggingTool() + cherrypy.config.update({'tools.request_logging.on': True}) + super(RequestLoggingToolTest, self).__init__(*args, **kwargs) + + @classmethod + def setup_server(cls): + cls.setup_controllers([FooResource]) + + def test_is_logged(self): + with patch('logging.Logger.debug') as mock_logger_debug: + self._put('/foo/0', {'newdata': 'xyz'}) + self.assertStatus(200) + call_args_list = mock_logger_debug.call_args_list + _, host, _, method, user, path = call_args_list[0][0] + self.assertEqual(host, '127.0.0.1') + self.assertEqual(method, 'PUT') + self.assertIsNone(user) + self.assertEqual(path, '/foo/0') + + class TestFunctions(unittest.TestCase): def test_is_valid_ipv6_address(self): diff --git a/src/pybind/mgr/dashboard/tools.py b/src/pybind/mgr/dashboard/tools.py index 662fc5cd955..ee8de3b9658 100644 --- a/src/pybind/mgr/dashboard/tools.py +++ b/src/pybind/mgr/dashboard/tools.py @@ -3,6 +3,7 @@ from __future__ import absolute_import import sys import inspect +import json import functools import collections @@ -15,8 +16,9 @@ import socket from six.moves import urllib import cherrypy -from . import logger +from . import logger, mgr from .exceptions import ViewCacheNoDataException +from .settings import Settings from .services.auth import JwtManager @@ -35,12 +37,31 @@ class RequestLoggingTool(cherrypy.Tool): def request_begin(self): req = cherrypy.request user = JwtManager.get_username() - if user: - logger.debug("[%s:%s] [%s] [%s] %s", req.remote.ip, - req.remote.port, req.method, user, req.path_info) - else: - logger.debug("[%s:%s] [%s] %s", req.remote.ip, - req.remote.port, req.method, req.path_info) + # Log the request. + logger.debug('[%s:%s] [%s] [%s] %s', req.remote.ip, req.remote.port, + req.method, user, req.path_info) + # Audit the request. + if Settings.AUDIT_API_ENABLED and req.method not in ['GET']: + url = build_url(req.remote.ip, scheme=req.scheme, + port=req.remote.port) + msg = '[DASHBOARD] from=\'{}\' path=\'{}\' method=\'{}\' ' \ + 'user=\'{}\''.format(url, req.path_info, req.method, user) + if Settings.AUDIT_API_LOG_PAYLOAD: + params = req.params if req.params else {} + params.update(get_request_body_params(req)) + # Hide sensitive data like passwords, secret keys, ... + # Extend the list of patterns to search for if necessary. + # Currently parameters like this are processed: + # - secret_key + # - user_password + # - new_passwd_to_login + keys = [] + for key in ['password', 'passwd', 'secret']: + keys.extend([x for x in params.keys() if key in x]) + for key in keys: + params[key] = '***' + msg = '{} params=\'{}\''.format(msg, json.dumps(params)) + mgr.cluster_log('audit', mgr.CLUSTER_LOG_PRIO_INFO, msg) def request_error(self): self._request_log(logger.error) @@ -718,3 +739,24 @@ def str_to_bool(val): if isinstance(val, bool): return val return bool(strtobool(val)) + + +def get_request_body_params(request): + """ + Helper function to get parameters from the request body. + :param request The CherryPy request object. + :type request: cherrypy.Request + :return: A dictionary containing the parameters. + :rtype: dict + """ + params = {} + if request.method not in request.methods_with_bodies: + return params + + content_type = request.headers.get('Content-Type', '') + if content_type in ['application/json', 'text/javascript']: + if not hasattr(request, 'json'): + raise cherrypy.HTTPError(400, 'Expected JSON body') + params.update(request.json.items()) + + return params -- 2.39.5