diff --git a/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py b/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py index 3622bee41..ed5cf750e 100644 --- a/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py +++ b/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py @@ -79,6 +79,8 @@ def _get_names(config): if line.strip().startswith("server_name"): names = line.partition("server_name")[2].rpartition(";")[0] for n in names.split(): - all_names.add(n) + # Filter out wildcards in both all_names and test_names + if not n.startswith("*."): + all_names.add(n) non_ip_names = set(n for n in all_names if not util.IP_REGEX.match(n)) return all_names, non_ip_names diff --git a/certbot-compatibility-test/certbot_compatibility_test/test_driver.py b/certbot-compatibility-test/certbot_compatibility_test/test_driver.py index 71100bb27..71a0ba574 100644 --- a/certbot-compatibility-test/certbot_compatibility_test/test_driver.py +++ b/certbot-compatibility-test/certbot_compatibility_test/test_driver.py @@ -1,7 +1,6 @@ """Tests Certbot plugins against different server configurations.""" import argparse import filecmp -import functools import logging import os import shutil @@ -64,17 +63,17 @@ def test_authenticator(plugin, config, temp_dir): type(achalls[i]), achalls[i].domain, config) success = False elif isinstance(responses[i], challenges.TLSSNI01Response): - verify = functools.partial(responses[i].simple_verify, achalls[i].chall, - achalls[i].domain, - util.JWK.public_key(), - host="127.0.0.1", - port=plugin.https_port) - if _try_until_true(verify): + verified = responses[i].simple_verify(achalls[i].chall, + achalls[i].domain, + util.JWK.public_key(), + host="127.0.0.1", + port=plugin.https_port) + if verified: logger.info( "tls-sni-01 verification for %s succeeded", achalls[i].domain) else: logger.error( - "tls-sni-01 verification for %s in %s failed", + "**** tls-sni-01 verification for %s in %s failed", achalls[i].domain, config) success = False @@ -122,7 +121,7 @@ def test_installer(args, plugin, config, temp_dir): if names_match: logger.info("get_all_names test succeeded") else: - logger.error("get_all_names test failed for config %s", config) + logger.error("**** get_all_names test failed for config %s", config) domains = list(plugin.get_testable_domain_names()) success = test_deploy_cert(plugin, temp_dir, domains) @@ -147,7 +146,7 @@ def test_deploy_cert(plugin, temp_dir, domains): plugin.deploy_cert(domain, cert_path, util.KEY_PATH, cert_path, cert_path) plugin.save() # Needed by the Apache plugin except le_errors.Error as error: - logger.error("Plugin failed to deploy certificate for %s:", domain) + logger.error("**** Plugin failed to deploy certificate for %s:", domain) logger.exception(error) return False @@ -155,11 +154,12 @@ def test_deploy_cert(plugin, temp_dir, domains): return False success = True + time.sleep(3) for domain in domains: - verify = functools.partial(validator.Validator().certificate, cert, - domain, "127.0.0.1", plugin.https_port) - if not _try_until_true(verify): - logger.error("Could not verify certificate for domain %s", domain) + verified = validator.Validator().certificate( + cert, domain, "127.0.0.1", plugin.https_port) + if not verified: + logger.error("**** Could not verify certificate for domain %s", domain) success = False if success: @@ -177,16 +177,21 @@ def test_enhancements(plugin, domains): "enhancements") return False - for domain in domains: + domains_and_info = [(domain, []) for domain in domains] + + for domain, info in domains_and_info: try: + previous_redirect = validator.Validator().any_redirect( + "localhost", plugin.http_port, headers={"Host": domain}) + info.append(previous_redirect) plugin.enhance(domain, "redirect") plugin.save() # Needed by the Apache plugin except le_errors.PluginError as error: # Don't immediately fail because a redirect may already be enabled - logger.warning("Plugin failed to enable redirect for %s:", domain) + logger.warning("*** Plugin failed to enable redirect for %s:", domain) logger.warning("%s", error) except le_errors.Error as error: - logger.error("An error occurred while enabling redirect for %s:", + logger.error("*** An error occurred while enabling redirect for %s:", domain) logger.exception(error) @@ -194,12 +199,14 @@ def test_enhancements(plugin, domains): return False success = True - for domain in domains: - verify = functools.partial(validator.Validator().redirect, "localhost", - plugin.http_port, headers={"Host": domain}) - if not _try_until_true(verify): - logger.error("Improper redirect for domain %s", domain) - success = False + for domain, info in domains_and_info: + previous_redirect = info[0] + if not previous_redirect: + verified = validator.Validator().redirect( + "localhost", plugin.http_port, headers={"Host": domain}) + if not verified: + logger.error("*** Improper redirect for domain %s", domain) + success = False if success: logger.info("Enhancements test succeeded") @@ -207,17 +214,6 @@ def test_enhancements(plugin, domains): return success -def _try_until_true(func, max_tries=5, sleep_time=0.5): - """Calls func up to max_tries times until it returns True""" - for _ in xrange(0, max_tries): - if func(): - return True - else: - time.sleep(sleep_time) - - return False - - def _save_and_restart(plugin, title=None): """Saves and restart the plugin, returning True if no errors occurred""" try: @@ -225,7 +221,7 @@ def _save_and_restart(plugin, title=None): plugin.restart() return True except le_errors.Error as error: - logger.error("Plugin failed to save and restart server:") + logger.error("*** Plugin failed to save and restart server:") logger.exception(error) return False @@ -235,12 +231,12 @@ def test_rollback(plugin, config, backup): try: plugin.rollback_checkpoints(1337) except le_errors.Error as error: - logger.error("Plugin raised an exception during rollback:") + logger.error("*** Plugin raised an exception during rollback:") logger.exception(error) return False if _dirs_are_unequal(config, backup): - logger.error("Rollback failed for config `%s`", config) + logger.error("*** Rollback failed for config `%s`", config) return False else: logger.info("Rollback succeeded") diff --git a/certbot-compatibility-test/certbot_compatibility_test/testdata/nginx.tar.gz b/certbot-compatibility-test/certbot_compatibility_test/testdata/nginx.tar.gz index 4ca5cc977..2f06add17 100644 Binary files a/certbot-compatibility-test/certbot_compatibility_test/testdata/nginx.tar.gz and b/certbot-compatibility-test/certbot_compatibility_test/testdata/nginx.tar.gz differ diff --git a/certbot-compatibility-test/certbot_compatibility_test/validator.py b/certbot-compatibility-test/certbot_compatibility_test/validator.py index 62dd466a1..0fd6efab5 100644 --- a/certbot-compatibility-test/certbot_compatibility_test/validator.py +++ b/certbot-compatibility-test/certbot_compatibility_test/validator.py @@ -45,19 +45,12 @@ class Validator(object): else: response = requests.get(url, allow_redirects=False) - if response.status_code not in (301, 303): - return False - redirect_location = response.headers.get("location", "") # We're checking that the redirect we added behaves correctly. # It's okay for some server configuration to redirect to an # http URL, as long as it's on some other domain. if not redirect_location.startswith("https://"): - if not redirect_location.startswith("http://"): - return False - else: - if redirect_location[len("http://"):] == name: - return False + return False if response.status_code != 301: logger.error("Server did not redirect with permanent code") @@ -65,6 +58,16 @@ class Validator(object): return True + def any_redirect(self, name, port=80, headers=None): + """Test whether webserver redirects.""" + url = "http://{0}:{1}".format(name, port) + if headers: + response = requests.get(url, headers=headers, allow_redirects=False) + else: + response = requests.get(url, allow_redirects=False) + + return response.status_code in xrange(300, 309) + def hsts(self, name): """Test for HTTP Strict Transport Security header""" headers = requests.get("https://" + name).headers diff --git a/tox.ini b/tox.ini index 7e7181528..299b7f20f 100644 --- a/tox.ini +++ b/tox.ini @@ -161,7 +161,7 @@ passenv = DOCKER_* commands = docker build -t certbot-compatibility-test -f certbot-compatibility-test/Dockerfile . docker build -t nginx-compat -f certbot-compatibility-test/Dockerfile-nginx . - docker run --rm -it nginx-compat -c nginx.tar.gz -vvvv + docker run --rm -it nginx-compat -c nginx.tar.gz -vv -aie whitelist_externals = docker passenv = DOCKER_*