Adding support for barbican host href to be derived from wsgi request

Currently barbican provides hostname part of hrefs returned in response
based on host_href value defined in barbican.conf.

This approach would not work if barbican API needs to be accessed via
public or internal endpoint as they can be different endpoints in
control planes. The endpoint used by client depends on which network client
is making the API request. For same reasons, keystone also allows different
endpoint for a service to expose as public or internal interface in service
catalog.

To allow that kind of deployment model for barbican service, now enhancing
its logic to derive this hostname (http_scheme+host+port) information from
wsgi requests when host_href value is not set in barbican.conf. So deployment
requiring this behavior can leave host_href blank in their barbican.conf. The
host_href needs to be set empty as not setting it results in default.

Generally in this kind of deployment, proxy (e.g. haproxy) will set
appropriate host, http scheme header. Request url received at barbican side
will have the client IP address and scheme inserted directly inside it.
Reference: https://en.wikipedia.org/wiki/X-Forwarded-For

Updated existing 'change host header' related functional test to skip when
host_href is not set in barbican server side. Added new functional tests when
hrefs are derived from wsgi request. New tests are skipped when host_href is
set at server side.

Added a flag in barbican-functional.conf to indicate barbican server setting
Default is to use CONF.host_href value. Explicit flag is added as functional
test setup may not always have barbican server conf available locally.

Change-Id: Idb8e62867f6cbd457eb64ea31500e93e74d247ea
Closes-Bug: 1541118

barbican/api/controllers/versions.py

@@ -44,7 +44,9 @@ def _version_not_found():
 def _get_versioned_url(version):
     if version[-1] != '/':
         version += '/'
-    return parse.urljoin(CONF.host_href, version)
+    # If host_href is not set in barbican conf, then derive it from request url
+    host_part = utils.get_base_url_from_request()
+    return parse.urljoin(host_part, version)
 
 
 class BaseVersionController(object):

barbican/common/utils.py

@@ -24,6 +24,7 @@ import uuid
 from oslo_log import log
 import pecan
 import six
+from six.moves.urllib import parse
 
 from barbican.common import config
 from barbican import i18n as u
@@ -55,9 +56,30 @@ def allow_all_content_types(f):
     return _do_allow_certain_content_types(f, mimetypes.types_map.values())
 
 
+def get_base_url_from_request():
+    """Derive base url from wsgi request if CONF.host_href is not set
+
+    Use host.href as base URL if its set in barbican.conf.
+    If its not set, then derives value from wsgi request. WSGI request uses
+    HOST header or HTTP_X_FORWARDED_FOR header (in case of proxy) for host +
+    port part of its url. Proxies can also set HTTP_X_FORWARDED_PROTO header
+    for indicating http vs https.
+
+    Some of unit tests does not have pecan context that's why using request
+    attr check on pecan instance.
+    """
+    if not CONF.host_href and hasattr(pecan.request, 'url'):
+        p_url = parse.urlsplit(pecan.request.url)
+        base_url = '%s://%s' % (p_url.scheme, p_url.netloc)
+        return base_url
+    else:  # when host_href is set or flow is not within wsgi request context
+        return CONF.host_href
+
+
 def hostname_for_refs(resource=None):
     """Return the HATEOAS-style return URI reference for this service."""
-    ref = ['{base}/{version}'.format(base=CONF.host_href, version=API_VERSION)]
+    base_url = get_base_url_from_request()
+    ref = ['{base}/{version}'.format(base=base_url, version=API_VERSION)]
     if resource:
         ref.append('/' + resource)
     return ''.join(ref)

barbican/common/validators.py

@@ -840,7 +840,7 @@ class ContainerValidator(ValidatorBase):
 
         # Ensure that our secret refs are valid relative to our config, no
         # spoofing allowed!
