Repository: boto/botocore
Pull request #479 (2015-03-04 17:12:54)

Validate that endpoint url has a valid hostname
Otherwise, we'll try to make a request with an invalid hostname
which will either fail at the http layer, or even worse, may
generate invalid signatures if the endpoint_url mistakenly has
an endpoint url.

This can also happen sometimes because of misconfigured region
values in the config file.

cc @kyleknap @danielgtaylor 


Patch:
diff --git a/botocore/endpoint.py b/botocore/endpoint.py
index ea2dc041a..84c8d5758 100644
--- a/botocore/endpoint.py
+++ b/botocore/endpoint.py
@@ -26,6 +26,7 @@ from botocore.awsrequest import AWSRequest
 from botocore.compat import filter_ssl_san_warnings, urlsplit
 from botocore.compat import urlunsplit
 from botocore.utils import percent_encode_sequence
+from botocore.utils import is_valid_endpoint_url
 from botocore.hooks import first_non_none_response
 from botocore.response import StreamingBody
 from botocore import parsers
@@ -343,6 +344,8 @@ class EndpointCreator(object):
             final_endpoint_url = endpoint_url
         else:
             final_endpoint_url = endpoint['uri']
+        if not is_valid_endpoint_url(final_endpoint_url):
+            raise ValueError("Invalid endpoint: %s" % final_endpoint_url)
         return self._get_endpoint(service_model, region_name,
                                   final_endpoint_url, verify,
                                   response_parser_factory)
diff --git a/botocore/utils.py b/botocore/utils.py
index 7ae0ae32c..f086cb653 100644
--- a/botocore/utils.py
+++ b/botocore/utils.py
@@ -10,6 +10,7 @@
 # distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF
 # ANY KIND, either express or implied. See the License for the specific
 # language governing permissions and limitations under the License.
+import re
 import logging
 import datetime
 import hashlib
@@ -21,7 +22,7 @@ import dateutil.parser
 from dateutil.tz import tzlocal, tzutc
 
 from botocore.exceptions import InvalidExpressionError, ConfigNotFound
-from botocore.compat import json, quote, zip_longest
+from botocore.compat import json, quote, zip_longest, urlsplit
 from botocore.vendored import requests
 from botocore.compat import OrderedDict
 
@@ -528,3 +529,29 @@ class ArgumentGenerator(object):
         return OrderedDict([
             ('KeyName', self._generate_skeleton(value_shape, stack)),
         ])
+
+
+def is_valid_endpoint_url(endpoint_url):
+    """Verify the endpoint_url is valid.
+
+    :type endpoint_url: string
+    :param endpoint_url: An endpoint_url.  Must have at least a scheme
+        and a hostname.
+
+    :return: True if the endpoint url is valid. False otherwise.
+
+    """
+    parts = urlsplit(endpoint_url)
+    hostname = parts.hostname
+    if hostname is None:
+        return False
+    # Hostname validation is based on http://tools.ietf.org/html/rfc952
+    # and http://tools.ietf.org/html/rfc1123#page-13
+    # with the validation taken from:
+    # http://stackoverflow.com/questions/2532053/validate-a-hostname-string
+    if len(hostname) > 255:
+        return False
+    if hostname[-1] == ".":
+        hostname = hostname[:-1]
+    allowed = re.compile("(?!-)[A-Z\d-]{1,63}(?<!-)$", re.IGNORECASE)
+    return all(allowed.match(x) for x in hostname.split("."))


Test patch:
diff --git a/tests/integration/test_client.py b/tests/integration/test_client.py
index 4512d78f9..db055a585 100644
--- a/tests/integration/test_client.py
+++ b/tests/integration/test_client.py
@@ -127,7 +127,7 @@ class TestAcceptedDateTimeFormats(unittest.TestCase):
         self.assertIn('Clusters', response)
 
 
-class TestClientCanBeCloned(unittest.TestCase):
+class TestCreateClients(unittest.TestCase):
     def setUp(self):
         self.session = botocore.session.get_session()
 
@@ -141,3 +141,8 @@ class TestClientCanBeCloned(unittest.TestCase):
         # We really just want to ensure create_client doesn't raise
         # an exception, but we'll double check that the client looks right.
         self.assertTrue(hasattr(client, 'list_buckets'))
