From b9bc8602c2bdc31f09b241f3612f7fda6d601ea9 Mon Sep 17 00:00:00 2001 From: Trishank K Kuppusamy Date: Thu, 30 Aug 2018 19:18:10 -0400 Subject: [PATCH 01/34] Update TUF to handle HTTPS proxies Signed-off-by: Trishank K Kuppusamy --- ci-requirements.txt | 7 +- dev-requirements.txt | 1 + requirements.in | 7 +- requirements.txt | 21 +- setup.py | 7 +- tests/test_download.py | 29 +-- tests/test_slow_retrieval_attack.py | 5 +- tuf/download.py | 314 ++++++---------------------- tuf/exceptions.py | 2 +- tuf/settings.py | 17 +- 10 files changed, 109 insertions(+), 301 deletions(-) diff --git a/ci-requirements.txt b/ci-requirements.txt index fc218efb..216ba4e0 100644 --- a/ci-requirements.txt +++ b/ci-requirements.txt @@ -1,6 +1,7 @@ -securesystemslib[crypto,pynacl] -six -iso8601 coverage coveralls +iso8601 pylint +requests +securesystemslib[crypto,pynacl] +six diff --git a/dev-requirements.txt b/dev-requirements.txt index d1573acd..10e78433 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -31,6 +31,7 @@ pylint==2.1.1 ; python_version >= "3.0" pylint==1.9.3 ; python_version < "3.0" # pyup: ignore pynacl==1.2.1 pyyaml==3.13 +requests==2.19.1 securesystemslib[crypto,pynacl]==0.11.2 singledispatch==3.4.0.3 six==1.11.0 diff --git a/requirements.in b/requirements.in index 667e20ee..9ab11b5d 100644 --- a/requirements.in +++ b/requirements.in @@ -1,8 +1,9 @@ # requirements.in for pip-compile. -securesystemslib cryptography colorama -pynacl -six iso8601 +pynacl +requests +securesystemslib +six diff --git a/requirements.txt b/requirements.txt index 6e422366..a1727570 100644 --- a/requirements.txt +++ b/requirements.txt @@ -8,6 +8,10 @@ asn1crypto==0.24.0 \ --hash=sha256:2f1adbb7546ed199e3c90ef23ec95c5cf3585bac7d11fb7eb562a3fe89c64e87 \ --hash=sha256:9d5c20441baf0cb60a4ac34cc447c6c189024b6b4c6cd7877034f4965c464e49 \ # via cryptography +certifi==2018.8.24 \ + --hash=sha256:376690d6f16d32f9d1fe8932551d80b23e9d393a8578c5633a2ed39a64861638 \ + --hash=sha256:456048c7e371c089d0a77a5212fb37a2c2dce1e24146e3b7e0261736aaeaa22a \ + # via requests cffi==1.11.5 \ --hash=sha256:151b7eefd035c56b2b2e1eb9963c90c6302dc15fbd8c1c0a83a163ff2c7d7743 \ --hash=sha256:1553d1e99f035ace1c0544050622b7bc963374a00c467edafac50ad7bd276aef \ @@ -37,6 +41,10 @@ cffi==1.11.5 \ --hash=sha256:edabd457cd23a02965166026fd9bfd196f4324fe6032e866d0f3bd0301cd486f \ --hash=sha256:fdf1c1dc5bafc32bc5d08b054f94d659422b05aba244d6be4ddc1c72d9aa70fb \ # via cryptography, pynacl +chardet==3.0.4 \ + --hash=sha256:84ab92ed1c4d4f16916e05906b6b75a6c0fb5db821cc65e70cbd64a3e2a5eaae \ + --hash=sha256:fc323ffcaeaed0e0a02bf4d117757b98aed530d9ed4531e3e15460124c106691 \ + # via requests colorama==0.3.9 \ --hash=sha256:463f8483208e921368c9f306094eb6f725c6ca42b0f97e313cb5d5512459feda \ --hash=sha256:48eb22f4f8461b1df5734a074b57042430fb06e1d61bd1e11b078c0fe6d7a1f1 @@ -69,11 +77,7 @@ enum34==1.1.6 \ idna==2.7 \ --hash=sha256:156a6814fb5ac1fc6850fb002e0852d56c0c8d2531923a51032d1b70760e186e \ --hash=sha256:684a38a6f903c1d71d6d5fac066b58d7768af4de2b832e426ec79c30daa94a16 \ - # via cryptography -ipaddress==1.0.22 \ - --hash=sha256:64b28eec5e78e7510698f6d4da08800a5c575caa4a286c93d651c5d3ff7b6794 \ - --hash=sha256:b146c751ea45cad6188dd6cf2d9b757f6f4f8d6ffb96a023e6f2e26eea02a72c \ - # via cryptography + # via cryptography, requests iso8601==0.1.12 \ --hash=sha256:210e0134677cc0d02f6028087fee1df1e1d76d372ee1db0bf30bf66c5c1c89a3 \ --hash=sha256:49c4b20e1f38aa5cf109ddcd39647ac419f928512c869dc01d5c7098eddede82 \ @@ -105,9 +109,16 @@ pynacl==1.2.1 \ --hash=sha256:eeee629828d0eb4f6d98ac41e9a3a6461d114d1d0aa111a8931c049359298da0 \ --hash=sha256:f5ce9e26d25eb0b2d96f3ef0ad70e1d3ae89b5d60255c462252a3e456a48c053 \ --hash=sha256:fabf73d5d0286f9e078774f3435601d2735c94ce9e514ac4fb945701edead7e4 +requests==2.19.1 \ + --hash=sha256:63b52e3c866428a224f97cab011de738c36aec0185aa91cfacd418b5d58911d1 \ + --hash=sha256:ec22d826a36ed72a7358ff3fe56cbd4ba69dd7a6718ffd450ff0e9df7a47ce6a securesystemslib==0.11.2 \ --hash=sha256:43554371feeef50196587aa066cffd6b9ceff6b484fa7b127e139fafb5c0e23e \ --hash=sha256:7fe1ed8a4139b12225986ff6f9ebab48c74eaa93265a73f988e8de10e6b237a8 six==1.11.0 \ --hash=sha256:70e8a77beed4562e7f14fe23a786b54f6296e34344c23bc42f07b15018ff98e9 \ --hash=sha256:832dc0e10feb1aa2c68dcc57dbb658f1c7e65b9b61af69048abc87a2db00a0eb +urllib3==1.23 \ + --hash=sha256:a68ac5e15e76e7e5dd2b8f94007233e01effe3e50e8daddf69acfd81cb686baf \ + --hash=sha256:b5725a0bd4ba422ab0e66e89e030c806576753ea3ee08554382c14e685d117b5 \ + # via requests diff --git a/setup.py b/setup.py index 12ffe460..880d11f7 100755 --- a/setup.py +++ b/setup.py @@ -109,7 +109,12 @@ 'Topic :: Security', 'Topic :: Software Development' ], - install_requires = ['iso8601>=0.1.12', 'six>=1.11.0', 'securesystemslib>=0.11.2'], + install_requires = [ + 'iso8601>=0.1.12', + 'requests>=2.19.1', + 'six>=1.11.0', + 'securesystemslib>=0.11.2' + ], packages = find_packages(exclude=['tests']), scripts = [ 'tuf/scripts/repo.py', diff --git a/tests/test_download.py b/tests/test_download.py index d631ff32..1688f265 100755 --- a/tests/test_download.py +++ b/tests/test_download.py @@ -45,6 +45,8 @@ import tuf.unittest_toolbox as unittest_toolbox import tuf.exceptions +import requests.exceptions + import securesystemslib import six @@ -168,44 +170,29 @@ def test_download_url_to_tempfileobj_and_urls(self): self.assertRaises(securesystemslib.exceptions.FormatError, download_file, None, self.target_data_length) - self.assertRaises(securesystemslib.exceptions.FormatError, + self.assertRaises(requests.exceptions.MissingSchema, download_file, self.random_string(), self.target_data_length) - self.assertRaises(six.moves.urllib.error.HTTPError, + self.assertRaises(requests.exceptions.HTTPError, download_file, 'http://localhost:' + str(self.PORT) + '/' + self.random_string(), self.target_data_length) - self.assertRaises(six.moves.urllib.error.URLError, + self.assertRaises(requests.exceptions.ConnectionError, download_file, 'http://localhost:' + str(self.PORT+1) + '/' + self.random_string(), self.target_data_length) # Specify an unsupported URI scheme. url_with_unsupported_uri = self.url.replace('http', 'file') - self.assertRaises(securesystemslib.exceptions.FormatError, download_file, url_with_unsupported_uri, + self.assertRaises(requests.exceptions.InvalidSchema, download_file, url_with_unsupported_uri, self.target_data_length) - self.assertRaises(securesystemslib.exceptions.FormatError, unsafe_download_file, + self.assertRaises(requests.exceptions.InvalidSchema, unsafe_download_file, url_with_unsupported_uri, self.target_data_length) - def test__get_opener(self): - # Test normal case. - # A simple https server should be used to test the rest of the optional - # ssl-related functions of 'tuf.download.py'. - fake_cacert = self.make_temp_data_file() - - with open(fake_cacert, 'wt') as file_object: - file_object.write('fake cacert') - - tuf.settings.ssl_certificates = fake_cacert - tuf.download._get_opener('https') - tuf.settings.ssl_certificates = None - - - def test_https_connection(self): # Make a temporary file to be served to the client. current_directory = os.getcwd() @@ -230,7 +217,7 @@ def test_https_connection(self): https_url = 'https://localhost:' + str(port) + '/' + relative_target_filepath # Download the target file using an https connection. - tuf.settings.ssl_certificates = 'ssl_cert.crt' + os.environ['REQUESTS_CA_BUNDLE'] = 'ssl_cert.crt' message = 'Downloading target file from https server: ' + https_url logger.info(message) try: diff --git a/tests/test_slow_retrieval_attack.py b/tests/test_slow_retrieval_attack.py index 290f7996..6fd985f8 100755 --- a/tests/test_slow_retrieval_attack.py +++ b/tests/test_slow_retrieval_attack.py @@ -157,10 +157,9 @@ def setUp(self): # The slow retrieval server, in mode 2 (1 byte per second), will only # sleep for a total of (target file size) seconds. Add a target file # that contains sufficient number of bytes to trigger a slow retrieval - # error. "sufficient number of bytes" assumed to be - # >> 'tuf.settings.SLOW_START_GRACE_PERIOD' bytes. + # error. "sufficient number of bytes" assumed to be 10x more. extra_bytes = 8 - total_bytes = tuf.settings.SLOW_START_GRACE_PERIOD + extra_bytes + total_bytes = 100 * extra_bytes repository = repo_tool.load_repository(self.repository_directory) file1_filepath = os.path.join(self.repository_directory, 'targets', diff --git a/tuf/download.py b/tuf/download.py index 492c10c9..0cd4d95c 100755 --- a/tuf/download.py +++ b/tuf/download.py @@ -35,29 +35,36 @@ from __future__ import unicode_literals import os -import socket import logging import time import timeit -import ssl -import tuf +import requests +import six import securesystemslib import securesystemslib.util -import six - -# 'ssl.match_hostname' was added in Python 3.2. The vendored version is needed -# for Python 2.7. -try: - from ssl import match_hostname - -except ImportError: # pragma: no cover - from securesystemslib._vendor.ssl_match_hostname import match_hostname +import tuf +import tuf.exceptions # See 'log.py' to learn how logging is handled in TUF. logger = logging.getLogger('tuf.download') +# From http://docs.python-requests.org/en/master/user/advanced/#session-objects: +# +# "The Session object allows you to persist certain parameters across requests. +# It also persists cookies across all requests made from the Session instance, +# and will use urllib3's connection pooling. So if you're making several +# requests to the same host, the underlying TCP connection will be reused, +# which can result in a significant performance increase (see HTTP persistent +# connection)." +# +# FIXME: There are very likely security implications to sharing the same +# session. Ideally, tuf.client.updater would use a separate Session for every +# mirror, and it would pass the Session to be used in this module, but that +# requires a much more invasive change. +# +_session = requests.Session() def safe_download(url, required_length): @@ -76,8 +83,7 @@ def safe_download(url, required_length): url: - A URL string that represents the location of the file. The URI scheme - component must be one of 'tuf.settings.SUPPORTED_URI_SCHEMES'. + A URL string that represents the location of the file. required_length: An integer value representing the length of the file. This is an exact @@ -106,20 +112,6 @@ def safe_download(url, required_length): securesystemslib.formats.URL_SCHEMA.check_match(url) securesystemslib.formats.LENGTH_SCHEMA.check_match(required_length) - # Ensure 'url' specifies one of the URI schemes in - # 'tuf.settings.SUPPORTED_URI_SCHEMES'. Be default, ['http', 'https'] is - # supported. If the URI scheme of 'url' is empty or "file", files on the - # local system can be accessed. Unexpected files may be accessed by - # compromised metadata (unlikely to happen if targets.json metadata is signed - # with offline keys). - parsed_url = six.moves.urllib.parse.urlparse(url) - - if parsed_url.scheme not in tuf.settings.SUPPORTED_URI_SCHEMES: - message = \ - repr(url) + ' specifies an unsupported URI scheme. Supported ' + \ - ' URI Schemes: ' + repr(tuf.settings.SUPPORTED_URI_SCHEMES) - raise securesystemslib.exceptions.FormatError(message) - return _download_file(url, required_length, STRICT_REQUIRED_LENGTH=True) @@ -142,8 +134,7 @@ def unsafe_download(url, required_length): url: - A URL string that represents the location of the file. The URI scheme - component must be one of 'tuf.settings.SUPPORTED_URI_SCHEMES'. + A URL string that represents the location of the file. required_length: An integer value representing the length of the file. This is an upper @@ -172,20 +163,6 @@ def unsafe_download(url, required_length): securesystemslib.formats.URL_SCHEMA.check_match(url) securesystemslib.formats.LENGTH_SCHEMA.check_match(required_length) - # Ensure 'url' specifies one of the URI schemes in - # 'tuf.settings.SUPPORTED_URI_SCHEMES'. Be default, ['http', 'https'] is - # supported. If the URI scheme of 'url' is empty or "file", files on the - # local system can be accessed. Unexpected files may be accessed by - # compromised metadata (unlikely to happen if targets.json metadata is signed - # with offline keys). - parsed_url = six.moves.urllib.parse.urlparse(url) - - if parsed_url.scheme not in tuf.settings.SUPPORTED_URI_SCHEMES: - message = \ - repr(url) + ' specifies an unsupported URI scheme. Supported ' + \ - ' URI Schemes: ' + repr(tuf.settings.SUPPORTED_URI_SCHEMES) - raise securesystemslib.exceptions.FormatError(message) - return _download_file(url, required_length, STRICT_REQUIRED_LENGTH=False) @@ -229,9 +206,6 @@ def _download_file(url, required_length, STRICT_REQUIRED_LENGTH=True): securesystemslib.exceptions.FormatError, if any of the arguments are improperly formatted. - tuf.exceptions.Error, if the certificates pointed to by - tuf.settings.ssl_certificates cannot be loaded. - Any other unforeseen runtime exception. @@ -257,14 +231,34 @@ def _download_file(url, required_length, STRICT_REQUIRED_LENGTH=True): temp_file = securesystemslib.util.TempFile() try: - # Open the connection to the remote file. _open_connection() can raise - # socket connection exceptions (such as SSLError). Connection errors of - # this kind can be minimized by adjusting the socket timeout in - # settings.py. - connection = _open_connection(url) + # Attach some default headers to every Session. + _session.headers.update({ + # Tell the server not to compress or modify anything. + # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Accept-Encoding#Directives + 'Accept-Encoding': 'identity', + # The TUF user agent. + # TODO: Attach version number, which cannot be easily found right now. + 'User-Agent': 'tuf' + }) + + # Get the requests.Response object for this URL. + # + # Always stream to control how requests are downloaded: + # http://docs.python-requests.org/en/master/user/advanced/#body-content-workflow + # + # We will always manually close Responses, so no need for a context + # manager. + # + # Always set the connect + read timeout: + # http://docs.python-requests.org/en/master/user/advanced/#timeouts + response = _session.get(url, stream=True, + timeout=tuf.settings.SOCKET_TIMEOUT) + + # Check response status. + response.raise_for_status() # We ask the server about how big it thinks this file should be. - reported_length = _get_content_length(connection) + reported_length = _get_content_length(response) # Then, we check whether the required length matches the reported length. _check_content_length(reported_length, required_length, @@ -273,7 +267,7 @@ def _download_file(url, required_length, STRICT_REQUIRED_LENGTH=True): # Download the contents of the URL, up to the required length, to a # temporary file, and get the total number of downloaded bytes. total_downloaded, average_download_speed = \ - _download_fixed_amount_of_data(connection, temp_file, required_length) + _download_fixed_amount_of_data(response, temp_file, required_length) # Does the total number of downloaded bytes match the required length? _check_downloaded_length(total_downloaded, required_length, @@ -293,21 +287,20 @@ def _download_file(url, required_length, STRICT_REQUIRED_LENGTH=True): -def _download_fixed_amount_of_data(connection, temp_file, required_length): +def _download_fixed_amount_of_data(response, temp_file, required_length): """ This is a helper function, where the download really happens. While-block - reads data from connection a fixed chunk of data at a time, or less, until + reads data from response a fixed chunk of data at a time, or less, until 'required_length' is reached. - connection: - The object that the _open_connection returns for communicating with the - server about the contents of a URL. + response: + The object for communicating with the server about the contents of a URL. temp_file: A temporary file where the contents at the URL specified by the - 'connection' object will be stored. + 'response' object will be stored. required_length: The number of bytes that we must download for the file. This is almost @@ -328,12 +321,6 @@ def _download_fixed_amount_of_data(connection, temp_file, required_length): attempt. """ - # Tolerate servers with a slow start by ignoring their delivery speed for - # 'tuf.settings.SLOW_START_GRACE_PERIOD' seconds. Set 'seconds_spent_receiving' - # to negative SLOW_START_GRACE_PERIOD seconds, and begin checking the average - # download speed once it is positive. - grace_period = -tuf.settings.SLOW_START_GRACE_PERIOD - # Keep track of total bytes downloaded. number_of_bytes_received = 0 average_download_speed = 0 @@ -350,20 +337,17 @@ def _download_fixed_amount_of_data(connection, temp_file, required_length): if tuf.settings.SLEEP_BEFORE_ROUND: time.sleep(tuf.settings.SLEEP_BEFORE_ROUND) - data = b'' read_amount = min(tuf.settings.CHUNK_SIZE, - required_length - number_of_bytes_received) + required_length - number_of_bytes_received) - try: - data = connection.read(read_amount) - - # Python 3.2 returns 'IOError' if the remote file object has timed out. - except (socket.error, IOError): - pass + # NOTE: This may not handle some servers adding a Content-Encoding + # header, which may cause urllib3 to misbehave: + # https://github.com/pypa/pip/blob/404838abcca467648180b358598c597b74d568c9/src/pip/_internal/download.py#L547-L582 + data = response.raw.read(read_amount) number_of_bytes_received = number_of_bytes_received + len(data) - # Data successfully read from the connection. Store it. + # Data successfully read from the response. Store it. temp_file.write(data) if number_of_bytes_received == required_length: @@ -372,9 +356,6 @@ def _download_fixed_amount_of_data(connection, temp_file, required_length): stop_time = timeit.default_timer() seconds_spent_receiving = stop_time - start_time - if (seconds_spent_receiving + grace_period) < 0: - continue - # Measure the average download speed. average_download_speed = number_of_bytes_received / seconds_spent_receiving @@ -397,119 +378,24 @@ def _download_fixed_amount_of_data(connection, temp_file, required_length): except: # Whatever happens, make sure that we always close the connection. - connection.close() + response.close() raise - connection.close() + response.close() return number_of_bytes_received, average_download_speed -def _get_request(url): - """ - Wraps the URL to retrieve to protects against "creative" - interpretation of the RFC: http://bugs.python.org/issue8732 - - https://github.com/pypa/pip/blob/d0fa66ecc03ab20b7411b35f7c7b423f31f77761/pip/download.py#L147 - """ - - return six.moves.urllib.request.Request(url, headers={'Accept-encoding': 'identity'}) - - - - - -def _get_opener(scheme=None): - """ - Build a urllib2 opener based on whether the user now wants SSL. - - https://github.com/pypa/pip/blob/d0fa66ecc03ab20b7411b35f7c7b423f31f77761/pip/download.py#L178 - Raises tuf.exceptions.Error if tuf.settings.ssl_certificates is not a valid - file. - """ - - if scheme == "https": - if not os.path.isfile(tuf.settings.ssl_certificates): - raise tuf.exceptions.Error('The SSL certificate specified in' - ' tuf.settings.ssl_certificates is not a valid file.' - ' ssl_certificates set to: ' + repr(tuf.settings.ssl_certificates)) - - else: - # If we are going over https, use an opener which will provide SSL - # certificate verification. - https_handler = VerifiedHTTPSHandler() - opener = six.moves.urllib.request.build_opener(https_handler) - - # Strip out HTTPHandler to prevent MITM spoof. - for handler in opener.handlers: - if isinstance(handler, six.moves.urllib.request.HTTPHandler): - opener.handlers.remove(handler) - - else: - # Otherwise, use the default opener. - opener = six.moves.urllib.request.build_opener() - - return opener - - - - - -def _open_connection(url): - """ - - Helper function that opens a connection to the url. urllib2 supports http, - ftp, and file. In python (2.6+) where the ssl module is available, urllib2 - also supports https. - - TODO: Determine whether this follows http redirects and decide if we like - that. For example, would we not want to allow redirection from ssl to - non-ssl urls? - - - url: - URL string (e.g., 'http://...' or 'ftp://...' or 'file://...') - - - tuf.exceptions.Error, if the certificates pointed by - tuf.settings.ssl_certificates cannot be loaded. - - - Opens a connection to a remote server. - - - File-like object. - """ - - # urllib2.Request produces a Request object that allows for a finer control - # of the requesting process. Request object allows to add headers or data to - # the HTTP request. For instance, request method add_header(key, val) can be - # used to change/spoof 'User-Agent' from default Python-urllib/x.y to - # 'Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 5.1)' this can be useful if - # servers do not recognize connections that originates from - # Python-urllib/x.y. - - parsed_url = six.moves.urllib.parse.urlparse(url) - opener = _get_opener(scheme=parsed_url.scheme) - request = _get_request(url) - - return opener.open(request, timeout = tuf.settings.SOCKET_TIMEOUT) - - - - - -def _get_content_length(connection): +def _get_content_length(response): """ A helper function that gets the purported file length from server. - connection: - The object that the _open_connection function returns for communicating - with the server about the contents of a URL. + response: + The object for communicating with the server about the contents of a URL. No known side effects. @@ -525,7 +411,7 @@ def _get_content_length(connection): try: # What is the length of this document according to the HTTP spec? - reported_length = connection.info().get('Content-Length') + reported_length = response.headers.get('Content-Length') # Try casting it as a decimal number. reported_length = int(reported_length, 10) @@ -536,7 +422,7 @@ def _get_content_length(connection): except Exception as e: logger.exception('Could not get content length' - ' about ' + str(connection) + ' from server: ' + str(e)) + ' about ' + str(response) + ' from server: ' + str(e)) return None return reported_length @@ -682,71 +568,3 @@ def _check_downloaded_length(total_downloaded, required_length, logger.info('Downloaded ' + str(total_downloaded) + ' bytes out of an' ' upper limit of ' + str(required_length) + ' bytes.') - - - - - -class VerifiedHTTPSConnection(six.moves.http_client.HTTPSConnection): - """ - A connection that wraps connections with ssl certificate verification. - - https://github.com/pypa/pip/blob/d0fa66ecc03ab20b7411b35f7c7b423f31f77761/pip/download.py#L72 - Raise tuf.exeptions.Error if the certificates specified in - tuf.settings.ssl_certificates cannot be loaded. - """ - - def connect(self): - - connection_kwargs = {} - connection_kwargs.update(timeout = self.timeout) - - # for >= py2.7 - if hasattr(self, 'source_address'): - connection_kwargs.update(source_address = self.source_address) - - sock = socket.create_connection((self.host, self.port), **connection_kwargs) - - # for >= py2.7 - if getattr(self, '_tunnel_host', None): - self.sock = sock - self._tunnel() - - # set location of certificate authorities - if not os.path.isfile(tuf.settings.ssl_certificates): - raise tuf.exceptions.Error('The SSL certificate specified in' - ' tuf.settings.ssl_certificates is not a valid file.' - ' ssl_certificates set to: ' + repr(tuf.settings.ssl_certificates)) - - else: - cert_path = tuf.settings.ssl_certificates - - # TODO: Disallow SSLv2. - # http://docs.python.org/dev/library/ssl.html#protocol-versions - # TODO: Select the right ciphers. - # http://docs.python.org/dev/library/ssl.html#cipher-selection - # ssl.PROTOCOL_SSLv23, the default value for 'ssl_version', is deprecated in - # Python 2.7.13, but becomes an alias for ssl.PROTOCOL_TLS. - self.sock = ssl.wrap_socket(sock, self.key_file, self.cert_file, - cert_reqs=ssl.CERT_REQUIRED, ca_certs=cert_path, - ssl_version=ssl.PROTOCOL_SSLv23) - - match_hostname(self.sock.getpeercert(), self.host) - - - - - -class VerifiedHTTPSHandler(six.moves.urllib.request.HTTPSHandler): - """ - A HTTPSHandler that uses our own VerifiedHTTPSConnection. - - https://github.com/pypa/pip/blob/d0fa66ecc03ab20b7411b35f7c7b423f31f77761/pip/download.py#L109 - """ - - def __init__(self, connection_class = VerifiedHTTPSConnection): - self.specialized_conn_class = connection_class - six.moves.urllib.request.HTTPSHandler.__init__(self) - - def https_open(self, req): - return self.do_open(self.specialized_conn_class, req) diff --git a/tuf/exceptions.py b/tuf/exceptions.py index 86c90a22..3f5dca1d 100755 --- a/tuf/exceptions.py +++ b/tuf/exceptions.py @@ -173,7 +173,7 @@ def __init__(self, expected_length, observed_length): def __str__(self): return 'Observed length (' + repr(self.observed_length)+\ - ') <= expected length (' + repr(self.expected_length) + ').' + ') != expected length (' + repr(self.expected_length) + ').' class SlowRetrievalError(DownloadError): diff --git a/tuf/settings.py b/tuf/settings.py index b33ce4ca..cf5ad588 100755 --- a/tuf/settings.py +++ b/tuf/settings.py @@ -46,11 +46,6 @@ # /tmp/repositories/django_repo/metadata/current/root.METADATA_EXTENSION repositories_directory = None -# A PEM (RFC 1422) file where you may find SSL certificate authorities -# https://en.wikipedia.org/wiki/Certificate_authority -# http://docs.python.org/2/library/ssl.html#certificates -ssl_certificates = None - # The 'log.py' module manages TUF's logging system. Users have the option to # enable/disable logging to a file via 'ENABLE_FILE_LOGGING', or # tuf.log.enable_file_logging() and tuf.log.disable_file_logging(). @@ -80,7 +75,7 @@ DEFAULT_TARGETS_REQUIRED_LENGTH = 5000000 #bytes # Set a timeout value in seconds (float) for non-blocking socket operations. -SOCKET_TIMEOUT = 4 #seconds +SOCKET_TIMEOUT = 5 #seconds # The maximum chunk of data, in bytes, we would download in every round. CHUNK_SIZE = 400000 #bytes @@ -89,16 +84,6 @@ # avoid being considered as a slow retrieval attack. MIN_AVERAGE_DOWNLOAD_SPEED = 50 #bytes/second -# The time (in seconds) we ignore a server with a slow initial retrieval speed. -SLOW_START_GRACE_PERIOD = 0.1 #seconds - -# Software updaters that integrate the framework are required to specify -# the URL prefix for the mirrors that clients can contact to download updates. -# The following URI schemes are those that download.py support. By default, -# the ['http', 'https'] URI schemes are supported, but may be modified by -# integrators to schemes that they wish to support for their integration. -SUPPORTED_URI_SCHEMES = ['http', 'https'] - # By default, limit number of delegatees we visit for any target. MAX_NUMBER_OF_DELEGATIONS = 2**5 From 1e97275b9a04079c86958d5ad295f4c569f0a1af Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Fri, 31 Aug 2018 13:28:43 -0400 Subject: [PATCH 02/34] minor: undo some import/dependency re-ordering (sorry -- just to keep things simple) Signed-off-by: Sebastien Awwad --- ci-requirements.txt | 10 +++++----- requirements.in | 4 ++-- tuf/download.py | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/ci-requirements.txt b/ci-requirements.txt index 216ba4e0..f737ed8d 100644 --- a/ci-requirements.txt +++ b/ci-requirements.txt @@ -1,7 +1,7 @@ -coverage -coveralls -iso8601 -pylint -requests securesystemslib[crypto,pynacl] six +iso8601 +coverage +coveralls +pylint +requests diff --git a/requirements.in b/requirements.in index 9ab11b5d..1781b032 100644 --- a/requirements.in +++ b/requirements.in @@ -1,9 +1,9 @@ # requirements.in for pip-compile. +securesystemslib cryptography colorama -iso8601 pynacl requests -securesystemslib six +iso8601 diff --git a/tuf/download.py b/tuf/download.py index 0cd4d95c..36811c0f 100755 --- a/tuf/download.py +++ b/tuf/download.py @@ -39,12 +39,12 @@ import time import timeit +import tuf import requests -import six import securesystemslib import securesystemslib.util -import tuf +import six import tuf.exceptions # See 'log.py' to learn how logging is handled in TUF. From 34e0ec7c62d3085e81056c1072577de8b48bf3c8 Mon Sep 17 00:00:00 2001 From: Trishank K Kuppusamy Date: Fri, 31 Aug 2018 14:51:31 -0400 Subject: [PATCH 03/34] Add TUF version number, and user agent Signed-off-by: Sebastien Awwad --- setup.py | 16 +++++++++++++++- tuf/__init__.py | 1 + tuf/download.py | 6 ++++-- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/setup.py b/setup.py index 880d11f7..9eb11e24 100755 --- a/setup.py +++ b/setup.py @@ -75,12 +75,26 @@ from setuptools import find_packages +def find_version(*file_paths): + """ + https://github.com/pypa/pip/blob/404838abcca467648180b358598c597b74d568c9/setup.py#L18-L28 + """ + + version_file = read(*file_paths) + version_match = re.search(r"^__version__ = ['\"]([^'\"]*)['\"]", + version_file, re.M) + if version_match: + return version_match.group(1) + raise RuntimeError("Unable to find version string.") + + with open('README.md') as file_object: long_description = file_object.read() + setup( name = 'tuf', - version = '0.11.1', + version = find_version("tuf", "__init__.py"), description = 'A secure updater framework for Python', long_description = long_description, long_description_content_type='text/markdown', diff --git a/tuf/__init__.py b/tuf/__init__.py index e69de29b..fee46bd8 100755 --- a/tuf/__init__.py +++ b/tuf/__init__.py @@ -0,0 +1 @@ +__version__ = "0.11.1" diff --git a/tuf/download.py b/tuf/download.py index 36811c0f..2298b122 100755 --- a/tuf/download.py +++ b/tuf/download.py @@ -232,13 +232,15 @@ def _download_file(url, required_length, STRICT_REQUIRED_LENGTH=True): try: # Attach some default headers to every Session. + requests_user_agent = _session.headers['User-Agent'] + # Follows the RFC: https://tools.ietf.org/html/rfc7231#section-5.5.3 + tuf_user_agent = 'tuf/' + tuf.__version__ + ' ' + requests_user_agent _session.headers.update({ # Tell the server not to compress or modify anything. # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Accept-Encoding#Directives 'Accept-Encoding': 'identity', # The TUF user agent. - # TODO: Attach version number, which cannot be easily found right now. - 'User-Agent': 'tuf' + 'User-Agent': tuf_user_agent }) # Get the requests.Response object for this URL. From 314f6e71b923c00be1c0b9c1e623e606f76854b0 Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Fri, 31 Aug 2018 17:04:27 -0400 Subject: [PATCH 04/34] Remove partial single-source version; add comments explaining Previous commit isn't going to work: read wasn't defined. Code provided was from here: https://packaging.python.org/guides/single-sourcing-package-version/ and is a little more complicated than is ideal. It'll also match comment lines if they exist. Single-sourcing version number isn't necessary for this pull request, but if I was going to do it, I'd probably add a VERSION file and have tuf/__init__.py and setup.py each read that in. There could be problems with that, too. I'm going to punt on this and keep the version in two places and we can fix that less urgently. (Also, the user agent reporting a version seems less critical in any case than the rest of the PR.) Version info will now be in two locations and require update in tandem. Signed-off-by: Sebastien Awwad --- setup.py | 15 +-------------- tuf/__init__.py | 4 ++++ 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/setup.py b/setup.py index 9eb11e24..bc8d8e1f 100755 --- a/setup.py +++ b/setup.py @@ -75,26 +75,13 @@ from setuptools import find_packages -def find_version(*file_paths): - """ - https://github.com/pypa/pip/blob/404838abcca467648180b358598c597b74d568c9/setup.py#L18-L28 - """ - - version_file = read(*file_paths) - version_match = re.search(r"^__version__ = ['\"]([^'\"]*)['\"]", - version_file, re.M) - if version_match: - return version_match.group(1) - raise RuntimeError("Unable to find version string.") - - with open('README.md') as file_object: long_description = file_object.read() setup( name = 'tuf', - version = find_version("tuf", "__init__.py"), + version = '0.11.1', # If updating version, also update it in tuf/__init__.py description = 'A secure updater framework for Python', long_description = long_description, long_description_content_type='text/markdown', diff --git a/tuf/__init__.py b/tuf/__init__.py index fee46bd8..e20a2f4a 100755 --- a/tuf/__init__.py +++ b/tuf/__init__.py @@ -1 +1,5 @@ +# This value is used in the requests user agent. +# setup.py has it hard-coded separately. +# Currently, when the version is changed, it must be set in both locations. +# TODO: Single-source the version number. __version__ = "0.11.1" From c25ce7c3be50e6c75d29f384e9160d7e9c5e8ee3 Mon Sep 17 00:00:00 2001 From: Trishank K Kuppusamy Date: Fri, 31 Aug 2018 16:44:39 -0400 Subject: [PATCH 05/34] use a different session per hostname Signed-off-by: Sebastien Awwad --- tuf/download.py | 54 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 35 insertions(+), 19 deletions(-) diff --git a/tuf/download.py b/tuf/download.py index 2298b122..2aa0d426 100755 --- a/tuf/download.py +++ b/tuf/download.py @@ -59,12 +59,10 @@ # which can result in a significant performance increase (see HTTP persistent # connection)." # -# FIXME: There are very likely security implications to sharing the same -# session. Ideally, tuf.client.updater would use a separate Session for every -# mirror, and it would pass the Session to be used in this module, but that -# requires a much more invasive change. -# -_session = requests.Session() +# NOTE: We use a separate requests.Session per hostname in order to reuse +# connections to the same hostname while avoiding sharing state between +# different hosts, and minimizing into subtle security issues. +_sessions = {} def safe_download(url, required_length): @@ -231,17 +229,35 @@ def _download_file(url, required_length, STRICT_REQUIRED_LENGTH=True): temp_file = securesystemslib.util.TempFile() try: - # Attach some default headers to every Session. - requests_user_agent = _session.headers['User-Agent'] - # Follows the RFC: https://tools.ietf.org/html/rfc7231#section-5.5.3 - tuf_user_agent = 'tuf/' + tuf.__version__ + ' ' + requests_user_agent - _session.headers.update({ - # Tell the server not to compress or modify anything. - # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Accept-Encoding#Directives - 'Accept-Encoding': 'identity', - # The TUF user agent. - 'User-Agent': tuf_user_agent - }) + # Use a different requests.Session per hostname to reuse connections while + # minimizing subtle security issues. + hostname = six.moves.urllib.parse.urlparse(url).hostname + + if not hostname: + raise securesystemslib.exceptions\ + .FormatError('Could not get hostname from: ' + url) + + session = _sessions.get(hostname) + + if not session: + session = requests.Session() + _sessions[hostname] = session + + # Attach some default headers to every Session. + requests_user_agent = session.headers['User-Agent'] + # Follows the RFC: https://tools.ietf.org/html/rfc7231#section-5.5.3 + tuf_user_agent = 'tuf/' + tuf.__version__ + ' ' + requests_user_agent + session.headers.update({ + # Tell the server not to compress or modify anything. + # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Accept-Encoding#Directives + 'Accept-Encoding': 'identity', + # The TUF user agent. + 'User-Agent': tuf_user_agent + }) + + logger.debug('Made new session for ' + hostname) + else: + logger.debug('Reusing session for ' + hostname) # Get the requests.Response object for this URL. # @@ -253,8 +269,8 @@ def _download_file(url, required_length, STRICT_REQUIRED_LENGTH=True): # # Always set the connect + read timeout: # http://docs.python-requests.org/en/master/user/advanced/#timeouts - response = _session.get(url, stream=True, - timeout=tuf.settings.SOCKET_TIMEOUT) + response = session.get(url, stream=True, + timeout=tuf.settings.SOCKET_TIMEOUT) # Check response status. response.raise_for_status() From f29622b2c60527223774c2e131e8750a5cacee17 Mon Sep 17 00:00:00 2001 From: Trishank K Kuppusamy Date: Tue, 4 Sep 2018 13:36:26 -0400 Subject: [PATCH 06/34] add debug statements Signed-off-by: Sebastien Awwad --- tuf/download.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tuf/download.py b/tuf/download.py index 2aa0d426..e55bdf40 100755 --- a/tuf/download.py +++ b/tuf/download.py @@ -236,6 +236,9 @@ def _download_file(url, required_length, STRICT_REQUIRED_LENGTH=True): if not hostname: raise securesystemslib.exceptions\ .FormatError('Could not get hostname from: ' + url) + else: + logger.debug('url: ' + url) + logger.debug('hostname: ' + hostname) session = _sessions.get(hostname) From 411c0de33e34ec27d7f34573a34574b7135372a1 Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Wed, 5 Sep 2018 11:55:51 -0400 Subject: [PATCH 07/34] minor: better use of junk variable in test_slow_retrieval_attack Also halves the duration of a failed test (from 800s to 400s). Otherwise, this is a code-style-only change. Signed-off-by: Sebastien Awwad --- tests/test_slow_retrieval_attack.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_slow_retrieval_attack.py b/tests/test_slow_retrieval_attack.py index 6fd985f8..78c8ac40 100755 --- a/tests/test_slow_retrieval_attack.py +++ b/tests/test_slow_retrieval_attack.py @@ -157,9 +157,9 @@ def setUp(self): # The slow retrieval server, in mode 2 (1 byte per second), will only # sleep for a total of (target file size) seconds. Add a target file # that contains sufficient number of bytes to trigger a slow retrieval - # error. "sufficient number of bytes" assumed to be 10x more. - extra_bytes = 8 - total_bytes = 100 * extra_bytes + # error. A transfer of 400 bytes should not be permitted to take 400 + # seconds. + total_bytes = 400 repository = repo_tool.load_repository(self.repository_directory) file1_filepath = os.path.join(self.repository_directory, 'targets', From a8debd7cc66ec79c89602c33354a888b2752a944 Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Wed, 5 Sep 2018 11:58:33 -0400 Subject: [PATCH 08/34] Test: improve failure behavior of test_slow_retrieval_attack and also clarify setup workflow in test_slow_retrieval_attack. Because the test was written incorrectly, the test was failing with a bad hash error after 31 seconds, when it should instead have been failing because the slow retrieval attack was not averted. In particular, a target file was updated, but metadata was not correctly updated on the repository and, further, the client's copy of the metadata was never updated. (The client continued to expect 31 bytes of target file instead of 400 or 800.) The way the test used to run, the target file change previously had no purpose. Signed-off-by: Sebastien Awwad --- tests/test_slow_retrieval_attack.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/test_slow_retrieval_attack.py b/tests/test_slow_retrieval_attack.py index 78c8ac40..8509dc6d 100755 --- a/tests/test_slow_retrieval_attack.py +++ b/tests/test_slow_retrieval_attack.py @@ -154,6 +154,12 @@ def setUp(self): shutil.copytree(original_client, self.client_directory) shutil.copytree(original_keystore, self.keystore_directory) + + # Produce a longer target file than exists in the other test repository + # data, to provide for a long-duration slow attack. Then we'll write new + # top-level metadata that includes a hash over that file, and provide that + # metadata to the client as well. + # The slow retrieval server, in mode 2 (1 byte per second), will only # sleep for a total of (target file size) seconds. Add a target file # that contains sufficient number of bytes to trigger a slow retrieval @@ -189,6 +195,19 @@ def setUp(self): shutil.copytree(os.path.join(self.repository_directory, 'metadata.staged'), os.path.join(self.repository_directory, 'metadata')) + # Since we've changed the repository metadata in this setup (by lengthening + # a target file and then writing new metadata), we also have to update the + # client metadata to get to the expected initial state, where the client + # knows the right target info (and so expects the right, longer target + # length. + # We'll skip using updater.refresh since we don't have a server running, + # and we'll update the metadata locally, manually. + shutil.rmtree(os.path.join( + self.client_directory, self.repository_name, 'metadata', 'current')) + shutil.copytree(os.path.join(self.repository_directory, 'metadata'), + os.path.join(self.client_directory, self.repository_name, 'metadata', + 'current')) + # Set the url prefix required by the 'tuf/client/updater.py' updater. # 'path/to/tmp/repository' -> 'localhost:8001/tmp/repository'. repository_basepath = self.repository_directory[len(os.getcwd()):] From cc4628735a2bd5da5954b9dee70a9450cc83277a Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Wed, 5 Sep 2018 12:04:44 -0400 Subject: [PATCH 09/34] Raise ReadTimeoutError from requests as TUF SlowRetrievalError so as to preserve 'API' of expected errors. Signed-off-by: Sebastien Awwad --- tuf/download.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tuf/download.py b/tuf/download.py index e55bdf40..300e7e17 100755 --- a/tuf/download.py +++ b/tuf/download.py @@ -47,6 +47,8 @@ import six import tuf.exceptions +import urllib3.exceptions + # See 'log.py' to learn how logging is handled in TUF. logger = logging.getLogger('tuf.download') @@ -333,7 +335,11 @@ def _download_fixed_amount_of_data(response, temp_file, required_length): Data from the server will be written to 'temp_file'. - Runtime or network exceptions will be raised without question. + tuf.exceptions.SlowRetrievalError + will be raised if urllib3.exceptions.ReadTimeoutError is caught (if the + download times out). + + Otherwise, runtime or network exceptions will be raised without question. A (total_downloaded, average_download_speed) tuple, where @@ -397,6 +403,11 @@ def _download_fixed_amount_of_data(response, temp_file, required_length): # Finally, we signal that the download is complete. break + except urllib3.exceptions.ReadTimeoutError as e: + # Whatever happens, make sure that we always close the connection. + response.close() + raise tuf.exceptions.SlowRetrievalError(str(e)) + except: # Whatever happens, make sure that we always close the connection. response.close() From 2f8782113642afc85f149d06b8e590eb7f6518b7 Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Mon, 10 Sep 2018 14:39:40 -0400 Subject: [PATCH 10/34] Test: mark slow retrieval test failure as expected... until a fix has been provided. This results from PR 781. See comments: https://github.com/theupdateframework/tuf/pull/781 Signed-off-by: Sebastien Awwad --- tests/test_slow_retrieval_attack.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/tests/test_slow_retrieval_attack.py b/tests/test_slow_retrieval_attack.py index 8509dc6d..66b11c53 100755 --- a/tests/test_slow_retrieval_attack.py +++ b/tests/test_slow_retrieval_attack.py @@ -163,9 +163,11 @@ def setUp(self): # The slow retrieval server, in mode 2 (1 byte per second), will only # sleep for a total of (target file size) seconds. Add a target file # that contains sufficient number of bytes to trigger a slow retrieval - # error. A transfer of 400 bytes should not be permitted to take 400 - # seconds. - total_bytes = 400 + # error. A transfer should not be permitted to take 1 second per byte + # transferred. Because this test is currently expected to fail, I'm + # limiting the size to 10 bytes (10 seconds) to avoid expected testing + # delays.... Consider increasing again after fix, to, e.g. 400. + total_bytes = 10 repository = repo_tool.load_repository(self.repository_directory) file1_filepath = os.path.join(self.repository_directory, 'targets', @@ -270,6 +272,13 @@ def test_with_tuf_mode_1(self): + # The following test fails as a result of a change to TUF's download code. + # Rather than constructing urllib2 requests, we now use the requests library. + # This solves an HTTPS proxy issue, but has for the moment deprived us of a + # way to prevent certain this kind of slow retrieval attack. + # See conversation in PR: https://github.com/theupdateframework/tuf/pull/781 + # TODO: Update download code to resolve the slow retrieval vulnerability. + @unittest.expectedFailure def test_with_tuf_mode_2(self): # Simulate a slow retrieval attack. # 'mode_2': During the download process, the server blocks the download From 8951e8b9a881266e770f25083618ce9b1267ec40 Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Mon, 10 Sep 2018 14:41:35 -0400 Subject: [PATCH 11/34] Style fixes in download.py within PR 781 changes - Remove trailing whitespace - Fix indent sizes (4 if continuing line, else 2) - Fix line continuation to match PEP 8 and lab code guidelines Also fixes one minor typo. Signed-off-by: Sebastien Awwad --- tuf/download.py | 50 ++++++++++++++++++++++++------------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/tuf/download.py b/tuf/download.py index 300e7e17..1ae0f79e 100755 --- a/tuf/download.py +++ b/tuf/download.py @@ -63,8 +63,8 @@ # # NOTE: We use a separate requests.Session per hostname in order to reuse # connections to the same hostname while avoiding sharing state between -# different hosts, and minimizing into subtle security issues. -_sessions = {} +# different hosts, and minimizing subtle security issues. +_sessions = {} def safe_download(url, required_length): @@ -236,33 +236,33 @@ def _download_file(url, required_length, STRICT_REQUIRED_LENGTH=True): hostname = six.moves.urllib.parse.urlparse(url).hostname if not hostname: - raise securesystemslib.exceptions\ - .FormatError('Could not get hostname from: ' + url) + raise securesystemslib.exceptions.FormatError( + 'Could not get hostname from: ' + url) else: - logger.debug('url: ' + url) - logger.debug('hostname: ' + hostname) + logger.debug('url: ' + url) + logger.debug('hostname: ' + hostname) session = _sessions.get(hostname) if not session: - session = requests.Session() - _sessions[hostname] = session + session = requests.Session() + _sessions[hostname] = session - # Attach some default headers to every Session. - requests_user_agent = session.headers['User-Agent'] - # Follows the RFC: https://tools.ietf.org/html/rfc7231#section-5.5.3 - tuf_user_agent = 'tuf/' + tuf.__version__ + ' ' + requests_user_agent - session.headers.update({ - # Tell the server not to compress or modify anything. - # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Accept-Encoding#Directives - 'Accept-Encoding': 'identity', - # The TUF user agent. - 'User-Agent': tuf_user_agent - }) + # Attach some default headers to every Session. + requests_user_agent = session.headers['User-Agent'] + # Follows the RFC: https://tools.ietf.org/html/rfc7231#section-5.5.3 + tuf_user_agent = 'tuf/' + tuf.__version__ + ' ' + requests_user_agent + session.headers.update({ + # Tell the server not to compress or modify anything. + # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Accept-Encoding#Directives + 'Accept-Encoding': 'identity', + # The TUF user agent. + 'User-Agent': tuf_user_agent}) + + logger.debug('Made new session for ' + hostname) - logger.debug('Made new session for ' + hostname) else: - logger.debug('Reusing session for ' + hostname) + logger.debug('Reusing session for ' + hostname) # Get the requests.Response object for this URL. # @@ -274,8 +274,8 @@ def _download_file(url, required_length, STRICT_REQUIRED_LENGTH=True): # # Always set the connect + read timeout: # http://docs.python-requests.org/en/master/user/advanced/#timeouts - response = session.get(url, stream=True, - timeout=tuf.settings.SOCKET_TIMEOUT) + response = session.get( + url, stream=True, timeout=tuf.settings.SOCKET_TIMEOUT) # Check response status. response.raise_for_status() @@ -364,8 +364,8 @@ def _download_fixed_amount_of_data(response, temp_file, required_length): if tuf.settings.SLEEP_BEFORE_ROUND: time.sleep(tuf.settings.SLEEP_BEFORE_ROUND) - read_amount = min(tuf.settings.CHUNK_SIZE, - required_length - number_of_bytes_received) + read_amount = min( + tuf.settings.CHUNK_SIZE, required_length - number_of_bytes_received) # NOTE: This may not handle some servers adding a Content-Encoding # header, which may cause urllib3 to misbehave: From d199610f949f8f8b3566665ea12a123e1cc8a152 Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Mon, 10 Sep 2018 14:45:42 -0400 Subject: [PATCH 12/34] DOC: Clarify and correct comments on download timeouts and call out need for more testing in a comment. Signed-off-by: Sebastien Awwad --- tests/slow_retrieval_server.py | 6 +----- tests/test_slow_retrieval_attack.py | 3 +++ tuf/download.py | 5 ++++- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/slow_retrieval_server.py b/tests/slow_retrieval_server.py index 576bb98d..6930c80a 100755 --- a/tests/slow_retrieval_server.py +++ b/tests/slow_retrieval_server.py @@ -73,11 +73,7 @@ def do_GET(self): # 'mode_2' else: DELAY = 1 - # Throttle the file by sending a character every few seconds. - # NOTE: The for-loop below completes early if the download file - # (len(data)) is small. 'download.py' waits at least - # 'settings.SLOW_START_GRACE_PERIOD' seconds before triggering a - # potential slow retrieval error. + # Throttle the file by sending a character every DELAY seconds. for i in range(len(data)): self.wfile.write(data[i].encode('utf-8')) time.sleep(DELAY) diff --git a/tests/test_slow_retrieval_attack.py b/tests/test_slow_retrieval_attack.py index 66b11c53..d3622ea8 100755 --- a/tests/test_slow_retrieval_attack.py +++ b/tests/test_slow_retrieval_attack.py @@ -31,6 +31,9 @@ does not fall below a required rate (tuf.settings.MIN_AVERAGE_DOWNLOAD_SPEED). Note: There is no difference between 'updates' and 'target' files. + + # TODO: Consider additional tests for slow metadata download. Tests here only + use slow target download. """ # Help with Python 3 compatibility, where the print statement is a function, an diff --git a/tuf/download.py b/tuf/download.py index 1ae0f79e..f2e47e5c 100755 --- a/tuf/download.py +++ b/tuf/download.py @@ -272,7 +272,10 @@ def _download_file(url, required_length, STRICT_REQUIRED_LENGTH=True): # We will always manually close Responses, so no need for a context # manager. # - # Always set the connect + read timeout: + # Always set the timeout. This timeout value is interpreted by requests as: + # - connect timeout (max delay before first byte is received) + # - read (gap) timeout (max delay between bytes received) + # These are NOT overall/total, wall-clock timeouts for any single read. # http://docs.python-requests.org/en/master/user/advanced/#timeouts response = session.get( url, stream=True, timeout=tuf.settings.SOCKET_TIMEOUT) From a5416d4baa8610408365fc449bb60051ecf542a3 Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Mon, 10 Sep 2018 16:00:13 -0400 Subject: [PATCH 13/34] Raise helpful error in download.py if cannot parse URL to extract hostname. After commit "use a different session per hostname", the code no longer raises MissingSchema if a URL is malformed in certain cases. Since it parses URLs to extract the hostname and would have raised securesystemslib.exceptions.FormatError, so the test would have to check for that error instead of requests's MissingSchema. However, it's best to use a different error type, since while that would be, true enough, a formatting error, FormatError is customarily reserved for the automatic detection based on schemas in formats.py (using .check_match()), and in any case it is not a securesystemslib error. So this commit adds error type tuf.exceptions.URLParsingError and raises it if the hostname cannot be isolated in a URL, and checks for it in test_download.py. Signed-off-by: Sebastien Awwad --- tests/test_download.py | 2 +- tuf/download.py | 2 +- tuf/exceptions.py | 4 ++++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/test_download.py b/tests/test_download.py index 1688f265..837b2e9e 100755 --- a/tests/test_download.py +++ b/tests/test_download.py @@ -170,7 +170,7 @@ def test_download_url_to_tempfileobj_and_urls(self): self.assertRaises(securesystemslib.exceptions.FormatError, download_file, None, self.target_data_length) - self.assertRaises(requests.exceptions.MissingSchema, + self.assertRaises(tuf.exceptions.URLParsingError, download_file, self.random_string(), self.target_data_length) diff --git a/tuf/download.py b/tuf/download.py index f2e47e5c..9170514f 100755 --- a/tuf/download.py +++ b/tuf/download.py @@ -236,7 +236,7 @@ def _download_file(url, required_length, STRICT_REQUIRED_LENGTH=True): hostname = six.moves.urllib.parse.urlparse(url).hostname if not hostname: - raise securesystemslib.exceptions.FormatError( + raise tuf.exceptions.URLParsingError( 'Could not get hostname from: ' + url) else: logger.debug('url: ' + url) diff --git a/tuf/exceptions.py b/tuf/exceptions.py index 3f5dca1d..3b72bd40 100755 --- a/tuf/exceptions.py +++ b/tuf/exceptions.py @@ -270,6 +270,10 @@ class URLMatchesNoPatternError(Error): """If a URL does not match a user-specified regular expression.""" pass +class URLParsingError(Error): + """If we are unable to parse a URL -- for example, if a hostname element + cannot be isoalted.""" + pass class InvalidConfigurationError(Error): """If a configuration object does not match the expected format.""" From 4595ab839a7806b8b9b2260c24c04ad6b195ada1 Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Mon, 10 Sep 2018 16:26:41 -0400 Subject: [PATCH 14/34] Remove unused os import in tuf.download It is not longer used, and pylint complains if it's left in. Signed-off-by: Sebastien Awwad --- tuf/download.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tuf/download.py b/tuf/download.py index 9170514f..54448997 100755 --- a/tuf/download.py +++ b/tuf/download.py @@ -34,7 +34,6 @@ from __future__ import division from __future__ import unicode_literals -import os import logging import time import timeit From 264186fa51d6092857cbfbc5d297add11d077b10 Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Mon, 10 Sep 2018 16:56:02 -0400 Subject: [PATCH 15/34] Session index: hostname-indexed --> scheme+hostname-indexed In order to avoid re-using session data from an HTTPS connection in an HTTP connection. Some cookies may not be HTTP-safe. Signed-off-by: Sebastien Awwad --- tuf/download.py | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/tuf/download.py b/tuf/download.py index 54448997..8de805e3 100755 --- a/tuf/download.py +++ b/tuf/download.py @@ -60,9 +60,10 @@ # which can result in a significant performance increase (see HTTP persistent # connection)." # -# NOTE: We use a separate requests.Session per hostname in order to reuse -# connections to the same hostname while avoiding sharing state between -# different hosts, and minimizing subtle security issues. +# NOTE: We use a separate requests.Session per scheme+hostname combination, in +# order to reuse connections to the same hostname to improve efficiency, but +# avoiding sharing state between different hosts-scheme combinations to +# minimize subtle security issues. Some cookies may not be HTTP-safe. _sessions = {} @@ -230,22 +231,24 @@ def _download_file(url, required_length, STRICT_REQUIRED_LENGTH=True): temp_file = securesystemslib.util.TempFile() try: - # Use a different requests.Session per hostname to reuse connections while - # minimizing subtle security issues. - hostname = six.moves.urllib.parse.urlparse(url).hostname + # Use a different requests.Session per schema+hostname combination, to + # reuse connections while minimizing subtle security issues. + parsed_url = six.moves.urllib.parse.urlparse(url) - if not hostname: + if not parsed_url.scheme or not parsed_url.hostname: raise tuf.exceptions.URLParsingError( - 'Could not get hostname from: ' + url) - else: - logger.debug('url: ' + url) - logger.debug('hostname: ' + hostname) + 'Could not get scheme and hostname from URL: ' + url) - session = _sessions.get(hostname) + session_index = parsed_url.scheme + '+' + parsed_url.hostname + + logger.debug('url: ' + url) + logger.debug('session index: ' + session_index) + + session = _sessions.get(session_index) if not session: session = requests.Session() - _sessions[hostname] = session + _sessions[session_index] = session # Attach some default headers to every Session. requests_user_agent = session.headers['User-Agent'] @@ -258,10 +261,10 @@ def _download_file(url, required_length, STRICT_REQUIRED_LENGTH=True): # The TUF user agent. 'User-Agent': tuf_user_agent}) - logger.debug('Made new session for ' + hostname) + logger.debug('Made new session for ' + session_index) else: - logger.debug('Reusing session for ' + hostname) + logger.debug('Reusing session for ' + session_index) # Get the requests.Response object for this URL. # From 8d64b5a2e102a0dba562cab5f248c6ab861cdd13 Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Tue, 11 Sep 2018 15:30:40 -0400 Subject: [PATCH 16/34] Test: test download.py w/ untrusted or bad-hostname SSL certs Rewrite test_https_connection to do a more thorough test, including the use of an unknown certificate and the use of a good certificate which lists a hostname not matching that expected in the connection. In the process, made some small changes to the simple_https_server module used in tests (takes an extra argument: certificate file to use). Given the extent of the changes to test_https_connection, I also made some style adjustments to better match our code style guidelines. I also reduced the length of a delay after the https servers started from 1s to 0.2s, as part of a general campaign to speed up the TUF tests. 200ms should do to start the servers, and if not, I'll adjust it upward. Signed-off-by: Sebastien Awwad --- tests/simple_https_server.py | 18 +++-- tests/ssl_cert_wronghost.crt | 31 +++++++++ tests/test_download.py | 124 +++++++++++++++++++++++++++++------ 3 files changed, 148 insertions(+), 25 deletions(-) create mode 100644 tests/ssl_cert_wronghost.crt diff --git a/tests/simple_https_server.py b/tests/simple_https_server.py index e260519c..adecbf6d 100755 --- a/tests/simple_https_server.py +++ b/tests/simple_https_server.py @@ -40,11 +40,14 @@ import sys import random import ssl - +import os import six PORT = 0 +keyfile = 'ssl_cert.key' +certfile = 'ssl_cert.crt' + def _generate_random_port(): return random.randint(30000, 45000) @@ -60,12 +63,19 @@ def _generate_random_port(): else: PORT = _generate_random_port() +if len(sys.argv) > 2: + + if os.path.exists(sys.argv[2]): + certfile = sys.argv[2] + else: + print('simple_https_server: cert file not found: ' + sys.argv[2] + + '; using default: ' + certfile) + httpd = six.moves.BaseHTTPServer.HTTPServer(('localhost', PORT), six.moves.SimpleHTTPServer.SimpleHTTPRequestHandler) -httpd.socket = ssl.wrap_socket(httpd.socket, keyfile='ssl_cert.key', - certfile='ssl_cert.crt', - server_side=True) +httpd.socket = ssl.wrap_socket( + httpd.socket, keyfile=keyfile, certfile=certfile, server_side=True) #print('Starting https server on port: ' + str(PORT)) httpd.serve_forever() diff --git a/tests/ssl_cert_wronghost.crt b/tests/ssl_cert_wronghost.crt new file mode 100644 index 00000000..df7bfa37 --- /dev/null +++ b/tests/ssl_cert_wronghost.crt @@ -0,0 +1,31 @@ +-----BEGIN CERTIFICATE----- +MIIFRTCCA62gAwIBAgIJAKY6b706lpuDMA0GCSqGSIb3DQEBCwUAMIGEMQswCQYD +VQQGEwJVUzERMA8GA1UECAwITmV3IFlvcmsxETAPBgNVBAcMCEJyb29rbHluMQww +CgYDVQQKDANOWVUxKTAnBgNVBAsMIENvbXB1dGVyIFNjaWVuY2UgYW5kIEVuZ2lu +ZWVyaW5nMRYwFAYDVQQDDA1ub3RteWhvc3RuYW1lMB4XDTE4MDkxMjE2NTkxN1oX +DTM4MDkwNzE2NTkxN1owgYQxCzAJBgNVBAYTAlVTMREwDwYDVQQIDAhOZXcgWW9y +azERMA8GA1UEBwwIQnJvb2tseW4xDDAKBgNVBAoMA05ZVTEpMCcGA1UECwwgQ29t +cHV0ZXIgU2NpZW5jZSBhbmQgRW5naW5lZXJpbmcxFjAUBgNVBAMMDW5vdG15aG9z +dG5hbWUwggGiMA0GCSqGSIb3DQEBAQUAA4IBjwAwggGKAoIBgQDHIVV5GxadvVSU +IoGSzZrMz1b0r4n9mTN1Jvp4LE4jG/v0z9SDngJ9aqTJJJpB6Oy7RT+AnGQVhG/B +ADLmUBOuyljaTKJZiKCBZcUNbP6LwRM+Qv3Ofn2/Xew0ilP8hdCPRLcyv1mexSfW +oLIQ44jEnFmXK5X8z9c/UM/i0CuES+t7HXZXoxAgr7x9jMSMcb9bu8gh637BNK0h +HwCu9VCLQq89tsYJ+tn576Cr5SkEsG4B8zMz3lNb5gnl24xyGJ7afosOY556ADiM +wS/aml46vxAkm1m4qldZVsm8uDzXKsOOvWsDAlH9ZCueTwZBY8LI8ud4ADiYluLv +kuqMvGfQCglveTMt8YQwkkRZeynE2DTpstBT3jyEHgvucoBGrqzsqgRKMRX78viS +gw9Ymg2rp/E5RXDb5LchM2sNy15xFDWPdEZU9qPmXpufTcLspX25Gt+oVjc5SOfs +DSQNzO9Geat7UFkil76Z8YUH5S7smJCgG/ojp+rKt0f2tfUtQqUCAwEAAaOBtzCB +tDCBowYDVR0jBIGbMIGYoYGKpIGHMIGEMQswCQYDVQQGEwJVUzERMA8GA1UECAwI +TmV3IFlvcmsxETAPBgNVBAcMCEJyb29rbHluMQwwCgYDVQQKDANOWVUxKTAnBgNV +BAsMIENvbXB1dGVyIFNjaWVuY2UgYW5kIEVuZ2luZWVyaW5nMRYwFAYDVQQDDA1u +b3RteWhvc3RuYW1lggkApjpvvTqWm4MwDAYDVR0TBAUwAwEB/zANBgkqhkiG9w0B +AQsFAAOCAYEAvpBMce3kxwo9W0o4RqezkSxnNyax0ezbUNodIkx5kbzX09qQLqhK +SkhQY3CNmtrpsczUg1W2nldxioEouwfTlhi15H98E/8XytpGaHO7Rnbtq8nkOp3E +N1+DMfFR95OynbHSd7bfK9UEmH1CmCnttvCuQkLTxDCpEsQNAxvmU/yDONoDr+cu +jGo80XTnYTqHl5/UtGbCS4SAIdWgrXTIqVvY/eF+mR+3nQEYjBuqW0cNfXLyYLXH +XMc6qtfGX1P+NRWtlrWgGQmc0fry+GczRHMJuKtJMV2xZzPJAJqwwvj3Fjz8HNGu +ZX3kVdbkDjf8is2cWgyZqDecqPHDBW4Ey539s/5eurgOkEvhriS4/9RnVhgdzduj +nRdXkD10ficrFcBQO0KaTWT+iFBc9duuYPuLRyRTye5p3t0liOikH2XrRXs4IBfz +2mT4npXQl1liNixcCf/yUEUOSQAJDG6aRjDjD4SZBUPDLjfqKLid8M0BpLQrks9L +5hAg1WZXorY6 +-----END CERTIFICATE----- diff --git a/tests/test_download.py b/tests/test_download.py index 837b2e9e..6e493aaa 100755 --- a/tests/test_download.py +++ b/tests/test_download.py @@ -193,43 +193,125 @@ def test_download_url_to_tempfileobj_and_urls(self): + + + # This test uses sites on the internet, requiring a net connection to succeed. + # Since this is the only such test in TUF, I'm not going to enable it... but + # it's here in case it's useful for diagnosis. + # def test_https_validation(self): + # """ + # Use some known URLs on the net to ensure that TUF download checks SSL + # certificates appropriately. + # """ + # # We should never get as far as the target file download itself, so the + # # length we pass to safe_download and unsafe_download shouldn't matter. + # irrelevant_length = 10 + # + # for bad_url in [ + # 'https://expired.badssl.com/', # expired certificate + # 'https://wrong.host.badssl.com/', ]: # hostname verification fail + # + # with self.assertRaises(requests.exceptions.SSLError): + # download.safe_download(bad_url, irrelevant_length) + # + # with self.assertRaises(requests.exceptions.SSLError): + # download.unsafe_download(bad_url, irrelevant_length) + + + + + def test_https_connection(self): + """ + Try various HTTPS downloads using trusted and untrusted certificates with + and without the correct hostname listed in the SSL certificate. + """ # Make a temporary file to be served to the client. current_directory = os.getcwd() target_filepath = self.make_temp_data_file(directory=current_directory) - target_data = None - target_data_length = 0 with open(target_filepath, 'r') as target_file_object: - target_data = target_file_object.read() - target_data_length = len(target_data) + target_data_length = len(target_file_object.read()) - # Launch an https server (serves files in the current dir). - port = random.randint(30000, 45000) - command = ['python', 'simple_https_server.py', str(port)] - https_server_process = subprocess.Popen(command, stderr=subprocess.PIPE) + # These cert files should both be valid, but "good" lists localhost and + # "bad" lists another hostname. We'll be trying to download from localhost, + # so we expect + good_cert_fname = 'ssl_cert.crt' + bad_cert_fname = 'ssl_cert_wronghost.crt' + + # Launch two https servers (serves files in the current dir). + # The first we expect to operate correctly, and the second we run with an + # HTTPS certificate with an unexpected hostname. + port1 = str(random.randint(30000, 45000)) + port2 = str(int(port1) + 1) + command1 = ['python', 'simple_https_server.py', port1, good_cert_fname] + command2 = ['python', 'simple_https_server.py', port2, bad_cert_fname] + good_https_server_proc = subprocess.Popen(command1, stderr=subprocess.PIPE) + bad_https_server_proc = subprocess.Popen(command2, stderr=subprocess.PIPE) # NOTE: Following error is raised if delay is not applied: # - time.sleep(1) + time.sleep(0.2) + + relative_target_fpath = os.path.basename(target_filepath) + good_https_url = 'https://localhost:' + port1 + '/' + relative_target_fpath + bad_https_url = good_https_url.replace(':' + port1, ':' + port2) - junk, relative_target_filepath = os.path.split(target_filepath) - https_url = 'https://localhost:' + str(port) + '/' + relative_target_filepath # Download the target file using an https connection. - os.environ['REQUESTS_CA_BUNDLE'] = 'ssl_cert.crt' - message = 'Downloading target file from https server: ' + https_url - logger.info(message) + + # Use try-finally solely to ensure that the server processes are killed. try: - download.safe_download(https_url, target_data_length) - download.unsafe_download(https_url, target_data_length) + # Trust the certfile that happens to use a different hostname than we + # will expect. + os.environ['REQUESTS_CA_BUNDLE'] = bad_cert_fname + + # Try connecting to the server process with the bad cert while trusting + # the bad cert. Expect failure because even though we trust it, the + # hostname we're connecting to does not match the hostname in the cert. + logger.info('Trying https download of target file: ' + bad_https_url) + with self.assertRaises(requests.exceptions.SSLError): + download.safe_download(bad_https_url, target_data_length) + with self.assertRaises(requests.exceptions.SSLError): + download.unsafe_download(bad_https_url, target_data_length) + + # Try connecting to the server process with the good cert while not + # trusting the good cert (trusting the bad cert instead). Expect failure + # because even though the server's cert file is otherwise OK, we don't + # trust it. + print('Trying https download of target file: ' + good_https_url) + with self.assertRaises(requests.exceptions.SSLError): + download.safe_download(good_https_url, target_data_length) + with self.assertRaises(requests.exceptions.SSLError): + download.unsafe_download(good_https_url, target_data_length) + + # Configure environment to trust the good cert file now instead. + os.environ['REQUESTS_CA_BUNDLE'] = good_cert_fname + + # Try connecting to the server process with the good cert while trusting + # that cert. Expect success. + # Note: running these OK downloads at the top of this try section causes + # a failure in a previous assertion: retrieving the same good URL + # again after no longer "trusting" the good certfile still succeeds + # if we had previously succeeded in retrieving that same URL while + # still trusting the good cert. Perhaps it's a caching issue....? + # I'm not especially concerned yet, but take note for later.... + logger.info('Trying https download of target file: ' + good_https_url) + download.safe_download(good_https_url, target_data_length) + download.unsafe_download(good_https_url, target_data_length) finally: - if https_server_process.returncode is None: - message = \ - 'Server process ' + str(https_server_process.pid) + ' terminated.' - logger.info(message) - https_server_process.kill() + if good_https_server_proc.returncode is None: + logger.info( + 'Terminating server process ' + str(good_https_server_proc.pid)) + good_https_server_proc.kill() + + if bad_https_server_proc.returncode is None: + logger.info( + 'Terminating server process ' + str(bad_https_server_proc.pid)) + bad_https_server_proc.kill() + + From 46b584d8eb301716a186fb0181a8e2cb5c9eba14 Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Thu, 20 Sep 2018 10:02:03 -0400 Subject: [PATCH 17/34] Test: try download.py through via various proxies - adds inaz2/proxy2.py, copied code. - adds dev dependency on twisted for a simple proxy test - adds a new test module, test_proxy_use, and runs those tests only in Python2.7 (as proxy2 only supports Python2.7) using new logic in aggregate_tests.py. Signed-off-by: Sebastien Awwad --- dev-requirements.txt | 2 + tests/aggregate_tests.py | 18 ++ tests/proxy2.py | 410 +++++++++++++++++++++++++++++++++++++ tests/simple_proxy.py | 130 ++++++++++++ tests/test_proxy_use.py | 428 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 988 insertions(+) create mode 100644 tests/proxy2.py create mode 100644 tests/simple_proxy.py create mode 100644 tests/test_proxy_use.py diff --git a/dev-requirements.txt b/dev-requirements.txt index 10e78433..cc155b16 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -23,6 +23,7 @@ iso8601==0.1.12 isort==4.3.4 lazy-object-proxy==1.3.1 mccabe==0.6.1 +# mitmproxy==4.0.4 ; python_version >= "3.6" pbr==4.2.0 pluggy==0.7.1 py==1.5.4 @@ -37,6 +38,7 @@ singledispatch==3.4.0.3 six==1.11.0 smmap2==2.0.4 stevedore==1.29.0 +twisted==18.7.0 tox==3.2.1 virtualenv==16.0.0 wrapt==1.10.11 diff --git a/tests/aggregate_tests.py b/tests/aggregate_tests.py index 44fd2588..495e8929 100755 --- a/tests/aggregate_tests.py +++ b/tests/aggregate_tests.py @@ -45,11 +45,29 @@ # and run in a test suite. tests_list = glob.glob('test_*.py') +# A dictionary of test modules that should only run in certain python versions. +# Carefully consider the impact of only testing these in a given version. +# test_proxy_use.py: uses a proxy that only runs in Python2.7. TUF's +# compatibility with proxies is not likely to vary based on the Python version +# in use, so this is OK for now. See comments in that module. +# The format here is: if major is included, limit version to that listed here. +# If minor is included, limit version to that listed here. Skip the test if any +# such listed constraints don't match the python version currently running. +VERSION_SPECIFIC_TESTS = { + 'test_proxy_use': {'major': 2, 'minor': 7}} # Run test only if Python2.7 + # Remove '.py' from each filename to allow loadTestsFromNames() (called below) # to properly load the file as a module. tests_without_extension = [] for test in tests_list: test = test[:-3] + if test in VERSION_SPECIFIC_TESTS: + if 'major' in VERSION_SPECIFIC_TESTS[test] \ + and sys.version_info.major != VERSION_SPECIFIC_TESTS[test]['major']: + continue + elif 'minor' in VERSION_SPECIFIC_TESTS[test] \ + and sys.version_info.minor != VERSION_SPECIFIC_TESTS[test]['minor']: + continue tests_without_extension.append(test) # Randomize the order in which the tests run. Randomization might catch errors diff --git a/tests/proxy2.py b/tests/proxy2.py new file mode 100644 index 00000000..975478ba --- /dev/null +++ b/tests/proxy2.py @@ -0,0 +1,410 @@ +#!/usr/bin/env python + +# This code is taken from: https://github.com/inaz2/proxy2 +# Credit goes to the author. It has been very slightly modified here solely to +# switch from IPv6 use to IPv4 use. (Modified lines are marked '# MODIFIED'.) +# +# Because this is a helper module for a test, the style is less important, and +# so to minimize changes from the source, it has NOT been changed to match the +# TUF project's code style. + +""" + + proxy2.py + + + Taken from a repository set to BSD 3-Clause "New" or "Revised" License. See: + https://github.com/inaz2/proxy2/blob/b2bab648173ac69f0a10421750125517accdfe26/LICENSE + + + Serves as an HTTP, HTTP CONNECT (TCP), and HTTPS proxy, for testing purposes. + This is used by test_proxy_use.py. + + This module requires Python2.7 and does not support Python3. +""" + +import sys +import os +import socket +import ssl +import select +import httplib +import urlparse +import threading +import gzip +import zlib +import time +import json +import re +from BaseHTTPServer import HTTPServer, BaseHTTPRequestHandler +from SocketServer import ThreadingMixIn +from cStringIO import StringIO +from subprocess import Popen, PIPE +from HTMLParser import HTMLParser + + +def with_color(c, s): + return "\x1b[%dm%s\x1b[0m" % (c, s) + +def join_with_script_dir(path): + return os.path.join(os.path.dirname(os.path.abspath(__file__)), path) + + +class ThreadingHTTPServer(ThreadingMixIn, HTTPServer): + address_family = socket.AF_INET # MODIFIED + daemon_threads = True + + def handle_error(self, request, client_address): + # surpress socket/ssl related errors + cls, e = sys.exc_info()[:2] + if cls is socket.error or cls is ssl.SSLError: + pass + else: + return HTTPServer.handle_error(self, request, client_address) + + +class ProxyRequestHandler(BaseHTTPRequestHandler): + cakey = join_with_script_dir('ca.key') + cacert = join_with_script_dir('ca.crt') + certkey = join_with_script_dir('cert.key') + certdir = join_with_script_dir('certs/') + timeout = 5 + lock = threading.Lock() + + def __init__(self, *args, **kwargs): + self.tls = threading.local() + self.tls.conns = {} + + BaseHTTPRequestHandler.__init__(self, *args, **kwargs) + + def log_error(self, format, *args): + # surpress "Request timed out: timeout('timed out',)" + if isinstance(args[0], socket.timeout): + return + + self.log_message(format, *args) + + def do_CONNECT(self): + if os.path.isfile(self.cakey) and os.path.isfile(self.cacert) and os.path.isfile(self.certkey) and os.path.isdir(self.certdir): + self.connect_intercept() + else: + self.connect_relay() + + def connect_intercept(self): + hostname = self.path.split(':')[0] + certpath = "%s/%s.crt" % (self.certdir.rstrip('/'), hostname) + + with self.lock: + if not os.path.isfile(certpath): + epoch = "%d" % (time.time() * 1000) + p1 = Popen(["openssl", "req", "-new", "-key", self.certkey, "-subj", "/CN=%s" % hostname], stdout=PIPE) + p2 = Popen(["openssl", "x509", "-req", "-days", "3650", "-CA", self.cacert, "-CAkey", self.cakey, "-set_serial", epoch, "-out", certpath], stdin=p1.stdout, stderr=PIPE) + p2.communicate() + + self.wfile.write("%s %d %s\r\n" % (self.protocol_version, 200, 'Connection Established')) + self.end_headers() + + self.connection = ssl.wrap_socket(self.connection, keyfile=self.certkey, certfile=certpath, server_side=True) + self.rfile = self.connection.makefile("rb", self.rbufsize) + self.wfile = self.connection.makefile("wb", self.wbufsize) + + conntype = self.headers.get('Proxy-Connection', '') + if self.protocol_version == "HTTP/1.1" and conntype.lower() != 'close': + self.close_connection = 0 + else: + self.close_connection = 1 + + def connect_relay(self): + address = self.path.split(':', 1) + address[1] = int(address[1]) or 443 + try: + s = socket.create_connection(address, timeout=self.timeout) + except Exception as e: + self.send_error(502) + return + self.send_response(200, 'Connection Established') + self.end_headers() + + conns = [self.connection, s] + self.close_connection = 0 + while not self.close_connection: + rlist, wlist, xlist = select.select(conns, [], conns, self.timeout) + if xlist or not rlist: + break + for r in rlist: + other = conns[1] if r is conns[0] else conns[0] + data = r.recv(8192) + if not data: + self.close_connection = 1 + break + other.sendall(data) + + def do_GET(self): + if self.path == 'http://proxy2.test/': + self.send_cacert() + return + + req = self + content_length = int(req.headers.get('Content-Length', 0)) + req_body = self.rfile.read(content_length) if content_length else None + + if req.path[0] == '/': + if isinstance(self.connection, ssl.SSLSocket): + req.path = "https://%s%s" % (req.headers['Host'], req.path) + else: + req.path = "http://%s%s" % (req.headers['Host'], req.path) + + req_body_modified = self.request_handler(req, req_body) + if req_body_modified is False: + self.send_error(403) + return + elif req_body_modified is not None: + req_body = req_body_modified + req.headers['Content-length'] = str(len(req_body)) + + u = urlparse.urlsplit(req.path) + scheme, netloc, path = u.scheme, u.netloc, (u.path + '?' + u.query if u.query else u.path) + assert scheme in ('http', 'https') + if netloc: + req.headers['Host'] = netloc + setattr(req, 'headers', self.filter_headers(req.headers)) + + try: + origin = (scheme, netloc) + if not origin in self.tls.conns: + if scheme == 'https': + self.tls.conns[origin] = httplib.HTTPSConnection(netloc, timeout=self.timeout) + else: + self.tls.conns[origin] = httplib.HTTPConnection(netloc, timeout=self.timeout) + conn = self.tls.conns[origin] + conn.request(self.command, path, req_body, dict(req.headers)) + res = conn.getresponse() + + version_table = {10: 'HTTP/1.0', 11: 'HTTP/1.1'} + setattr(res, 'headers', res.msg) + setattr(res, 'response_version', version_table[res.version]) + + # support streaming + if not 'Content-Length' in res.headers and 'no-store' in res.headers.get('Cache-Control', ''): + self.response_handler(req, req_body, res, '') + setattr(res, 'headers', self.filter_headers(res.headers)) + self.relay_streaming(res) + with self.lock: + self.save_handler(req, req_body, res, '') + return + + res_body = res.read() + except Exception as e: + if origin in self.tls.conns: + del self.tls.conns[origin] + self.send_error(502) + return + + content_encoding = res.headers.get('Content-Encoding', 'identity') + res_body_plain = self.decode_content_body(res_body, content_encoding) + + res_body_modified = self.response_handler(req, req_body, res, res_body_plain) + if res_body_modified is False: + self.send_error(403) + return + elif res_body_modified is not None: + res_body_plain = res_body_modified + res_body = self.encode_content_body(res_body_plain, content_encoding) + res.headers['Content-Length'] = str(len(res_body)) + + setattr(res, 'headers', self.filter_headers(res.headers)) + + self.wfile.write("%s %d %s\r\n" % (self.protocol_version, res.status, res.reason)) + for line in res.headers.headers: + self.wfile.write(line) + self.end_headers() + self.wfile.write(res_body) + self.wfile.flush() + + with self.lock: + self.save_handler(req, req_body, res, res_body_plain) + + def relay_streaming(self, res): + self.wfile.write("%s %d %s\r\n" % (self.protocol_version, res.status, res.reason)) + for line in res.headers.headers: + self.wfile.write(line) + self.end_headers() + try: + while True: + chunk = res.read(8192) + if not chunk: + break + self.wfile.write(chunk) + self.wfile.flush() + except socket.error: + # connection closed by client + pass + + do_HEAD = do_GET + do_POST = do_GET + do_PUT = do_GET + do_DELETE = do_GET + do_OPTIONS = do_GET + + def filter_headers(self, headers): + # http://tools.ietf.org/html/rfc2616#section-13.5.1 + hop_by_hop = ('connection', 'keep-alive', 'proxy-authenticate', 'proxy-authorization', 'te', 'trailers', 'transfer-encoding', 'upgrade') + for k in hop_by_hop: + del headers[k] + + # accept only supported encodings + if 'Accept-Encoding' in headers: + ae = headers['Accept-Encoding'] + filtered_encodings = [x for x in re.split(r',\s*', ae) if x in ('identity', 'gzip', 'x-gzip', 'deflate')] + headers['Accept-Encoding'] = ', '.join(filtered_encodings) + + return headers + + def encode_content_body(self, text, encoding): + if encoding == 'identity': + data = text + elif encoding in ('gzip', 'x-gzip'): + io = StringIO() + with gzip.GzipFile(fileobj=io, mode='wb') as f: + f.write(text) + data = io.getvalue() + elif encoding == 'deflate': + data = zlib.compress(text) + else: + raise Exception("Unknown Content-Encoding: %s" % encoding) + return data + + def decode_content_body(self, data, encoding): + if encoding == 'identity': + text = data + elif encoding in ('gzip', 'x-gzip'): + io = StringIO(data) + with gzip.GzipFile(fileobj=io) as f: + text = f.read() + elif encoding == 'deflate': + try: + text = zlib.decompress(data) + except zlib.error: + text = zlib.decompress(data, -zlib.MAX_WBITS) + else: + raise Exception("Unknown Content-Encoding: %s" % encoding) + return text + + def send_cacert(self): + with open(self.cacert, 'rb') as f: + data = f.read() + + self.wfile.write("%s %d %s\r\n" % (self.protocol_version, 200, 'OK')) + self.send_header('Content-Type', 'application/x-x509-ca-cert') + self.send_header('Content-Length', len(data)) + self.send_header('Connection', 'close') + self.end_headers() + self.wfile.write(data) + + def print_info(self, req, req_body, res, res_body): + def parse_qsl(s): + return '\n'.join("%-20s %s" % (k, v) for k, v in urlparse.parse_qsl(s, keep_blank_values=True)) + + req_header_text = "%s %s %s\n%s" % (req.command, req.path, req.request_version, req.headers) + res_header_text = "%s %d %s\n%s" % (res.response_version, res.status, res.reason, res.headers) + + print with_color(33, req_header_text) + + u = urlparse.urlsplit(req.path) + if u.query: + query_text = parse_qsl(u.query) + print with_color(32, "==== QUERY PARAMETERS ====\n%s\n" % query_text) + + cookie = req.headers.get('Cookie', '') + if cookie: + cookie = parse_qsl(re.sub(r';\s*', '&', cookie)) + print with_color(32, "==== COOKIE ====\n%s\n" % cookie) + + auth = req.headers.get('Authorization', '') + if auth.lower().startswith('basic'): + token = auth.split()[1].decode('base64') + print with_color(31, "==== BASIC AUTH ====\n%s\n" % token) + + if req_body is not None: + req_body_text = None + content_type = req.headers.get('Content-Type', '') + + if content_type.startswith('application/x-www-form-urlencoded'): + req_body_text = parse_qsl(req_body) + elif content_type.startswith('application/json'): + try: + json_obj = json.loads(req_body) + json_str = json.dumps(json_obj, indent=2) + if json_str.count('\n') < 50: + req_body_text = json_str + else: + lines = json_str.splitlines() + req_body_text = "%s\n(%d lines)" % ('\n'.join(lines[:50]), len(lines)) + except ValueError: + req_body_text = req_body + elif len(req_body) < 1024: + req_body_text = req_body + + if req_body_text: + print with_color(32, "==== REQUEST BODY ====\n%s\n" % req_body_text) + + print with_color(36, res_header_text) + + cookies = res.headers.getheaders('Set-Cookie') + if cookies: + cookies = '\n'.join(cookies) + print with_color(31, "==== SET-COOKIE ====\n%s\n" % cookies) + + if res_body is not None: + res_body_text = None + content_type = res.headers.get('Content-Type', '') + + if content_type.startswith('application/json'): + try: + json_obj = json.loads(res_body) + json_str = json.dumps(json_obj, indent=2) + if json_str.count('\n') < 50: + res_body_text = json_str + else: + lines = json_str.splitlines() + res_body_text = "%s\n(%d lines)" % ('\n'.join(lines[:50]), len(lines)) + except ValueError: + res_body_text = res_body + elif content_type.startswith('text/html'): + m = re.search(r']*>\s*([^<]+?)\s*', res_body, re.I) + if m: + h = HTMLParser() + print with_color(32, "==== HTML TITLE ====\n%s\n" % h.unescape(m.group(1).decode('utf-8'))) + elif content_type.startswith('text/') and len(res_body) < 1024: + res_body_text = res_body + + if res_body_text: + print with_color(32, "==== RESPONSE BODY ====\n%s\n" % res_body_text) + + def request_handler(self, req, req_body): + pass + + def response_handler(self, req, req_body, res, res_body): + pass + + def save_handler(self, req, req_body, res, res_body): + self.print_info(req, req_body, res, res_body) + + +def test(HandlerClass=ProxyRequestHandler, ServerClass=ThreadingHTTPServer, protocol="HTTP/1.1"): + if sys.argv[1:]: + port = int(sys.argv[1]) + else: + port = 8080 + server_address = ('127.0.0.1', port) # <~> changed from '::1' + + HandlerClass.protocol_version = protocol + httpd = ServerClass(server_address, HandlerClass) + + sa = httpd.socket.getsockname() + print "Serving HTTP Proxy on", sa[0], "port", sa[1], "..." + httpd.serve_forever() + + +if __name__ == '__main__': + test() diff --git a/tests/simple_proxy.py b/tests/simple_proxy.py new file mode 100644 index 00000000..7e07ecf0 --- /dev/null +++ b/tests/simple_proxy.py @@ -0,0 +1,130 @@ +#!/usr/bin/env python + +# Copyright 2018, New York University and the TUF contributors +# SPDX-License-Identifier: MIT OR Apache-2.0 + +""" + + simple_proxy.py + + + See LICENSE-MIT OR LICENSE for licensing information. + + + Provide a simple http proxy server that can be used by TUF tests. + See test_proxy_use.py to see how it's used in the tests. + + Arguments are optional, but: + If provided, the first argument is assumed to choose HTTP vs HTTPS proxy. + If the first argument is 'https' (case insensitive), an HTTPS proxy will be + run, else an HTTP proxy will be run. + If provided, the second argument is assumed to indicate the port on which + the proxy should run. It must be 1023 1: + if sys.argv[1].lower() in ['https', 'http_smart', 'http_dumb']: + proxy_type = sys.argv[1].lower() + else: + print('\n\nUNEXPECTED PROTOCOL FOR PROXY SERVER. Using ' + proxy_type + + ' instead.\n\n') + + if len(sys.argv) > 2: + try: + port = int(sys.argv[2]) + except: + print('Ignoring argument. Second argument expected to be port, an int ' + 'i > 1023 and i < 65536. Generating randomly between 8090 and 30000.') + pass + + if port < 1024 or port > 65536: + port = _generate_random_port() + + if proxy_type == 'https': + run_https_proxy(port) + elif proxy_type == 'http_dumb': + run_http_dumb_proxy(port) + elif proxy_type == 'http_smart': + run_http_smart_proxy(port) + else: + raise Exception('Unexpected proxy type requested....') + + + + + +def run_http_dumb_proxy(port): + """ + This proxy doesn't know what to do with HTTP CONNECT requests, so it can't + pass on HTTPS requests. + """ + + twisted.python.log.startLogging(sys.stdout) + + class ProxyFactory(twisted.web.http.HTTPFactory): + protocol = twisted.web.proxy.Proxy + + twisted.internet.reactor.listenTCP(port, ProxyFactory()) + twisted.internet.reactor.run() + + + + + +def run_http_smart_proxy(port): + """ + This proxy needs to support HTTP CONNECT requests so that it can create a + TCP tunnel for HTTPS requests to the target server to go through. + """ + raise NotImplementedError() + + + + + +def run_https_proxy(port): + """ + Run a proxy that connects using HTTPS (has its own validated certificate + that the client accepts, etc.) before passing the request on to the target + server. + """ + raise NotImplementedError() + + + + + +if __name__ == '__main__': + main() diff --git a/tests/test_proxy_use.py b/tests/test_proxy_use.py new file mode 100644 index 00000000..0b36adb5 --- /dev/null +++ b/tests/test_proxy_use.py @@ -0,0 +1,428 @@ +#!/usr/bin/env python + +# Copyright 2018, New York University and the TUF contributors +# SPDX-License-Identifier: MIT OR Apache-2.0 + +""" + + test_proxy_use.py + + + See LICENSE-MIT OR LICENSE for licensing information. + + + Integration/regression test of TUF downloads through proxies. + + NOTE: Make sure test_proxy_use.py is run in 'tuf/tests/' directory. + Otherwise, test data or scripts may not be found. + + THIS module requires Python3.6+, as it uses mitmproxy, which only supports + Python3.6+. + + So long as the tests succeed in Python 3.6+, it is unlikely that TUF + behaves differently with respect to proxies when it runs in other Python + versions. + + As a result of this dependency on a modern Python version, this test does + not run with the other unit tests, being specifically excluded from + aggregate_tests.py. +""" + +# Help with Python 3 compatibility, where the print statement is a function, an +# implicit relative import is invalid, and the '/' operator performs true +# division. Example: print 'hello world' raises a 'SyntaxError' exception. +from __future__ import print_function +from __future__ import absolute_import +from __future__ import division +from __future__ import unicode_literals + +import hashlib +import logging +import os +import random +import subprocess +import time +import unittest + +import tuf +import tuf.download as download +import tuf.log +import tuf.unittest_toolbox as unittest_toolbox +import tuf.exceptions + +import requests.exceptions + +import securesystemslib +import six + +logger = logging.getLogger('tuf.test_download') + +class TestWithProxies(unittest_toolbox.Modified_TestCase): + + @classmethod + def setUpClass(cls): + """ + Setup performed before the first test function (TestWithProxies class + method) runs. + Launch http, https, and proxy servers in the current working directory. + """ + + unittest_toolbox.Modified_TestCase.setUpClass() + + # Launch a simple HTTP server (serves files in the current dir). + cls.http_port = random.randint(30000, 45000) + cls.http_server_proc = subprocess.Popen( + ['python', 'simple_server.py', str(cls.http_port)], + stderr=subprocess.PIPE) + # logger.info('\n\tHTTP server process started.') + # logger.info('\tHTTP server process id: ' + str(cls.http_server_proc.pid)) + # logger.info('\tServing HTTP on port: ' + str(cls.http_port)) + + # Launch an HTTPS server (serves files in the current dir). + cls.https_port = cls.http_port + 1 + cls.https_server_proc = subprocess.Popen( + ['python', 'simple_https_server.py', str(cls.https_port)], + stderr=subprocess.PIPE) + # logger.info('\n\tHTTPS server process started.') + # logger.info('\tHTTPS server process id: ' + str(cls.https_server_proc.pid)) + # logger.info('\tServing HTTPS on port: ' + str(cls.https_port)) + + # Launch a very basic HTTP proxy server. I think this one won't even handle + # HTTP CONNECT (and so can't pass HTTPS requests on) + cls.http_proxy_port = cls.http_port + 2 + cls.http_proxy_proc = subprocess.Popen( + ['python', 'simple_proxy.py', 'http_dumb', str(cls.http_proxy_port)], + stderr=subprocess.PIPE) + # logger.info('\n\tProxy server process started.') + # logger.info('\tHTTP Proxy process id: ' + str(cls.http_proxy_proc.pid)) + # logger.info('\tHTTP Proxy listening on port: ' + str(cls.http_proxy_proc)) + + # Launch a less basic HTTP proxy server. This one should be able to handle + # HTTP CONNECT requests, and so should be able to pass HTTPS requests on to + # the target server. + + cls.http_proxy_port2 = cls.http_port + 4 + + # Try proxy.py: + # Nope, proxy.py doesn't support HTTP CONNECT. + # cls.http_proxy_proc2 = subprocess.Popen( + # ['proxy.py', '--port', str(cls.http_proxy_port2)], + # stderr=subprocess.PIPE) + + # Once a working proxy is chosen, it should probably be run this way instead: + # cls.http_proxy_proc2 = subprocess.Popen( + # ['python', 'simple_proxy.py', 'http_smart', str(cls.http_proxy_port2)], + # stderr=subprocess.PIPE) + + # So, mitm_proxy doesn't support HTTP CONNECT, either, because it expects to + # tinker with things and isn't interested in being blind to the data. + # mitm_proxy can't be started this way; it expects a console. + # NOTE: Start it manually outside of this tester for now, using: + # >>> mitmproxy --listen-host 127.0.0.1 --listen-port 8899 + # cls.http_proxy_proc2 = subprocess.Popen( + # ['mitmproxy', '--listen-host', '127.0.0.1', + # '--listen-port', str(cls.http_proxy_port2)], + # stderr=subprocess.PIPE) + # logger.info('\n\tProxy server process started.') + # logger.info('\tHTTP Proxy process id: ' + str(cls.http_proxy_proc2.pid)) + # logger.info('\tHTTP Proxy listening on port: ' + str(cls.http_proxy_proc2)) + + # Let's try inaz2/proxy2.... + # This seems to work, once modified to use IPv4. + cls.http_proxy_proc2 = subprocess.Popen( + ['python', 'proxy2.py', str(cls.http_proxy_port2)], + stderr=subprocess.PIPE) + + + + # TODO: Launch a basic HTTPS proxy server. + # # Launch a basic HTTPS proxy server. + # cls.https_proxy_port = cls.http_port + 11 + # cls.https_proxy_proc = subprocess.Popen( + # ['python', 'simple_proxy.py', 'https', str(cls.https_proxy_port)], + # stderr=subprocess.PIPE) + # logger.info('\n\tProxy server process started.') + # logger.info('\tHTTPS Proxy process id: '+str(cls.http_proxy_proc.pid)) + # logger.info('\tHTTPS Proxy listening on port: '+str(cls.http_proxy_proc)) + + + # The first here is for an http proxy that cannot support HTTP CONNECT, and + # so cannot pass on HTTPS connections. + cls.http_proxy_addr = 'http://127.0.0.1:' + str(cls.http_proxy_port) + + # The second is for an http proxy that can support HTTP CONNECT, and so can + # pass on HTTPS connections. Because it is an http proxy, the address will + # begin with http:// whether the connection to the final target server is + # HTTP or HTTPS. + cls.http_proxy_addr2 = 'http://127.0.0.1:' + str(cls.http_proxy_port2) + + # # This is to the HTTPS proxy server. + # cls.https_proxy_addr = 'https://localhost:' + str(cls.https_proxy_port) + + # Give the HTTP server and proxy server processes a little bit of time to + # start listening before allowing tests to begin, lest we get "Connection + # refused" errors. + time.sleep(1.5) + + + + + + @classmethod + def tearDownClass(cls): + """ + Cleanup performed after the last of the tests (TestWithProxies methods) + has been run. + Stop server process and perform clean up. + """ + unittest_toolbox.Modified_TestCase.tearDownClass() + + for proc in [ + cls.http_server_proc, + cls.https_server_proc, + cls.http_proxy_proc, + cls.http_proxy_proc2, + #cls.https_proxy_proc, + ]: + if proc.returncode is None: + logger.info('\tTerminating process ' + str(proc.pid) + ' in cleanup.') + proc.kill() + + + + def setUp(self): + """ + Setup performed before EACH test function (TestWithProxies class method) + runs. + """ + unittest_toolbox.Modified_TestCase.setUp(self) + + # Dictionary for saving environment values to restore. + self.old_env_values = {} + + # Make a temporary file to serve on the server, and determine its length, + # and its url on the server. + current_dir = os.getcwd() + target_filepath = self.make_temp_data_file(directory=current_dir) + rel_target_filepath = os.path.basename(target_filepath) + + with open(target_filepath, 'r') as target_file_object: + self.target_data_length = len(target_file_object.read()) + + self.url = \ + 'http://localhost:' + str(self.http_port) + '/' + rel_target_filepath + + self.url_https = \ + 'https://localhost:' + str(self.https_port) + '/' + rel_target_filepath + + + + + + def tearDown(self): + """ + Cleanup performed after each test (each TestWithProxies method). + Reset environment variables (for next test, etc.). + """ + unittest_toolbox.Modified_TestCase.tearDown(self) + + self.restore_all_modified_env_values() + + + + + + def test_baseline_no_proxy(self): + """ + Test a length-validating TUF download of a file through a proxy. Use an + HTTP proxy, and perform an HTTP connection with the final server. + """ + + logger.info('Trying http download with no proxy: ' + self.url) + download.safe_download(self.url, self.target_data_length) + download.unsafe_download(self.url, self.target_data_length) + + + + + + def test_http_dl_via_dumb_http_proxy(self): + """ + Test a length-validating TUF download of a file through a proxy. Use an + HTTP proxy, and make an HTTP connection with the final server. + """ + + self.set_env_value('HTTP_PROXY', self.http_proxy_addr) + + logger.info('Trying http download via http proxy: ' + self.url) + download.safe_download(self.url, self.target_data_length) + download.unsafe_download(self.url, self.target_data_length) + + + + + + @unittest.expectedFailure + def test_httpS_dl_via_dumb_http_proxy(self): + """ + Test a length-validating TUF download of a file through a proxy. Use an + HTTP proxy, and try to use HTTP CONNECT, even though this HTTP proxy does + not happen to support HTTP CONNECT. (Consequently, this test fails.) + + Note that the proxy address is still http://... even though the connection + with the target server is an HTTPS connection. + """ + self.set_env_value('HTTPS_PROXY', self.http_proxy_addr) # http as intended + + logger.info('Trying httpS download via http proxy: ' + self.url_https) + download.safe_download(self.url_https, self.target_data_length) + download.unsafe_download(self.url_https, self.target_data_length) + + + + + + def test_http_dl_via_smart_http_proxy(self): + """ + Test a length-validating TUF download of a file through a proxy. Use an + HTTP proxy normally, and make an HTTP connection with the final server. + """ + + self.set_env_value('HTTP_PROXY', self.http_proxy_addr2) + + logger.info('Trying http download via http proxy: ' + self.url) + download.safe_download(self.url, self.target_data_length) + download.unsafe_download(self.url, self.target_data_length) + + + + + + def test_httpS_dl_via_smart_http_proxy(self): + """ + Test a length-validating TUF download of a file through a proxy. Use an + HTTP proxy that supports HTTP CONNECT (which essentially causes it to act + as a TCP proxy), and perform an HTTPS connection through with the final + server. + + Note that the proxy address is still http://... even though the connection + with the target server is an HTTPS connection. The proxy itself will act as + a TCP proxy via HTTP CONNECT. + + TEMPORARY NOTE: It turns out that mitmproxy doesn't support HTTP CONNECT, + so I need a new proxy option for this.... + + """ + self.set_env_value('HTTP_PROXY', self.http_proxy_addr2) # http as intended + self.set_env_value('HTTPS_PROXY', self.http_proxy_addr2) # http as intended + + self.set_env_value('REQUESTS_CA_BUNDLE', 'ssl_cert.crt') + + logger.info('Trying httpS download via http proxy: ' + self.url_https) + download.safe_download(self.url_https, self.target_data_length) + download.unsafe_download(self.url_https, self.target_data_length) + + + + + + # def test_http_dl_via_httpS_proxy(self): + # """ + # Test a length-validating TUF download of a file through a proxy. Use an + # HTTP proxy, and perform an HTTPS connection with the final server. + + # Note that the proxy address is still http://... even though the connection + # with the target server is an HTTPS connection. The proxy itself is reached + # in an HTTP connection. + # """ + # pass + + + + + + # def test_httpS_dl_via_httpS_proxy(self): + # pass + + + + + + # def test_transparent_https_proxy(self): + # pass + + + + + + + def set_env_value(self, key, value): + """ + Set an environment variable after noting what the original value was, if it + was set, and add it to the queue for restoring to its original value / lack + of a value after the test finishes. + + Safe for multiple uses in one test: does not overwrite original saved value + with new saved values. + """ + + if key in self.old_env_values: + # Do not save the current value. We already saved an older value, and + # the original one is the one we'll restore to, not whatever we most + # recently overwrote it with. + pass + + elif key in os.environ: + self.old_env_values[key] = os.environ[key] + + else: + self.old_env_values[key] = None # Note that it was previously unset. + + # Actually set the new value. + os.environ[key] = value + + + + + + def restore_env_value(self, key): + # Save old values for environment variables for restoration after the test. + # Save the pre-existing value of the environment variables HTTP_PROXY and + # HTTPS_PROXY so that we can restore them in tearDown() after the test. + # If the value was not originally set at all, we'll try to unset it again, + # too. + assert key in self.old_env_values, 'Test coding mistake: something is ' \ + 'trying to restore environment variable ' + key + ', but that ' \ + 'variable does not appear in the list of values to restore. ' \ + 'Please make sure to use _set_env_value().' + + if self.old_env_values[key] is None: + # If it was not previously set, try to unset it. + # If the platform provides a way to unset environment variables, + # del os.environ[key] should unset the variable. Otherwise, we'll just + # have to settle for setting it to an empty string. + # See os.environ in: + # https://docs.python.org/2/library/os.html#process-parameters) + os.environ[key] = '' + del os.environ[key] + + else: + # If it was previously set, restore the original value from when the + # test was being set up. + os.environ[key] = self.old_env_values[key] + + + + + + def restore_all_modified_env_values(self): + for key in self.old_env_values: + self.restore_env_value(key) + + + + + +# Run unit test. +if __name__ == '__main__': + unittest.main() From b7b73e592ef317ac6c469f26524a64138988da20 Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Thu, 20 Sep 2018 14:56:57 -0400 Subject: [PATCH 18/34] Test: add flag to toggle relay-only/intercept proxy behavior in proxy2. And use it to run both relaying and intercepting proxies. True: normal HTTP proxy. Support HTTP & HTTPS connections to target server False: intercepting MITM transparent HTTPS proxy. Makes own TLS connections and has its own cert; must be trusted by the client and is able to modify requests. Also perform some cleanup of test_proxy_use.py Signed-off-by: Sebastien Awwad --- tests/proxy2.py | 41 +++++++++++++++++++++++------ tests/test_proxy_use.py | 58 ++++++++++++----------------------------- 2 files changed, 49 insertions(+), 50 deletions(-) diff --git a/tests/proxy2.py b/tests/proxy2.py index 975478ba..61ea86c3 100644 --- a/tests/proxy2.py +++ b/tests/proxy2.py @@ -1,12 +1,14 @@ #!/usr/bin/env python # This code is taken from: https://github.com/inaz2/proxy2 -# Credit goes to the author. It has been very slightly modified here solely to -# switch from IPv6 use to IPv4 use. (Modified lines are marked '# MODIFIED'.) +# Credit goes to the author. It has been very slightly modified here to use +# IPv4 instead of IPv6, and to only attempt interception of HTTPS traffic +# (instead of relaying via HTTP CONNECT) if new global variable INTERCEPT is +# set to True. (Modified sections are marked '# MODIFIED'.) # # Because this is a helper module for a test, the style is less important, and # so to minimize changes from the source, it has NOT been changed to match the -# TUF project's code style. +# TUF project's code style outside of rewritten sections. """ @@ -42,6 +44,12 @@ from subprocess import Popen, PIPE from HTMLParser import HTMLParser +# MODIFIED: (added) A boolean: +# False: normal HTTP proxy. Support HTTP & HTTPS connections to target server +# True: intercepting MITM transparent HTTPS proxy. Makes own TLS connections +# and has its own cert; must be trusted by the client and is able to +# modify requests. +INTERCEPT = False def with_color(c, s): return "\x1b[%dm%s\x1b[0m" % (c, s) @@ -85,10 +93,20 @@ def log_error(self, format, *args): self.log_message(format, *args) def do_CONNECT(self): - if os.path.isfile(self.cakey) and os.path.isfile(self.cacert) and os.path.isfile(self.certkey) and os.path.isdir(self.certdir): - self.connect_intercept() - else: - self.connect_relay() + # MODIFIED: This function has been modified to use new global INTERCEPT + # and to issue an error if the necessary certificate/key files are + # missing for interception attempts. + if not INTERCEPT: + print('\n\nRELAYING\n\n') + self.connect_relay() + + else: + assert os.path.isfile(self.cakey) and os.path.isfile(self.cacert) \ + and os.path.isfile(self.certkey) and os.path.isdir(self.certdir), \ + '\nMissing key or certificate files; unable to perform TLS ' \ + 'handshake with client to intercept traffic.\n' + print('\n\nINTERCEPTING\n\n') + self.connect_intercept() def connect_intercept(self): hostname = self.path.split(':')[0] @@ -396,7 +414,14 @@ def test(HandlerClass=ProxyRequestHandler, ServerClass=ThreadingHTTPServer, prot port = int(sys.argv[1]) else: port = 8080 - server_address = ('127.0.0.1', port) # <~> changed from '::1' + server_address = ('127.0.0.1', port) # MODIFIED: changed from '::1' + + # MODIFIED: Conditional below added to control INTERCEPT setting. + if len(sys.argv > 2): + if sys.argv[2].lower() == 'intercept': + global INTERCEPT + INTERCEPT = True + HandlerClass.protocol_version = protocol httpd = ServerClass(server_address, HandlerClass) diff --git a/tests/test_proxy_use.py b/tests/test_proxy_use.py index 0b36adb5..b04c8ffc 100644 --- a/tests/test_proxy_use.py +++ b/tests/test_proxy_use.py @@ -74,18 +74,12 @@ def setUpClass(cls): cls.http_server_proc = subprocess.Popen( ['python', 'simple_server.py', str(cls.http_port)], stderr=subprocess.PIPE) - # logger.info('\n\tHTTP server process started.') - # logger.info('\tHTTP server process id: ' + str(cls.http_server_proc.pid)) - # logger.info('\tServing HTTP on port: ' + str(cls.http_port)) # Launch an HTTPS server (serves files in the current dir). cls.https_port = cls.http_port + 1 cls.https_server_proc = subprocess.Popen( ['python', 'simple_https_server.py', str(cls.https_port)], stderr=subprocess.PIPE) - # logger.info('\n\tHTTPS server process started.') - # logger.info('\tHTTPS server process id: ' + str(cls.https_server_proc.pid)) - # logger.info('\tServing HTTPS on port: ' + str(cls.https_port)) # Launch a very basic HTTP proxy server. I think this one won't even handle # HTTP CONNECT (and so can't pass HTTPS requests on) @@ -93,57 +87,37 @@ def setUpClass(cls): cls.http_proxy_proc = subprocess.Popen( ['python', 'simple_proxy.py', 'http_dumb', str(cls.http_proxy_port)], stderr=subprocess.PIPE) - # logger.info('\n\tProxy server process started.') - # logger.info('\tHTTP Proxy process id: ' + str(cls.http_proxy_proc.pid)) - # logger.info('\tHTTP Proxy listening on port: ' + str(cls.http_proxy_proc)) # Launch a less basic HTTP proxy server. This one should be able to handle # HTTP CONNECT requests, and so should be able to pass HTTPS requests on to # the target server. - cls.http_proxy_port2 = cls.http_port + 4 + cls.http_proxy_port2 = cls.http_port + 3 - # Try proxy.py: - # Nope, proxy.py doesn't support HTTP CONNECT. - # cls.http_proxy_proc2 = subprocess.Popen( - # ['proxy.py', '--port', str(cls.http_proxy_port2)], - # stderr=subprocess.PIPE) + # proxy.py doesn't support HTTP CONNECT. + # mitm_proxy doesn't support HTTP CONNECT either, because it expects to + # tinker with things and isn't interested in being blind to the data. + # mitm_proxy also can't be started this way; it expects a console. # Once a working proxy is chosen, it should probably be run this way instead: # cls.http_proxy_proc2 = subprocess.Popen( # ['python', 'simple_proxy.py', 'http_smart', str(cls.http_proxy_port2)], # stderr=subprocess.PIPE) - # So, mitm_proxy doesn't support HTTP CONNECT, either, because it expects to - # tinker with things and isn't interested in being blind to the data. - # mitm_proxy can't be started this way; it expects a console. - # NOTE: Start it manually outside of this tester for now, using: - # >>> mitmproxy --listen-host 127.0.0.1 --listen-port 8899 - # cls.http_proxy_proc2 = subprocess.Popen( - # ['mitmproxy', '--listen-host', '127.0.0.1', - # '--listen-port', str(cls.http_proxy_port2)], - # stderr=subprocess.PIPE) - # logger.info('\n\tProxy server process started.') - # logger.info('\tHTTP Proxy process id: ' + str(cls.http_proxy_proc2.pid)) - # logger.info('\tHTTP Proxy listening on port: ' + str(cls.http_proxy_proc2)) - # Let's try inaz2/proxy2.... + # inaz2/proxy2 provides HTTP CONNECT # This seems to work, once modified to use IPv4. cls.http_proxy_proc2 = subprocess.Popen( ['python', 'proxy2.py', str(cls.http_proxy_port2)], stderr=subprocess.PIPE) - - # TODO: Launch a basic HTTPS proxy server. - # # Launch a basic HTTPS proxy server. - # cls.https_proxy_port = cls.http_port + 11 - # cls.https_proxy_proc = subprocess.Popen( - # ['python', 'simple_proxy.py', 'https', str(cls.https_proxy_port)], - # stderr=subprocess.PIPE) - # logger.info('\n\tProxy server process started.') - # logger.info('\tHTTPS Proxy process id: '+str(cls.http_proxy_proc.pid)) - # logger.info('\tHTTPS Proxy listening on port: '+str(cls.http_proxy_proc)) + # Launch a basic HTTPS proxy server. (This performs its own TLS connection + # with the client and must be trusted by it, and is capable of tampering.) + cls.https_proxy_port = cls.http_port + 4 + cls.https_proxy_proc = subprocess.Popen( + ['python', 'proxy2.py', str(cls.https_proxy_port), 'intercept'], + stderr=subprocess.PIPE) # The first here is for an http proxy that cannot support HTTP CONNECT, and @@ -156,13 +130,13 @@ def setUpClass(cls): # HTTP or HTTPS. cls.http_proxy_addr2 = 'http://127.0.0.1:' + str(cls.http_proxy_port2) - # # This is to the HTTPS proxy server. - # cls.https_proxy_addr = 'https://localhost:' + str(cls.https_proxy_port) + # This is to the HTTPS proxy server. + cls.https_proxy_addr = 'https://127.0.0.1:' + str(cls.https_proxy_port) # Give the HTTP server and proxy server processes a little bit of time to # start listening before allowing tests to begin, lest we get "Connection # refused" errors. - time.sleep(1.5) + time.sleep(2) @@ -182,7 +156,7 @@ def tearDownClass(cls): cls.https_server_proc, cls.http_proxy_proc, cls.http_proxy_proc2, - #cls.https_proxy_proc, + cls.https_proxy_proc, ]: if proc.returncode is None: logger.info('\tTerminating process ' + str(proc.pid) + ' in cleanup.') From 2b019f65f769e3b51fa0cf10d973b61ac125f7f4 Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Fri, 21 Sep 2018 12:08:44 -0400 Subject: [PATCH 19/34] Test: add proxy certs and reorganize certs in test data dir for proxy testing. Also update the test scripts to point to the new location of ssl certificates and ssl keys. Signed-off-by: Sebastien Awwad --- tests/proxy2.py | 16 +++++++----- tests/simple_https_server.py | 4 +-- tests/ssl_certs/proxy_ca.crt | 17 ++++++++++++ tests/ssl_certs/proxy_ca.key | 27 ++++++++++++++++++++ tests/ssl_certs/proxy_cert.key | 27 ++++++++++++++++++++ tests/{ => ssl_certs}/ssl_cert.crt | 0 tests/{ => ssl_certs}/ssl_cert.key | 0 tests/{ => ssl_certs}/ssl_cert_wronghost.crt | 0 tests/test_download.py | 4 +-- tests/test_proxy_use.py | 3 ++- 10 files changed, 87 insertions(+), 11 deletions(-) create mode 100644 tests/ssl_certs/proxy_ca.crt create mode 100644 tests/ssl_certs/proxy_ca.key create mode 100644 tests/ssl_certs/proxy_cert.key rename tests/{ => ssl_certs}/ssl_cert.crt (100%) rename tests/{ => ssl_certs}/ssl_cert.key (100%) rename tests/{ => ssl_certs}/ssl_cert_wronghost.crt (100%) diff --git a/tests/proxy2.py b/tests/proxy2.py index 61ea86c3..df24a1e7 100644 --- a/tests/proxy2.py +++ b/tests/proxy2.py @@ -54,8 +54,10 @@ def with_color(c, s): return "\x1b[%dm%s\x1b[0m" % (c, s) -def join_with_script_dir(path): - return os.path.join(os.path.dirname(os.path.abspath(__file__)), path) +# MODIFIED: from join_with_script_dir to include ssl_certs directory +def get_cert_filepath(path): + return os.path.join( + os.path.dirname(os.path.abspath(__file__)), 'ssl_certs', path) class ThreadingHTTPServer(ThreadingMixIn, HTTPServer): @@ -72,10 +74,12 @@ def handle_error(self, request, client_address): class ProxyRequestHandler(BaseHTTPRequestHandler): - cakey = join_with_script_dir('ca.key') - cacert = join_with_script_dir('ca.crt') - certkey = join_with_script_dir('cert.key') - certdir = join_with_script_dir('certs/') + # MODIFIED: Calls below modified: filenames changed, function changed to + # include ssl_certs directory. + cakey = get_cert_filepath('proxy_ca.key') + cacert = get_cert_filepath('proxy_ca.crt') + certkey = get_cert_filepath('proxy_cert.key') + certdir = get_cert_filepath('proxy_certs') timeout = 5 lock = threading.Lock() diff --git a/tests/simple_https_server.py b/tests/simple_https_server.py index adecbf6d..4cb9d860 100755 --- a/tests/simple_https_server.py +++ b/tests/simple_https_server.py @@ -45,8 +45,8 @@ PORT = 0 -keyfile = 'ssl_cert.key' -certfile = 'ssl_cert.crt' +keyfile = os.path.join('ssl_certs', 'ssl_cert.key') +certfile = os.path.join('ssl_certs', 'ssl_cert.crt') def _generate_random_port(): return random.randint(30000, 45000) diff --git a/tests/ssl_certs/proxy_ca.crt b/tests/ssl_certs/proxy_ca.crt new file mode 100644 index 00000000..f079e58b --- /dev/null +++ b/tests/ssl_certs/proxy_ca.crt @@ -0,0 +1,17 @@ +-----BEGIN CERTIFICATE----- +MIICpDCCAYwCCQCFr/EhHmzVajANBgkqhkiG9w0BAQsFADAUMRIwEAYDVQQDDAlw +cm94eTIgQ0EwHhcNMTgwOTIwMTkyOTQ2WhcNMjgwOTE3MTkyOTQ2WjAUMRIwEAYD +VQQDDAlwcm94eTIgQ0EwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQC/ +rVOeqSzJb01Vyliw3dnfLJsWfDfs/Lq5HLn+Xqnzl6MqnYirDqHzTErD3vl8lo/o +OJrziO0vYCWGXEylRQlZp+P37bLToSWiVqWZ8pH6CAh+AhA3WtegN5JwTgIUSP7A +aDlxuZrXlJM50QVlXJIPkc74M8ALz0nu5zmyWkGFvmTYS8503T8cXs9Alr4Bo++9 +Ilixv6lW4QS7FKTeQXlI49K4TeGGGsfmEO6Uj4WTUkwMZym9wfiqtaWc6I9ZMese +WmU3LuufY+pFCdjsdMWDJpYc+HabTSrbgXSF5Iq9a84Xuum39qhVpYhBwBtLk3ye +cxZmIxde1vnkWAitJFETAgMBAAEwDQYJKoZIhvcNAQELBQADggEBAKV09r/x3WyO +McH0RU4WRVzvQN5F0e7swpDlLUX7YnfvpPEkavqQfmrL1cYyEDgsm/347Gvcs1Aa +iaT77axYroXOvCEJ3DxZdzUErKH6Jr3MmHKcZ/L35u6ZXKnmx/edFjdWr6ENkjuZ +NVvKbTrm4cl6Wy4bXkp6b24rBa9IFJncOouSkIvHENEcH//OD4xeTK8vSJTJ9nmw +TiJ0TjCRujtJWC6yb03ZV32VbeiHa1zLlZhcyKqUtt81dLti5t5+L2hAAVCcnEgI +DBWQdlRs/wilHGWVBo/9srOoMNsmvecTBpLH2JyC5VZ1+faYLPrNlgkWgHIFOTTi +h4ByR95Wbi8= +-----END CERTIFICATE----- diff --git a/tests/ssl_certs/proxy_ca.key b/tests/ssl_certs/proxy_ca.key new file mode 100644 index 00000000..0e08b82d --- /dev/null +++ b/tests/ssl_certs/proxy_ca.key @@ -0,0 +1,27 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIEpQIBAAKCAQEAv61TnqksyW9NVcpYsN3Z3yybFnw37Py6uRy5/l6p85ejKp2I +qw6h80xKw975fJaP6Dia84jtL2AlhlxMpUUJWafj9+2y06ElolalmfKR+ggIfgIQ +N1rXoDeScE4CFEj+wGg5cbma15STOdEFZVySD5HO+DPAC89J7uc5slpBhb5k2EvO +dN0/HF7PQJa+AaPvvSJYsb+pVuEEuxSk3kF5SOPSuE3hhhrH5hDulI+Fk1JMDGcp +vcH4qrWlnOiPWTHrHlplNy7rn2PqRQnY7HTFgyaWHPh2m00q24F0heSKvWvOF7rp +t/aoVaWIQcAbS5N8nnMWZiMXXtb55FgIrSRREwIDAQABAoIBACxJObbA064+3xlh +RRioSXx86+BIFwvUYLgAYSDacl3rvTFNcJRFLznteKDE1dPpXZqD6Zk3G8YEauce +UD8nMj/awJs5+kVXSEC30E8/cmbYkE284E5J2OQVsunrvCM/skx2SD90aMhCdbm4 +B40h1EVwpOdH3alc3XIrTnNc0yK5MWAu41qwkxYxXHmW9Y0L8AjZve9JBrnKsJMB +ETEZFhHgi/IWtfh5PLbJO2dbSe7Nqo4ikyWo3r5b3yvuphFz1il88ZLjJ5nDmtlH +is7sk7pd0tYNsK1Di5G1ku50XvcbOE4F7mOVCxICTwjN+sdyG8o+AVlgbTKBo/JF +uEhthCECgYEA/3YXS9mAEujlstrV4VOksYWtySSrLHC56tLjj8cHVPJ1qkzT4OOC +X9TsWReDG4J8/t0DOHn+5dnhnqGcYjMMAQx095KHU1bQGrcRdmi6cjnNLTvfEbge +IcJTYG5P7NpLfLjB3DOGqFR4o0iz4K9ZLTYJc+BaCB9qJBEw6nuoP+sCgYEAwBTN +WpRDrmch0+LFPQwboLwtEPiFscTj8SInV0KsI/MK8+5Sm+tXS8PQHYJYcECEQxQM +2gfyM8vy33UP4yn4edJGWlaz7a4hyDxn944vv2fBQ3vjJTNz3X3skkhZ2/F+ZW9e +SFxPj+Vbif8VTEU+wK0f5SUmpRec4E7y3fq+kXkCgYEAib8ZbLLI1mlygfBx51/8 +rCRSwuTcz8ew2CgCwGInV+ys+bkXfmnuwNHE531AGrNPxvVRaUCO602C1NB7zI+N +53raDyyZf5yN9fnElr592l3EfqGL9Lf8t2NbJeIVgrdqgMP29E9sSpPRwOnQ5FRo +l3JNwoe0xDB8QRpr7+PhoyUCgYEAp+GGmmR7wzLgnhDV00WB4DqYKP0N3RH5KAhx +2hKr4b/LEuh5y00mP1Il06TZJ0M8VmRv1yCa0CqxXB00hZdpVRAz7UFagaJwZFJn +jDb6BJDqmdDt9tXBrxUgb7pMz6+CiaWNAjGsWFheaX5JXyAmeMDX369Y13KL6oEW +RG2jogECgYEA/1vLZcWNK/0yd4ClU+Xbu8xC29q82JUMsaazHtbgSNlOfo9LMQlH +z6xBiMYfHZ/SiHCy9RsO8GD4caXiF0RsTVnhqjSRJf3EARamufelNsu2ApLclkSN +fzSoB7ZHddGaYKYpXkGzcwFcKd/QjAlHm1yIsZu4B52AhCxC/WS2X54= +-----END RSA PRIVATE KEY----- diff --git a/tests/ssl_certs/proxy_cert.key b/tests/ssl_certs/proxy_cert.key new file mode 100644 index 00000000..76938656 --- /dev/null +++ b/tests/ssl_certs/proxy_cert.key @@ -0,0 +1,27 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIEpAIBAAKCAQEAzZO36nZvb9wLxBNB2cZyHqcX5poChJd1YnFBtxbtQwiISxid +eGdiWImQE80vpUyTQbI7TxM+w1xZeEeu4PXuYrOgdTDRFEnjM2mteG+3WpHQBN4H +xoah0msp3046fMkYqcEvhvHbsc5DAWgLK4JFHQPtG/+CIH0ZY+lBBPQhFIhBLYkt +YxNVqwpsXOGreASSw6mO6cVehCuVFJQO5NnI1sCAvp3SeosMKeIcDZxpZWmZhSwH +n3Rj6RMNM66C8zG4YlpvIniGzgV4UiW8XrTUG8HmzQ2295IcfB4No2DZeJDSR9oq +jOkyqJXll+tSiAMuzBRtTQKvGZ5bpZWW4XELEQIDAQABAoIBAQCAfW2cjD4GimCI +QwkLlq9JXWLg7S3ZtdjWmLdcOmY9WZ3mYhI6aVPcxs5Ysgyvonb/vui2+e5mqNf7 +B8LUNKK06lTGKqbjqXLqdYjJF/pgD3cXM7dkbE3EeNqJChogWIijwW11SMHqFmNn +A6LHpPqRshyHPWIV8FroSagr8nKio5BjUEuUiQUUAmSJPGN5qUhdIWXcQu8R1JB8 +9qqqtwPR4FELbFVGI2vYHaSWGnf9V0boPOsfFXWbSq/Ksj3Lm3gAqMtlAeOFu84l +fhP9RkgeXfaCXq0VaOM83UDgLqXm4Ni4wAMKRLwNs4LzumqMM/dfUTn+mGncj33q +idp5qnDhAoGBAOXkwuf60F7aBbo98A0vWZli2CbkspsJz2J573pf+lVWI+ZHBZLI +MOM2DgCOEIUfa2TIMkwFr2t9x6uXlACEwFbEtEBpM4J5qUHgGtXZIsnTsv3qUg/C +L89cNrMddOuuRkxQbyK1QMYZZmZQjSKG2jW6m1KING+shtkOzQ/P9ildAoGBAOTs +DLyyPeEZPj1UMqxVNmeYYRfWnt+YyTPulOIbSuFN0DhZPNLsjrhSxvDwe/3sYH/p +nKdjnlFlx8frz9wtkCt0hWvY0pG2Zam4IBCvreFN7rSvpzHwUAK3oXic2TRKKu1m +xUPZqMJwnWAPX+XxGFn0m7UJj+95VTEOJ2d12ClFAoGAdexXMgmM8uqg/3yf8xNz +wWNbfu/W0gJBN8FWXw52aWmrNob9y+IWeaYTnqNAxBhuzR6H9kkAR4IYduNkzrNJ +ufhigZu1CVuAv8LF4SXlW2PVL7wPZff08Efb4xrcC7y0YJbtuv8Af90tkpQFIU3N +Brx2yeoGA7aa4SJfe5nwKh0CgYAo1yP+lh4MBqDf+CGCNUGbgcfwpM17PprGtQ3C +uPPG9kbrhqAfUSy1Ha94VK8KQh2FNHxKMK+R/gKCXEOdGFPcLNGQyAHpFQ1WFg9C +atUumOS5P40oj6L2mSQpjHIDrieyat9Ol4pQBh9Nf/Cv6S9a/RS6W5ZeNttIASpu +fsutsQKBgQCq+BFeDYJH4f+C1233W3PXM0P1ivj+9TJMRUP63RRay6rv2ZTZXyPc +Rx6Lv4OVWh9VMfv1kHRloJ1GKEBo/uD3nid1WqoNxpXv1iwxeGtjXkFHfvCB7Ruu +vTyQhJQQ7WSCJJOfarstusIn0udOG3MLRgG4X1pPQghyS1AT8NUglw== +-----END RSA PRIVATE KEY----- diff --git a/tests/ssl_cert.crt b/tests/ssl_certs/ssl_cert.crt similarity index 100% rename from tests/ssl_cert.crt rename to tests/ssl_certs/ssl_cert.crt diff --git a/tests/ssl_cert.key b/tests/ssl_certs/ssl_cert.key similarity index 100% rename from tests/ssl_cert.key rename to tests/ssl_certs/ssl_cert.key diff --git a/tests/ssl_cert_wronghost.crt b/tests/ssl_certs/ssl_cert_wronghost.crt similarity index 100% rename from tests/ssl_cert_wronghost.crt rename to tests/ssl_certs/ssl_cert_wronghost.crt diff --git a/tests/test_download.py b/tests/test_download.py index 6e493aaa..9026a906 100755 --- a/tests/test_download.py +++ b/tests/test_download.py @@ -236,8 +236,8 @@ def test_https_connection(self): # These cert files should both be valid, but "good" lists localhost and # "bad" lists another hostname. We'll be trying to download from localhost, # so we expect - good_cert_fname = 'ssl_cert.crt' - bad_cert_fname = 'ssl_cert_wronghost.crt' + good_cert_fname = os.path.join('ssl_certs', 'ssl_cert.crt') + bad_cert_fname = os.path.join('ssl_certs', 'ssl_cert_wronghost.crt') # Launch two https servers (serves files in the current dir). # The first we expect to operate correctly, and the second we run with an diff --git a/tests/test_proxy_use.py b/tests/test_proxy_use.py index b04c8ffc..f479c156 100644 --- a/tests/test_proxy_use.py +++ b/tests/test_proxy_use.py @@ -290,7 +290,8 @@ def test_httpS_dl_via_smart_http_proxy(self): self.set_env_value('HTTP_PROXY', self.http_proxy_addr2) # http as intended self.set_env_value('HTTPS_PROXY', self.http_proxy_addr2) # http as intended - self.set_env_value('REQUESTS_CA_BUNDLE', 'ssl_cert.crt') + self.set_env_value('REQUESTS_CA_BUNDLE', + os.path.join('ssl_certs', 'ssl_cert.crt')) logger.info('Trying httpS download via http proxy: ' + self.url_https) download.safe_download(self.url_https, self.target_data_length) From 7288b71917476a3cbcda9e85271e2c3d39db7d54 Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Fri, 21 Sep 2018 12:14:49 -0400 Subject: [PATCH 20/34] Test: fix a bug in arg processing for the proxy server Fixes a typo in arg processing for test script proxy2.py. Also removes an outdated comment and clarifies another. Signed-off-by: Sebastien Awwad --- tests/proxy2.py | 4 ++-- tests/test_proxy_use.py | 4 ---- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/proxy2.py b/tests/proxy2.py index df24a1e7..457866e7 100644 --- a/tests/proxy2.py +++ b/tests/proxy2.py @@ -61,7 +61,7 @@ def get_cert_filepath(path): class ThreadingHTTPServer(ThreadingMixIn, HTTPServer): - address_family = socket.AF_INET # MODIFIED + address_family = socket.AF_INET # MODIFIED to use IPv4 instead of IPv6 daemon_threads = True def handle_error(self, request, client_address): @@ -421,7 +421,7 @@ def test(HandlerClass=ProxyRequestHandler, ServerClass=ThreadingHTTPServer, prot server_address = ('127.0.0.1', port) # MODIFIED: changed from '::1' # MODIFIED: Conditional below added to control INTERCEPT setting. - if len(sys.argv > 2): + if len(sys.argv) > 2: if sys.argv[2].lower() == 'intercept': global INTERCEPT INTERCEPT = True diff --git a/tests/test_proxy_use.py b/tests/test_proxy_use.py index f479c156..6a783c13 100644 --- a/tests/test_proxy_use.py +++ b/tests/test_proxy_use.py @@ -282,10 +282,6 @@ def test_httpS_dl_via_smart_http_proxy(self): Note that the proxy address is still http://... even though the connection with the target server is an HTTPS connection. The proxy itself will act as a TCP proxy via HTTP CONNECT. - - TEMPORARY NOTE: It turns out that mitmproxy doesn't support HTTP CONNECT, - so I need a new proxy option for this.... - """ self.set_env_value('HTTP_PROXY', self.http_proxy_addr2) # http as intended self.set_env_value('HTTPS_PROXY', self.http_proxy_addr2) # http as intended From 46fe1900b54474365f894c5b27646b145aa3c817 Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Fri, 21 Sep 2018 12:16:59 -0400 Subject: [PATCH 21/34] Test: add tests of HTTPS proxy compatibility - client makes HTTPS connection to proxy; proxy makes HTTP connection to target server - client makes HTTPS connection to proxy; proxy makes HTTPS connection to target server Added functionality to the proxy2.py script to allow it to take and use a certificate to use to validate the target server. Also added clarifying comments in test_proxy_use.py. Signed-off-by: Sebastien Awwad --- tests/proxy2.py | 25 +++++++++++++++--- tests/test_proxy_use.py | 56 ++++++++++++++++++++++++++++++++--------- 2 files changed, 66 insertions(+), 15 deletions(-) diff --git a/tests/proxy2.py b/tests/proxy2.py index 457866e7..b434b706 100644 --- a/tests/proxy2.py +++ b/tests/proxy2.py @@ -50,6 +50,7 @@ # and has its own cert; must be trusted by the client and is able to # modify requests. INTERCEPT = False +CA_FPATH_FOR_TARGET_SERVER = None def with_color(c, s): return "\x1b[%dm%s\x1b[0m" % (c, s) @@ -195,7 +196,12 @@ def do_GET(self): origin = (scheme, netloc) if not origin in self.tls.conns: if scheme == 'https': - self.tls.conns[origin] = httplib.HTTPSConnection(netloc, timeout=self.timeout) + # MODIFIED: Added CA override in line below, fed from + # global variable set by command line argument. + self.tls.conns[origin] = httplib.HTTPSConnection( + netloc, timeout=self.timeout, + context=ssl.create_default_context( + cafile=CA_FPATH_FOR_TARGET_SERVER)) else: self.tls.conns[origin] = httplib.HTTPConnection(netloc, timeout=self.timeout) conn = self.tls.conns[origin] @@ -414,18 +420,31 @@ def save_handler(self, req, req_body, res, res_body): def test(HandlerClass=ProxyRequestHandler, ServerClass=ThreadingHTTPServer, protocol="HTTP/1.1"): + # MODIFIED: Added these globals. + global INTERCEPT + global CA_FPATH_FOR_TARGET_SERVER + if sys.argv[1:]: port = int(sys.argv[1]) else: port = 8080 server_address = ('127.0.0.1', port) # MODIFIED: changed from '::1' - # MODIFIED: Conditional below added to control INTERCEPT setting. + # MODIFIED: Argument added, conditional below added to control INTERCEPT + # setting. if len(sys.argv) > 2: if sys.argv[2].lower() == 'intercept': - global INTERCEPT INTERCEPT = True + # MODIFIED: Argument added to control certificate(s) the proxy expects of + # the target server(s), and added default value. + CA_FPATH_FOR_TARGET_SERVER = get_cert_filepath('ssl_cert.crt') + if len(sys.argv) > 3: + if not os.path.isabs(sys.argv[3]): + print('Path for target server cert is not absolute. Ignoring.') + elif os.path.exists(sys.argv[3]): + CA_FPATH_FOR_TARGET_SERVER = sys.argv[3] + HandlerClass.protocol_version = protocol httpd = ServerClass(server_address, HandlerClass) diff --git a/tests/test_proxy_use.py b/tests/test_proxy_use.py index 6a783c13..77468a86 100644 --- a/tests/test_proxy_use.py +++ b/tests/test_proxy_use.py @@ -114,9 +114,19 @@ def setUpClass(cls): # Launch a basic HTTPS proxy server. (This performs its own TLS connection # with the client and must be trusted by it, and is capable of tampering.) + # We instruct the proxy server to expect certain certificates from the + # target server. + # 1st arg: port + # 2nd arg: whether to intercept (HTTPS proxy) or relay (TCP tunnel using + # HTTP CONNECT verb, to facilitate an HTTPS connection between the client + # and server which the proxy cannot inspect) + # 3rd arg: (optional) certificate file for telling the proxy what target + # server certs to accept in its HTTPS connection to the target server. + # This is only relevant if the proxy is in intercept mode. cls.https_proxy_port = cls.http_port + 4 cls.https_proxy_proc = subprocess.Popen( - ['python', 'proxy2.py', str(cls.https_proxy_port), 'intercept'], + ['python', 'proxy2.py', str(cls.https_proxy_port), 'intercept', + os.path.join('ssl_certs', 'ssl_cert.crt')], stderr=subprocess.PIPE) @@ -297,23 +307,45 @@ def test_httpS_dl_via_smart_http_proxy(self): - # def test_http_dl_via_httpS_proxy(self): - # """ - # Test a length-validating TUF download of a file through a proxy. Use an - # HTTP proxy, and perform an HTTPS connection with the final server. + def test_http_dl_via_httpS_proxy(self): + """ + Test a length-validating TUF download of a file through a proxy. Use an + HTTPS proxy, and perform an HTTP connection with the final server. + """ + self.set_env_value('HTTP_PROXY', self.https_proxy_addr) + self.set_env_value('HTTPS_PROXY', self.https_proxy_addr) # unnecessary - # Note that the proxy address is still http://... even though the connection - # with the target server is an HTTPS connection. The proxy itself is reached - # in an HTTP connection. - # """ - # pass + # We're making an HTTPS connection with the proxy. The proxy will make a + # plain HTTP connection to the target server. + self.set_env_value('REQUESTS_CA_BUNDLE', + os.path.join('ssl_certs', 'proxy_ca.crt')) + + logger.info('Trying http download via httpS proxy: ' + self.url_https) + download.safe_download(self.url, self.target_data_length) + download.unsafe_download(self.url, self.target_data_length) - # def test_httpS_dl_via_httpS_proxy(self): - # pass + def test_httpS_dl_via_httpS_proxy(self): + """ + Test a length-validating TUF download of a file through a proxy. Use an + HTTPS proxy, and perform an HTTPS connection with the final server. + """ + self.set_env_value('HTTP_PROXY', self.https_proxy_addr) # unnecessary + self.set_env_value('HTTPS_PROXY', self.https_proxy_addr) + + # We're making an HTTPS connection with the proxy. The proxy will make its + # own HTTPS connection with the target server, and will have to know what + # certificate to trust. It was told what certs to trust when it was + # started in setUpClass(). + self.set_env_value('REQUESTS_CA_BUNDLE', + os.path.join('ssl_certs', 'proxy_ca.crt')) + + logger.info('Trying httpS download via httpS proxy: ' + self.url_https) + download.safe_download(self.url_https, self.target_data_length) + download.unsafe_download(self.url_https, self.target_data_length) From 34db5095aad20925b352acf54539f712efe81536 Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Tue, 25 Sep 2018 13:55:22 -0400 Subject: [PATCH 22/34] Test: clarify simple_proxy.py test script and disable unused sections for clarity as well. Signed-off-by: Sebastien Awwad --- tests/simple_proxy.py | 121 ++++++++++++++++++++++-------------------- 1 file changed, 64 insertions(+), 57 deletions(-) diff --git a/tests/simple_proxy.py b/tests/simple_proxy.py index 7e07ecf0..6ed743c4 100644 --- a/tests/simple_proxy.py +++ b/tests/simple_proxy.py @@ -14,16 +14,23 @@ Provide a simple http proxy server that can be used by TUF tests. See test_proxy_use.py to see how it's used in the tests. - Arguments are optional, but: - If provided, the first argument is assumed to choose HTTP vs HTTPS proxy. - If the first argument is 'https' (case insensitive), an HTTPS proxy will be - run, else an HTTP proxy will be run. - If provided, the second argument is assumed to indicate the port on which - the proxy should run. It must be 1023 1: - if sys.argv[1].lower() in ['https', 'http_smart', 'http_dumb']: - proxy_type = sys.argv[1].lower() - else: - print('\n\nUNEXPECTED PROTOCOL FOR PROXY SERVER. Using ' + proxy_type + - ' instead.\n\n') + # if len(sys.argv) > 1: + # if sys.argv[1].lower() in ['https', 'http_smart', 'http_dumb']: + # proxy_type = sys.argv[1].lower() + # else: + # print('\n\nUNEXPECTED PROTOCOL FOR PROXY SERVER. Using ' + proxy_type + + # ' instead.\n\n') - if len(sys.argv) > 2: - try: - port = int(sys.argv[2]) - except: - print('Ignoring argument. Second argument expected to be port, an int ' - 'i > 1023 and i < 65536. Generating randomly between 8090 and 30000.') - pass + if len(sys.argv) > 2: + try: + port = int(sys.argv[2]) + except: + print('Ignoring argument. Second argument expected to be port, an int ' + 'i > 1023 and i < 65536. Generating randomly between 8090 and 30000.') + pass - if port < 1024 or port > 65536: - port = _generate_random_port() + if port < 1024 or port > 65536: + port = _generate_random_port() - if proxy_type == 'https': - run_https_proxy(port) - elif proxy_type == 'http_dumb': - run_http_dumb_proxy(port) - elif proxy_type == 'http_smart': - run_http_smart_proxy(port) - else: - raise Exception('Unexpected proxy type requested....') + if proxy_type == 'http_dumb': + run_http_dumb_proxy(port) + # elif proxy_type == 'https': + # run_https_proxy(port) + # elif proxy_type == 'http_smart': + # run_http_smart_proxy(port) + else: + raise Exception('Unexpected proxy type requested....') def run_http_dumb_proxy(port): - """ - This proxy doesn't know what to do with HTTP CONNECT requests, so it can't - pass on HTTPS requests. - """ + """ + This proxy doesn't know what to do with HTTP CONNECT requests, so it can't + pass on HTTPS requests. + """ - twisted.python.log.startLogging(sys.stdout) + twisted.python.log.startLogging(sys.stdout) - class ProxyFactory(twisted.web.http.HTTPFactory): - protocol = twisted.web.proxy.Proxy + class ProxyFactory(twisted.web.http.HTTPFactory): + protocol = twisted.web.proxy.Proxy - twisted.internet.reactor.listenTCP(port, ProxyFactory()) - twisted.internet.reactor.run() + twisted.internet.reactor.listenTCP(port, ProxyFactory()) + twisted.internet.reactor.run() -def run_http_smart_proxy(port): - """ - This proxy needs to support HTTP CONNECT requests so that it can create a - TCP tunnel for HTTPS requests to the target server to go through. - """ - raise NotImplementedError() +# def run_http_smart_proxy(port): +# """ +# This proxy needs to support HTTP CONNECT requests so that it can create a +# TCP tunnel for HTTPS requests to the target server to go through. +# """ +# raise NotImplementedError() -def run_https_proxy(port): - """ - Run a proxy that connects using HTTPS (has its own validated certificate - that the client accepts, etc.) before passing the request on to the target - server. - """ - raise NotImplementedError() +# def run_https_proxy(port): +# """ +# Run a proxy that connects using HTTPS (has its own validated certificate +# that the client accepts, etc.) before passing the request on to the target +# server. +# """ +# raise NotImplementedError() if __name__ == '__main__': - main() + main() From d29e4d1aada1c4361dec529445c59200cc277aaf Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Tue, 25 Sep 2018 14:28:08 -0400 Subject: [PATCH 23/34] Test: refine proxy2.py script a bit: - move some configuration values up to the module level (proxy certs dir, proxy ca key and cert, general certs dir). - add explanatory comments for these values - create the proxy's host-specific client certificates directory if it does not yet exist. - note that the module is not thread-safe - fix a Windows-incompatible line (explicit path separator) Signed-off-by: Sebastien Awwad --- tests/proxy2.py | 64 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 42 insertions(+), 22 deletions(-) diff --git a/tests/proxy2.py b/tests/proxy2.py index b434b706..2f2576b3 100644 --- a/tests/proxy2.py +++ b/tests/proxy2.py @@ -23,6 +23,8 @@ This is used by test_proxy_use.py. This module requires Python2.7 and does not support Python3. + + Note that this is not thread-safe, in part due to its use of globals. """ import sys @@ -44,21 +46,34 @@ from subprocess import Popen, PIPE from HTMLParser import HTMLParser -# MODIFIED: (added) A boolean: +# MODIFIED: (added) three globals +# INTERCEPT: A boolean: # False: normal HTTP proxy. Support HTTP & HTTPS connections to target server # True: intercepting MITM transparent HTTPS proxy. Makes own TLS connections # and has its own cert; must be trusted by the client and is able to # modify requests. +# TARGET_SERVER_CA_FILEPATH: location of certificate to use as CA for +# connections to target servers (to constrain certs to trust from target +# servers). +# The remaining globals define the certs and keys to be used in communications +# with the client, with the proxy's CA signing new certs for individual hosts +# the client wishes to connect to, and placing them in dir PROXY_CERTS_DIR. INTERCEPT = False -CA_FPATH_FOR_TARGET_SERVER = None +CERTS_DIR = os.path.join( + os.path.dirname(os.path.abspath(__file__)), 'ssl_certs') +TARGET_SERVER_CA_FILEPATH = os.path.join(CERTS_DIR, 'ssl_cert.crt') +PROXY_CA_KEY = os.path.join(CERTS_DIR, 'proxy_ca.key') # was cakey +PROXY_CA_CERT = os.path.join(CERTS_DIR, 'proxy_ca.crt') # was cacert +PROXY_CERTS_KEY = os.path.join(CERTS_DIR, 'proxy_cert.key') # was certkey +PROXY_CERTS_DIR = os.path.join(CERTS_DIR, 'proxy_certs') # was certdir + def with_color(c, s): return "\x1b[%dm%s\x1b[0m" % (c, s) -# MODIFIED: from join_with_script_dir to include ssl_certs directory -def get_cert_filepath(path): - return os.path.join( - os.path.dirname(os.path.abspath(__file__)), 'ssl_certs', path) +# MODIFIED: removed join_with_script_dir +# def get_cert_filepath(path): +# return os.path.join(CERTS_DIR, path) class ThreadingHTTPServer(ThreadingMixIn, HTTPServer): @@ -75,12 +90,9 @@ def handle_error(self, request, client_address): class ProxyRequestHandler(BaseHTTPRequestHandler): - # MODIFIED: Calls below modified: filenames changed, function changed to + # MODIFIED: Variables here made into globals. + #Calls below modified: filenames changed, function changed to # include ssl_certs directory. - cakey = get_cert_filepath('proxy_ca.key') - cacert = get_cert_filepath('proxy_ca.crt') - certkey = get_cert_filepath('proxy_cert.key') - certdir = get_cert_filepath('proxy_certs') timeout = 5 lock = threading.Lock() @@ -106,8 +118,10 @@ def do_CONNECT(self): self.connect_relay() else: - assert os.path.isfile(self.cakey) and os.path.isfile(self.cacert) \ - and os.path.isfile(self.certkey) and os.path.isdir(self.certdir), \ + assert os.path.isfile(PROXY_CA_KEY) \ + and os.path.isfile(PROXY_CA_CERT) \ + and os.path.isfile(PROXY_CERTS_KEY) \ + and os.path.isdir(PROXY_CERTS_DIR), \ '\nMissing key or certificate files; unable to perform TLS ' \ 'handshake with client to intercept traffic.\n' print('\n\nINTERCEPTING\n\n') @@ -115,19 +129,19 @@ def do_CONNECT(self): def connect_intercept(self): hostname = self.path.split(':')[0] - certpath = "%s/%s.crt" % (self.certdir.rstrip('/'), hostname) + certpath = os.path.join(PROXY_CERTS_DIR, hostname + '.crt') # MODIFIED for Windows compatibility and to use new globals with self.lock: if not os.path.isfile(certpath): epoch = "%d" % (time.time() * 1000) - p1 = Popen(["openssl", "req", "-new", "-key", self.certkey, "-subj", "/CN=%s" % hostname], stdout=PIPE) - p2 = Popen(["openssl", "x509", "-req", "-days", "3650", "-CA", self.cacert, "-CAkey", self.cakey, "-set_serial", epoch, "-out", certpath], stdin=p1.stdout, stderr=PIPE) + p1 = Popen(["openssl", "req", "-new", "-key", PROXY_CERTS_KEY, "-subj", "/CN=%s" % hostname], stdout=PIPE) + p2 = Popen(["openssl", "x509", "-req", "-days", "3650", "-CA", PROXY_CA_CERT, "-CAkey", PROXY_CA_KEY, "-set_serial", epoch, "-out", certpath], stdin=p1.stdout, stderr=PIPE) # MODIFIED to use the new globals p2.communicate() self.wfile.write("%s %d %s\r\n" % (self.protocol_version, 200, 'Connection Established')) self.end_headers() - self.connection = ssl.wrap_socket(self.connection, keyfile=self.certkey, certfile=certpath, server_side=True) + self.connection = ssl.wrap_socket(self.connection, keyfile=PROXY_CERTS_KEY, certfile=certpath, server_side=True) # MODIFIED: Updated to use new globals self.rfile = self.connection.makefile("rb", self.rbufsize) self.wfile = self.connection.makefile("wb", self.wbufsize) @@ -201,7 +215,7 @@ def do_GET(self): self.tls.conns[origin] = httplib.HTTPSConnection( netloc, timeout=self.timeout, context=ssl.create_default_context( - cafile=CA_FPATH_FOR_TARGET_SERVER)) + cafile=TARGET_SERVER_CA_FILEPATH)) else: self.tls.conns[origin] = httplib.HTTPConnection(netloc, timeout=self.timeout) conn = self.tls.conns[origin] @@ -319,7 +333,7 @@ def decode_content_body(self, data, encoding): return text def send_cacert(self): - with open(self.cacert, 'rb') as f: + with open(PROXY_CA_CERT, 'rb') as f: # MODIFIED to use new globals data = f.read() self.wfile.write("%s %d %s\r\n" % (self.protocol_version, 200, 'OK')) @@ -422,7 +436,7 @@ def save_handler(self, req, req_body, res, res_body): def test(HandlerClass=ProxyRequestHandler, ServerClass=ThreadingHTTPServer, protocol="HTTP/1.1"): # MODIFIED: Added these globals. global INTERCEPT - global CA_FPATH_FOR_TARGET_SERVER + global TARGET_SERVER_CA_FILEPATH if sys.argv[1:]: port = int(sys.argv[1]) @@ -438,12 +452,18 @@ def test(HandlerClass=ProxyRequestHandler, ServerClass=ThreadingHTTPServer, prot # MODIFIED: Argument added to control certificate(s) the proxy expects of # the target server(s), and added default value. - CA_FPATH_FOR_TARGET_SERVER = get_cert_filepath('ssl_cert.crt') if len(sys.argv) > 3: if not os.path.isabs(sys.argv[3]): print('Path for target server cert is not absolute. Ignoring.') elif os.path.exists(sys.argv[3]): - CA_FPATH_FOR_TARGET_SERVER = sys.argv[3] + TARGET_SERVER_CA_FILEPATH = sys.argv[3] + else: + print('Target server cert not found. Ignoring.') + + # MODIFIED: Create the target-host-specific proxy certificates directory if + # it doesn't already exist. + if not os.path.exists(PROXY_CERTS_DIR): + os.mkdir(PROXY_CERTS_DIR) HandlerClass.protocol_version = protocol From 75e126ac96fe69ac1e07a4401ebc0286d6cdfee9 Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Tue, 25 Sep 2018 15:04:08 -0400 Subject: [PATCH 24/34] Test: remove redundant proxy tests and their requirements, add more explanatory comments in test_proxy.use, and prepare for rename of proxy2.py to proxy_server.py in next commit (separate so that the rename can be seen as such). Signed-off-by: Sebastien Awwad --- dev-requirements.txt | 2 - tests/proxy2.py | 2 +- tests/test_proxy_use.py | 119 +++++++++------------------------------- 3 files changed, 28 insertions(+), 95 deletions(-) diff --git a/dev-requirements.txt b/dev-requirements.txt index cc155b16..10e78433 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -23,7 +23,6 @@ iso8601==0.1.12 isort==4.3.4 lazy-object-proxy==1.3.1 mccabe==0.6.1 -# mitmproxy==4.0.4 ; python_version >= "3.6" pbr==4.2.0 pluggy==0.7.1 py==1.5.4 @@ -38,7 +37,6 @@ singledispatch==3.4.0.3 six==1.11.0 smmap2==2.0.4 stevedore==1.29.0 -twisted==18.7.0 tox==3.2.1 virtualenv==16.0.0 wrapt==1.10.11 diff --git a/tests/proxy2.py b/tests/proxy2.py index 2f2576b3..b1404128 100644 --- a/tests/proxy2.py +++ b/tests/proxy2.py @@ -12,7 +12,7 @@ """ - proxy2.py + proxy_server.py Taken from a repository set to BSD 3-Clause "New" or "Revised" License. See: diff --git a/tests/test_proxy_use.py b/tests/test_proxy_use.py index 77468a86..c18abcdb 100644 --- a/tests/test_proxy_use.py +++ b/tests/test_proxy_use.py @@ -65,6 +65,13 @@ def setUpClass(cls): Setup performed before the first test function (TestWithProxies class method) runs. Launch http, https, and proxy servers in the current working directory. + We'll set up four servers: + - HTTP server (simple_server.py) + - HTTPS server (simple_https_server.py) + - HTTP proxy server (proxy_server.py) + (that supports HTTP CONNECT to funnel HTTPS connections) + - HTTPS proxy server (proxy_server.py) + (trusted by the client to intercept and resign connections) """ unittest_toolbox.Modified_TestCase.setUpClass() @@ -81,39 +88,22 @@ def setUpClass(cls): ['python', 'simple_https_server.py', str(cls.https_port)], stderr=subprocess.PIPE) - # Launch a very basic HTTP proxy server. I think this one won't even handle - # HTTP CONNECT (and so can't pass HTTPS requests on) + + # Launch an HTTP proxy server derived from inaz2/proxy2. + # This one is able to handle HTTP CONNECT requests, and so can pass HTTPS + # requests on to the target server. cls.http_proxy_port = cls.http_port + 2 cls.http_proxy_proc = subprocess.Popen( - ['python', 'simple_proxy.py', 'http_dumb', str(cls.http_proxy_port)], + ['python', 'proxy2.py', str(cls.http_proxy_port)], stderr=subprocess.PIPE) - - # Launch a less basic HTTP proxy server. This one should be able to handle - # HTTP CONNECT requests, and so should be able to pass HTTPS requests on to - # the target server. - - cls.http_proxy_port2 = cls.http_port + 3 - - # proxy.py doesn't support HTTP CONNECT. - # mitm_proxy doesn't support HTTP CONNECT either, because it expects to - # tinker with things and isn't interested in being blind to the data. - # mitm_proxy also can't be started this way; it expects a console. - - # Once a working proxy is chosen, it should probably be run this way instead: - # cls.http_proxy_proc2 = subprocess.Popen( - # ['python', 'simple_proxy.py', 'http_smart', str(cls.http_proxy_port2)], - # stderr=subprocess.PIPE) + # Note that the http proxy server's address uses http://, regardless of the + # type of connection used with the target server. + cls.http_proxy_addr = 'http://127.0.0.1:' + str(cls.http_proxy_port) - # inaz2/proxy2 provides HTTP CONNECT - # This seems to work, once modified to use IPv4. - cls.http_proxy_proc2 = subprocess.Popen( - ['python', 'proxy2.py', str(cls.http_proxy_port2)], - stderr=subprocess.PIPE) - - - # Launch a basic HTTPS proxy server. (This performs its own TLS connection - # with the client and must be trusted by it, and is capable of tampering.) + # Launch an HTTPS proxy server, also derived from inaz2/proxy2. + # (An HTTPS proxy performs its own TLS connection with the client and must + # be trusted by it, and is capable of tampering.) # We instruct the proxy server to expect certain certificates from the # target server. # 1st arg: port @@ -123,30 +113,20 @@ def setUpClass(cls): # 3rd arg: (optional) certificate file for telling the proxy what target # server certs to accept in its HTTPS connection to the target server. # This is only relevant if the proxy is in intercept mode. - cls.https_proxy_port = cls.http_port + 4 + cls.https_proxy_port = cls.http_port + 3 cls.https_proxy_proc = subprocess.Popen( ['python', 'proxy2.py', str(cls.https_proxy_port), 'intercept', os.path.join('ssl_certs', 'ssl_cert.crt')], stderr=subprocess.PIPE) - - - # The first here is for an http proxy that cannot support HTTP CONNECT, and - # so cannot pass on HTTPS connections. - cls.http_proxy_addr = 'http://127.0.0.1:' + str(cls.http_proxy_port) - - # The second is for an http proxy that can support HTTP CONNECT, and so can - # pass on HTTPS connections. Because it is an http proxy, the address will - # begin with http:// whether the connection to the final target server is - # HTTP or HTTPS. - cls.http_proxy_addr2 = 'http://127.0.0.1:' + str(cls.http_proxy_port2) - - # This is to the HTTPS proxy server. + # Note that the https proxy server's address uses https://, regardless of + # the type of connection used with the target server. cls.https_proxy_addr = 'https://127.0.0.1:' + str(cls.https_proxy_port) # Give the HTTP server and proxy server processes a little bit of time to # start listening before allowing tests to begin, lest we get "Connection - # refused" errors. - time.sleep(2) + # refused" errors. On the first test system. 0.1s was too short and 0.15s + # was long enough. Use 0.5s to be safe, and if issues arise, increase it. + time.sleep(0.5) @@ -165,7 +145,6 @@ def tearDownClass(cls): cls.http_server_proc, cls.https_server_proc, cls.http_proxy_proc, - cls.http_proxy_proc2, cls.https_proxy_proc, ]: if proc.returncode is None: @@ -230,49 +209,13 @@ def test_baseline_no_proxy(self): - def test_http_dl_via_dumb_http_proxy(self): - """ - Test a length-validating TUF download of a file through a proxy. Use an - HTTP proxy, and make an HTTP connection with the final server. - """ - - self.set_env_value('HTTP_PROXY', self.http_proxy_addr) - - logger.info('Trying http download via http proxy: ' + self.url) - download.safe_download(self.url, self.target_data_length) - download.unsafe_download(self.url, self.target_data_length) - - - - - - @unittest.expectedFailure - def test_httpS_dl_via_dumb_http_proxy(self): - """ - Test a length-validating TUF download of a file through a proxy. Use an - HTTP proxy, and try to use HTTP CONNECT, even though this HTTP proxy does - not happen to support HTTP CONNECT. (Consequently, this test fails.) - - Note that the proxy address is still http://... even though the connection - with the target server is an HTTPS connection. - """ - self.set_env_value('HTTPS_PROXY', self.http_proxy_addr) # http as intended - - logger.info('Trying httpS download via http proxy: ' + self.url_https) - download.safe_download(self.url_https, self.target_data_length) - download.unsafe_download(self.url_https, self.target_data_length) - - - - - def test_http_dl_via_smart_http_proxy(self): """ Test a length-validating TUF download of a file through a proxy. Use an HTTP proxy normally, and make an HTTP connection with the final server. """ - self.set_env_value('HTTP_PROXY', self.http_proxy_addr2) + self.set_env_value('HTTP_PROXY', self.http_proxy_addr) logger.info('Trying http download via http proxy: ' + self.url) download.safe_download(self.url, self.target_data_length) @@ -293,8 +236,8 @@ def test_httpS_dl_via_smart_http_proxy(self): with the target server is an HTTPS connection. The proxy itself will act as a TCP proxy via HTTP CONNECT. """ - self.set_env_value('HTTP_PROXY', self.http_proxy_addr2) # http as intended - self.set_env_value('HTTPS_PROXY', self.http_proxy_addr2) # http as intended + self.set_env_value('HTTP_PROXY', self.http_proxy_addr) # http as intended + self.set_env_value('HTTPS_PROXY', self.http_proxy_addr) # http as intended self.set_env_value('REQUESTS_CA_BUNDLE', os.path.join('ssl_certs', 'ssl_cert.crt')) @@ -351,14 +294,6 @@ def test_httpS_dl_via_httpS_proxy(self): - # def test_transparent_https_proxy(self): - # pass - - - - - - def set_env_value(self, key, value): """ Set an environment variable after noting what the original value was, if it From e5a50a6831f8cdae372f5a8405e73638f32a75f1 Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Tue, 25 Sep 2018 15:05:53 -0400 Subject: [PATCH 25/34] Test: rename proxy2.py as proxy_server.py (from the original name in the source repository to a more useful name here). Signed-off-by: Sebastien Awwad --- tests/{proxy2.py => proxy_server.py} | 0 tests/test_proxy_use.py | 4 ++-- 2 files changed, 2 insertions(+), 2 deletions(-) rename tests/{proxy2.py => proxy_server.py} (100%) diff --git a/tests/proxy2.py b/tests/proxy_server.py similarity index 100% rename from tests/proxy2.py rename to tests/proxy_server.py diff --git a/tests/test_proxy_use.py b/tests/test_proxy_use.py index c18abcdb..08206bc1 100644 --- a/tests/test_proxy_use.py +++ b/tests/test_proxy_use.py @@ -94,7 +94,7 @@ def setUpClass(cls): # requests on to the target server. cls.http_proxy_port = cls.http_port + 2 cls.http_proxy_proc = subprocess.Popen( - ['python', 'proxy2.py', str(cls.http_proxy_port)], + ['python', 'proxy_server.py', str(cls.http_proxy_port)], stderr=subprocess.PIPE) # Note that the http proxy server's address uses http://, regardless of the # type of connection used with the target server. @@ -115,7 +115,7 @@ def setUpClass(cls): # This is only relevant if the proxy is in intercept mode. cls.https_proxy_port = cls.http_port + 3 cls.https_proxy_proc = subprocess.Popen( - ['python', 'proxy2.py', str(cls.https_proxy_port), 'intercept', + ['python', 'proxy_server.py', str(cls.https_proxy_port), 'intercept', os.path.join('ssl_certs', 'ssl_cert.crt')], stderr=subprocess.PIPE) # Note that the https proxy server's address uses https://, regardless of From 5312703fc77089614b15c7ca5c61fd48b7db274f Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Tue, 25 Sep 2018 15:06:40 -0400 Subject: [PATCH 26/34] Test: remove no-longer-used simple_proxy.py as that set of tests is now redundant, and depended on Twisted, which we need not depend on. Signed-off-by: Sebastien Awwad --- tests/simple_proxy.py | 137 ------------------------------------------ 1 file changed, 137 deletions(-) delete mode 100644 tests/simple_proxy.py diff --git a/tests/simple_proxy.py b/tests/simple_proxy.py deleted file mode 100644 index 6ed743c4..00000000 --- a/tests/simple_proxy.py +++ /dev/null @@ -1,137 +0,0 @@ -#!/usr/bin/env python - -# Copyright 2018, New York University and the TUF contributors -# SPDX-License-Identifier: MIT OR Apache-2.0 - -""" - - simple_proxy.py - - - See LICENSE-MIT OR LICENSE for licensing information. - - - Provide a simple http proxy server that can be used by TUF tests. - See test_proxy_use.py to see how it's used in the tests. - - Arguments are optional, but: - - 1. Proxy type: - If provided, the first argument is assumed to choose HTTP vs HTTPS proxy - vs HTTP CONNECT supporting HTTP proxy. - Currently, only basic HTTP is supported. The tests use proxy2.py for - the other two types of proxy. - The basic HTTP proxy is currently provided by dependency twisted. - - 2. Port: - If provided, the port on which the proxy should run. Userspace ports - only (1023 1: - # if sys.argv[1].lower() in ['https', 'http_smart', 'http_dumb']: - # proxy_type = sys.argv[1].lower() - # else: - # print('\n\nUNEXPECTED PROTOCOL FOR PROXY SERVER. Using ' + proxy_type + - # ' instead.\n\n') - - if len(sys.argv) > 2: - try: - port = int(sys.argv[2]) - except: - print('Ignoring argument. Second argument expected to be port, an int ' - 'i > 1023 and i < 65536. Generating randomly between 8090 and 30000.') - pass - - if port < 1024 or port > 65536: - port = _generate_random_port() - - if proxy_type == 'http_dumb': - run_http_dumb_proxy(port) - # elif proxy_type == 'https': - # run_https_proxy(port) - # elif proxy_type == 'http_smart': - # run_http_smart_proxy(port) - else: - raise Exception('Unexpected proxy type requested....') - - - - - -def run_http_dumb_proxy(port): - """ - This proxy doesn't know what to do with HTTP CONNECT requests, so it can't - pass on HTTPS requests. - """ - - twisted.python.log.startLogging(sys.stdout) - - class ProxyFactory(twisted.web.http.HTTPFactory): - protocol = twisted.web.proxy.Proxy - - twisted.internet.reactor.listenTCP(port, ProxyFactory()) - twisted.internet.reactor.run() - - - - - -# def run_http_smart_proxy(port): -# """ -# This proxy needs to support HTTP CONNECT requests so that it can create a -# TCP tunnel for HTTPS requests to the target server to go through. -# """ -# raise NotImplementedError() - - - - - -# def run_https_proxy(port): -# """ -# Run a proxy that connects using HTTPS (has its own validated certificate -# that the client accepts, etc.) before passing the request on to the target -# server. -# """ -# raise NotImplementedError() - - - - - -if __name__ == '__main__': - main() From d69f9a2160caa1b047a219a5e3fa59b0c744e665 Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Wed, 26 Sep 2018 12:13:40 -0400 Subject: [PATCH 27/34] Test: changed target server cert checking in test proxy script Added Python version checking and changed behavior in Python2.7.9+ to use custom certificate for target server inherited from command line argument. In Python versions < 2.7.9, proxy_server.py does not perform certificate validation of the target server. As that is not part of what the current tests using this script require, that is currently OK. In Python versions > 2.7.9 (SSLContext was added in 2.7.9), the same code actually does check the certificate, using the system's trusted CAs. As a result, since we are using custom certificates, we need to either disable certificate checking in 2.7.9 or load the specific CA for target test server, using the SSLContext and create_default_context functionality also added in 2.7.9. It is easier to do the latter, so the behavior in 2.7.9+ is to check the cert and below 2.7.9 is not to. Note that we do not support Python < 2.7. SSLContext is also available in all Python3 versions that we support. Signed-off-by: Sebastien Awwad --- tests/proxy_server.py | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/tests/proxy_server.py b/tests/proxy_server.py index b1404128..093020e6 100644 --- a/tests/proxy_server.py +++ b/tests/proxy_server.py @@ -22,6 +22,18 @@ Serves as an HTTP, HTTP CONNECT (TCP), and HTTPS proxy, for testing purposes. This is used by test_proxy_use.py. + In Python versions < 2.7.9, this proxy does not perform certificate + validation of the target server. As that is not part of what the current + tests using this script require, that is currently OK. In Python + versions > 2.7.9 (SSLContext was added in 2.7.9), the same code actually does + check the certificate, using the system's trusted CAs. As a result, since we + are using custom certificates, we need to either disable certificate + checking in 2.7.9 or load the specific CA for target test server, using the + SSLContext and create_default_context functionality also added in 2.7.9. It + is easier to do the latter, so the behavior in 2.7.9+ is to check the cert + and below 2.7.9 is not to. Note that we do not support Python < 2.7. + SSLContext is also available in all Python3 versions that we support. + This module requires Python2.7 and does not support Python3. Note that this is not thread-safe, in part due to its use of globals. @@ -210,11 +222,23 @@ def do_GET(self): origin = (scheme, netloc) if not origin in self.tls.conns: if scheme == 'https': - # MODIFIED: Added CA override in line below, fed from - # global variable set by command line argument. + # MODIFIED: Added Python version checking and changed behavior + # in Python2.7.9+ to use custom certificate for target server + # inherited from command line argument. + # In Python versions < 2.7.9, there is no certificate + # validation through this method of the target server. + # In supported Python versions > 2.7.9, we check the target + # server's certificate against our expected custom cert. + # See this script's docstring. + if sys.version_info.major == 2 \ + and sys.version_info.minor == 7 \ + and sys.version_info.micro < 9: + self.tls.conns[origin] = httplib.HTTPSConnection( + netloc, timeout=self.timeout) + else: self.tls.conns[origin] = httplib.HTTPSConnection( netloc, timeout=self.timeout, - context=ssl.create_default_context( + context=ssl.create_default_context( # reqs Python2.7.9+ cafile=TARGET_SERVER_CA_FILEPATH)) else: self.tls.conns[origin] = httplib.HTTPConnection(netloc, timeout=self.timeout) From 2b97c0e59c142a3fd21eca9930ab6c3e8a39255d Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Wed, 26 Sep 2018 13:10:17 -0400 Subject: [PATCH 28/34] Test: adjust proxy_server.py argument handling: - if it is provided, don't require the certificate filename to be provided as an absolute path - raise an error if the provided certificate filename does not point to an existing file, rather than just printing and ignoring (to avoid possible future diagnostic headaches) Signed-off-by: Sebastien Awwad --- tests/proxy_server.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/proxy_server.py b/tests/proxy_server.py index 093020e6..68ad8256 100644 --- a/tests/proxy_server.py +++ b/tests/proxy_server.py @@ -477,12 +477,10 @@ def test(HandlerClass=ProxyRequestHandler, ServerClass=ThreadingHTTPServer, prot # MODIFIED: Argument added to control certificate(s) the proxy expects of # the target server(s), and added default value. if len(sys.argv) > 3: - if not os.path.isabs(sys.argv[3]): - print('Path for target server cert is not absolute. Ignoring.') - elif os.path.exists(sys.argv[3]): + if os.path.exists(sys.argv[3]): TARGET_SERVER_CA_FILEPATH = sys.argv[3] else: - print('Target server cert not found. Ignoring.') + raise Exception('Target server cert file not found: ' + sys.argv[3]) # MODIFIED: Create the target-host-specific proxy certificates directory if # it doesn't already exist. From b163caa29b37f2794201cbec43be2f6697ade745 Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Wed, 26 Sep 2018 13:52:39 -0400 Subject: [PATCH 29/34] Test: add https test with expired server certificate in test_download.py. In the process, added another test cert and generalized the server process killer in test_download.py. Additionally, I added another expected-to-be-good certificate that was generated in the same way as the new bad certificates (but for their individual flaws of course). This is because the new certs aren't exactly like the old good cert, so that we have another cert to test against in case the way the certs were generated turns out to matter at some point in the future. Also slightly increased a start-servers delay in the test in response to one test system taking too long and seeing connection issues. Probably not helped by the number of processes. Clarified a related comment in the test code. Also made a note that environment variable cleanup would be good to add to test_download.py, either copied from or moved somewhere accessible from test_proxy_use.py Signed-off-by: Sebastien Awwad --- tests/ssl_certs/ssl_cert_2.crt | 30 +++++++++ tests/ssl_certs/ssl_cert_expired.crt | 30 +++++++++ tests/test_download.py | 98 ++++++++++++++++++++-------- 3 files changed, 130 insertions(+), 28 deletions(-) create mode 100644 tests/ssl_certs/ssl_cert_2.crt create mode 100644 tests/ssl_certs/ssl_cert_expired.crt diff --git a/tests/ssl_certs/ssl_cert_2.crt b/tests/ssl_certs/ssl_cert_2.crt new file mode 100644 index 00000000..6d6fb63a --- /dev/null +++ b/tests/ssl_certs/ssl_cert_2.crt @@ -0,0 +1,30 @@ +-----BEGIN CERTIFICATE----- +MIIFOTCCA6GgAwIBAgIJAO+bbero+zKtMA0GCSqGSIb3DQEBCwUAMIGAMQswCQYD +VQQGEwJVUzERMA8GA1UECAwITmV3IFlvcmsxETAPBgNVBAcMCEJyb29rbHluMQww +CgYDVQQKDANOWVUxKTAnBgNVBAsMIENvbXB1dGVyIFNjaWVuY2UgYW5kIEVuZ2lu +ZWVyaW5nMRIwEAYDVQQDDAlsb2NhbGhvc3QwHhcNMTgwOTI2MTgwMDAzWhcNMzgw +OTIxMTgwMDAzWjCBgDELMAkGA1UEBhMCVVMxETAPBgNVBAgMCE5ldyBZb3JrMREw +DwYDVQQHDAhCcm9va2x5bjEMMAoGA1UECgwDTllVMSkwJwYDVQQLDCBDb21wdXRl +ciBTY2llbmNlIGFuZCBFbmdpbmVlcmluZzESMBAGA1UEAwwJbG9jYWxob3N0MIIB +ojANBgkqhkiG9w0BAQEFAAOCAY8AMIIBigKCAYEAxyFVeRsWnb1UlCKBks2azM9W +9K+J/ZkzdSb6eCxOIxv79M/Ug54CfWqkySSaQejsu0U/gJxkFYRvwQAy5lATrspY +2kyiWYiggWXFDWz+i8ETPkL9zn59v13sNIpT/IXQj0S3Mr9ZnsUn1qCyEOOIxJxZ +lyuV/M/XP1DP4tArhEvrex12V6MQIK+8fYzEjHG/W7vIIet+wTStIR8ArvVQi0Kv +PbbGCfrZ+e+gq+UpBLBuAfMzM95TW+YJ5duMchie2n6LDmOeegA4jMEv2ppeOr8Q +JJtZuKpXWVbJvLg81yrDjr1rAwJR/WQrnk8GQWPCyPLneAA4mJbi75LqjLxn0AoJ +b3kzLfGEMJJEWXspxNg06bLQU948hB4L7nKARq6s7KoESjEV+/L4koMPWJoNq6fx +OUVw2+S3ITNrDctecRQ1j3RGVPaj5l6bn03C7KV9uRrfqFY3OUjn7A0kDczvRnmr +e1BZIpe+mfGFB+Uu7JiQoBv6I6fqyrdH9rX1LUKlAgMBAAGjgbMwgbAwgZ8GA1Ud +IwSBlzCBlKGBhqSBgzCBgDELMAkGA1UEBhMCVVMxETAPBgNVBAgMCE5ldyBZb3Jr +MREwDwYDVQQHDAhCcm9va2x5bjEMMAoGA1UECgwDTllVMSkwJwYDVQQLDCBDb21w +dXRlciBTY2llbmNlIGFuZCBFbmdpbmVlcmluZzESMBAGA1UEAwwJbG9jYWxob3N0 +ggkA75tt6uj7Mq0wDAYDVR0TBAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAYEAFWcl +1tAmt/3DJDjk0ppF62jbwcEOu1N9Nono9a70ojAQYYuMC7Ditw6rLbeXS8tP8ae/ +drlci3VxlE5PpmAjuP67Uv2CuGu/2iMqa99AWZ4mVN+x4YL6awvYs8ea6I1Xe8tQ +5+RqvNA+QtnjtfOeb6yWQBAGrc2eTX87IzqvV/EewkdKAs4GZUWG1Zjv3effqjTO +qRX94ltW1GWud7fVcqpZLOaK9U+4IaI2nNHuCtWODoyQmMoVApXyig/YQqFe0eyj +76m1T+2SZLRtn0xn1fTHuLZ2bdtTMZ7k5PTAKnBNEn1Rr9MAS+WEASN1ZyoQ3reL +VYrgkMTrrXPO8bdDTvP7z1Jzv5Cq9WMHFvOLfnj/vN9ZPH6w4QT3Zb97SAAOSPK/ +gzOzRtIe+hqCYBh/cwMoeeoAzes/nJgorj3IOTu8JXmtZrZGrdLIhu2Q8U+yKasf ++TUrr6xdcJI/fyVM5BVelpGhqHzzOQe1tO4VYQlAVaaVvFidDPHqTI2/S272 +-----END CERTIFICATE----- diff --git a/tests/ssl_certs/ssl_cert_expired.crt b/tests/ssl_certs/ssl_cert_expired.crt new file mode 100644 index 00000000..f0b79cb9 --- /dev/null +++ b/tests/ssl_certs/ssl_cert_expired.crt @@ -0,0 +1,30 @@ +-----BEGIN CERTIFICATE----- +MIIFOTCCA6GgAwIBAgIJALtyUsChEIJpMA0GCSqGSIb3DQEBCwUAMIGAMQswCQYD +VQQGEwJVUzERMA8GA1UECAwITmV3IFlvcmsxETAPBgNVBAcMCEJyb29rbHluMQww +CgYDVQQKDANOWVUxKTAnBgNVBAsMIENvbXB1dGVyIFNjaWVuY2UgYW5kIEVuZ2lu +ZWVyaW5nMRIwEAYDVQQDDAlsb2NhbGhvc3QwHhcNMTgwOTI2MTc0NTM2WhcNMTgw +OTI1MTc0NTM2WjCBgDELMAkGA1UEBhMCVVMxETAPBgNVBAgMCE5ldyBZb3JrMREw +DwYDVQQHDAhCcm9va2x5bjEMMAoGA1UECgwDTllVMSkwJwYDVQQLDCBDb21wdXRl +ciBTY2llbmNlIGFuZCBFbmdpbmVlcmluZzESMBAGA1UEAwwJbG9jYWxob3N0MIIB +ojANBgkqhkiG9w0BAQEFAAOCAY8AMIIBigKCAYEAxyFVeRsWnb1UlCKBks2azM9W +9K+J/ZkzdSb6eCxOIxv79M/Ug54CfWqkySSaQejsu0U/gJxkFYRvwQAy5lATrspY +2kyiWYiggWXFDWz+i8ETPkL9zn59v13sNIpT/IXQj0S3Mr9ZnsUn1qCyEOOIxJxZ +lyuV/M/XP1DP4tArhEvrex12V6MQIK+8fYzEjHG/W7vIIet+wTStIR8ArvVQi0Kv +PbbGCfrZ+e+gq+UpBLBuAfMzM95TW+YJ5duMchie2n6LDmOeegA4jMEv2ppeOr8Q +JJtZuKpXWVbJvLg81yrDjr1rAwJR/WQrnk8GQWPCyPLneAA4mJbi75LqjLxn0AoJ +b3kzLfGEMJJEWXspxNg06bLQU948hB4L7nKARq6s7KoESjEV+/L4koMPWJoNq6fx +OUVw2+S3ITNrDctecRQ1j3RGVPaj5l6bn03C7KV9uRrfqFY3OUjn7A0kDczvRnmr +e1BZIpe+mfGFB+Uu7JiQoBv6I6fqyrdH9rX1LUKlAgMBAAGjgbMwgbAwgZ8GA1Ud +IwSBlzCBlKGBhqSBgzCBgDELMAkGA1UEBhMCVVMxETAPBgNVBAgMCE5ldyBZb3Jr +MREwDwYDVQQHDAhCcm9va2x5bjEMMAoGA1UECgwDTllVMSkwJwYDVQQLDCBDb21w +dXRlciBTY2llbmNlIGFuZCBFbmdpbmVlcmluZzESMBAGA1UEAwwJbG9jYWxob3N0 +ggkAu3JSwKEQgmkwDAYDVR0TBAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAYEAW4I1 +TacdFv3L9ENFkSLciPb7zFMckLUZfk/P+4VjdapWrfuydO4W/ogMxA4DK09thTsK +N/BgcExyKjDldGUfUv57Tqv3v2E5kbygNcNtP53fwMz3y+7QourzkDE5HWciw1Lb +hmbnCBTzt/UioSBdJnAH29GWpSS+Jzu745sRaI48AS/J5ApH2aVEnNQTCE7v1LNH +2bTTPYl3eDXiD8yOhvyiW1F4y2BSFbQRH/3aE6Goe4A75m8sX50+JlOgjyyQnAMf +vbfvZsjGfqdXv9Qpci50qKCFxHJLXXNAUbX3fDgKE+RoZUNZnmn2VDgJYnToz6on +RcVnppV09kmSjHXZBT04XXUA0vG3p+oU0TO4puJlePVf4Oz23/DRCPHSfVWgMeB2 +c1PpKit4+Bz7mypnsWVw8kk//l0GJ1cHnkkZElKJtPEB7I587jgTCDcN811TGNBc +rLLd/JwtYAvi1CPFt2ICGDvA4AKLY3rBNg5z1DrSE/iom1NTC00SFZJztYiX +-----END CERTIFICATE----- diff --git a/tests/test_download.py b/tests/test_download.py index 9026a906..5176a6a5 100755 --- a/tests/test_download.py +++ b/tests/test_download.py @@ -21,6 +21,8 @@ NOTE: Make sure test_download.py is ran in 'tuf/tests/' directory. Otherwise, module that launches simple server would not be found. + + TODO: Adopt the environment variable management from test_proxy_use.py here. """ # Help with Python 3 compatibility, where the print statement is a function, an @@ -233,30 +235,50 @@ def test_https_connection(self): with open(target_filepath, 'r') as target_file_object: target_data_length = len(target_file_object.read()) - # These cert files should both be valid, but "good" lists localhost and - # "bad" lists another hostname. We'll be trying to download from localhost, - # so we expect + # These cert files provide various test cases: + # good: A valid cert from an older generation of test_download.py tests. + # good2: A valid cert made simultaneous to the bad certs below, with the + # same settings otherwise, tested here in case the difference + # between the way the new bad certs and the old good cert were + # generated turns out to matter at some point. + # bad: An otherwise-valid cert with the wrong hostname. The good certs + # list "localhost", but this lists "notmyhostname". + # expired: An otherwise-valid cert but which is expired (no valid dates + # exist, fwiw: startdate > enddate). good_cert_fname = os.path.join('ssl_certs', 'ssl_cert.crt') + good2_cert_fname = os.path.join('ssl_certs', 'ssl_cert_2.crt') bad_cert_fname = os.path.join('ssl_certs', 'ssl_cert_wronghost.crt') + expired_cert_fname = os.path.join('ssl_certs', 'ssl_cert_expired.crt') - # Launch two https servers (serves files in the current dir). - # The first we expect to operate correctly, and the second we run with an - # HTTPS certificate with an unexpected hostname. + # Launch three https servers (serve files in the current dir). + # The first we expect to operate correctly. + # The second we run with an HTTPS certificate with an unexpected hostname. + # The third we run with an HTTPS certificate that is expired. port1 = str(random.randint(30000, 45000)) port2 = str(int(port1) + 1) + port3 = str(int(port1) + 2) + port4 = str(int(port1) + 3) command1 = ['python', 'simple_https_server.py', port1, good_cert_fname] - command2 = ['python', 'simple_https_server.py', port2, bad_cert_fname] + command2 = ['python', 'simple_https_server.py', port2, good2_cert_fname] + command3 = ['python', 'simple_https_server.py', port3, bad_cert_fname] + command4 = ['python', 'simple_https_server.py', port4, expired_cert_fname] good_https_server_proc = subprocess.Popen(command1, stderr=subprocess.PIPE) - bad_https_server_proc = subprocess.Popen(command2, stderr=subprocess.PIPE) + good2_https_server_proc = subprocess.Popen(command2, stderr=subprocess.PIPE) + bad_https_server_proc = subprocess.Popen(command3, stderr=subprocess.PIPE) + expd_https_server_proc = subprocess.Popen(command4, stderr=subprocess.PIPE) - # NOTE: Following error is raised if delay is not applied: - # - time.sleep(0.2) + # Provide a delay long enough to allow the https servers to start. + # Encountered an error on one test system at delay value of 0.2s, so + # increasing to 0.5s. + # Expect to see "Connection refused" if this delay is not long enough + # (though other issues could cause that). + time.sleep(0.5) relative_target_fpath = os.path.basename(target_filepath) good_https_url = 'https://localhost:' + port1 + '/' + relative_target_fpath - bad_https_url = good_https_url.replace(':' + port1, ':' + port2) - + good2_https_url = good_https_url.replace(':' + port1, ':' + port2) + bad_https_url = good_https_url.replace(':' + port1, ':' + port3) + expired_https_url = good_https_url.replace(':' + port1, ':' + port4) # Download the target file using an https connection. @@ -275,8 +297,8 @@ def test_https_connection(self): with self.assertRaises(requests.exceptions.SSLError): download.unsafe_download(bad_https_url, target_data_length) - # Try connecting to the server process with the good cert while not - # trusting the good cert (trusting the bad cert instead). Expect failure + # Try connecting to the server processes with the good certs while not + # trusting the good certs (trusting the bad cert instead). Expect failure # because even though the server's cert file is otherwise OK, we don't # trust it. print('Trying https download of target file: ' + good_https_url) @@ -285,31 +307,51 @@ def test_https_connection(self): with self.assertRaises(requests.exceptions.SSLError): download.unsafe_download(good_https_url, target_data_length) - # Configure environment to trust the good cert file now instead. - os.environ['REQUESTS_CA_BUNDLE'] = good_cert_fname + print('Trying https download of target file: ' + good2_https_url) + with self.assertRaises(requests.exceptions.SSLError): + download.safe_download(good2_https_url, target_data_length) + with self.assertRaises(requests.exceptions.SSLError): + download.unsafe_download(good2_https_url, target_data_length) - # Try connecting to the server process with the good cert while trusting - # that cert. Expect success. + # Configure environment to now trust the certfile that is expired. + os.environ['REQUESTS_CA_BUNDLE'] = expired_cert_fname + + # Try connecting to the server process with the expired cert while + # trusting the expired cert. Expect failure because even though we trust + # it, it is expired. + logger.info('Trying https download of target file: ' + expired_https_url) + with self.assertRaises(requests.exceptions.SSLError): + download.safe_download(expired_https_url, target_data_length) + with self.assertRaises(requests.exceptions.SSLError): + download.unsafe_download(expired_https_url, target_data_length) + + # Try connecting to the server processes with the good certs while + # trusting the appropriate good certs. Expect success. # Note: running these OK downloads at the top of this try section causes # a failure in a previous assertion: retrieving the same good URL # again after no longer "trusting" the good certfile still succeeds # if we had previously succeeded in retrieving that same URL while # still trusting the good cert. Perhaps it's a caching issue....? # I'm not especially concerned yet, but take note for later.... + os.environ['REQUESTS_CA_BUNDLE'] = good_cert_fname logger.info('Trying https download of target file: ' + good_https_url) download.safe_download(good_https_url, target_data_length) download.unsafe_download(good_https_url, target_data_length) - finally: - if good_https_server_proc.returncode is None: - logger.info( - 'Terminating server process ' + str(good_https_server_proc.pid)) - good_https_server_proc.kill() + os.environ['REQUESTS_CA_BUNDLE'] = good2_cert_fname + logger.info('Trying https download of target file: ' + good2_https_url) + download.safe_download(good2_https_url, target_data_length) + download.unsafe_download(good2_https_url, target_data_length) - if bad_https_server_proc.returncode is None: - logger.info( - 'Terminating server process ' + str(bad_https_server_proc.pid)) - bad_https_server_proc.kill() + finally: + for proc in [ + good_https_server_proc, + good2_https_server_proc, + bad_https_server_proc, + expd_https_server_proc]: + if proc.returncode is None: + logger.info('Terminating server process ' + str(proc.pid)) + proc.kill() From ec27630a480eedabfe38b8997ad196f40c902df6 Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Tue, 2 Oct 2018 15:01:38 -0400 Subject: [PATCH 30/34] minor: PR tweaks based on review: doc, casing, typos, updates - two reversions to unnecessary changes - some typo fixes - capitalization of HTTP/S where reasonable - commenting out code section with ''' rather than # Signed-off-by: Sebastien Awwad --- tests/proxy_server.py | 2 +- tests/test_download.py | 63 +++++++++++++++++++++-------------------- tests/test_proxy_use.py | 26 ++++++++--------- tuf/exceptions.py | 4 +-- tuf/settings.py | 2 +- 5 files changed, 49 insertions(+), 48 deletions(-) diff --git a/tests/proxy_server.py b/tests/proxy_server.py index 68ad8256..80be450b 100644 --- a/tests/proxy_server.py +++ b/tests/proxy_server.py @@ -1,6 +1,6 @@ #!/usr/bin/env python -# This code is taken from: https://github.com/inaz2/proxy2 +# This code is taken from: github.com/inaz2/proxy2 # Credit goes to the author. It has been very slightly modified here to use # IPv4 instead of IPv6, and to only attempt interception of HTTPS traffic # (instead of relaying via HTTP CONNECT) if new global variable INTERCEPT is diff --git a/tests/test_download.py b/tests/test_download.py index 5176a6a5..79d1e50e 100755 --- a/tests/test_download.py +++ b/tests/test_download.py @@ -197,28 +197,29 @@ def test_download_url_to_tempfileobj_and_urls(self): + ''' # This test uses sites on the internet, requiring a net connection to succeed. # Since this is the only such test in TUF, I'm not going to enable it... but # it's here in case it's useful for diagnosis. - # def test_https_validation(self): - # """ - # Use some known URLs on the net to ensure that TUF download checks SSL - # certificates appropriately. - # """ - # # We should never get as far as the target file download itself, so the - # # length we pass to safe_download and unsafe_download shouldn't matter. - # irrelevant_length = 10 - # - # for bad_url in [ - # 'https://expired.badssl.com/', # expired certificate - # 'https://wrong.host.badssl.com/', ]: # hostname verification fail - # - # with self.assertRaises(requests.exceptions.SSLError): - # download.safe_download(bad_url, irrelevant_length) - # - # with self.assertRaises(requests.exceptions.SSLError): - # download.unsafe_download(bad_url, irrelevant_length) + def test_https_validation(self): + """ + Use some known URLs on the net to ensure that TUF download checks SSL + certificates appropriately. + """ + # We should never get as far as the target file download itself, so the + # length we pass to safe_download and unsafe_download shouldn't matter. + irrelevant_length = 10 + for bad_url in [ + 'https://expired.badssl.com/', # expired certificate + 'https://wrong.host.badssl.com/', ]: # hostname verification fail + + with self.assertRaises(requests.exceptions.SSLError): + download.safe_download(bad_url, irrelevant_length) + + with self.assertRaises(requests.exceptions.SSLError): + download.unsafe_download(bad_url, irrelevant_length) + ''' @@ -250,10 +251,12 @@ def test_https_connection(self): bad_cert_fname = os.path.join('ssl_certs', 'ssl_cert_wronghost.crt') expired_cert_fname = os.path.join('ssl_certs', 'ssl_cert_expired.crt') - # Launch three https servers (serve files in the current dir). - # The first we expect to operate correctly. - # The second we run with an HTTPS certificate with an unexpected hostname. - # The third we run with an HTTPS certificate that is expired. + # Launch four HTTPS servers (serve files in the current dir). + # 1: we expect to operate correctly + # 2: also good; uses a slightly different cert (controls for the cert + # generation method used for the next two, in case it comes to matter) + # 3: run with an HTTPS certificate with an unexpected hostname + # 4: run with an HTTPS certificate that is expired port1 = str(random.randint(30000, 45000)) port2 = str(int(port1) + 1) port3 = str(int(port1) + 2) @@ -267,7 +270,7 @@ def test_https_connection(self): bad_https_server_proc = subprocess.Popen(command3, stderr=subprocess.PIPE) expd_https_server_proc = subprocess.Popen(command4, stderr=subprocess.PIPE) - # Provide a delay long enough to allow the https servers to start. + # Provide a delay long enough to allow the HTTPS servers to start. # Encountered an error on one test system at delay value of 0.2s, so # increasing to 0.5s. # Expect to see "Connection refused" if this delay is not long enough @@ -280,7 +283,7 @@ def test_https_connection(self): bad_https_url = good_https_url.replace(':' + port1, ':' + port3) expired_https_url = good_https_url.replace(':' + port1, ':' + port4) - # Download the target file using an https connection. + # Download the target file using an HTTPS connection. # Use try-finally solely to ensure that the server processes are killed. try: @@ -291,7 +294,7 @@ def test_https_connection(self): # Try connecting to the server process with the bad cert while trusting # the bad cert. Expect failure because even though we trust it, the # hostname we're connecting to does not match the hostname in the cert. - logger.info('Trying https download of target file: ' + bad_https_url) + logger.info('Trying HTTPS download of target file: ' + bad_https_url) with self.assertRaises(requests.exceptions.SSLError): download.safe_download(bad_https_url, target_data_length) with self.assertRaises(requests.exceptions.SSLError): @@ -301,13 +304,13 @@ def test_https_connection(self): # trusting the good certs (trusting the bad cert instead). Expect failure # because even though the server's cert file is otherwise OK, we don't # trust it. - print('Trying https download of target file: ' + good_https_url) + print('Trying HTTPS download of target file: ' + good_https_url) with self.assertRaises(requests.exceptions.SSLError): download.safe_download(good_https_url, target_data_length) with self.assertRaises(requests.exceptions.SSLError): download.unsafe_download(good_https_url, target_data_length) - print('Trying https download of target file: ' + good2_https_url) + print('Trying HTTPS download of target file: ' + good2_https_url) with self.assertRaises(requests.exceptions.SSLError): download.safe_download(good2_https_url, target_data_length) with self.assertRaises(requests.exceptions.SSLError): @@ -319,7 +322,7 @@ def test_https_connection(self): # Try connecting to the server process with the expired cert while # trusting the expired cert. Expect failure because even though we trust # it, it is expired. - logger.info('Trying https download of target file: ' + expired_https_url) + logger.info('Trying HTTPS download of target file: ' + expired_https_url) with self.assertRaises(requests.exceptions.SSLError): download.safe_download(expired_https_url, target_data_length) with self.assertRaises(requests.exceptions.SSLError): @@ -334,12 +337,12 @@ def test_https_connection(self): # still trusting the good cert. Perhaps it's a caching issue....? # I'm not especially concerned yet, but take note for later.... os.environ['REQUESTS_CA_BUNDLE'] = good_cert_fname - logger.info('Trying https download of target file: ' + good_https_url) + logger.info('Trying HTTPS download of target file: ' + good_https_url) download.safe_download(good_https_url, target_data_length) download.unsafe_download(good_https_url, target_data_length) os.environ['REQUESTS_CA_BUNDLE'] = good2_cert_fname - logger.info('Trying https download of target file: ' + good2_https_url) + logger.info('Trying HTTPS download of target file: ' + good2_https_url) download.safe_download(good2_https_url, target_data_length) download.unsafe_download(good2_https_url, target_data_length) diff --git a/tests/test_proxy_use.py b/tests/test_proxy_use.py index 08206bc1..181bbe78 100644 --- a/tests/test_proxy_use.py +++ b/tests/test_proxy_use.py @@ -64,7 +64,7 @@ def setUpClass(cls): """ Setup performed before the first test function (TestWithProxies class method) runs. - Launch http, https, and proxy servers in the current working directory. + Launch HTTP, HTTPS, and proxy servers in the current working directory. We'll set up four servers: - HTTP server (simple_server.py) - HTTPS server (simple_https_server.py) @@ -95,8 +95,7 @@ def setUpClass(cls): cls.http_proxy_port = cls.http_port + 2 cls.http_proxy_proc = subprocess.Popen( ['python', 'proxy_server.py', str(cls.http_proxy_port)], - stderr=subprocess.PIPE) - # Note that the http proxy server's address uses http://, regardless of the + # Note that the HTTP proxy server's address uses http://, regardless of the # type of connection used with the target server. cls.http_proxy_addr = 'http://127.0.0.1:' + str(cls.http_proxy_port) @@ -117,8 +116,7 @@ def setUpClass(cls): cls.https_proxy_proc = subprocess.Popen( ['python', 'proxy_server.py', str(cls.https_proxy_port), 'intercept', os.path.join('ssl_certs', 'ssl_cert.crt')], - stderr=subprocess.PIPE) - # Note that the https proxy server's address uses https://, regardless of + # Note that the HTTPS proxy server's address uses https://, regardless of # the type of connection used with the target server. cls.https_proxy_addr = 'https://127.0.0.1:' + str(cls.https_proxy_port) @@ -201,7 +199,7 @@ def test_baseline_no_proxy(self): HTTP proxy, and perform an HTTP connection with the final server. """ - logger.info('Trying http download with no proxy: ' + self.url) + logger.info('Trying HTTP download with no proxy: ' + self.url) download.safe_download(self.url, self.target_data_length) download.unsafe_download(self.url, self.target_data_length) @@ -217,7 +215,7 @@ def test_http_dl_via_smart_http_proxy(self): self.set_env_value('HTTP_PROXY', self.http_proxy_addr) - logger.info('Trying http download via http proxy: ' + self.url) + logger.info('Trying HTTP download via HTTP proxy: ' + self.url) download.safe_download(self.url, self.target_data_length) download.unsafe_download(self.url, self.target_data_length) @@ -225,7 +223,7 @@ def test_http_dl_via_smart_http_proxy(self): - def test_httpS_dl_via_smart_http_proxy(self): + def test_https_dl_via_smart_http_proxy(self): """ Test a length-validating TUF download of a file through a proxy. Use an HTTP proxy that supports HTTP CONNECT (which essentially causes it to act @@ -242,7 +240,7 @@ def test_httpS_dl_via_smart_http_proxy(self): self.set_env_value('REQUESTS_CA_BUNDLE', os.path.join('ssl_certs', 'ssl_cert.crt')) - logger.info('Trying httpS download via http proxy: ' + self.url_https) + logger.info('Trying HTTPS download via HTTP proxy: ' + self.url_https) download.safe_download(self.url_https, self.target_data_length) download.unsafe_download(self.url_https, self.target_data_length) @@ -250,7 +248,7 @@ def test_httpS_dl_via_smart_http_proxy(self): - def test_http_dl_via_httpS_proxy(self): + def test_http_dl_via_https_proxy(self): """ Test a length-validating TUF download of a file through a proxy. Use an HTTPS proxy, and perform an HTTP connection with the final server. @@ -263,7 +261,7 @@ def test_http_dl_via_httpS_proxy(self): self.set_env_value('REQUESTS_CA_BUNDLE', os.path.join('ssl_certs', 'proxy_ca.crt')) - logger.info('Trying http download via httpS proxy: ' + self.url_https) + logger.info('Trying HTTP download via HTTPS proxy: ' + self.url_https) download.safe_download(self.url, self.target_data_length) download.unsafe_download(self.url, self.target_data_length) @@ -271,7 +269,7 @@ def test_http_dl_via_httpS_proxy(self): - def test_httpS_dl_via_httpS_proxy(self): + def test_https_dl_via_https_proxy(self): """ Test a length-validating TUF download of a file through a proxy. Use an HTTPS proxy, and perform an HTTPS connection with the final server. @@ -286,7 +284,7 @@ def test_httpS_dl_via_httpS_proxy(self): self.set_env_value('REQUESTS_CA_BUNDLE', os.path.join('ssl_certs', 'proxy_ca.crt')) - logger.info('Trying httpS download via httpS proxy: ' + self.url_https) + logger.info('Trying HTTPS download via HTTPS proxy: ' + self.url_https) download.safe_download(self.url_https, self.target_data_length) download.unsafe_download(self.url_https, self.target_data_length) @@ -340,7 +338,7 @@ def restore_env_value(self, key): # del os.environ[key] should unset the variable. Otherwise, we'll just # have to settle for setting it to an empty string. # See os.environ in: - # https://docs.python.org/2/library/os.html#process-parameters) + # https://docs.python.org/2/library/os.html#process-parameters os.environ[key] = '' del os.environ[key] diff --git a/tuf/exceptions.py b/tuf/exceptions.py index 3b72bd40..86f60723 100755 --- a/tuf/exceptions.py +++ b/tuf/exceptions.py @@ -172,8 +172,8 @@ def __init__(self, expected_length, observed_length): self.observed_length = observed_length #bytes def __str__(self): - return 'Observed length (' + repr(self.observed_length)+\ - ') != expected length (' + repr(self.expected_length) + ').' + return 'Observed length (' + repr(self.observed_length) + \ + ') < expected length (' + repr(self.expected_length) + ').' class SlowRetrievalError(DownloadError): diff --git a/tuf/settings.py b/tuf/settings.py index cf5ad588..83d28924 100755 --- a/tuf/settings.py +++ b/tuf/settings.py @@ -75,7 +75,7 @@ DEFAULT_TARGETS_REQUIRED_LENGTH = 5000000 #bytes # Set a timeout value in seconds (float) for non-blocking socket operations. -SOCKET_TIMEOUT = 5 #seconds +SOCKET_TIMEOUT = 4 #seconds # The maximum chunk of data, in bytes, we would download in every round. CHUNK_SIZE = 400000 #bytes From 15b33b82770a78ee1a0a2f93aae4338cea232a38 Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Tue, 2 Oct 2018 15:05:42 -0400 Subject: [PATCH 31/34] Test: modularize: add func that spawns Python intepreter process that draws from sys.executable (the currently running Python interpreter) instead of assuming 'python' is correct. Use this function instead of having many individual subprocess calls written out. Slightly simplifies code, too. This should eventually be moved to a common test module instead of appearing in two places in the test code. Signed-off-by: Sebastien Awwad --- tests/test_download.py | 41 +++++++++++++++++++++++++++++++---------- tests/test_proxy_use.py | 37 ++++++++++++++++++++++++++----------- 2 files changed, 57 insertions(+), 21 deletions(-) diff --git a/tests/test_download.py b/tests/test_download.py index 79d1e50e..9a041088 100755 --- a/tests/test_download.py +++ b/tests/test_download.py @@ -38,6 +38,7 @@ import os import random import subprocess +import sys import time import unittest @@ -73,8 +74,7 @@ def setUp(self): # Launch a SimpleHTTPServer (serves files in the current dir). self.PORT = random.randint(30000, 45000) - command = ['python', 'simple_server.py', str(self.PORT)] - self.server_proc = subprocess.Popen(command, stderr=subprocess.PIPE) + self.server_proc = popen_python(['simple_server.py', str(self.PORT)]) logger.info('\n\tServer process started.') logger.info('\tServer process id: '+str(self.server_proc.pid)) logger.info('\tServing on port: '+str(self.PORT)) @@ -261,14 +261,14 @@ def test_https_connection(self): port2 = str(int(port1) + 1) port3 = str(int(port1) + 2) port4 = str(int(port1) + 3) - command1 = ['python', 'simple_https_server.py', port1, good_cert_fname] - command2 = ['python', 'simple_https_server.py', port2, good2_cert_fname] - command3 = ['python', 'simple_https_server.py', port3, bad_cert_fname] - command4 = ['python', 'simple_https_server.py', port4, expired_cert_fname] - good_https_server_proc = subprocess.Popen(command1, stderr=subprocess.PIPE) - good2_https_server_proc = subprocess.Popen(command2, stderr=subprocess.PIPE) - bad_https_server_proc = subprocess.Popen(command3, stderr=subprocess.PIPE) - expd_https_server_proc = subprocess.Popen(command4, stderr=subprocess.PIPE) + good_https_server_proc = popen_python( + ['simple_https_server.py', port1, good_cert_fname]) + good2_https_server_proc = popen_python( + ['simple_https_server.py', port2, good2_cert_fname]) + bad_https_server_proc = popen_python( + ['simple_https_server.py', port3, bad_cert_fname]) + expd_https_server_proc = popen_python( + ['simple_https_server.py', port4, expired_cert_fname]) # Provide a delay long enough to allow the HTTPS servers to start. # Encountered an error on one test system at delay value of 0.2s, so @@ -366,6 +366,27 @@ def test__get_content_length(self): self.assertEqual(content_length, None) + + + +# TODO: Move this to a common test module (tests/common.py?) +# and strip it test_proxy_use.py and test_download.py. +def popen_python(command_arg_list): + """ + Run subprocess.Popen() to produce a process running a Python interpreter. + Uses the same Python interpreter that the current process is using, via + sys.executable. + """ + assert sys.executable, 'Test cannot function: unable to determine ' \ + 'current Python interpreter via sys.executable.' + + return subprocess.Popen( + [sys.executable] + command_arg_list, stderr=subprocess.PIPE) + + + + + # Run unit test. if __name__ == '__main__': unittest.main() diff --git a/tests/test_proxy_use.py b/tests/test_proxy_use.py index 181bbe78..91430c91 100644 --- a/tests/test_proxy_use.py +++ b/tests/test_proxy_use.py @@ -41,6 +41,7 @@ import os import random import subprocess +import sys import time import unittest @@ -78,23 +79,21 @@ def setUpClass(cls): # Launch a simple HTTP server (serves files in the current dir). cls.http_port = random.randint(30000, 45000) - cls.http_server_proc = subprocess.Popen( - ['python', 'simple_server.py', str(cls.http_port)], - stderr=subprocess.PIPE) + cls.http_server_proc = popen_python( + ['simple_server.py', str(cls.http_port)]) # Launch an HTTPS server (serves files in the current dir). cls.https_port = cls.http_port + 1 - cls.https_server_proc = subprocess.Popen( - ['python', 'simple_https_server.py', str(cls.https_port)], - stderr=subprocess.PIPE) + cls.https_server_proc = popen_python( + ['simple_https_server.py', str(cls.https_port)]) # Launch an HTTP proxy server derived from inaz2/proxy2. # This one is able to handle HTTP CONNECT requests, and so can pass HTTPS # requests on to the target server. cls.http_proxy_port = cls.http_port + 2 - cls.http_proxy_proc = subprocess.Popen( - ['python', 'proxy_server.py', str(cls.http_proxy_port)], + cls.http_proxy_proc = popen_python( + ['proxy_server.py', str(cls.http_proxy_port)]) # Note that the HTTP proxy server's address uses http://, regardless of the # type of connection used with the target server. cls.http_proxy_addr = 'http://127.0.0.1:' + str(cls.http_proxy_port) @@ -113,9 +112,9 @@ def setUpClass(cls): # server certs to accept in its HTTPS connection to the target server. # This is only relevant if the proxy is in intercept mode. cls.https_proxy_port = cls.http_port + 3 - cls.https_proxy_proc = subprocess.Popen( - ['python', 'proxy_server.py', str(cls.https_proxy_port), 'intercept', - os.path.join('ssl_certs', 'ssl_cert.crt')], + cls.https_proxy_proc = popen_python( + ['proxy_server.py', str(cls.https_proxy_port), 'intercept', + os.path.join('ssl_certs', 'ssl_cert.crt')]) # Note that the HTTPS proxy server's address uses https://, regardless of # the type of connection used with the target server. cls.https_proxy_addr = 'https://127.0.0.1:' + str(cls.https_proxy_port) @@ -359,6 +358,22 @@ def restore_all_modified_env_values(self): +# TODO: Move this to a common test module (tests/common.py?) +# and strip it test_proxy_use.py and test_download.py. +def popen_python(command_arg_list): + """ + Run subprocess.Popen() to produce a process running a Python interpreter. + Uses the same Python interpreter that the current process is using, via + sys.executable. + """ + assert sys.executable, 'Test cannot function: unable to determine ' \ + 'current Python interpreter via sys.executable.' + + return subprocess.Popen( + [sys.executable] + command_arg_list, stderr=subprocess.PIPE) + + + # Run unit test. if __name__ == '__main__': unittest.main() From e8a1ab1395335681739681504ccaaeded61f60c1 Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Tue, 2 Oct 2018 15:28:23 -0400 Subject: [PATCH 32/34] Test: simplify env variable overwrite/restore code in test_proxy_use Signed-off-by: Sebastien Awwad --- tests/test_proxy_use.py | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/tests/test_proxy_use.py b/tests/test_proxy_use.py index 91430c91..bed8ea43 100644 --- a/tests/test_proxy_use.py +++ b/tests/test_proxy_use.py @@ -301,17 +301,13 @@ def set_env_value(self, key, value): with new saved values. """ - if key in self.old_env_values: - # Do not save the current value. We already saved an older value, and - # the original one is the one we'll restore to, not whatever we most - # recently overwrote it with. - pass - - elif key in os.environ: - self.old_env_values[key] = os.environ[key] - - else: - self.old_env_values[key] = None # Note that it was previously unset. + # Only save the current value if we have not previously saved an older + # value. The original one is the one we'll restore to, not whatever we + # most recently overwrote. + if key not in self.old_env_values: + # If the value was previously unset in os.environ, save the old value + # as None so that we know to unset it. + self.old_env_values[key] = os.environ.get(key, None) # Actually set the new value. os.environ[key] = value From ebcb17bbef18d3d0dc1a7462c3aeb1d4c8ba58e4 Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Tue, 2 Oct 2018 15:49:39 -0400 Subject: [PATCH 33/34] Test: clear requests sessions when updating expected ssl certs to make sure that the test uses the intended certificate. (There's some indirect indication that the updated environment variable might not always have been used.) Signed-off-by: Sebastien Awwad --- tests/test_download.py | 22 ++++++++++++++++------ tests/test_proxy_use.py | 11 ++++++++++- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/tests/test_download.py b/tests/test_download.py index 9a041088..5cf07af8 100755 --- a/tests/test_download.py +++ b/tests/test_download.py @@ -290,6 +290,9 @@ def test_https_connection(self): # Trust the certfile that happens to use a different hostname than we # will expect. os.environ['REQUESTS_CA_BUNDLE'] = bad_cert_fname + # Clear sessions to ensure that the certificate we just specified is used. + # TODO: Confirm necessity of this session clearing and lay out mechanics. + tuf.download._sessions = {} # Try connecting to the server process with the bad cert while trusting # the bad cert. Expect failure because even though we trust it, the @@ -316,8 +319,12 @@ def test_https_connection(self): with self.assertRaises(requests.exceptions.SSLError): download.unsafe_download(good2_https_url, target_data_length) + # Configure environment to now trust the certfile that is expired. os.environ['REQUESTS_CA_BUNDLE'] = expired_cert_fname + # Clear sessions to ensure that the certificate we just specified is used. + # TODO: Confirm necessity of this session clearing and lay out mechanics. + tuf.download._sessions = {} # Try connecting to the server process with the expired cert while # trusting the expired cert. Expect failure because even though we trust @@ -328,20 +335,23 @@ def test_https_connection(self): with self.assertRaises(requests.exceptions.SSLError): download.unsafe_download(expired_https_url, target_data_length) + # Try connecting to the server processes with the good certs while # trusting the appropriate good certs. Expect success. - # Note: running these OK downloads at the top of this try section causes - # a failure in a previous assertion: retrieving the same good URL - # again after no longer "trusting" the good certfile still succeeds - # if we had previously succeeded in retrieving that same URL while - # still trusting the good cert. Perhaps it's a caching issue....? - # I'm not especially concerned yet, but take note for later.... + # TODO: expand testing to switch expected certificates back and forth a + # bit more while clearing / not clearing sessions. os.environ['REQUESTS_CA_BUNDLE'] = good_cert_fname + # Clear sessions to ensure that the certificate we just specified is used. + # TODO: Confirm necessity of this session clearing and lay out mechanics. + tuf.download._sessions = {} logger.info('Trying HTTPS download of target file: ' + good_https_url) download.safe_download(good_https_url, target_data_length) download.unsafe_download(good_https_url, target_data_length) os.environ['REQUESTS_CA_BUNDLE'] = good2_cert_fname + # Clear sessions to ensure that the certificate we just specified is used. + # TODO: Confirm necessity of this session clearing and lay out mechanics. + tuf.download._sessions = {} logger.info('Trying HTTPS download of target file: ' + good2_https_url) download.safe_download(good2_https_url, target_data_length) download.unsafe_download(good2_https_url, target_data_length) diff --git a/tests/test_proxy_use.py b/tests/test_proxy_use.py index bed8ea43..90832ae7 100644 --- a/tests/test_proxy_use.py +++ b/tests/test_proxy_use.py @@ -238,6 +238,9 @@ def test_https_dl_via_smart_http_proxy(self): self.set_env_value('REQUESTS_CA_BUNDLE', os.path.join('ssl_certs', 'ssl_cert.crt')) + # Clear sessions to ensure that the certificate we just specified is used. + # TODO: Confirm necessity of this session clearing and lay out mechanics. + tuf.download._sessions = {} logger.info('Trying HTTPS download via HTTP proxy: ' + self.url_https) download.safe_download(self.url_https, self.target_data_length) @@ -259,6 +262,9 @@ def test_http_dl_via_https_proxy(self): # plain HTTP connection to the target server. self.set_env_value('REQUESTS_CA_BUNDLE', os.path.join('ssl_certs', 'proxy_ca.crt')) + # Clear sessions to ensure that the certificate we just specified is used. + # TODO: Confirm necessity of this session clearing and lay out mechanics. + tuf.download._sessions = {} logger.info('Trying HTTP download via HTTPS proxy: ' + self.url_https) download.safe_download(self.url, self.target_data_length) @@ -282,6 +288,9 @@ def test_https_dl_via_https_proxy(self): # started in setUpClass(). self.set_env_value('REQUESTS_CA_BUNDLE', os.path.join('ssl_certs', 'proxy_ca.crt')) + # Clear sessions to ensure that the certificate we just specified is used. + # TODO: Confirm necessity of this session clearing and lay out mechanics. + tuf.download._sessions = {} logger.info('Trying HTTPS download via HTTPS proxy: ' + self.url_https) download.safe_download(self.url_https, self.target_data_length) @@ -325,7 +334,7 @@ def restore_env_value(self, key): assert key in self.old_env_values, 'Test coding mistake: something is ' \ 'trying to restore environment variable ' + key + ', but that ' \ 'variable does not appear in the list of values to restore. ' \ - 'Please make sure to use _set_env_value().' + 'Please make sure to use set_env_value().' if self.old_env_values[key] is None: # If it was not previously set, try to unset it. From 01d8d9e78007072185642cf79edc809fd5628dac Mon Sep 17 00:00:00 2001 From: Sebastien Awwad Date: Tue, 2 Oct 2018 16:48:09 -0400 Subject: [PATCH 34/34] Test: tighten test-skip conditions and lengthen a subprocess sleep After seeing some AppVeyor failures, I've increased the wait after starting test HTTP, HTTPS, and proxy servers from 0.5s to 1s, to make it less likely that tests will fail because the servers weren't done starting up yet. After some review comments by @aaaaalbert, I've tightened the logic in aggregate_tests.py around which tests to skip unless a certain Python version is running, and added some consistency checks. This also involved a bit of clarification of comments and variable names. Signed-off-by: Sebastien Awwad --- tests/aggregate_tests.py | 42 ++++++++++++++++++++++++++-------------- tests/test_proxy_use.py | 3 ++- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/tests/aggregate_tests.py b/tests/aggregate_tests.py index 495e8929..2654bee6 100755 --- a/tests/aggregate_tests.py +++ b/tests/aggregate_tests.py @@ -43,40 +43,54 @@ # 'test_' and end with '.py'. A shell-style wildcard is used with glob() to # match desired filenames. All the tests matching the pattern will be loaded # and run in a test suite. -tests_list = glob.glob('test_*.py') +available_tests = glob.glob('test_*.py') # A dictionary of test modules that should only run in certain python versions. # Carefully consider the impact of only testing these in a given version. # test_proxy_use.py: uses a proxy that only runs in Python2.7. TUF's # compatibility with proxies is not likely to vary based on the Python version # in use, so this is OK for now. See comments in that module. -# The format here is: if major is included, limit version to that listed here. -# If minor is included, limit version to that listed here. Skip the test if any -# such listed constraints don't match the python version currently running. +# The semantics here are: only add to this list the particular tests that are +# to be run in a single major version or a single minor version. An entry must +# include major version, and may include minor version. +# Skip the test if any such listed constraints don't match the python version +# currently running. +# Note that aggregate_tests.py is run for each version of Python that tox is +# configured to use. Note also that this TUF implementation does not support +# any Python versions <2.7 or any Python3 versions <3.4. VERSION_SPECIFIC_TESTS = { 'test_proxy_use': {'major': 2, 'minor': 7}} # Run test only if Python2.7 +# Further example: +# 'test_abc': {'major': 2} # Run test only if Python2 -# Remove '.py' from each filename to allow loadTestsFromNames() (called below) -# to properly load the file as a module. -tests_without_extension = [] -for test in tests_list: +# Determine which tests should be run. +test_modules_to_run = [] +for test in available_tests: + # Remove '.py' from each filename to allow loadTestsFromNames() (called below) + # to properly load the file as a module. + assert test[-3:] == '.py', 'aggregate_tests.py is inconsistent; fix.' test = test[:-3] + if test in VERSION_SPECIFIC_TESTS: - if 'major' in VERSION_SPECIFIC_TESTS[test] \ - and sys.version_info.major != VERSION_SPECIFIC_TESTS[test]['major']: + # Consistency checks. + assert 'major' in VERSION_SPECIFIC_TESTS[test], 'Empty/illogical constraint' + for keyword in VERSION_SPECIFIC_TESTS[test]: + assert keyword in ['major', 'minor'], 'Unrecognized test constraint' + + if sys.version_info.major != VERSION_SPECIFIC_TESTS[test]['major']: continue - elif 'minor' in VERSION_SPECIFIC_TESTS[test] \ + if 'minor' in VERSION_SPECIFIC_TESTS[test] \ and sys.version_info.minor != VERSION_SPECIFIC_TESTS[test]['minor']: continue - tests_without_extension.append(test) + test_modules_to_run.append(test) # Randomize the order in which the tests run. Randomization might catch errors # with unit tests that do not properly clean up or restore monkey-patched # modules. -random.shuffle(tests_without_extension) +random.shuffle(test_modules_to_run) if __name__ == '__main__': - suite = unittest.TestLoader().loadTestsFromNames(tests_without_extension) + suite = unittest.TestLoader().loadTestsFromNames(test_modules_to_run) all_tests_passed = unittest.TextTestRunner(verbosity=1).run(suite).wasSuccessful() if not all_tests_passed: sys.exit(1) diff --git a/tests/test_proxy_use.py b/tests/test_proxy_use.py index 90832ae7..b245f840 100644 --- a/tests/test_proxy_use.py +++ b/tests/test_proxy_use.py @@ -123,7 +123,8 @@ def setUpClass(cls): # start listening before allowing tests to begin, lest we get "Connection # refused" errors. On the first test system. 0.1s was too short and 0.15s # was long enough. Use 0.5s to be safe, and if issues arise, increase it. - time.sleep(0.5) + # Observed some occasional AppVeyor failures, so increasing this to 1s. + time.sleep(1)