From 08fad7496612962418c81c2d981069bde275157b Mon Sep 17 00:00:00 2001 From: Michael Johnson Date: Thu, 30 Jul 2020 13:32:19 -0700 Subject: [PATCH] Fix accepting 'insert_headers' when unsupported This patch fixes the Octavia listener API from accepting the 'insert_headers' parameter for protocols that do not support header insertion. Change-Id: I4ec2299b64b180f8b2d8f0b8485a6be9fe32d2eb Story: 2007967 Task: 40464 --- octavia/api/v2/controllers/listener.py | 11 ++- octavia/common/constants.py | 5 ++ .../tests/functional/api/v2/test_listener.py | 73 +++++++++++++++++-- ...r-insertion-mismatch-e3aeb5f5fee0348b.yaml | 5 ++ 4 files changed, 83 insertions(+), 11 deletions(-) create mode 100644 releasenotes/notes/fix-protocol-header-insertion-mismatch-e3aeb5f5fee0348b.yaml diff --git a/octavia/api/v2/controllers/listener.py b/octavia/api/v2/controllers/listener.py index bcfee80e79..53bf4f2ccd 100644 --- a/octavia/api/v2/controllers/listener.py +++ b/octavia/api/v2/controllers/listener.py @@ -130,6 +130,11 @@ class ListenersController(base.BaseController): listener_dict.get('insert_headers')) def _validate_insert_headers(self, insert_header_list, listener_protocol): + if (listener_protocol not in + constants.LISTENER_PROTOCOLS_SUPPORTING_HEADER_INSERTION): + raise exceptions.InvalidOption( + value='insert-headers', + option=('a %s protocol listener.' % listener_protocol)) if list(set(insert_header_list) - ( set(constants.SUPPORTED_HTTP_HEADERS + constants.SUPPORTED_SSL_HEADERS))): @@ -173,9 +178,9 @@ class ListenersController(base.BaseController): # Check for UDP compatibility if (listener_protocol == constants.PROTOCOL_UDP and self._is_tls_or_insert_header(listener_dict)): - raise exceptions.ValidationException(detail=_( - "%s protocol listener does not support TLS or header " - "insertion.") % constants.PROTOCOL_UDP) + raise exceptions.ValidationException( + detail=_("%s protocol listener does not " + "support TLS.") % constants.PROTOCOL_UDP) # Check for TLS disabled if (not CONF.api_settings.allow_tls_terminated_listeners and diff --git a/octavia/common/constants.py b/octavia/common/constants.py index 2251b7d71e..41e7429079 100644 --- a/octavia/common/constants.py +++ b/octavia/common/constants.py @@ -844,3 +844,8 @@ VIP_SECURITY_GROUP_PREFIX = 'lb-' AMP_BASE_PORT_PREFIX = 'octavia-lb-vrrp-' OCTAVIA_OWNED = 'octavia_owned' + +# Sadly in the LBaaS v2 API, header insertions are on the listener objects +# but they should be on the pool. Dealing with it until v3. +LISTENER_PROTOCOLS_SUPPORTING_HEADER_INSERTION = [PROTOCOL_HTTP, + PROTOCOL_TERMINATED_HTTPS] diff --git a/octavia/tests/functional/api/v2/test_listener.py b/octavia/tests/functional/api/v2/test_listener.py index 70b3f0d552..1671b1f5ed 100644 --- a/octavia/tests/functional/api/v2/test_listener.py +++ b/octavia/tests/functional/api/v2/test_listener.py @@ -672,13 +672,10 @@ class TestListener(base.BaseAPITest): 'protocol_port': 6666, 'connection_limit': 10, 'default_tls_container_ref': uuidutils.generate_uuid(), 'sni_container_refs': [sni1, sni2], - 'insert_headers': { - "X-Forwarded-Port": "true", - "X-Forwarded-For": "true"}, + 'insert_headers': {}, 'loadbalancer_id': self.lb_id} - expect_error_msg = ( - "Validation failure: %s protocol listener does not support TLS or " - "header insertion.") % constants.PROTOCOL_UDP + expect_error_msg = ("Validation failure: %s protocol listener does " + "not support TLS.") % constants.PROTOCOL_UDP res = self.post(self.LISTENERS_PATH, self._build_body(req_dict), status=400, expect_errors=True) self.assertEqual(expect_error_msg, res.json['faultstring']) @@ -1208,6 +1205,33 @@ class TestListener(base.BaseAPITest): self.create_listener(constants.PROTOCOL_TCP, 80, self.lb_id, allowed_cidrs=allowed_cidrs) + def _test_negative_create_with_headers(self, protocol): + req_dict = {'name': 'listener1', 'default_pool_id': None, + 'description': 'desc1', + 'admin_state_up': False, + 'protocol': protocol, + 'protocol_port': 6666, 'connection_limit': 10, + 'insert_headers': { + "X-Forwarded-Port": "true", + "X-Forwarded-For": "true"}, + 'loadbalancer_id': self.lb_id} + res = self.post(self.LISTENERS_PATH, self._build_body(req_dict), + status=400) + self.assertIn(protocol, res.json['faultstring']) + self.assert_correct_status(lb_id=self.lb_id) + + def test_negative_create_HTTPS_with_headers(self): + self._test_negative_create_with_headers(constants.PROTOCOL_HTTPS) + + def test_negative_create_PROXY_with_headers(self): + self._test_negative_create_with_headers(constants.PROTOCOL_PROXY) + + def test_negative_create_TCP_with_headers(self): + self._test_negative_create_with_headers(constants.PROTOCOL_TCP) + + def test_negative_create_UDP_with_headers(self): + self._test_negative_create_with_headers(constants.PROTOCOL_UDP) + def test_update_allowed_cidrs(self): allowed_cidrs = ['10.0.1.0/24', '10.0.2.0/24'] new_cidrs = ['10.0.1.0/24', '10.0.3.0/24'] @@ -2301,7 +2325,9 @@ class TestListener(base.BaseAPITest): lb_listener = {'protocol': 'HTTP', 'protocol_port': 80, 'loadbalancer_id': self.lb_id, - 'insert_headers': {'X-Forwarded-For': 'true'}} + 'insert_headers': {'X-Forwarded-For': 'true', + 'X-Forwarded-Port': 'true', + 'X-Forwarded-Proto': 'true'}} body = self._build_body(lb_listener) self.post(self.LISTENERS_PATH, body, status=201) @@ -2346,7 +2372,9 @@ class TestListener(base.BaseAPITest): constants.PROTOCOL_HTTP, 80, self.lb_id) self.set_lb_status(self.lb_id) new_listener = self._build_body( - {'insert_headers': {'X-Forwarded-For': 'true'}}) + {'insert_headers': {'X-Forwarded-For': 'true', + 'X-Forwarded-Port': 'true', + 'X-Forwarded-Proto': 'true'}}) listener_path = self.LISTENER_PATH.format( listener_id=listener['listener'].get('id')) update_listener = self.put( @@ -2397,6 +2425,35 @@ class TestListener(base.BaseAPITest): # the status. self.put(listener_path, new_listener, status=400).json + def _test_update_protocol_insert_headers_mismatch(self, protocol): + listener = self.create_listener( + protocol, 80, self.lb_id) + self.set_lb_status(self.lb_id) + new_listener = self._build_body( + {'insert_headers': {'X-Forwarded-Port': 'true'}}) + listener_path = self.LISTENER_PATH.format( + listener_id=listener['listener'].get('id')) + update_listener = self.put( + listener_path, new_listener, status=400).json + self.assertIn(protocol, update_listener['faultstring']) + self.assert_correct_status(lb_id=self.lb_id) + + def test_update_protocol_HTTPS_insert_headers(self): + self._test_update_protocol_insert_headers_mismatch( + constants.PROTOCOL_HTTPS) + + def test_update_protocol_PROXY_insert_headers(self): + self._test_update_protocol_insert_headers_mismatch( + constants.PROTOCOL_PROXY) + + def test_update_protocol_TCP_insert_headers(self): + self._test_update_protocol_insert_headers_mismatch( + constants.PROTOCOL_TCP) + + def test_update_protocol_UDP_insert_headers(self): + self._test_update_protocol_insert_headers_mismatch( + constants.PROTOCOL_UDP) + def _getStats(self, listener_id): res = self.get(self.LISTENER_PATH.format( listener_id=listener_id + "/stats")) diff --git a/releasenotes/notes/fix-protocol-header-insertion-mismatch-e3aeb5f5fee0348b.yaml b/releasenotes/notes/fix-protocol-header-insertion-mismatch-e3aeb5f5fee0348b.yaml new file mode 100644 index 0000000000..25a268ea41 --- /dev/null +++ b/releasenotes/notes/fix-protocol-header-insertion-mismatch-e3aeb5f5fee0348b.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Fixed an issue where listener "insert_headers" parameter was accepted for + protocols that do not support header insertion.