+
+    def test_client_raises_exception_invalid_region(self):
+        with self.assertRaisesRegexp(ValueError, 'Invalid endpoint'):
+            self.session.create_client('cloudformation',
+                                       region_name='invalid region name')
diff --git a/tests/unit/test_endpoint.py b/tests/unit/test_endpoint.py
index 701e2ff25..054419ee2 100644
--- a/tests/unit/test_endpoint.py
+++ b/tests/unit/test_endpoint.py
@@ -147,7 +147,6 @@ class TestEndpointFeatures(TestEndpointBase):
             prepared_request, verify=True, stream=False,
             proxies=proxies, timeout=DEFAULT_TIMEOUT)
 
-
     def test_make_request_with_no_auth(self):
         self.endpoint.auth = None
         self.endpoint.make_request(self.op, request_dict())
diff --git a/tests/unit/test_s3_addressing.py b/tests/unit/test_s3_addressing.py
index e7545e452..89b7c4222 100644
--- a/tests/unit/test_s3_addressing.py
+++ b/tests/unit/test_s3_addressing.py
@@ -200,13 +200,17 @@ class TestS3Addressing(BaseSessionTest):
             prepared_request.url,
             'https://s3-us-west-2.amazonaws.com/192.168.5.256/mykeyname')
 
+    def test_invalid_endpoint_raises_exception(self):
+        with self.assertRaisesRegexp(ValueError, 'Invalid endpoint'):
+            self.s3.get_endpoint('bad region name')
+
     def test_non_existent_region(self):
         # If I ask for a region that does not
         # exist on a global endpoint, such as:
-        endpoint = self.s3.get_endpoint('REGION DOES NOT EXIST')
+        endpoint = self.s3.get_endpoint('REGION-DOES-NOT-EXIST')
         # Then the default endpoint heuristic will apply and we'll
         # get the region name as specified.
-        self.assertEqual(endpoint.region_name, 'REGION DOES NOT EXIST')
+        self.assertEqual(endpoint.region_name, 'REGION-DOES-NOT-EXIST')
         # Why not fixed this?  Well backwards compatability for one thing.
         # The other reason is because it was intended to accomodate this
         # use case.  Let's say I have us-west-2 set as my default region,
diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py
index 052fc379a..314f1b014 100644
--- a/tests/unit/test_utils.py
+++ b/tests/unit/test_utils.py
@@ -32,6 +32,7 @@ from botocore.utils import CachedProperty
 from botocore.utils import ArgumentGenerator
 from botocore.utils import calculate_tree_hash
 from botocore.utils import calculate_sha256
+from botocore.utils import is_valid_endpoint_url
 from botocore.model import DenormalizedStructureBuilder
 from botocore.model import ShapeResolver
 
@@ -461,5 +462,39 @@ class TestTreeHash(unittest.TestCase):
             '12f3cbd6101b981cde074039f6f728071da8879d6f632de8afc7cdf00661b08f')
 
 
+class TestIsValidEndpointURL(unittest.TestCase):
+    def test_dns_name_is_valid(self):
+        self.assertTrue(is_valid_endpoint_url('https://s3.amazonaws.com/'))
+
+    def test_ip_address_is_allowed(self):
+        self.assertTrue(is_valid_endpoint_url('https://10.10.10.10/'))
+
+    def test_path_component_ignored(self):
+        self.assertTrue(
+            is_valid_endpoint_url('https://foo.bar.com/other/path/'))
+
+    def test_can_have_port(self):
+        self.assertTrue(is_valid_endpoint_url('https://foo.bar.com:12345/'))
+
+    def test_ip_can_have_port(self):
+        self.assertTrue(is_valid_endpoint_url('https://10.10.10.10:12345/'))
+
+    def test_cannot_have_spaces(self):
+        self.assertFalse(is_valid_endpoint_url('https://my invalid name/'))
+
+    def test_missing_scheme(self):
+        self.assertFalse(is_valid_endpoint_url('foo.bar.com'))
+
+    def test_no_new_lines(self):
+        self.assertFalse(is_valid_endpoint_url('https://foo.bar.com\nbar/'))
+
+    def test_long_hostname(self):
+        long_hostname = 'htps://%s.com' % ('a' * 256)
+        self.assertFalse(is_valid_endpoint_url(long_hostname))
+
+    def test_hostname_can_end_with_dot(self):
+        self.assertTrue(is_valid_endpoint_url('https://foo.bar.com./'))
+
+
 if __name__ == '__main__':
     unittest.main()