-        configured_host_href = CONF.host_href
+        configured_host_href = utils.get_base_url_from_request()
         for secret_ref in secret_refs:
             if configured_host_href not in secret_ref.get('secret_ref'):
                 raise exception.UnsupportedField(

barbican/tests/api/controllers/test_versions.py

@@ -13,12 +13,17 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 from barbican.api import controllers
+from barbican.common import utils as cmn_utils
 from barbican.tests import utils
 
 
 class WhenTestingVersionsResource(utils.BarbicanAPIBaseTestCase):
     root_controller = controllers.versions.VersionsController()
 
+    def tearDown(self):
+        super(WhenTestingVersionsResource, self).tearDown()
+        cmn_utils.CONF.clear_override('host_href')
+
     def test_should_return_multiple_choices_on_get(self):
         resp = self.app.get('/')
         self.assertEqual(300, resp.status_int)
@@ -46,6 +51,34 @@ class WhenTestingVersionsResource(utils.BarbicanAPIBaseTestCase):
         self.assertEqual(1, len(v1_info['media-types']))
         self.assertEqual('application/json', v1_info['media-types'][0]['base'])
 
+    def test_when_host_href_is_not_set_in_conf(self):
+        cmn_utils.CONF.set_override('host_href', '', enforce_type=True)
+        host_hdr = 'http://myproxy.server.com:9311'
+        utils.mock_pecan_request(self, host=host_hdr)
+        dummy_root = 'http://mylocalhost:9999'
+        resp = self.app.get(dummy_root)
+
+        versions_response = resp.json['versions']['values']
+
+        for v_info in versions_response:
+            self.assertIn(host_hdr, v_info['links'][0]['href'])
+            self.assertNotIn(dummy_root, v_info['links'][0]['href'])
+
+    def test_when_host_href_is_set_in_conf(self):
+        host_href = 'http://myapp.server.com:9311/'
+        cmn_utils.CONF.set_override('host_href', host_href, enforce_type=True)
+        host_hdr = 'http://myproxy.server.com:9311'
+        utils.mock_pecan_request(self, host=host_hdr)
+        dummy_root = 'http://mylocalhost:9999'
+        resp = self.app.get(dummy_root)
+
+        versions_response = resp.json['versions']['values']
+
+        for v_info in versions_response:
+            self.assertIn(host_href, v_info['links'][0]['href'])
+            self.assertNotIn(dummy_root, v_info['links'][0]['href'])
+            self.assertNotIn(host_hdr, v_info['links'][0]['href'])
+
 
 class WhenTestingV1Resource(utils.BarbicanAPIBaseTestCase):
 

barbican/tests/common/test_utils.py

@@ -51,6 +51,43 @@ class WhenTestingHostnameForRefsGetter(test_utils.BaseTestCase):
         self.assertEqual("{0}/{1}".format(self.host, self.version), uri)
 
 
+class WhenTestingHostByWsgiRequestForRefsGetter(test_utils.BaseTestCase):
+
+    def setUp(self):
+        super(WhenTestingHostByWsgiRequestForRefsGetter, self).setUp()
+
+        self._old_version = utils.API_VERSION
+        self.host = 'http://my_host:9311'
+        self.version = 'version1'
+        self.external_project_id = 'external_project_id'
+        self.resource = 'my_resource'
+
+        test_utils.mock_pecan_request(self, host=self.host)
+
+        utils.CONF.set_override('host_href', None, enforce_type=True)
+        utils.API_VERSION = self.version
+
+    def tearDown(self):
+        super(WhenTestingHostByWsgiRequestForRefsGetter, self).tearDown()
+        utils.CONF.clear_override('host_href')
+        utils.API_VERSION = self._old_version
+
+    def test_hostname_for_refs(self):
+        uri = utils.hostname_for_refs(resource=self.resource)
+        self.assertEqual("{0}/{1}/{2}".format(self.host, self.version,
+                                              self.resource), uri)
+
+    def test_blank_conf_hosthref_for_refs(self):
+        utils.CONF.set_override('host_href', '', enforce_type=True)
+        uri = utils.hostname_for_refs(resource=self.resource)
+        self.assertEqual("{0}/{1}/{2}".format(self.host, self.version,
+                                              self.resource), uri)
+
+    def test_hostname_for_refs_no_resource(self):
+        uri = utils.hostname_for_refs()
+        self.assertEqual("{0}/{1}".format(self.host, self.version), uri)
+
+
 class WhenTestingAcceptEncodingGetter(test_utils.BaseTestCase):
 
     def setUp(self):

barbican/tests/utils.py

@@ -12,6 +12,7 @@
 # implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
+from contextlib import contextmanager
 import datetime
 import functools
 import os
@@ -33,6 +34,19 @@ import barbican.context
 from barbican.tests import database_utils
 
 
+def mock_pecan_request(test_instance, host=None):
+    patcher_obj = mock.patch('pecan.request')
+    mock_req = patcher_obj.start()
+    test_instance.addCleanup(patcher_obj.stop)
+    mock_req.url = host
+
+
+@contextmanager
+def pecan_context(test_instance, host=None):
+    mock_pecan_request(test_instance, host=host)
+    yield
+
+
 class BarbicanAPIBaseTestCase(oslotest.BaseTestCase):
     """Base TestCase for all tests needing to interact with a Barbican app."""
     root_controller = None

etc/barbican/barbican-functional.conf

@@ -61,6 +61,12 @@ auditor_b_password=barbican
 # override_url=http://localhost:9311
 # override_url_version=v1
 
+# Flag to indicate if (when True) the server is setting the href's returned in
+# requests via barbican.conf's 'host_href' setting, or else (when False) the
+# server is setting the href's from the wsgi request.
+# Default value is True.
+server_host_href_set = True
+
 [quotas]
 # For each resource, the default maximum number that can be used for
 # a project is set below.  This value can be overridden for each

etc/barbican/barbican.conf

@@ -14,6 +14,9 @@ bind_port = 9311
 # Host name, for use in HATEOAS-style references
 #  Note: Typically this would be the load balanced endpoint that clients would use
 #  communicate back with this service.
+# If a deployment wants to derive host from wsgi request instead then make this
+# blank. Blank is needed to override default config value which is
+# 'http://localhost:9311'.
 host_href = http://localhost:9311
 
 # Log to this file. Make sure you do not set the same log

functionaltests/api/base.py

@@ -30,6 +30,8 @@ from functionaltests.common import config
 
 CONF = config.get_config()
 
+conf_host_href_used = CONF.keymanager.server_host_href_set
+
 
 class TestCase(oslotest.BaseTestCase):
     max_payload_size = 10000

functionaltests/api/v1/behaviors/container_behaviors.py

@@ -55,7 +55,7 @@ class ContainerBehaviors(base_behaviors.BaseBehaviors):
         """
         resp = self.client.get(
             container_ref, response_model_type=container_models.ContainerModel,
-            user_name=user_name)
+            user_name=user_name, extra_headers=extra_headers)
 
         return resp
 

functionaltests/api/v1/functional/test_containers.py

@@ -212,8 +212,10 @@ class GenericContainersTestCase(BaseContainerTestCase):
         resp = self.behaviors.delete_container("not_a_ref", expected_fail=True)
         self.assertEqual(resp.status_code, 404)
 
+    @testcase.skipIf(not base.conf_host_href_used, 'response href using '
+                     'wsgi request instead of CONF.host_href')
     @testcase.attr('positive')
-    def test_create_change_host_header(self, **kwargs):
+    def test_create_change_host_with_header_not_allowed(self, **kwargs):
         """Create a container with a (possibly) malicious host name header."""
 
         test_model = container_models.ContainerModel(**self.default_data)
@@ -231,6 +233,29 @@ class GenericContainersTestCase(BaseContainerTestCase):
         regex = '.*{0}.*'.format(malicious_hostname)
         self.assertNotRegexpMatches(resp.headers['location'], regex)
 
+    @testcase.skipIf(base.conf_host_href_used, 'response href using '
+                     'CONF.host_href instead of wsgi request')
+    @testcase.attr('positive')
+    def test_get_change_host_with_header_allowed(self, **kwargs):
+        """Get a container with a alternative proxy host name header."""
+
+        test_model = container_models.ContainerModel(**self.default_data)
+
+        another_proxy_hostname = 'proxy2.server.com'
+        changed_host_header = {'Host': another_proxy_hostname}
+
+        # In test, cannot pass different host header during create as returned
+        # container_href in response contains that host in url. That url is
+        # used in deleting that container during cleanup step.
+        resp, container_href = self.behaviors.create_container(test_model)
+        self.assertEqual(resp.status_code, 201)
+
+        resp = self.behaviors.get_container(container_href,
+                                            extra_headers=changed_host_header)
+        # Assert that returned href has provided proxy hostname
+        regex = '.*{0}.*'.format(another_proxy_hostname)
+        self.assertRegexpMatches(resp.model.container_ref, regex)
+
 
 @utils.parameterized_test_case
 class RSAContainersTestCase(BaseContainerTestCase):
@@ -304,8 +329,10 @@ class RSAContainersTestCase(BaseContainerTestCase):
         resp, container_ref = self.behaviors.create_container(test_model)
         self.assertEqual(resp.status_code, 400)
 
+    @testcase.skipIf(not base.conf_host_href_used, 'response href using '
+                     'wsgi request instead of CONF.host_href')
     @testcase.attr('positive')
-    def test_create_rsa_change_host_header(self, **kwargs):
+    def test_create_rsa_change_host_with_header_not_allowed(self, **kwargs):
         """Create a container with a (possibly) malicious host name header."""
 
         test_model = container_models.ContainerModel(**self.default_data)
@@ -323,6 +350,29 @@ class RSAContainersTestCase(BaseContainerTestCase):
         regex = '.*{0}.*'.format(malicious_hostname)
         self.assertNotRegexpMatches(resp.headers['location'], regex)
 
+    @testcase.skipIf(base.conf_host_href_used, 'response href using '
+                     'CONF.host_href instead of wsgi request')
+    @testcase.attr('positive')
+    def test_get_rsa_change_host_with_header_allowed(self, **kwargs):
+        """Get a container with a alternative proxy host name header."""
+
+        test_model = container_models.ContainerModel(**self.default_data)
+
+        another_proxy_hostname = 'proxy2.server.com'
+        changed_host_header = {'Host': another_proxy_hostname}
+
+        # In test, cannot pass different host header during create as returned
+        # container_href in response contains that host in url. That url is
+        # used in deleting that container during cleanup step.
+        resp, container_href = self.behaviors.create_container(test_model)
+        self.assertEqual(resp.status_code, 201)
+
+        resp = self.behaviors.get_container(container_href,
+                                            extra_headers=changed_host_header)
+        # Assert that returned href has provided proxy hostname
+        regex = '.*{0}.*'.format(another_proxy_hostname)
+        self.assertRegexpMatches(resp.model.container_ref, regex)
+
 
 class ContainersPagingTestCase(base.PagingTestCase):
 

functionaltests/api/v1/functional/test_orders.py

@@ -485,8 +485,10 @@ class OrdersTestCase(base.TestCase):
         create_resp, order_ref = self.behaviors.create_order(test_model)
         self.assertEqual(create_resp.status_code, 400)
 
+    @testcase.skipIf(not base.conf_host_href_used, 'response href using '
+                     'wsgi request instead of CONF.host_href')
     @testcase.attr('positive')
-    def test_order_create_change_host_header(self, **kwargs):
+    def test_order_create_change_host_with_header_not_allowed(self, **kwargs):
         """Create an order with a (possibly) malicious host name in header."""
 
         test_model = order_models.OrderModel(**self.create_default_data)
@@ -504,6 +506,29 @@ class OrdersTestCase(base.TestCase):
         regex = '.*{0}.*'.format(malicious_hostname)
         self.assertNotRegexpMatches(resp.headers['location'], regex)
 
+    @testcase.skipIf(base.conf_host_href_used, 'response href using '
+                     'CONF.host_href instead of wsgi request')
+    @testcase.attr('positive')
+    def test_order_get_change_host_with_header_allowed(self, **kwargs):
+        """Get an order with a alternative proxy host name in header."""
+
+        test_model = order_models.OrderModel(**self.create_default_data)
+
+        another_proxy_hostname = 'proxy2.server.com'
+        changed_host_header = {'Host': another_proxy_hostname}
+
+        # In test, cannot pass different host header during create as returned
+        # order_ref in response contains that host in url. That url is
+        # used in deleting that order during cleanup step.
+        resp, order_ref = self.behaviors.create_order(test_model)
+        self.assertEqual(resp.status_code, 202)
+
+        order_resp = self.behaviors.get_order(
+            order_ref, extra_headers=changed_host_header)
+        # Assert that returned href has provided proxy hostname
+        regex = '.*{0}.*'.format(another_proxy_hostname)
+        self.assertRegexpMatches(order_resp.model.order_ref, regex)
+
     @testcase.attr('positive')
     def test_encryption_using_generated_key(self):
         """Tests functionality of a generated asymmetric key pair."""

functionaltests/api/v1/functional/test_secrets.py

@@ -941,8 +941,10 @@ class SecretsTestCase(base.TestCase):
         resp, secret_ref = self.behaviors.create_secret(test_model)
         self.assertEqual(resp.status_code, 400)
 
+    @testcase.skipIf(not base.conf_host_href_used, 'response href using '
+                     'wsgi request instead of CONF.host_href')
     @testcase.attr('positive')
-    def test_secret_create_change_host_header(self, **kwargs):
+    def test_secret_create_change_host_with_header_not_allowed(self, **kwargs):
         """Create a secret with a (possibly) malicious host name in header."""
 
         test_model = secret_models.SecretModel(
@@ -961,6 +963,31 @@ class SecretsTestCase(base.TestCase):
         regex = '.*{0}.*'.format(malicious_hostname)
         self.assertNotRegexpMatches(resp.headers['location'], regex)
 
+    @testcase.skipIf(base.conf_host_href_used, 'response href using '
+                     'CONF.host_href instead of wsgi request')
+    @testcase.attr('positive')
+    def test_secret_get_change_host_with_header_allowed(self, **kwargs):
+        """Get secret metadata with alternative proxy host name in header."""
+
+        test_model = secret_models.SecretModel(
+            **self.default_secret_create_data)
+
+        another_proxy_hostname = 'proxy2.server.com'
+        changed_host_header = {'Host': another_proxy_hostname}
+
+        # In test, cannot pass different host header during create as returned
+        # secret_ref in response contains that host in url. That url is used in
+        # deleting that secret during cleanup step
+        resp, secret_ref = self.behaviors.create_secret(
+            test_model)
+        self.assertEqual(resp.status_code, 201)
+
+        resp = self.behaviors.get_secret_metadata(
+            secret_ref, extra_headers=changed_host_header)
+        # Check returned href has provided proxy hostname
+        regex = '.*{0}.*'.format(another_proxy_hostname)
+        self.assertRegexpMatches(resp.model.secret_ref, regex)
+
     @utils.parameterized_dataset({
         'symmetric': ['symmetric',
                       base64.b64decode(

functionaltests/api/v1/smoke/test_consumers.py

@@ -216,8 +216,11 @@ class ConsumersTestCase(base.TestCase):
         self.assertIn(self.consumer_data, consumer_data)
         self.assertEqual(1, count)
 
+    @testcase.skipIf(not base.conf_host_href_used, 'response href using '
+                     'wsgi request instead of CONF.host_href')
     @testcase.attr('positive')
-    def test_create_consumer_change_host_header(self, **kwargs):
+    def test_create_consumer_change_host_with_header_not_allowed(self,
+                                                                 **kwargs):
         """Create a consumer with a (possibly) malicious host name header."""
 
         test_model = consumer_model.ConsumerModel(**self.consumer_data)

functionaltests/common/config.py

@@ -73,7 +73,8 @@ def setup_config(config_file=''):
         cfg.IntOpt('timeout', default=10),
         cfg.StrOpt('override_url', default=''),
         cfg.StrOpt('override_url_version', default=''),
-        cfg.BoolOpt('verify_ssl', default=True)
+        cfg.BoolOpt('verify_ssl', default=True),
+        cfg.BoolOpt('server_host_href_set', default=True)
     ]
     TEST_CONF.register_group(keymanager_group)
     TEST_CONF.register_opts(keymanager_options, group=keymanager_group)