From 375a8452bbb2b3d12fbf82f725e977c3ce4b09b7 Mon Sep 17 00:00:00 2001 From: Rajat Jain Date: Sun, 18 Apr 2021 17:56:37 +0530 Subject: [PATCH] api: Log correct client IP if load balancer in use When Nova-Api runs behind the load balancer or Reverse proxy, Loadbalancer IP is getting logged in nova_api.log instead of end user source ip by RequestLog It should check for CONF.api.use_forwarded_for and then uses key 'HTTP_X_FORWARDED_FOR' to get the client ip. Co-Authored-By: melanie witt Closes-Bug: #1913605 Change-Id: Id2703ea4439d587a1a7a878796a79709fae5ea61 --- nova/api/openstack/requestlog.py | 14 ++++++++++++- .../unit/api/openstack/test_requestlog.py | 21 ++++++++++++++++++- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/nova/api/openstack/requestlog.py b/nova/api/openstack/requestlog.py index 3c41f114f0a5..dd0d207e34f4 100644 --- a/nova/api/openstack/requestlog.py +++ b/nova/api/openstack/requestlog.py @@ -21,6 +21,9 @@ import webob.exc from nova.api.openstack import wsgi from nova.api import wsgi as base_wsgi +import nova.conf + +CONF = nova.conf.CONF # TODO(sdague) maybe we can use a better name here for the logger LOG = logging.getLogger(__name__) @@ -65,8 +68,17 @@ class RequestLog(base_wsgi.Middleware): # wsgi stack, res is going to be an empty dict for the # fallback logging. So never count on it having attributes. status = getattr(res, "status", "500 Error").split(None, 1)[0] + + remote_address = req.environ.get('REMOTE_ADDR', '-') + + # If the API is configured to treat the X-Forwarded-For header as the + # canonical remote address, use its value instead. + if CONF.api.use_forwarded_for: + remote_address = req.environ.get( + 'HTTP_X_FORWARDED_FOR', remote_address) + data = { - 'REMOTE_ADDR': req.environ.get('REMOTE_ADDR', '-'), + 'REMOTE_ADDR': remote_address, 'REQUEST_METHOD': req.environ['REQUEST_METHOD'], 'REQUEST_URI': self._get_uri(req.environ), 'status': status, diff --git a/nova/tests/unit/api/openstack/test_requestlog.py b/nova/tests/unit/api/openstack/test_requestlog.py index 75ce6a2b6c2d..0ea91439cc24 100644 --- a/nova/tests/unit/api/openstack/test_requestlog.py +++ b/nova/tests/unit/api/openstack/test_requestlog.py @@ -58,7 +58,7 @@ class TestRequestLogMiddleware(testtools.TestCase): """ emit.return_value = True - self.useFixture(fixtures.ConfFixture()) + conf = self.useFixture(fixtures.ConfFixture()).conf self.useFixture(fixtures.RPCFixture('nova.test')) api = self.useFixture(fixtures.OSAPIFixture()).api @@ -73,6 +73,25 @@ class TestRequestLogMiddleware(testtools.TestCase): '"GET /" status: 200 len: %s' % content_length) self.assertIn(log1, self.stdlog.logger.output) + # Verify handling of X-Forwarded-For header, example: load balancer. + # First, try without setting CONF.api.use_forwarded_for, it should not + # use the header value. + headers = {'X-Forwarded-For': '1.2.3.4'} + resp = api.api_request('/', strip_version=True, headers=headers) + content_length = resp.headers['content-length'] + log2 = ('INFO [nova.api.openstack.requestlog] 127.0.0.1 ' + '"GET /" status: 200 len: %s' % content_length) + self.assertIn(log2, self.stdlog.logger.output) + + # Now set CONF.api.use_forwarded_for, it should use the header value. + conf.set_override('use_forwarded_for', True, 'api') + headers = {'X-Forwarded-For': '1.2.3.4'} + resp = api.api_request('/', strip_version=True, headers=headers) + content_length = resp.headers['content-length'] + log3 = ('INFO [nova.api.openstack.requestlog] 1.2.3.4 ' + '"GET /" status: 200 len: %s' % content_length) + self.assertIn(log3, self.stdlog.logger.output) + @mock.patch('nova.api.openstack.requestlog.RequestLog._should_emit') def test_logs_mv(self, emit): """Ensure logs register microversion if passed.