diff --git a/Dockerfile-dev b/Dockerfile-dev index 2a89b2ff5..607aa3441 100644 --- a/Dockerfile-dev +++ b/Dockerfile-dev @@ -20,10 +20,10 @@ WORKDIR /opt/certbot/src # If doesn't exist, it is created along with all missing # directories in its path. -# TODO: Install non-default Python versions for tox. # TODO: Install Apache/Nginx for plugin development. COPY letsencrypt-auto-source/letsencrypt-auto /opt/certbot/src/letsencrypt-auto-source/letsencrypt-auto RUN /opt/certbot/src/letsencrypt-auto-source/letsencrypt-auto --os-packages-only && \ + apt-get install python3-dev -y && \ apt-get clean && \ rm -rf /var/lib/apt/lists/* \ /tmp/* \ @@ -51,6 +51,8 @@ COPY certbot-compatibility-test /opt/certbot/src/certbot-compatibility-test/ COPY tests /opt/certbot/src/tests/ RUN virtualenv --no-site-packages -p python2 /opt/certbot/venv && \ + /opt/certbot/venv/bin/pip install -U pip && \ + /opt/certbot/venv/bin/pip install -U setuptools && \ /opt/certbot/venv/bin/pip install \ -e /opt/certbot/src/acme \ -e /opt/certbot/src \ @@ -58,8 +60,7 @@ RUN virtualenv --no-site-packages -p python2 /opt/certbot/venv && \ -e /opt/certbot/src/certbot-nginx \ -e /opt/certbot/src/letshelp-certbot \ -e /opt/certbot/src/certbot-compatibility-test \ - -e /opt/certbot/src[dev,docs] && \ - /opt/certbot/venv/bin/pip install -U setuptools + -e /opt/certbot/src[dev,docs] # install in editable mode (-e) to save space: it's not possible to # "rm -rf /opt/certbot/src" (it's stays in the underlaying image); diff --git a/README.rst b/README.rst index ab12562df..e44a61021 100644 --- a/README.rst +++ b/README.rst @@ -129,19 +129,7 @@ email to client-dev+subscribe@letsencrypt.org) System Requirements =================== -The Let's Encrypt Client presently only runs on Unix-ish OSes that include -Python 2.6 or 2.7; Python 3.x support will hopefully be added in the future. The -client requires root access in order to write to ``/etc/letsencrypt``, -``/var/log/letsencrypt``, ``/var/lib/letsencrypt``; to bind to ports 80 and 443 -(if you use the ``standalone`` plugin) and to read and modify webserver -configurations (if you use the ``apache`` or ``nginx`` plugins). If none of -these apply to you, it is theoretically possible to run without root privileges, -but for most users who want to avoid running an ACME client as root, either -`letsencrypt-nosudo `_ or -`simp_le `_ are more appropriate choices. - -The Apache plugin currently requires a Debian-based OS with augeas version -1.0; this includes Ubuntu 12.04+ and Debian 7+. +See https://certbot.eff.org/docs/install.html#system-requirements. .. Do not modify this comment unless you know what you're doing. tag:intro-end diff --git a/acme/acme/challenges.py b/acme/acme/challenges.py index ac4e3d60a..14641af10 100644 --- a/acme/acme/challenges.py +++ b/acme/acme/challenges.py @@ -5,7 +5,7 @@ import hashlib import logging import socket -from cryptography.hazmat.primitives import hashes +from cryptography.hazmat.primitives import hashes # type: ignore import OpenSSL import requests @@ -23,7 +23,7 @@ logger = logging.getLogger(__name__) class Challenge(jose.TypedJSONObjectWithFields): # _fields_to_partial_json | pylint: disable=abstract-method """ACME challenge.""" - TYPES = {} + TYPES = {} # type: dict @classmethod def from_json(cls, jobj): @@ -37,7 +37,7 @@ class Challenge(jose.TypedJSONObjectWithFields): class ChallengeResponse(jose.TypedJSONObjectWithFields): # _fields_to_partial_json | pylint: disable=abstract-method """ACME challenge response.""" - TYPES = {} + TYPES = {} # type: dict resource_type = 'challenge' resource = fields.Resource(resource_type) diff --git a/acme/acme/client.py b/acme/acme/client.py index d6166960d..0c8886fc6 100644 --- a/acme/acme/client.py +++ b/acme/acme/client.py @@ -28,11 +28,13 @@ logger = logging.getLogger(__name__) # https://urllib3.readthedocs.org/en/latest/security.html#insecureplatformwarning if sys.version_info < (2, 7, 9): # pragma: no cover try: - requests.packages.urllib3.contrib.pyopenssl.inject_into_urllib3() + requests.packages.urllib3.contrib.pyopenssl.inject_into_urllib3() # type: ignore except AttributeError: import urllib3.contrib.pyopenssl # pylint: disable=import-error urllib3.contrib.pyopenssl.inject_into_urllib3() +DEFAULT_NETWORK_TIMEOUT = 45 + DER_CONTENT_TYPE = 'application/pkix-cert' @@ -503,13 +505,14 @@ class ClientNetwork(object): # pylint: disable=too-many-instance-attributes REPLAY_NONCE_HEADER = 'Replay-Nonce' def __init__(self, key, alg=jose.RS256, verify_ssl=True, - user_agent='acme-python'): + user_agent='acme-python', timeout=DEFAULT_NETWORK_TIMEOUT): self.key = key self.alg = alg self.verify_ssl = verify_ssl self._nonces = set() self.user_agent = user_agent self.session = requests.Session() + self._default_timeout = timeout def __del__(self): self.session.close() @@ -608,7 +611,7 @@ class ClientNetwork(object): # pylint: disable=too-many-instance-attributes kwargs['verify'] = self.verify_ssl kwargs.setdefault('headers', {}) kwargs['headers'].setdefault('User-Agent', self.user_agent) - kwargs.setdefault('timeout', 45) # timeout after 45 seconds + kwargs.setdefault('timeout', self._default_timeout) response = self.session.request(method, url, *args, **kwargs) # If content is DER, log the base64 of it instead of raw bytes, to keep # binary data out of the logs. diff --git a/acme/acme/crypto_util.py b/acme/acme/crypto_util.py index 266f2c0c7..6a33b3e52 100644 --- a/acme/acme/crypto_util.py +++ b/acme/acme/crypto_util.py @@ -23,7 +23,7 @@ logger = logging.getLogger(__name__) # https://www.openssl.org/docs/ssl/SSLv23_method.html). _serve_sni # should be changed to use "set_options" to disable SSLv2 and SSLv3, # in case it's used for things other than probing/serving! -_DEFAULT_TLSSNI01_SSL_METHOD = OpenSSL.SSL.SSLv23_METHOD +_DEFAULT_TLSSNI01_SSL_METHOD = OpenSSL.SSL.SSLv23_METHOD # type: ignore class SSLSocket(object): # pylint: disable=too-few-public-methods diff --git a/acme/acme/crypto_util_test.py b/acme/acme/crypto_util_test.py index ebb4010a6..9cf1f7deb 100644 --- a/acme/acme/crypto_util_test.py +++ b/acme/acme/crypto_util_test.py @@ -6,7 +6,7 @@ import time import unittest import six -from six.moves import socketserver # pylint: disable=import-error +from six.moves import socketserver #type: ignore # pylint: disable=import-error import OpenSSL diff --git a/acme/acme/jose/jwa.py b/acme/acme/jose/jwa.py index 1853e0107..9b682ecab 100644 --- a/acme/acme/jose/jwa.py +++ b/acme/acme/jose/jwa.py @@ -9,9 +9,9 @@ import logging import cryptography.exceptions from cryptography.hazmat.backends import default_backend -from cryptography.hazmat.primitives import hashes -from cryptography.hazmat.primitives import hmac -from cryptography.hazmat.primitives.asymmetric import padding +from cryptography.hazmat.primitives import hashes # type: ignore +from cryptography.hazmat.primitives import hmac # type: ignore +from cryptography.hazmat.primitives.asymmetric import padding # type: ignore from acme.jose import errors from acme.jose import interfaces @@ -28,9 +28,9 @@ class JWA(interfaces.JSONDeSerializable): # pylint: disable=abstract-method """JSON Web Algorithm.""" -class JWASignature(JWA, collections.Hashable): +class JWASignature(JWA, collections.Hashable): # type: ignore """JSON Web Signature Algorithm.""" - SIGNATURES = {} + SIGNATURES = {} # type: dict def __init__(self, name): self.name = name diff --git a/acme/acme/jose/jwk.py b/acme/acme/jose/jwk.py index 5b6965c4d..54423f670 100644 --- a/acme/acme/jose/jwk.py +++ b/acme/acme/jose/jwk.py @@ -6,9 +6,9 @@ import logging import cryptography.exceptions from cryptography.hazmat.backends import default_backend -from cryptography.hazmat.primitives import hashes +from cryptography.hazmat.primitives import hashes # type: ignore from cryptography.hazmat.primitives import serialization -from cryptography.hazmat.primitives.asymmetric import ec +from cryptography.hazmat.primitives.asymmetric import ec # type: ignore from cryptography.hazmat.primitives.asymmetric import rsa import six @@ -25,8 +25,8 @@ class JWK(json_util.TypedJSONObjectWithFields): # pylint: disable=too-few-public-methods """JSON Web Key.""" type_field_name = 'kty' - TYPES = {} - cryptography_key_types = () + TYPES = {} # type: dict + cryptography_key_types = () # type: tuple """Subclasses should override.""" required = NotImplemented diff --git a/acme/acme/jose/jws.py b/acme/acme/jose/jws.py index 9c14cf729..8fa8d7670 100644 --- a/acme/acme/jose/jws.py +++ b/acme/acme/jose/jws.py @@ -121,12 +121,12 @@ class Header(json_util.JSONObjectWithFields): # x5c does NOT use JOSE Base64 (4.1.6) - @x5c.encoder + @x5c.encoder # type: ignore def x5c(value): # pylint: disable=missing-docstring,no-self-argument return [base64.b64encode(OpenSSL.crypto.dump_certificate( OpenSSL.crypto.FILETYPE_ASN1, cert.wrapped)) for cert in value] - @x5c.decoder + @x5c.decoder # type: ignore def x5c(value): # pylint: disable=missing-docstring,no-self-argument try: return tuple(util.ComparableX509(OpenSSL.crypto.load_certificate( @@ -157,12 +157,12 @@ class Signature(json_util.JSONObjectWithFields): 'signature', decoder=json_util.decode_b64jose, encoder=json_util.encode_b64jose) - @protected.encoder + @protected.encoder # type: ignore def protected(value): # pylint: disable=missing-docstring,no-self-argument # wrong type guess (Signature, not bytes) | pylint: disable=no-member return json_util.encode_b64jose(value.encode('utf-8')) - @protected.decoder + @protected.decoder # type: ignore def protected(value): # pylint: disable=missing-docstring,no-self-argument return json_util.decode_b64jose(value).decode('utf-8') diff --git a/acme/acme/jose/util.py b/acme/acme/jose/util.py index 6be9a6602..26b7e0c5a 100644 --- a/acme/acme/jose/util.py +++ b/acme/acme/jose/util.py @@ -134,7 +134,7 @@ class ComparableRSAKey(ComparableKey): # pylint: disable=too-few-public-methods return hash((self.__class__, pub.n, pub.e)) -class ImmutableMap(collections.Mapping, collections.Hashable): +class ImmutableMap(collections.Mapping, collections.Hashable): # type: ignore # pylint: disable=too-few-public-methods """Immutable key to value mapping with attribute access.""" @@ -180,7 +180,7 @@ class ImmutableMap(collections.Mapping, collections.Hashable): for key, value in six.iteritems(self))) -class frozendict(collections.Mapping, collections.Hashable): +class frozendict(collections.Mapping, collections.Hashable): # type: ignore # pylint: disable=invalid-name,too-few-public-methods """Frozen dictionary.""" __slots__ = ('_items', '_keys') diff --git a/acme/acme/messages.py b/acme/acme/messages.py index f7670dd72..4070290ad 100644 --- a/acme/acme/messages.py +++ b/acme/acme/messages.py @@ -98,7 +98,7 @@ class Error(jose.JSONObjectWithFields, errors.Error): if part is not None) -class _Constant(jose.JSONDeSerializable, collections.Hashable): +class _Constant(jose.JSONDeSerializable, collections.Hashable): # type: ignore """ACME constant.""" __slots__ = ('name',) POSSIBLE_NAMES = NotImplemented @@ -132,7 +132,7 @@ class _Constant(jose.JSONDeSerializable, collections.Hashable): class Status(_Constant): """ACME "status" field.""" - POSSIBLE_NAMES = {} + POSSIBLE_NAMES = {} # type: dict STATUS_UNKNOWN = Status('unknown') STATUS_PENDING = Status('pending') STATUS_PROCESSING = Status('processing') @@ -143,7 +143,7 @@ STATUS_REVOKED = Status('revoked') class IdentifierType(_Constant): """ACME identifier type.""" - POSSIBLE_NAMES = {} + POSSIBLE_NAMES = {} # type: dict IDENTIFIER_FQDN = IdentifierType('dns') # IdentifierDNS in Boulder @@ -161,7 +161,7 @@ class Identifier(jose.JSONObjectWithFields): class Directory(jose.JSONDeSerializable): """Directory.""" - _REGISTERED_TYPES = {} + _REGISTERED_TYPES = {} # type: dict class Meta(jose.JSONObjectWithFields): """Directory Meta.""" diff --git a/acme/acme/standalone.py b/acme/acme/standalone.py index 02cc2daf5..087240c15 100644 --- a/acme/acme/standalone.py +++ b/acme/acme/standalone.py @@ -6,9 +6,9 @@ import logging import os import sys -from six.moves import BaseHTTPServer # pylint: disable=import-error +from six.moves import BaseHTTPServer # type: ignore # pylint: disable=import-error from six.moves import http_client # pylint: disable=import-error -from six.moves import socketserver # pylint: disable=import-error +from six.moves import socketserver # type: ignore # pylint: disable=import-error import OpenSSL diff --git a/acme/acme/standalone_test.py b/acme/acme/standalone_test.py index 58469d470..613258c97 100644 --- a/acme/acme/standalone_test.py +++ b/acme/acme/standalone_test.py @@ -7,7 +7,7 @@ import time import unittest from six.moves import http_client # pylint: disable=import-error -from six.moves import socketserver # pylint: disable=import-error +from six.moves import socketserver # type: ignore # pylint: disable=import-error import requests diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py index cdfc01626..39d25619d 100644 --- a/certbot-apache/certbot_apache/configurator.py +++ b/certbot-apache/certbot_apache/configurator.py @@ -174,6 +174,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # Set Version if self.version is None: self.version = self.get_version() + logger.debug('Apache version is %s', + '.'.join(str(i) for i in self.version)) if self.version < (2, 2): raise errors.NotSupportedError( "Apache Version %s not supported.", str(self.version)) @@ -254,9 +256,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): raise errors.PluginError( "Unable to find cert and/or key directives") - logger.info("Deploying Certificate to VirtualHost %s", vhost.filep) - logger.debug("Apache version is %s", - ".".join(str(i) for i in self.version)) + logger.info("Deploying Certificate for %s to VirtualHost %s", domain, vhost.filep) if self.version < (2, 4, 8) or (chain_path and not fullchain_path): # install SSLCertificateFile, SSLCertificateKeyFile, diff --git a/certbot-apache/certbot_apache/parser.py b/certbot-apache/certbot_apache/parser.py index 275a01e7f..67984a26c 100644 --- a/certbot-apache/certbot_apache/parser.py +++ b/certbot-apache/certbot_apache/parser.py @@ -136,7 +136,8 @@ class ApacheParser(object): proc = subprocess.Popen( constants.os_constant("define_cmd"), stdout=subprocess.PIPE, - stderr=subprocess.PIPE) + stderr=subprocess.PIPE, + universal_newlines=True) stdout, stderr = proc.communicate() except (OSError, ValueError): diff --git a/certbot-apache/setup.py b/certbot-apache/setup.py index db8cb11db..cb35d2686 100644 --- a/certbot-apache/setup.py +++ b/certbot-apache/setup.py @@ -11,7 +11,7 @@ install_requires = [ 'acme=={0}'.format(version), 'certbot=={0}'.format(version), 'mock', - 'python-augeas', + 'python-augeas<=0.5.0', # For pkg_resources. >=1.0 so pip resolves it to a version cryptography # will tolerate; see #2599: 'setuptools>=1.0', diff --git a/certbot-compatibility-test/certbot_compatibility_test/interfaces.py b/certbot-compatibility-test/certbot_compatibility_test/interfaces.py index cd367d9af..7d3daee09 100644 --- a/certbot-compatibility-test/certbot_compatibility_test/interfaces.py +++ b/certbot-compatibility-test/certbot_compatibility_test/interfaces.py @@ -20,20 +20,20 @@ class IPluginProxy(zope.interface.Interface): def __init__(args): """Initializes the plugin with the given command line args""" - def cleanup_from_tests(): + def cleanup_from_tests(): # type: ignore """Performs any necessary cleanup from running plugin tests. This is guaranteed to be called before the program exits. """ - def has_more_configs(): + def has_more_configs(): # type: ignore """Returns True if there are more configs to test""" - def load_config(): + def load_config(): # type: ignore """Loads the next config and returns its name""" - def get_testable_domain_names(): + def get_testable_domain_names(): # type: ignore """Returns the domain names that can be used in testing""" @@ -44,7 +44,7 @@ class IAuthenticatorProxy(IPluginProxy, certbot.interfaces.IAuthenticator): class IInstallerProxy(IPluginProxy, certbot.interfaces.IInstaller): """Wraps a Certbot installer""" - def get_all_names_answer(): + def get_all_names_answer(): # type: ignore """Returns all names that should be found by the installer""" diff --git a/certbot-compatibility-test/nginx/nginx-roundtrip-testdata/simplepythonfcgi/weird-spacing.conf b/certbot-compatibility-test/nginx/nginx-roundtrip-testdata/simplepythonfcgi/weird-spacing.conf index a201fe659..5fbc76676 100644 --- a/certbot-compatibility-test/nginx/nginx-roundtrip-testdata/simplepythonfcgi/weird-spacing.conf +++ b/certbot-compatibility-test/nginx/nginx-roundtrip-testdata/simplepythonfcgi/weird-spacing.conf @@ -1,16 +1,16 @@ # static files location ~ ^/(images|javascript|js|css|flash|media|static)/ { - root ${PROJECTBASE}/${PROJECTNAME}/static; + root "${PROJECTBASE}/${PROJECTNAME}/static"; } location = /favicon.ico { - root ${PROJECTBASE}/${PROJECTNAME}/static/images; + root "${PROJECTBASE}/${PROJECTNAME}/static/images"; } # pass all requests to FastCGI TG server listening on ${HOST}:${PORT} # location / { - fastcgi_pass ${HOST}:${PORT}; + fastcgi_pass "${HOST}:${PORT}"; fastcgi_index index; fastcgi_param SCRIPT_FILENAME /scripts$fastcgi_script_name; include conf/fastcgi_params; diff --git a/certbot-nginx/certbot_nginx/configurator.py b/certbot-nginx/certbot_nginx/configurator.py index 7348def2f..b36f638ab 100644 --- a/certbot-nginx/certbot_nginx/configurator.py +++ b/certbot-nginx/certbot_nginx/configurator.py @@ -5,9 +5,11 @@ import re import shutil import socket import subprocess +import tempfile import time import OpenSSL +import six import zope.interface from acme import challenges @@ -30,16 +32,16 @@ from certbot_nginx import parser logger = logging.getLogger(__name__) REDIRECT_BLOCK = [[ - ['\n ', 'if', ' ', '($scheme != "https") '], - [['\n ', 'return', ' ', '301 https://$host$request_uri'], + ['\n ', 'if', ' ', '($scheme', ' ', '!=', ' ', '"https") '], + [['\n ', 'return', ' ', '301', ' ', 'https://$host$request_uri'], '\n '] ], ['\n']] TEST_REDIRECT_BLOCK = [ [ - ['if', '($scheme != "https")'], + ['if', '($scheme', '!=', '"https")'], [ - ['return', '301 https://$host$request_uri'] + ['return', '301', 'https://$host$request_uri'] ] ], ['#', ' managed by Certbot'] @@ -262,7 +264,7 @@ class NginxConfigurator(common.Plugin): """ if not matches: return None - elif matches[0]['rank'] in xrange(2, 6): + elif matches[0]['rank'] in six.moves.range(2, 6): # Wildcard match - need to find the longest one rank = matches[0]['rank'] wildcards = [x for x in matches if x['rank'] == rank] @@ -818,7 +820,7 @@ class NginxConfigurator(common.Plugin): self.restart() -def nginx_restart(nginx_ctl, nginx_conf="/etc/nginx.conf"): +def nginx_restart(nginx_ctl, nginx_conf): """Restarts the Nginx Server. .. todo:: Nginx restart is fatal if the configuration references @@ -829,22 +831,22 @@ def nginx_restart(nginx_ctl, nginx_conf="/etc/nginx.conf"): """ try: - proc = subprocess.Popen([nginx_ctl, "-c", nginx_conf, "-s", "reload"], - stdout=subprocess.PIPE, - stderr=subprocess.PIPE) - stdout, stderr = proc.communicate() + proc = subprocess.Popen([nginx_ctl, "-c", nginx_conf, "-s", "reload"]) + proc.communicate() if proc.returncode != 0: # Maybe Nginx isn't running - nginx_proc = subprocess.Popen([nginx_ctl, "-c", nginx_conf], - stdout=subprocess.PIPE, - stderr=subprocess.PIPE) - stdout, stderr = nginx_proc.communicate() - - if nginx_proc.returncode != 0: - # Enter recovery routine... - raise errors.MisconfigurationError( - "nginx restart failed:\n%s\n%s" % (stdout, stderr)) + # Write to temporary files instead of piping because of communication issues on Arch + # https://github.com/certbot/certbot/issues/4324 + with tempfile.TemporaryFile() as out: + with tempfile.TemporaryFile() as err: + nginx_proc = subprocess.Popen([nginx_ctl, "-c", nginx_conf], + stdout=out, stderr=err) + nginx_proc.communicate() + if nginx_proc.returncode != 0: + # Enter recovery routine... + raise errors.MisconfigurationError( + "nginx restart failed:\n%s\n%s" % (out.read(), err.read())) except (OSError, ValueError): raise errors.MisconfigurationError("nginx restart failed") diff --git a/certbot-nginx/certbot_nginx/nginxparser.py b/certbot-nginx/certbot_nginx/nginxparser.py index f6437c589..20aeeb554 100644 --- a/certbot-nginx/certbot_nginx/nginxparser.py +++ b/certbot-nginx/certbot_nginx/nginxparser.py @@ -2,11 +2,9 @@ # Forked from https://github.com/fatiherikli/nginxparser (MIT Licensed) import copy import logging -import string from pyparsing import ( - Literal, White, Word, alphanums, CharsNotIn, Combine, Forward, Group, - Optional, OneOrMore, Regex, ZeroOrMore) + Literal, White, Forward, Group, Optional, OneOrMore, QuotedString, Regex, ZeroOrMore, Combine) from pyparsing import stringEnd from pyparsing import restOfLine @@ -14,73 +12,42 @@ logger = logging.getLogger(__name__) class RawNginxParser(object): # pylint: disable=expression-not-assigned + # pylint: disable=pointless-statement """A class that parses nginx configuration with pyparsing.""" # constants - space = Optional(White()) - nonspace = Regex(r"\S+") + space = Optional(White()).leaveWhitespace() + required_space = White().leaveWhitespace() + left_bracket = Literal("{").suppress() - right_bracket = space.leaveWhitespace() + Literal("}").suppress() + right_bracket = space + Literal("}").suppress() semicolon = Literal(";").suppress() - key = Word(alphanums + "_/+-.") - dollar_var = Combine(Literal('$') + Regex(r"[^\{\};,\s]+")) - condition = Regex(r"\(.+\)") - # Matches anything that is not a special character, and ${SHELL_VARS}, AND - # any chars in single or double quotes - # All of these COULD be upgraded to something like - # https://stackoverflow.com/a/16130746 - dquoted = Regex(r'(\".*\")') - squoted = Regex(r"(\'.*\')") - nonspecial = Regex(r"[^\{\};,]") - varsub = Regex(r"(\$\{\w+\})") - # nonspecial nibbles one character at a time, but the other objects take - # precedence. We use ZeroOrMore to allow entries like "break ;" to be - # parsed as assignments - value = Combine(ZeroOrMore(dquoted | squoted | varsub | nonspecial)) + dquoted = QuotedString('"', multiline=True, unquoteResults=False, escChar='\\') + squoted = QuotedString("'", multiline=True, unquoteResults=False, escChar='\\') + quoted = dquoted | squoted + head_tokenchars = Regex(r"[^{};\s'\"]") # if (last_space) + tail_tokenchars = Regex(r"(\$\{)|[^{;\s]") # else + tokenchars = Combine(head_tokenchars + ZeroOrMore(tail_tokenchars)) + paren_quote_extend = Combine(quoted + Literal(')') + ZeroOrMore(tail_tokenchars)) + # note: ')' allows extension, but then we fall into else, not last_space. - location = CharsNotIn("{};," + string.whitespace) - # modifier for location uri [ = | ~ | ~* | ^~ ] - modifier = Literal("=") | Literal("~*") | Literal("~") | Literal("^~") + token = paren_quote_extend | tokenchars | quoted + + whitespace_token_group = space + token + ZeroOrMore(required_space + token) + space + assignment = whitespace_token_group + semicolon - # rules comment = space + Literal('#') + restOfLine - assignment = space + key + Optional(space + value, default=None) + semicolon - location_statement = space + Optional(modifier) + Optional(space + location + space) - if_statement = space + Literal("if") + space + condition + space - charset_map_statement = space + Literal("charset_map") + space + value + space + value - - map_statement = space + Literal("map") + space + nonspace + space + dollar_var + space - # This is NOT an accurate way to parse nginx map entries; it's almost - # certainly too permissive and may be wrong in other ways, but it should - # preserve things correctly in mmmmost or all cases. - # - # - I can neither prove nor disprove that it is correct wrt all escaped - # semicolon situations - # Addresses https://github.com/fatiherikli/nginxparser/issues/19 - map_pattern = Regex(r'".*"') | Regex(r"'.*'") | nonspace - map_entry = space + map_pattern + space + value + space + semicolon - map_block = Group( - Group(map_statement).leaveWhitespace() + - left_bracket + - Group(ZeroOrMore(Group(comment | map_entry)) + space).leaveWhitespace() + - right_bracket) - block = Forward() - # key could for instance be "server" or "http", or "location" (in which case - # location_statement needs to have a non-empty location) + # order matters! see issue 518, and also http { # server { \n} + contents = Group(comment) | Group(block) | Group(assignment) - block_begin = (Group(space + key + location_statement) ^ - Group(if_statement) ^ - Group(charset_map_statement)).leaveWhitespace() + block_begin = Group(whitespace_token_group) + block_innards = Group(ZeroOrMore(contents) + space).leaveWhitespace() + block << block_begin + left_bracket + block_innards + right_bracket - block_innards = Group(ZeroOrMore(Group(comment | assignment) | block | map_block) - + space).leaveWhitespace() - - block << Group(block_begin + left_bracket + block_innards + right_bracket) - - script = OneOrMore(Group(comment | assignment) ^ block ^ map_block) + space + stringEnd + script = OneOrMore(contents) + space + stringEnd script.parseWithTabs().leaveWhitespace() def __init__(self, source): @@ -107,30 +74,23 @@ class RawNginxDumper(object): if isinstance(b0, str): yield b0 continue - b = copy.deepcopy(b0) - if spacey(b[0]): - yield b.pop(0) # indentation - if not b: + item = copy.deepcopy(b0) + if spacey(item[0]): + yield item.pop(0) # indentation + if not item: continue - key, values = b.pop(0), b.pop(0) - if isinstance(key, list): - yield "".join(key) + '{' - for parameter in values: + if isinstance(item[0], list): # block + yield "".join(item.pop(0)) + '{' + for parameter in item.pop(0): for line in self.__iter__([parameter]): # negate "for b0 in blocks" yield line yield '}' - else: - if isinstance(key, str) and key.strip() == '#': # comment - yield key + values - else: # assignment - gap = "" - # Sometimes the parser has stuck some gap whitespace in here; - # if so rotate it into gap - if values and spacey(values): - gap = values - values = b.pop(0) - yield key + gap + values + ';' + else: # not a block - list of strings + semicolon = ";" + if isinstance(item[0], str) and item[0].strip() == '#': # comment + semicolon = "" + yield "".join(item) + semicolon def __str__(self): """Return the parsed block as a string.""" diff --git a/certbot-nginx/certbot_nginx/obj.py b/certbot-nginx/certbot_nginx/obj.py index 29fa976f3..849cefe1f 100644 --- a/certbot-nginx/certbot_nginx/obj.py +++ b/certbot-nginx/certbot_nginx/obj.py @@ -1,6 +1,8 @@ """Module contains classes used by the Nginx Configurator.""" import re +import six + from certbot.plugins import common REDIRECT_DIRECTIVES = ['return', 'rewrite'] @@ -97,6 +99,11 @@ class Addr(common.Addr): def __repr__(self): return "Addr(" + self.__str__() + ")" + def __hash__(self): + # Python 3 requires explicit overridden for __hash__ + # See certbot-apache/certbot_apache/obj.py for more information + return super(Addr, self).__hash__() + def super_eq(self, other): """Check ip/port equality, with IPv6 support. """ @@ -147,13 +154,15 @@ class VirtualHost(object): # pylint: disable=too-few-public-methods self.path = path def __str__(self): - addr_str = ", ".join(str(addr) for addr in self.addrs) + addr_str = ", ".join(str(addr) for addr in sorted(self.addrs, key=str)) + # names might be a set, and it has different representations in Python + # 2 and 3. Force it to be a list here for consistent outputs return ("file: %s\n" "addrs: %s\n" "names: %s\n" "ssl: %s\n" "enabled: %s" % (self.filep, addr_str, - self.names, self.ssl, self.enabled)) + list(self.names), self.ssl, self.enabled)) def __repr__(self): return "VirtualHost(" + self.__str__().replace("\n", ", ") + ")\n" @@ -161,7 +170,7 @@ class VirtualHost(object): # pylint: disable=too-few-public-methods def __eq__(self, other): if isinstance(other, self.__class__): return (self.filep == other.filep and - list(self.addrs) == list(other.addrs) and + sorted(self.addrs, key=str) == sorted(other.addrs, key=str) and self.names == other.names and self.ssl == other.ssl and self.enabled == other.enabled and @@ -181,7 +190,7 @@ class VirtualHost(object): # pylint: disable=too-few-public-methods def contains_list(self, test): """Determine if raw server block contains test list at top level """ - for i in xrange(0, len(self.raw) - len(test)): + for i in six.moves.range(0, len(self.raw) - len(test)): if self.raw[i:i + len(test)] == test: return True return False diff --git a/certbot-nginx/certbot_nginx/options-ssl-nginx.conf b/certbot-nginx/certbot_nginx/options-ssl-nginx.conf index 89c920b3e..e1839909d 100644 --- a/certbot-nginx/certbot_nginx/options-ssl-nginx.conf +++ b/certbot-nginx/certbot_nginx/options-ssl-nginx.conf @@ -4,4 +4,4 @@ ssl_session_timeout 1440m; ssl_protocols TLSv1 TLSv1.1 TLSv1.2; ssl_prefer_server_ciphers on; -ssl_ciphers "ECDHE-ECDSA-AES128-GCM-SHA256 ECDHE-ECDSA-AES256-GCM-SHA384 ECDHE-ECDSA-AES128-SHA ECDHE-ECDSA-AES256-SHA ECDHE-ECDSA-AES128-SHA256 ECDHE-ECDSA-AES256-SHA384 ECDHE-RSA-AES128-GCM-SHA256 ECDHE-RSA-AES256-GCM-SHA384 ECDHE-RSA-AES128-SHA ECDHE-RSA-AES128-SHA256 ECDHE-RSA-AES256-SHA384 DHE-RSA-AES128-GCM-SHA256 DHE-RSA-AES256-GCM-SHA384 DHE-RSA-AES128-SHA DHE-RSA-AES256-SHA DHE-RSA-AES128-SHA256 DHE-RSA-AES256-SHA256 EDH-RSA-DES-CBC3-SHA"; +ssl_ciphers "ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES128-SHA:DHE-RSA-AES256-SHA256:DHE-RSA-AES256-SHA:ECDHE-ECDSA-DES-CBC3-SHA:ECDHE-RSA-DES-CBC3-SHA:EDH-RSA-DES-CBC3-SHA:AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-SHA256:AES256-SHA256:AES128-SHA:AES256-SHA:DES-CBC3-SHA:!DSS"; diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py index eddc7b9b0..6f3f344db 100644 --- a/certbot-nginx/certbot_nginx/parser.py +++ b/certbot-nginx/certbot_nginx/parser.py @@ -298,9 +298,9 @@ class NginxParser(object): """ server = vhost.raw for directive in server: - if not directive or len(directive) < 2: + if not directive: continue - elif directive[0] == 'ssl' and directive[1] == 'on': + elif _is_ssl_on_directive(directive): return True return False @@ -342,7 +342,7 @@ class NginxParser(object): vhost.names = parsed_server['names'] vhost.raw = new_server except errors.MisconfigurationError as err: - raise errors.MisconfigurationError("Problem in %s: %s" % (filename, err.message)) + raise errors.MisconfigurationError("Problem in %s: %s" % (filename, str(err))) def _do_for_subarray(entry, condition, func, path=None): @@ -468,17 +468,17 @@ def _is_include_directive(entry): len(entry) == 2 and entry[0] == 'include' and isinstance(entry[1], str)) +def _is_ssl_on_directive(entry): + """Checks if an nginx parsed entry is an 'ssl on' directive. -def _get_servernames(names): - """Turns a server_name string into a list of server names - - :param str names: server names - :rtype: list + :param list entry: the parsed entry + :returns: Whether it's an 'ssl on' directive + :rtype: bool """ - whitespace_re = re.compile(r'\s+') - names = re.sub(whitespace_re, ' ', names) - return names.split(' ') + return (isinstance(entry, list) and + len(entry) == 2 and entry[0] == 'ssl' and + entry[1] == 'on') def _add_directives(block, directives, replace): """Adds or replaces directives in a config block. @@ -550,12 +550,11 @@ def _add_directive(block, directive, replace): # and there is already a copy of that directive with a different value # in the config file. directive_name = directive[0] - directive_value = directive[1] if location is None or (isinstance(directive_name, str) and directive_name in REPEATABLE_DIRECTIVES): block.append(directive) _comment_directive(block, len(block) - 1) - elif block[location][1] != directive_value: + elif block[location] != directive: raise errors.MisconfigurationError( 'tried to insert directive "{0}" but found ' 'conflicting "{1}".'.format(directive, block[location])) @@ -585,15 +584,14 @@ def _parse_server_raw(server): if not directive: continue if directive[0] == 'listen': - addr = obj.Addr.fromstring(directive[1]) + addr = obj.Addr.fromstring(" ".join(directive[1:])) if addr: parsed_server['addrs'].add(addr) if addr.ssl: parsed_server['ssl'] = True elif directive[0] == 'server_name': - parsed_server['names'].update( - _get_servernames(directive[1])) - elif directive[0] == 'ssl' and directive[1] == 'on': + parsed_server['names'].update(directive[1:]) + elif _is_ssl_on_directive(directive): parsed_server['ssl'] = True apply_ssl_to_all_addrs = True diff --git a/certbot-nginx/certbot_nginx/tests/configurator_test.py b/certbot-nginx/certbot_nginx/tests/configurator_test.py index cc36aa0de..b9e70cd59 100644 --- a/certbot-nginx/certbot_nginx/tests/configurator_test.py +++ b/certbot-nginx/certbot_nginx/tests/configurator_test.py @@ -27,12 +27,13 @@ class NginxConfiguratorTest(util.NginxTest): super(NginxConfiguratorTest, self).setUp() self.config = util.get_nginx_configurator( - self.config_path, self.config_dir, self.work_dir) + self.config_path, self.config_dir, self.work_dir, self.logs_dir) def tearDown(self): shutil.rmtree(self.temp_dir) shutil.rmtree(self.config_dir) shutil.rmtree(self.work_dir) + shutil.rmtree(self.logs_dir) @mock.patch("certbot_nginx.configurator.util.exe_exists") def test_prepare_no_install(self, mock_exe_exists): @@ -93,7 +94,7 @@ class NginxConfiguratorTest(util.NginxTest): None, [0]) self.config.parser.add_server_directives( mock_vhost, - [['listen', ' ', '5001 ssl']], + [['listen', ' ', '5001', ' ', 'ssl']], replace=False) self.config.save() @@ -104,7 +105,7 @@ class NginxConfiguratorTest(util.NginxTest): ['listen', '127.0.0.1'], ['server_name', '.example.com'], ['server_name', 'example.*'], - ['listen', '5001 ssl'], + ['listen', '5001', 'ssl'], ['#', parser.COMMENT]]]], parsed[0]) @@ -204,13 +205,13 @@ class NginxConfiguratorTest(util.NginxTest): ['server_name', '.example.com'], ['server_name', 'example.*'], - ['listen', '5001 ssl'], + ['listen', '5001', 'ssl'], ['ssl_certificate', 'example/fullchain.pem'], ['ssl_certificate_key', 'example/key.pem']] + util.filter_comments(self.config.parser.loc["ssl_options"]) ]], parsed_example_conf) - self.assertEqual([['server_name', 'somename alias another.alias']], + self.assertEqual([['server_name', 'somename', 'alias', 'another.alias']], parsed_server_conf) self.assertTrue(util.contains_at_depth( parsed_nginx_conf, @@ -221,8 +222,8 @@ class NginxConfiguratorTest(util.NginxTest): ['include', 'server.conf'], [['location', '/'], [['root', 'html'], - ['index', 'index.html index.htm']]], - ['listen', '5001 ssl'], + ['index', 'index.html', 'index.htm']]], + ['listen', '5001', 'ssl'], ['ssl_certificate', '/etc/nginx/fullchain.pem'], ['ssl_certificate_key', '/etc/nginx/key.pem']] + util.filter_comments(self.config.parser.loc["ssl_options"]) @@ -246,7 +247,7 @@ class NginxConfiguratorTest(util.NginxTest): ['server_name', 'summer.com'], ['listen', '80'], - ['listen', '5001 ssl'], + ['listen', '5001', 'ssl'], ['ssl_certificate', 'summer/fullchain.pem'], ['ssl_certificate_key', 'summer/key.pem']] + util.filter_comments(self.config.parser.loc["ssl_options"]) @@ -261,13 +262,13 @@ class NginxConfiguratorTest(util.NginxTest): # Note: As more challenges are offered this will have to be expanded achall1 = achallenges.KeyAuthorizationAnnotatedChallenge( challb=messages.ChallengeBody( - chall=challenges.TLSSNI01(token="kNdwjwOeX0I_A8DXt9Msmg"), + chall=challenges.TLSSNI01(token=b"kNdwjwOeX0I_A8DXt9Msmg"), uri="https://ca.org/chall0_uri", status=messages.Status("pending"), ), domain="localhost", account_key=self.rsa512jwk) achall2 = achallenges.KeyAuthorizationAnnotatedChallenge( challb=messages.ChallengeBody( - chall=challenges.TLSSNI01(token="m8TdO1qik4JVFtgPPurJmg"), + chall=challenges.TLSSNI01(token=b"m8TdO1qik4JVFtgPPurJmg"), uri="https://ca.org/chall1_uri", status=messages.Status("pending"), ), domain="example.com", account_key=self.rsa512jwk) @@ -407,8 +408,8 @@ class NginxConfiguratorTest(util.NginxTest): # Test that we successfully add a redirect when there is # a listen directive expected = [ - ['if', '($scheme != "https") '], - [['return', '301 https://$host$request_uri']] + ['if', '($scheme', '!=', '"https") '], + [['return', '301', 'https://$host$request_uri']] ] example_conf = self.config.parser.abs_path('sites-enabled/example.com') diff --git a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py index e83b414cf..dd31ebac8 100644 --- a/certbot-nginx/certbot_nginx/tests/nginxparser_test.py +++ b/certbot-nginx/certbot_nginx/tests/nginxparser_test.py @@ -25,15 +25,15 @@ class TestRawNginxParser(unittest.TestCase): def test_blocks(self): parsed = RawNginxParser.block.parseString('foo {}').asList() - self.assertEqual(parsed, [[['foo', ' '], []]]) + self.assertEqual(parsed, [['foo', ' '], []]) parsed = RawNginxParser.block.parseString('location /foo{}').asList() - self.assertEqual(parsed, [[['location', ' ', '/foo'], []]]) + self.assertEqual(parsed, [['location', ' ', '/foo'], []]) parsed = RawNginxParser.block.parseString('foo { bar foo ; }').asList() - self.assertEqual(parsed, [[['foo', ' '], [[' ', 'bar', ' ', 'foo '], ' ']]]) + self.assertEqual(parsed, [['foo', ' '], [[' ', 'bar', ' ', 'foo', ' '], ' ']]) def test_nested_blocks(self): parsed = RawNginxParser.block.parseString('foo { bar {} }').asList() - block, content = FIRST(parsed) + block, content = parsed self.assertEqual(FIRST(content), [[' ', 'bar', ' '], []]) self.assertEqual(FIRST(block), 'foo') @@ -72,8 +72,8 @@ class TestRawNginxParser(unittest.TestCase): [['user', 'www-data'], [['http'], [[['server'], [ - ['listen', '*:80 default_server ssl'], - ['server_name', '*.www.foo.com *.www.example.com'], + ['listen', '*:80', 'default_server', 'ssl'], + ['server_name', '*.www.foo.com', '*.www.example.com'], ['root', '/home/ubuntu/sites/foo/'], [['location', '/status'], [ [['types'], [['image/jpeg', 'jpg']]], @@ -97,17 +97,35 @@ class TestRawNginxParser(unittest.TestCase): [['server'], [['server_name', 'with.if'], [['location', '~', '^/services/.+$'], - [[['if', '($request_filename ~* \\.(ttf|woff)$)'], - [['add_header', 'Access-Control-Allow-Origin "*"']]]]]]], + [[['if', '($request_filename', '~*', '\\.(ttf|woff)$)'], + [['add_header', 'Access-Control-Allow-Origin', '"*"']]]]]]], [['server'], [['server_name', 'with.complicated.headers'], [['location', '~*', '\\.(?:gif|jpe?g|png)$'], - [['add_header', 'Pragma public'], + [['add_header', 'Pragma', 'public'], ['add_header', - 'Cache-Control \'public, must-revalidate, proxy-revalidate\'' - ' "test,;{}" foo'], + 'Cache-Control', '\'public, must-revalidate, proxy-revalidate\'', + '"test,;{}"', 'foo'], ['blah', '"hello;world"'], - ['try_files', '$uri @rewrites']]]]]]) + ['try_files', '$uri', '@rewrites']]]]]]) + + def test_parse_from_file3(self): + with open(util.get_data_filename('multiline_quotes.conf')) as handle: + parsed = util.filter_comments(load(handle)) + self.assertEqual( + parsed, + [[['http'], + [[['server'], + [['listen', '*:443'], + [['location', '/'], + [['body_filter_by_lua', + '\'ngx.ctx.buffered = (ngx.ctx.buffered or "")' + ' .. string.sub(ngx.arg[1], 1, 1000)\n' + ' ' + 'if ngx.arg[2] then\n' + ' ' + 'ngx.var.resp_body = ngx.ctx.buffered\n' + ' end\'']]]]]]]]) def test_abort_on_parse_failure(self): with open(util.get_data_filename('broken.conf')) as handle: @@ -117,7 +135,7 @@ class TestRawNginxParser(unittest.TestCase): with open(util.get_data_filename('nginx.conf')) as handle: parsed = load(handle) parsed[-1][-1].append(UnspacedList([['server'], - [['listen', ' ', '443 ssl'], + [['listen', ' ', '443', ' ', 'ssl'], ['server_name', ' ', 'localhost'], ['ssl_certificate', ' ', 'cert.pem'], ['ssl_certificate_key', ' ', 'cert.key'], @@ -126,9 +144,9 @@ class TestRawNginxParser(unittest.TestCase): ['ssl_ciphers', ' ', 'HIGH:!aNULL:!MD5'], [['location', ' ', '/'], [['root', ' ', 'html'], - ['index', ' ', 'index.html index.htm']]]]])) + ['index', ' ', 'index.html', ' ', 'index.htm']]]]])) - with tempfile.TemporaryFile() as f: + with tempfile.TemporaryFile(mode='w+t') as f: dump(parsed, f) f.seek(0) parsed_new = load(f) @@ -138,7 +156,7 @@ class TestRawNginxParser(unittest.TestCase): with open(util.get_data_filename('minimalistic_comments.conf')) as handle: parsed = load(handle) - with tempfile.TemporaryFile() as f: + with tempfile.TemporaryFile(mode='w+t') as f: dump(parsed, f) f.seek(0) parsed_new = load(f) @@ -161,10 +179,177 @@ class TestRawNginxParser(unittest.TestCase): parsed = loads('if ($http_accept ~* "webp") { set $webp "true"; }') self.assertEqual(parsed, [ - [['if', '($http_accept ~* "webp")'], - [['set', '$webp "true"']]] + [['if', '($http_accept', '~*', '"webp")'], + [['set', '$webp', '"true"']]] ]) + def test_comment_in_block(self): + parsed = loads("""http { + # server{ + }""") + + self.assertEqual(parsed, [ + [['http'], + [['#', ' server{']]] + ]) + + def test_access_log(self): + # see issue #3798 + parsed = loads('access_log syslog:server=unix:/dev/log,facility=auth,' + 'tag=nginx_post,severity=info custom;') + + self.assertEqual(parsed, [ + ['access_log', + 'syslog:server=unix:/dev/log,facility=auth,tag=nginx_post,severity=info', + 'custom'] + ]) + + def test_add_header(self): + # see issue #3798 + parsed = loads('add_header Cache-Control no-cache,no-store,must-revalidate,max-age=0;') + + self.assertEqual(parsed, [ + ['add_header', 'Cache-Control', 'no-cache,no-store,must-revalidate,max-age=0'] + ]) + + def test_map_then_assignment_in_block(self): + # see issue #3798 + test_str = """http { + map $http_upgrade $connection_upgrade { + default upgrade; + '' close; + "~Opera Mini" 1; + *.example.com 1; + } + one; + }""" + parsed = loads(test_str) + self.assertEqual(parsed, [ + [['http'], [ + [['map', '$http_upgrade', '$connection_upgrade'], [ + ['default', 'upgrade'], + ["''", 'close'], + ['"~Opera Mini"', '1'], + ['*.example.com', '1'] + ]], + ['one'] + ]] + ]) + + def test_variable_name(self): + parsed = loads('try_files /typo3temp/tx_ncstaticfilecache/' + '$host${request_uri}index.html @nocache;') + + self.assertEqual(parsed, [ + ['try_files', + '/typo3temp/tx_ncstaticfilecache/$host${request_uri}index.html', + '@nocache'] + ]) + + def test_weird_blocks(self): + test = r""" + if ($http_user_agent ~ MSIE) { + rewrite ^(.*)$ /msie/$1 break; + } + + if ($http_cookie ~* "id=([^;]+)(?:;|$)") { + set $id $1; + } + + if ($request_method = POST) { + return 405; + } + + if ($request_method) { + return 403; + } + + if ($args ~ post=140){ + rewrite ^ http://example.com/; + } + + location ~ ^/users/(.+\.(?:gif|jpe?g|png))$ { + alias /data/w3/images/$1; + } + """ + parsed = loads(test) + self.assertEqual(parsed, [[['if', '($http_user_agent', '~', 'MSIE)'], + [['rewrite', '^(.*)$', '/msie/$1', 'break']]], + [['if', '($http_cookie', '~*', '"id=([^;]+)(?:;|$)")'], [['set', '$id', '$1']]], + [['if', '($request_method', '=', 'POST)'], [['return', '405']]], + [['if', '($request_method)'], + [['return', '403']]], [['if', '($args', '~', 'post=140)'], + [['rewrite', '^', 'http://example.com/']]], + [['location', '~', '^/users/(.+\\.(?:gif|jpe?g|png))$'], + [['alias', '/data/w3/images/$1']]]] + ) + + def test_edge_cases(self): + # quotes + parsed = loads(r'"hello\""; # blah "heh heh"') + self.assertEqual(parsed, [['"hello\\""'], ['#', ' blah "heh heh"']]) + + # empty var as block + parsed = loads(r"${}") + self.assertEqual(parsed, [[['$'], []]]) + + # if with comment + parsed = loads("""if ($http_cookie ~* "id=([^;]+)(?:;|$)") { # blah ) + }""") + self.assertEqual(parsed, [[['if', '($http_cookie', '~*', '"id=([^;]+)(?:;|$)")'], + [['#', ' blah )']]]]) + + # end paren + test = """ + one"test"; + ("two"); + "test")red; + "test")"blue"; + "test")"three; + (one"test")one; + one"; + one"test; + one"test"one; + """ + parsed = loads(test) + self.assertEqual(parsed, [ + ['one"test"'], + ['("two")'], + ['"test")red'], + ['"test")"blue"'], + ['"test")"three'], + ['(one"test")one'], + ['one"'], + ['one"test'], + ['one"test"one'] + ]) + self.assertRaises(ParseException, loads, r'"test"one;') # fails + self.assertRaises(ParseException, loads, r'"test;') # fails + + # newlines + test = """ + server_name foo.example.com bar.example.com \ + baz.example.com qux.example.com; + server_name foo.example.com bar.example.com + baz.example.com qux.example.com; + """ + parsed = loads(test) + self.assertEqual(parsed, [ + ['server_name', 'foo.example.com', 'bar.example.com', + 'baz.example.com', 'qux.example.com'], + ['server_name', 'foo.example.com', 'bar.example.com', + 'baz.example.com', 'qux.example.com'] + ]) + + # variable weirdness + parsed = loads("directive $var;") + self.assertEqual(parsed, [['directive', '$var']]) + self.assertRaises(ParseException, loads, "server {server_name test.com};") + self.assertRaises(ParseException, loads, "directive ${var};") + self.assertEqual(loads("blag${dfgdfg};"), [['blag${dfgdfg}']]) + self.assertRaises(ParseException, loads, "blag${dfgdf{g};") + + class TestUnspacedList(unittest.TestCase): """Test the UnspacedList data structure""" def setUp(self): @@ -219,18 +404,18 @@ class TestUnspacedList(unittest.TestCase): ['\n ', 'listen', ' ', '127.0.0.1'], ['\n ', 'server_name', ' ', '.example.com'], ['\n ', 'server_name', ' ', 'example.*'], '\n', - ['listen', ' ', '5001 ssl']]) + ['listen', ' ', '5001', ' ', 'ssl']]) x.insert(5, "FROGZ") self.assertEqual(x, [['listen', '69.50.225.155:9000'], ['listen', '127.0.0.1'], ['server_name', '.example.com'], ['server_name', 'example.*'], - ['listen', '5001 ssl'], 'FROGZ']) + ['listen', '5001', 'ssl'], 'FROGZ']) self.assertEqual(x.spaced, [['\n ', 'listen', ' ', '69.50.225.155:9000'], ['\n ', 'listen', ' ', '127.0.0.1'], ['\n ', 'server_name', ' ', '.example.com'], ['\n ', 'server_name', ' ', 'example.*'], '\n', - ['listen', ' ', '5001 ssl'], + ['listen', ' ', '5001', ' ', 'ssl'], 'FROGZ']) def test_rawlists(self): diff --git a/certbot-nginx/certbot_nginx/tests/obj_test.py b/certbot-nginx/certbot_nginx/tests/obj_test.py index b0a2d5ad8..ba136bb78 100644 --- a/certbot-nginx/certbot_nginx/tests/obj_test.py +++ b/certbot-nginx/certbot_nginx/tests/obj_test.py @@ -108,8 +108,8 @@ class VirtualHostTest(unittest.TestCase): from certbot_nginx.obj import Addr raw1 = [ ['listen', '69.50.225.155:9000'], - [['if', '($scheme != "https") '], - [['return', '301 https://$host$request_uri']] + [['if', '($scheme', '!=', '"https") '], + [['return', '301', 'https://$host$request_uri']] ], ['#', ' managed by Certbot'] ] @@ -119,8 +119,8 @@ class VirtualHostTest(unittest.TestCase): set(['localhost']), raw1, []) raw2 = [ ['listen', '69.50.225.155:9000'], - [['if', '($scheme != "https") '], - [['return', '301 https://$host$request_uri']] + [['if', '($scheme', '!=', '"https") '], + [['return', '301', 'https://$host$request_uri']] ] ] self.vhost2 = VirtualHost( @@ -129,7 +129,7 @@ class VirtualHostTest(unittest.TestCase): set(['localhost']), raw2, []) raw3 = [ ['listen', '69.50.225.155:9000'], - ['rewrite', '^(.*)$ $scheme://www.domain.com$1 permanent;'] + ['rewrite', '^(.*)$', '$scheme://www.domain.com$1', 'permanent'] ] self.vhost3 = VirtualHost( "filep", @@ -158,7 +158,7 @@ class VirtualHostTest(unittest.TestCase): def test_str(self): stringified = '\n'.join(['file: filep', 'addrs: localhost', - "names: set(['localhost'])", 'ssl: False', + "names: ['localhost']", 'ssl: False', 'enabled: False']) self.assertEqual(stringified, str(self.vhost1)) @@ -181,7 +181,9 @@ class VirtualHostTest(unittest.TestCase): ['#', ' managed by Certbot'], ['ssl_certificate_key', '/etc/letsencrypt/live/two.functorkitten.xyz/privkey.pem'], ['#', ' managed by Certbot'], - [['if', '($scheme != "https")'], [['return', '301 https://$host$request_uri']]], + [['if', '($scheme', '!=', '"https")'], + [['return', '301', 'https://$host$request_uri']] + ], ['#', ' managed by Certbot'], []] vhost_haystack = VirtualHost( "filp", @@ -195,7 +197,9 @@ class VirtualHostTest(unittest.TestCase): ['#', ' managed by Certbot'], ['ssl_certificate_key', '/etc/letsencrypt/live/two.functorkitten.xyz/privkey.pem'], ['#', ' managed by Certbot'], - [['if', '($scheme != "https")'], [['return', '302 https://$host$request_uri']]], + [['if', '($scheme', '!=', '"https")'], + [['return', '302', 'https://$host$request_uri']] + ], ['#', ' managed by Certbot'], []] vhost_bad_haystack = VirtualHost( "filp", diff --git a/certbot-nginx/certbot_nginx/tests/parser_test.py b/certbot-nginx/certbot_nginx/tests/parser_test.py index 6a3f2f1de..9c2b8656e 100644 --- a/certbot-nginx/certbot_nginx/tests/parser_test.py +++ b/certbot-nginx/certbot_nginx/tests/parser_test.py @@ -52,7 +52,7 @@ class NginxParserTest(util.NginxTest): 'sites-enabled/sslon.com', 'sites-enabled/globalssl.com']]), set(nparser.parsed.keys())) - self.assertEqual([['server_name', 'somename alias another.alias']], + self.assertEqual([['server_name', 'somename', 'alias', 'another.alias']], nparser.parsed[nparser.abs_path('server.conf')]) self.assertEqual([[['server'], [['listen', '69.50.225.155:9000'], ['listen', '127.0.0.1'], @@ -168,16 +168,16 @@ class NginxParserTest(util.NginxTest): [['location', '/'], [['root', 'html'], ['index', 'index.html index.htm']]] ], None) self.assertFalse(nparser.has_ssl_on_directive(mock_vhost)) - mock_vhost.raw = [['listen', '*:80 default_server ssl'], - ['server_name', '*.www.foo.com *.www.example.com'], + mock_vhost.raw = [['listen', '*:80', 'default_server', 'ssl'], + ['server_name', '*.www.foo.com', '*.www.example.com'], ['root', '/home/ubuntu/sites/foo/']] self.assertFalse(nparser.has_ssl_on_directive(mock_vhost)) mock_vhost.raw = [['listen', '80 ssl'], - ['server_name', '*.www.foo.com *.www.example.com']] + ['server_name', '*.www.foo.com', '*.www.example.com']] self.assertFalse(nparser.has_ssl_on_directive(mock_vhost)) mock_vhost.raw = [['listen', '80'], ['ssl', 'on'], - ['server_name', '*.www.foo.com *.www.example.com']] + ['server_name', '*.www.foo.com', '*.www.example.com']] self.assertTrue(nparser.has_ssl_on_directive(mock_vhost)) def test_add_server_directives(self): @@ -309,7 +309,7 @@ class NginxParserTest(util.NginxTest): self.assertFalse(server['ssl']) server = parser._parse_server_raw([ #pylint: disable=protected-access - ['listen', '443 ssl'] + ['listen', '443', 'ssl'] ]) self.assertTrue(server['ssl']) @@ -341,16 +341,21 @@ class NginxParserTest(util.NginxTest): self.assertEqual(nginxparser.UnspacedList(nparser.loc["ssl_options"]), [['ssl_session_cache', 'shared:le_nginx_SSL:1m'], ['ssl_session_timeout', '1440m'], - ['ssl_protocols', 'TLSv1 TLSv1.1 TLSv1.2'], + ['ssl_protocols', 'TLSv1', 'TLSv1.1', 'TLSv1.2'], ['ssl_prefer_server_ciphers', 'on'], - ['ssl_ciphers', '"ECDHE-ECDSA-AES128-GCM-SHA256 ECDHE-ECDSA-'+ - 'AES256-GCM-SHA384 ECDHE-ECDSA-AES128-SHA ECDHE-ECDSA-AES256'+ - '-SHA ECDHE-ECDSA-AES128-SHA256 ECDHE-ECDSA-AES256-SHA384'+ - ' ECDHE-RSA-AES128-GCM-SHA256 ECDHE-RSA-AES256-GCM-SHA384'+ - ' ECDHE-RSA-AES128-SHA ECDHE-RSA-AES128-SHA256 ECDHE-RSA-'+ - 'AES256-SHA384 DHE-RSA-AES128-GCM-SHA256 DHE-RSA-AES256-GCM'+ - '-SHA384 DHE-RSA-AES128-SHA DHE-RSA-AES256-SHA DHE-RSA-'+ - 'AES128-SHA256 DHE-RSA-AES256-SHA256 EDH-RSA-DES-CBC3-SHA"'] + ['ssl_ciphers', '"ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-'+ + 'RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES128-GCM-SHA256:'+ + 'ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-'+ + 'SHA384:ECDHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-'+ + 'SHA256:DHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES128-'+ + 'SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA:'+ + 'ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-'+ + 'AES256-SHA384:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA:'+ + 'DHE-RSA-AES128-SHA256:DHE-RSA-AES128-SHA:DHE-RSA-AES256-'+ + 'SHA256:DHE-RSA-AES256-SHA:ECDHE-ECDSA-DES-CBC3-SHA:ECDHE-'+ + 'RSA-DES-CBC3-SHA:EDH-RSA-DES-CBC3-SHA:AES128-GCM-SHA256:'+ + 'AES256-GCM-SHA384:AES128-SHA256:AES256-SHA256:AES128-SHA:'+ + 'AES256-SHA:DES-CBC3-SHA:!DSS"'] ]) if __name__ == "__main__": diff --git a/certbot-nginx/certbot_nginx/tests/testdata/etc_nginx/multiline_quotes.conf b/certbot-nginx/certbot_nginx/tests/testdata/etc_nginx/multiline_quotes.conf new file mode 100644 index 000000000..74cd84bcd --- /dev/null +++ b/certbot-nginx/certbot_nginx/tests/testdata/etc_nginx/multiline_quotes.conf @@ -0,0 +1,16 @@ +# Test nginx configuration file with multiline quoted strings. +# Good example of usage for multilined quoted values is when +# using Openresty's Lua directives and you wish to keep the +# inline Lua code readable. +http { + server { + listen *:443; # because there should be no other port open. + + location / { + body_filter_by_lua 'ngx.ctx.buffered = (ngx.ctx.buffered or "") .. string.sub(ngx.arg[1], 1, 1000) + if ngx.arg[2] then + ngx.var.resp_body = ngx.ctx.buffered + end'; + } + } +} diff --git a/certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py b/certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py index e7dacb400..7a2de44a2 100644 --- a/certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py +++ b/certbot-nginx/certbot_nginx/tests/tls_sni_01_test.py @@ -3,6 +3,7 @@ import unittest import shutil import mock +import six from acme import challenges @@ -23,25 +24,25 @@ class TlsSniPerformTest(util.NginxTest): achalls = [ achallenges.KeyAuthorizationAnnotatedChallenge( challb=acme_util.chall_to_challb( - challenges.TLSSNI01(token="kNdwjwOeX0I_A8DXt9Msmg"), "pending"), + challenges.TLSSNI01(token=b"kNdwjwOeX0I_A8DXt9Msmg"), "pending"), domain="www.example.com", account_key=account_key), achallenges.KeyAuthorizationAnnotatedChallenge( challb=acme_util.chall_to_challb( challenges.TLSSNI01( - token="\xba\xa9\xda?= self.red_level: - return ''.join((util.ANSI_SGR_RED, out, util.ANSI_SGR_RESET)) - else: - return out diff --git a/certbot/configuration.py b/certbot/configuration.py index d25378922..30c6f0437 100644 --- a/certbot/configuration.py +++ b/certbot/configuration.py @@ -42,7 +42,7 @@ class NamespaceConfig(object): """ def __init__(self, namespace): - self.namespace = namespace + object.__setattr__(self, 'namespace', namespace) self.namespace.config_dir = os.path.abspath(self.namespace.config_dir) self.namespace.work_dir = os.path.abspath(self.namespace.work_dir) @@ -54,6 +54,9 @@ class NamespaceConfig(object): def __getattr__(self, name): return getattr(self.namespace, name) + def __setattr__(self, name, value): + setattr(self.namespace, name, value) + @property def server_path(self): """File path based on ``server``.""" diff --git a/certbot/constants.py b/certbot/constants.py index b286ca26a..382b0afb3 100644 --- a/certbot/constants.py +++ b/certbot/constants.py @@ -18,6 +18,7 @@ CLI_DEFAULTS = dict( os.path.join(os.environ.get("XDG_CONFIG_HOME", "~/.config"), "letsencrypt", "cli.ini"), ], + dry_run=False, verbose_count=-int(logging.INFO / 10), server="https://acme-v01.api.letsencrypt.org/directory", rsa_key_size=2048, @@ -32,6 +33,7 @@ CLI_DEFAULTS = dict( auth_cert_path="./cert.pem", auth_chain_path="./chain.pem", strict_permissions=False, + debug_challenges=False, ) STAGING_URI = "https://acme-staging.api.letsencrypt.org/directory" diff --git a/certbot/crypto_util.py b/certbot/crypto_util.py index 65e3de345..1ad76d503 100644 --- a/certbot/crypto_util.py +++ b/certbot/crypto_util.py @@ -53,12 +53,15 @@ def init_save_key(key_size, key_dir, keyname="key-certbot.pem"): # Save file util.make_or_verify_dir(key_dir, 0o700, os.geteuid(), config.strict_permissions) - key_f, key_path = util.unique_file( - os.path.join(key_dir, keyname), 0o600, "wb") - with key_f: - key_f.write(key_pem) - - logger.info("Generating key (%d bits): %s", key_size, key_path) + if config.dry_run: + key_path = None + logger.info("Generating key (%d bits), not saving to file", key_size) + else: + key_f, key_path = util.unique_file( + os.path.join(key_dir, keyname), 0o600, "wb") + with key_f: + key_f.write(key_pem) + logger.info("Generating key (%d bits): %s", key_size, key_path) return util.Key(key_path, key_pem) @@ -85,12 +88,15 @@ def init_save_csr(privkey, names, path, csrname="csr-certbot.pem"): # Save CSR util.make_or_verify_dir(path, 0o755, os.geteuid(), config.strict_permissions) - csr_f, csr_filename = util.unique_file( - os.path.join(path, csrname), 0o644, "wb") - csr_f.write(csr_pem) - csr_f.close() - - logger.info("Creating CSR: %s", csr_filename) + if config.dry_run: + csr_filename = None + logger.info("Creating CSR: not saving to file") + else: + csr_f, csr_filename = util.unique_file( + os.path.join(path, csrname), 0o644, "wb") + with csr_f: + csr_f.write(csr_pem) + logger.info("Creating CSR: %s", csr_filename) return util.CSR(csr_filename, csr_der, "der") diff --git a/certbot/display/completer.py b/certbot/display/completer.py index 37564954a..08b55fdea 100644 --- a/certbot/display/completer.py +++ b/certbot/display/completer.py @@ -4,7 +4,7 @@ import glob try: import readline except ImportError: - import certbot.display.dummy_readline as readline + import certbot.display.dummy_readline as readline # type: ignore class Completer(object): diff --git a/certbot/hooks.py b/certbot/hooks.py index ada3d3aaa..75d7a3b20 100644 --- a/certbot/hooks.py +++ b/certbot/hooks.py @@ -13,12 +13,14 @@ from certbot.plugins import util as plug_util logger = logging.getLogger(__name__) + def validate_hooks(config): """Check hook commands are executable.""" validate_hook(config.pre_hook, "pre") validate_hook(config.post_hook, "post") validate_hook(config.renew_hook, "renew") + def _prog(shell_cmd): """Extract the program run by a shell command. @@ -52,6 +54,7 @@ def validate_hook(shell_cmd, hook_name): raise errors.HookCommandNotFound(msg) + def pre_hook(config): "Run pre-hook if it's defined and hasn't been run." cmd = config.pre_hook @@ -62,7 +65,7 @@ def pre_hook(config): elif cmd: logger.info("Pre-hook command already run, skipping: %s", cmd) -pre_hook.already = set() +pre_hook.already = set() # type: ignore def post_hook(config): @@ -82,7 +85,8 @@ def post_hook(config): logger.info("Running post-hook command: %s", cmd) _run_hook(cmd) -post_hook.eventually = [] +post_hook.eventually = [] # type: ignore + def run_saved_post_hooks(): """Run any post hooks that were saved up in the course of the 'renew' verb""" diff --git a/certbot/interfaces.py b/certbot/interfaces.py index a2767121b..213992993 100644 --- a/certbot/interfaces.py +++ b/certbot/interfaces.py @@ -99,7 +99,7 @@ class IPluginFactory(zope.interface.Interface): class IPlugin(zope.interface.Interface): """Certbot plugin.""" - def prepare(): + def prepare(): # type: ignore """Prepare the plugin. Finish up any additional initialization. @@ -118,7 +118,7 @@ class IPlugin(zope.interface.Interface): """ - def more_info(): + def more_info(): # type: ignore """Human-readable string to help the user. Should describe the steps taken and any relevant info to help the user @@ -251,7 +251,7 @@ class IInstaller(IPlugin): """ - def get_all_names(): + def get_all_names(): # type: ignore """Returns all names that may be authenticated. :rtype: `collections.Iterable` of `str` @@ -288,7 +288,7 @@ class IInstaller(IPlugin): """ - def supported_enhancements(): + def supported_enhancements(): # type: ignore """Returns a `collections.Iterable` of supported enhancements. :returns: supported enhancements which should be a subset of @@ -326,7 +326,7 @@ class IInstaller(IPlugin): """ - def recovery_routine(): + def recovery_routine(): # type: ignore """Revert configuration to most recent finalized checkpoint. Remove all changes (temporary and permanent) that have not been @@ -337,21 +337,21 @@ class IInstaller(IPlugin): """ - def view_config_changes(): + def view_config_changes(): # type: ignore """Display all of the LE config changes. :raises .PluginError: when config changes cannot be parsed """ - def config_test(): + def config_test(): # type: ignore """Make sure the configuration is valid. :raises .MisconfigurationError: when the config is not in a usable state """ - def restart(): + def restart(): # type: ignore """Restart or refresh the server content. :raises .PluginError: when server cannot be restarted diff --git a/certbot/log.py b/certbot/log.py new file mode 100644 index 000000000..92b35ed51 --- /dev/null +++ b/certbot/log.py @@ -0,0 +1,180 @@ +"""Logging utilities for Certbot.""" +from __future__ import print_function +import functools +import logging +import os +import sys +import time +import traceback + +from acme import messages + +from certbot import cli +from certbot import constants +from certbot import errors +from certbot import util + +# Logging format +CLI_FMT = "%(message)s" +FILE_FMT = "%(asctime)s:%(levelname)s:%(name)s:%(message)s" + + +logger = logging.getLogger(__name__) + + +def pre_arg_parse_setup(): + """Ensures fatal exceptions are logged and reported to the user.""" + sys.excepthook = functools.partial(except_hook, config=None) + + +def post_arg_parse_setup(config): + """Setup logging after command line arguments are parsed. + + :param certbot.interface.IConfig config: Configuration object + + """ + file_handler, file_path = setup_log_file_handler( + config, 'letsencrypt.log', FILE_FMT) + + if config.quiet: + level = constants.QUIET_LOGGING_LEVEL + else: + level = -config.verbose_count * 10 + stderr_handler = ColoredStreamHandler() + stderr_handler.setFormatter(logging.Formatter(CLI_FMT)) + stderr_handler.setLevel(level) + + root_logger = logging.getLogger() + root_logger.setLevel(logging.DEBUG) # send all records to handlers + root_logger.addHandler(stderr_handler) + root_logger.addHandler(file_handler) + + logger.debug('Root logging level set at %d', level) + logger.info('Saving debug log to %s', file_path) + + sys.excepthook = functools.partial(except_hook, config=config) + + +def setup_log_file_handler(config, logfile, fmt): + """Setup file debug logging. + + :param certbot.interface.IConfig config: Configuration object + :param str logfile: basename for the log file + :param str fmt: logging format string + + :returns: file handler and absolute path to the log file + :rtype: tuple + + """ + # TODO: logs might contain sensitive data such as contents of the + # private key! #525 + util.make_or_verify_core_dir( + config.logs_dir, 0o700, os.geteuid(), config.strict_permissions) + log_file_path = os.path.join(config.logs_dir, logfile) + try: + handler = logging.handlers.RotatingFileHandler( + log_file_path, maxBytes=2 ** 20, backupCount=1000) + except IOError as error: + raise errors.Error(util.PERM_ERR_FMT.format(error)) + # rotate on each invocation, rollover only possible when maxBytes + # is nonzero and backupCount is nonzero, so we set maxBytes as big + # as possible not to overrun in single CLI invocation (1MB). + handler.doRollover() # TODO: creates empty letsencrypt.log.1 file + handler.setLevel(logging.DEBUG) + handler_formatter = logging.Formatter(fmt=fmt) + handler_formatter.converter = time.gmtime # don't use localtime + handler.setFormatter(handler_formatter) + return handler, log_file_path + + +class ColoredStreamHandler(logging.StreamHandler): + """Sends colored logging output to a stream. + + If the specified stream is not a tty, the class works like the + standard logging.StreamHandler. Default red_level is logging.WARNING. + + :ivar bool colored: True if output should be colored + :ivar bool red_level: The level at which to output + + """ + + def __init__(self, stream=None): + if sys.version_info < (2, 7): + # pragma: no cover + # pylint: disable=non-parent-init-called + logging.StreamHandler.__init__(self, stream) + else: + super(ColoredStreamHandler, self).__init__(stream) + self.colored = (sys.stderr.isatty() if stream is None else + stream.isatty()) + self.red_level = logging.WARNING + + def format(self, record): + """Formats the string representation of record. + + :param logging.LogRecord record: Record to be formatted + + :returns: Formatted, string representation of record + :rtype: str + + """ + out = (logging.StreamHandler.format(self, record) + if sys.version_info < (2, 7) + else super(ColoredStreamHandler, self).format(record)) + if self.colored and record.levelno >= self.red_level: + return ''.join((util.ANSI_SGR_RED, out, util.ANSI_SGR_RESET)) + else: + return out + + +def except_hook(exc_type, exc_value, trace, config): + """Logs exceptions and reports them to the user. + + Config is used to determine how to display exceptions to the user. In + general, if config.debug is True, then the full exception and traceback is + shown to the user, otherwise it is suppressed. If config itself is None, + then the traceback and exception is attempted to be written to a logfile. + If this is successful, the traceback is suppressed, otherwise it is shown + to the user. sys.exit is always called with a nonzero status. + + """ + tb_str = "".join(traceback.format_exception(exc_type, exc_value, trace)) + logger.debug("Exiting abnormally:%s%s", os.linesep, tb_str) + + if issubclass(exc_type, Exception) and (config is None or not config.debug): + if config is None: + logfile = "certbot.log" + try: + with open(logfile, "w") as logfd: + traceback.print_exception( + exc_type, exc_value, trace, file=logfd) + assert "--debug" not in sys.argv # config is None if this explodes + except: # pylint: disable=bare-except + sys.exit(tb_str) + if "--debug" in sys.argv: + sys.exit(tb_str) + + if issubclass(exc_type, errors.Error): + sys.exit(exc_value) + else: + # Here we're passing a client or ACME error out to the client at the shell + # Tell the user a bit about what happened, without overwhelming + # them with a full traceback + err = traceback.format_exception_only(exc_type, exc_value)[0] + # Typical error from the ACME module: + # acme.messages.Error: urn:ietf:params:acme:error:malformed :: The + # request message was malformed :: Error creating new registration + # :: Validation of contact mailto:none@longrandomstring.biz failed: + # Server failure at resolver + if (messages.is_acme_error(err) and ":: " in err and + config.verbose_count <= cli.flag_default("verbose_count")): + # prune ACME error code, we have a human description + _code, _sep, err = err.partition(":: ") + msg = "An unexpected error occurred:\n" + err + "Please see the " + if config is None: + msg += "logfile '{0}' for more details.".format(logfile) + else: + msg += "logfiles in {0} for more details.".format(config.logs_dir) + sys.exit(msg) + else: + sys.exit(tb_str) diff --git a/certbot/main.py b/certbot/main.py index 6b6ff06b2..ee178aa50 100644 --- a/certbot/main.py +++ b/certbot/main.py @@ -1,47 +1,37 @@ """Certbot main entry point.""" from __future__ import print_function -import atexit -import functools import logging.handlers import os import sys -import time -import traceback import zope.component from acme import jose -from acme import messages from acme import errors as acme_errors import certbot from certbot import account from certbot import cert_manager -from certbot import client from certbot import cli -from certbot import crypto_util -from certbot import colored_logging +from certbot import client from certbot import configuration from certbot import constants +from certbot import crypto_util from certbot import eff from certbot import errors from certbot import hooks from certbot import interfaces -from certbot import util -from certbot import reporter +from certbot import log from certbot import renewal +from certbot import reporter +from certbot import util from certbot.display import util as display_util, ops as display_ops from certbot.plugins import disco as plugins_disco from certbot.plugins import selection as plug_sel -_PERM_ERR_FMT = os.linesep.join(( - "The following error was encountered:", "{0}", - "If running as non-root, set --config-dir, " - "--work-dir, and --logs-dir to writeable paths.")) - USER_CANCELLED = ("User chose to cancel the operation and may " "reinvoke the client.") @@ -363,7 +353,7 @@ def _determine_account(config): acc = accounts[0] else: # no account registered yet if config.email is None and not config.register_unsafely_without_email: - config.namespace.email = display_ops.get_email() + config.email = display_ops.get_email() def _tos_cb(regr): if config.tos: @@ -386,7 +376,7 @@ def _determine_account(config): raise errors.Error( "Unable to register an account with ACME server") - config.namespace.account = acc.id + config.account = acc.id return acc, acme @@ -463,7 +453,7 @@ def register(config, unused_plugins): return ("--register-unsafely-without-email provided, however, a " "new e-mail address must\ncurrently be provided when " "updating a registration.") - config.namespace.email = display_ops.get_email(optional=False) + config.email = display_ops.get_email(optional=False) acc, acme = _determine_account(config) acme_client = client.Client(config, acc, None, None, acme=acme) @@ -474,15 +464,16 @@ def register(config, unused_plugins): eff.handle_subscription(config) add_msg("Your e-mail address was updated to {0}.".format(config.email)) -def _install_cert(config, le_client, domains, lineage=None): +def _install_cert(config, le_client, domains, lineage=None, enhance=True): path_provider = lineage if lineage else config assert path_provider.cert_path is not None le_client.deploy_certificate(domains, path_provider.key_path, path_provider.cert_path, path_provider.chain_path, path_provider.fullchain_path) - le_client.enhance_config(domains, path_provider.chain_path) + if enhance: + le_client.enhance_config(domains, path_provider.chain_path) -def install(config, plugins): +def install(config, plugins, lineage=None, enhance=True): """Install a previously obtained cert in a server.""" # XXX: Update for renewer/RenewableCert # FIXME: be consistent about whether errors are raised or returned from @@ -491,11 +482,11 @@ def install(config, plugins): try: installer, _ = plug_sel.choose_configurator_plugins(config, plugins, "install") except errors.PluginSelectionError as e: - return e.message + return str(e) domains, _ = _find_domains_or_certname(config, installer) le_client = _init_le_client(config, authenticator=None, installer=installer) - _install_cert(config, le_client, domains) + _install_cert(config, le_client, domains, lineage, enhance) def plugins_cmd(config, plugins): # TODO: Use IDisplay rather than print @@ -569,7 +560,7 @@ def certificates(config, unused_plugins): def revoke(config, unused_plugins): # TODO: coop with renewal config """Revoke a previously obtained certificate.""" # For user-agent construction - config.namespace.installer = config.namespace.authenticator = "None" + config.installer = config.authenticator = "None" if config.key_path is not None: # revocation by cert key logger.debug("Revoking %s using cert key %s", config.cert_path[0], config.key_path[0]) @@ -710,138 +701,11 @@ def renew(config, unused_plugins): hooks.run_saved_post_hooks() -def setup_log_file_handler(config, logfile, fmt): - """Setup file debug logging.""" - log_file_path = os.path.join(config.logs_dir, logfile) - try: - handler = logging.handlers.RotatingFileHandler( - log_file_path, maxBytes=2 ** 20, backupCount=1000) - except IOError as error: - raise errors.Error(_PERM_ERR_FMT.format(error)) - # rotate on each invocation, rollover only possible when maxBytes - # is nonzero and backupCount is nonzero, so we set maxBytes as big - # as possible not to overrun in single CLI invocation (1MB). - handler.doRollover() # TODO: creates empty letsencrypt.log.1 file - handler.setLevel(logging.DEBUG) - handler_formatter = logging.Formatter(fmt=fmt) - handler_formatter.converter = time.gmtime # don't use localtime - handler.setFormatter(handler_formatter) - return handler, log_file_path - - -def _cli_log_handler(level, fmt): - handler = colored_logging.StreamHandler() - handler.setFormatter(logging.Formatter(fmt)) - handler.setLevel(level) - return handler - - -def setup_logging(config): - """Sets up logging to logfiles and the terminal. - - :param certbot.interface.IConfig config: Configuration object - - """ - cli_fmt = "%(message)s" - file_fmt = "%(asctime)s:%(levelname)s:%(name)s:%(message)s" - logfile = "letsencrypt.log" - if config.quiet: - level = constants.QUIET_LOGGING_LEVEL - else: - level = -config.verbose_count * 10 - file_handler, log_file_path = setup_log_file_handler( - config, logfile=logfile, fmt=file_fmt) - cli_handler = _cli_log_handler(level, cli_fmt) - - # TODO: use fileConfig? - - root_logger = logging.getLogger() - root_logger.setLevel(logging.DEBUG) # send all records to handlers - root_logger.addHandler(cli_handler) - root_logger.addHandler(file_handler) - - logger.debug("Root logging level set at %d", level) - logger.info("Saving debug log to %s", log_file_path) - - -def _handle_exception(exc_type, exc_value, trace, config): - """Logs exceptions and reports them to the user. - - Config is used to determine how to display exceptions to the user. In - general, if config.debug is True, then the full exception and traceback is - shown to the user, otherwise it is suppressed. If config itself is None, - then the traceback and exception is attempted to be written to a logfile. - If this is successful, the traceback is suppressed, otherwise it is shown - to the user. sys.exit is always called with a nonzero status. - - """ - tb_str = "".join(traceback.format_exception(exc_type, exc_value, trace)) - logger.debug("Exiting abnormally:%s%s", os.linesep, tb_str) - - if issubclass(exc_type, Exception) and (config is None or not config.debug): - if config is None: - logfile = "certbot.log" - try: - with open(logfile, "w") as logfd: - traceback.print_exception( - exc_type, exc_value, trace, file=logfd) - assert "--debug" not in sys.argv # config is None if this explodes - except: # pylint: disable=bare-except - sys.exit(tb_str) - if "--debug" in sys.argv: - sys.exit(tb_str) - - if issubclass(exc_type, errors.Error): - sys.exit(exc_value) - else: - # Here we're passing a client or ACME error out to the client at the shell - # Tell the user a bit about what happened, without overwhelming - # them with a full traceback - err = traceback.format_exception_only(exc_type, exc_value)[0] - # Typical error from the ACME module: - # acme.messages.Error: urn:ietf:params:acme:error:malformed :: The - # request message was malformed :: Error creating new registration - # :: Validation of contact mailto:none@longrandomstring.biz failed: - # Server failure at resolver - if (messages.is_acme_error(err) and ":: " in err and - config.verbose_count <= cli.flag_default("verbose_count")): - # prune ACME error code, we have a human description - _code, _sep, err = err.partition(":: ") - msg = "An unexpected error occurred:\n" + err + "Please see the " - if config is None: - msg += "logfile '{0}' for more details.".format(logfile) - else: - msg += "logfiles in {0} for more details.".format(config.logs_dir) - sys.exit(msg) - else: - sys.exit(tb_str) - - -def make_or_verify_core_dir(directory, mode, uid, strict): - """Make sure directory exists with proper permissions. - - :param str directory: Path to a directory. - :param int mode: Directory mode. - :param int uid: Directory owner. - :param bool strict: require directory to be owned by current user - - :raises .errors.Error: if the directory cannot be made or verified - - """ - try: - util.make_or_verify_dir(directory, mode, uid, strict) - except OSError as error: - raise errors.Error(_PERM_ERR_FMT.format(error)) - def make_or_verify_needed_dirs(config): - """Create or verify existence of config, work, or logs directories""" - make_or_verify_core_dir(config.config_dir, constants.CONFIG_DIRS_MODE, + """Create or verify existence of config and work directories""" + util.make_or_verify_core_dir(config.config_dir, constants.CONFIG_DIRS_MODE, os.geteuid(), config.strict_permissions) - make_or_verify_core_dir(config.work_dir, constants.CONFIG_DIRS_MODE, - os.geteuid(), config.strict_permissions) - # TODO: logs might contain sensitive data such as contents of the - # private key! #525 - make_or_verify_core_dir(config.logs_dir, 0o700, + util.make_or_verify_core_dir(config.work_dir, constants.CONFIG_DIRS_MODE, os.geteuid(), config.strict_permissions) @@ -874,7 +738,7 @@ def _post_logging_setup(config, plugins, cli_args): def main(cli_args=sys.argv[1:]): """Command line argument parsing and main script execution.""" - sys.excepthook = functools.partial(_handle_exception, config=None) + log.pre_arg_parse_setup() plugins = plugins_disco.PluginsRegistry.find_all() # note: arg parser internally handles --help (and exits afterwards) @@ -884,20 +748,16 @@ def main(cli_args=sys.argv[1:]): make_or_verify_needed_dirs(config) - # Setup logging ASAP, otherwise "No handlers could be found for - # logger ..." TODO: this should be done before plugins discovery - setup_logging(config) + log.post_arg_parse_setup(config) _post_logging_setup(config, plugins, cli_args) - sys.excepthook = functools.partial(_handle_exception, config=config) - set_displayer(config) # Reporter report = reporter.Reporter(config) zope.component.provideUtility(report) - atexit.register(report.atexit_print_messages) + util.atexit_register(report.print_messages) return config.func(config, plugins) diff --git a/certbot/plugins/disco.py b/certbot/plugins/disco.py index e567422e2..a17f8d7b3 100644 --- a/certbot/plugins/disco.py +++ b/certbot/plugins/disco.py @@ -27,7 +27,7 @@ class PluginEntryPoint(object): """Distributions for which prefix will be omitted.""" # this object is mutable, don't allow it to be hashed! - __hash__ = None + __hash__ = None # type: ignore def __init__(self, entry_point): self.name = self.entry_point_to_plugin_name(entry_point) diff --git a/certbot/plugins/selection.py b/certbot/plugins/selection.py index 81387c435..d138001e6 100644 --- a/certbot/plugins/selection.py +++ b/certbot/plugins/selection.py @@ -137,9 +137,8 @@ noninstaller_plugins = ["webroot", "manual", "standalone"] def record_chosen_plugins(config, plugins, auth, inst): "Update the config entries to reflect the plugins we actually selected." - cn = config.namespace - cn.authenticator = plugins.find_init(auth).name if auth else "None" - cn.installer = plugins.find_init(inst).name if inst else "None" + config.authenticator = plugins.find_init(auth).name if auth else "None" + config.installer = plugins.find_init(inst).name if inst else "None" def choose_configurator_plugins(config, plugins, verb): diff --git a/certbot/renewal.py b/certbot/renewal.py index a0cc872a0..6eb171763 100644 --- a/certbot/renewal.py +++ b/certbot/renewal.py @@ -103,13 +103,13 @@ def _restore_webroot_config(config, renewalparams): """ if "webroot_map" in renewalparams: if not cli.set_by_cli("webroot_map"): - config.namespace.webroot_map = renewalparams["webroot_map"] + config.webroot_map = renewalparams["webroot_map"] elif "webroot_path" in renewalparams: logger.debug("Ancient renewal conf file without webroot-map, restoring webroot-path") wp = renewalparams["webroot_path"] if isinstance(wp, str): # prior to 0.1.0, webroot_path was a string wp = [wp] - config.namespace.webroot_path = wp + config.webroot_path = wp def _restore_plugin_configs(config, renewalparams): @@ -148,10 +148,10 @@ def _restore_plugin_configs(config, renewalparams): if config_value in ("None", "True", "False"): # bool("False") == True # pylint: disable=eval-used - setattr(config.namespace, config_item, eval(config_value)) + setattr(config, config_item, eval(config_value)) else: cast = cli.argparse_type(config_item) - setattr(config.namespace, config_item, cast(config_value)) + setattr(config, config_item, cast(config_value)) def restore_required_config_elements(config, renewalparams): @@ -172,7 +172,7 @@ def restore_required_config_elements(config, renewalparams): for item_name, restore_func in required_items: if item_name in renewalparams and not cli.set_by_cli(item_name): value = restore_func(item_name, renewalparams[item_name]) - setattr(config.namespace, item_name, value) + setattr(config, item_name, value) def _restore_pref_challs(unused_name, value): diff --git a/certbot/reporter.py b/certbot/reporter.py index 118b13166..e0063d8e5 100644 --- a/certbot/reporter.py +++ b/certbot/reporter.py @@ -3,11 +3,10 @@ from __future__ import print_function import collections import logging -import os import sys import textwrap -from six.moves import queue # pylint: disable=import-error +from six.moves import queue # type: ignore # pylint: disable=import-error import zope.interface from certbot import interfaces @@ -16,11 +15,6 @@ from certbot import util logger = logging.getLogger(__name__) -# Store the pid of the process that first imported this module so that -# atexit_print_messages side-effects such as error reporting can be limited to -# this process and not any fork()'d children. -INITIAL_PID = os.getpid() - @zope.interface.implementer(interfaces.IReporter) class Reporter(object): @@ -60,19 +54,6 @@ class Reporter(object): self.messages.put(self._msg_type(priority, msg, on_crash)) logger.debug("Reporting to user: %s", msg) - def atexit_print_messages(self, pid=None): - """Function to be registered with atexit to print messages. - - :param int pid: Process ID - - """ - if pid is None: - pid = INITIAL_PID - # This ensures that messages are only printed from the process that - # created the Reporter. - if pid == os.getpid(): - self.print_messages() - def print_messages(self): """Prints messages to the user and clears the message queue. diff --git a/certbot/storage.py b/certbot/storage.py index dacc73c4c..34dc57884 100644 --- a/certbot/storage.py +++ b/certbot/storage.py @@ -19,6 +19,9 @@ from certbot import errors from certbot import error_handler from certbot import util +from certbot.plugins import common as plugins_common +from certbot.plugins import disco as plugins_disco + logger = logging.getLogger(__name__) ALL_FOUR = ("cert", "privkey", "chain", "fullchain") @@ -31,7 +34,9 @@ def renewal_conf_files(config): return glob.glob(os.path.join(config.renewal_configs_dir, "*.conf")) def renewal_file_for_certname(config, certname): - """Return /path/to/certname.conf in the renewal conf directory""" + """Return /path/to/certname.conf in the renewal conf directory + :raises .CertStorageError: if file is missing + """ path = os.path.join(config.renewal_configs_dir, "{0}.conf".format(certname)) if not os.path.exists(path): raise errors.CertStorageError("No certificate found with name {0} (expected " @@ -127,6 +132,8 @@ def rename_renewal_config(prev_name, new_name, cli_config): except OSError: raise errors.ConfigurationError("Please specify a valid filename " "for the new certificate name.") + else: + return new_filename def update_configuration(lineagename, archive_dir, target, cli_config): @@ -143,19 +150,25 @@ def update_configuration(lineagename, archive_dir, target, cli_config): """ config_filename = renewal_filename_for_lineagename(cli_config, lineagename) - temp_filename = config_filename + ".new" + + def _save_renewal_values(unused_config, temp_filename): + # Save only the config items that are relevant to renewal + values = relevant_values(vars(cli_config.namespace)) + write_renewal_config(config_filename, temp_filename, archive_dir, target, values) + _modify_config_with_tempfile(config_filename, _save_renewal_values) + + return configobj.ConfigObj(config_filename) + +def _modify_config_with_tempfile(filename, function): + temp_filename = filename + ".new" # If an existing tempfile exists, delete it if os.path.exists(temp_filename): os.unlink(temp_filename) - # Save only the config items that are relevant to renewal - values = relevant_values(vars(cli_config.namespace)) - write_renewal_config(config_filename, temp_filename, archive_dir, target, values) - os.rename(temp_filename, config_filename) - - return configobj.ConfigObj(config_filename) - + config = configobj.ConfigObj(filename) + function(config, temp_filename) + os.rename(temp_filename, filename) def get_link_target(link): """Get an absolute path to the target of link. @@ -179,13 +192,12 @@ def _relevant(option): :rtype: bool """ - # The list() here produces a list of the plugin names as strings. from certbot import renewal - from certbot.plugins import disco as plugins_disco - plugins = list(plugins_disco.PluginsRegistry.find_all()) + plugins = plugins_disco.PluginsRegistry.find_all() + namespaces = [plugins_common.dest_namespace(plugin) for plugin in plugins] return (option in renewal.CONFIG_ITEMS or - any(option.startswith(x + "_") for x in plugins)) + any(option.startswith(namespace) for namespace in namespaces)) def relevant_values(all_values): @@ -241,6 +253,7 @@ def delete_files(config, certname): """Delete all files related to the certificate. If some files are not found, ignore them and continue. + :raises .CertStorageError: if lineage is missing """ renewal_filename = renewal_file_for_certname(config, certname) # file exists @@ -302,6 +315,79 @@ def delete_files(config, certname): except OSError: logger.debug("Unable to remove %s", archive_path) +def duplicate_lineage(config, certname, new_certname): + """Create a duplicate of certname with name new_certname + + :raises .CertStorageError: for storage errors + :raises .ConfigurationError: for cli and renewal configuration errors + :raises IOError: for filename errors + :raises OSError: for OS errors + """ + + # copy renewal config file + prev_filename = renewal_filename_for_lineagename(config, certname) + new_filename = renewal_filename_for_lineagename(config, new_certname) + if os.path.exists(new_filename): + raise errors.ConfigurationError("The new certificate name " + "is already in use.") + try: + shutil.copy2(prev_filename, new_filename) + except (OSError, IOError): + raise errors.ConfigurationError("Please specify a valid filename " + "for the new certificate name.") + logger.debug("Copied %s to %s", prev_filename, new_filename) + + # load config file + try: + renewal_config = configobj.ConfigObj(new_filename) + except configobj.ConfigObjError: + # config is corrupted + logger.warning("Could not parse %s. Only the certificate has been renamed.", + new_filename) + raise errors.CertStorageError( + "error parsing {0}".format(new_filename)) + + def copy_to_new_dir(prev_dir): + """Replace certname with new_certname in prev_dir""" + new_dir = prev_dir.replace(certname, new_certname) + # make dir iff it doesn't exist + shutil.copytree(prev_dir, new_dir, symlinks=True) + logger.debug("Copied %s to %s", prev_dir, new_dir) + return new_dir + + # archive dir + prev_archive_dir = _full_archive_path(renewal_config, config, certname) + new_archive_dir = prev_archive_dir + if not certname in prev_archive_dir: + raise errors.CertStorageError("Archive directory does not conform to defaults: " + "{0} not in {1}", certname, prev_archive_dir) + else: + new_archive_dir = copy_to_new_dir(prev_archive_dir) + + # live dir + # if things aren't in their default places, don't try to change things. + prev_live_dir = _full_live_path(config, certname) + prev_links = dict((kind, renewal_config.get(kind)) for kind in ALL_FOUR) + if (certname not in prev_live_dir or + len(set(os.path.dirname(renewal_config.get(kind)) for kind in ALL_FOUR)) != 1): + raise errors.CertStorageError("Live directory does not conform to defaults.") + else: + copy_to_new_dir(prev_live_dir) + new_links = dict((k, prev_links[k].replace(certname, new_certname)) for k in prev_links) + + # Update renewal config file + def _update_and_write(renewal_config, temp_filename): + renewal_config["archive_dir"] = new_archive_dir + renewal_config["version"] = certbot.__version__ + for kind in ALL_FOUR: + renewal_config[kind] = new_links[kind] + with open(temp_filename, "wb") as f: + renewal_config.write(outfile=f) + _modify_config_with_tempfile(new_filename, _update_and_write) + + # Update symlinks + return RenewableCert(new_filename, config, update_symlinks=True) + class RenewableCert(object): # pylint: disable=too-many-instance-attributes,too-many-public-methods diff --git a/certbot/tests/account_test.py b/certbot/tests/account_test.py index 7d335b09b..11d14132d 100644 --- a/certbot/tests/account_test.py +++ b/certbot/tests/account_test.py @@ -4,7 +4,6 @@ import json import os import shutil import stat -import tempfile import unittest import mock @@ -17,6 +16,8 @@ from certbot import errors from certbot.tests import util +from certbot.tests.util import TempDirTestCase + KEY = jose.JWKRSA.load(util.load_vector("rsa512_key_2.pem")) @@ -98,13 +99,14 @@ class AccountMemoryStorageTest(unittest.TestCase): self.assertEqual([account], self.storage.find_all()) -class AccountFileStorageTest(unittest.TestCase): +class AccountFileStorageTest(TempDirTestCase): """Tests for certbot.account.AccountFileStorage.""" def setUp(self): - self.tmp = tempfile.mkdtemp() + super(AccountFileStorageTest, self).setUp() + self.config = mock.MagicMock( - accounts_dir=os.path.join(self.tmp, "accounts")) + accounts_dir=os.path.join(self.tempdir, "accounts")) from certbot.account import AccountFileStorage self.storage = AccountFileStorage(self.config) @@ -118,9 +120,6 @@ class AccountFileStorageTest(unittest.TestCase): self.mock_client = mock.MagicMock() self.mock_client.directory.new_authz = new_authzr_uri - def tearDown(self): - shutil.rmtree(self.tmp) - def test_init_creates_dir(self): self.assertTrue(os.path.isdir(self.config.accounts_dir)) diff --git a/certbot/tests/auth_handler_test.py b/certbot/tests/auth_handler_test.py index 9d22843db..32c4c0d3b 100644 --- a/certbot/tests/auth_handler_test.py +++ b/certbot/tests/auth_handler_test.py @@ -5,6 +5,7 @@ import unittest import mock import six +import zope.component from acme import challenges from acme import client as acme_client @@ -12,6 +13,7 @@ from acme import messages from certbot import achallenges from certbot import errors +from certbot import interfaces from certbot import util from certbot.tests import acme_util @@ -65,6 +67,12 @@ class GetAuthorizationsTest(unittest.TestCase): def setUp(self): from certbot.auth_handler import AuthHandler + self.mock_display = mock.Mock() + zope.component.provideUtility( + self.mock_display, interfaces.IDisplay) + zope.component.provideUtility( + mock.Mock(debug_challenges=False), interfaces.IConfig) + self.mock_auth = mock.MagicMock(name="ApacheConfigurator") self.mock_auth.get_chall_pref.return_value = [challenges.TLSSNI01] @@ -157,6 +165,20 @@ class GetAuthorizationsTest(unittest.TestCase): self.assertEqual(len(authzr), 3) + @mock.patch("certbot.auth_handler.AuthHandler._poll_challenges") + def test_debug_challenges(self, mock_poll): + zope.component.provideUtility( + mock.Mock(debug_challenges=True), interfaces.IConfig) + self.mock_net.request_domain_challenges.side_effect = functools.partial( + gen_dom_authzr, challs=acme_util.CHALLENGES) + + mock_poll.side_effect = self._validate_all + + self.handler.get_authorizations(["0"]) + + self.assertEqual(self.mock_net.answer_challenge.call_count, 1) + self.assertEqual(self.mock_display.notification.call_count, 1) + def test_perform_failure(self): self.mock_net.request_domain_challenges.side_effect = functools.partial( gen_dom_authzr, challs=acme_util.CHALLENGES) diff --git a/certbot/tests/cert_manager_test.py b/certbot/tests/cert_manager_test.py index 473970870..d0e563979 100644 --- a/certbot/tests/cert_manager_test.py +++ b/certbot/tests/cert_manager_test.py @@ -18,11 +18,14 @@ from certbot.storage import ALL_FOUR from certbot.tests import storage_test from certbot.tests import util as test_util -class BaseCertManagerTest(unittest.TestCase): +from certbot.tests.util import TempDirTestCase + + +class BaseCertManagerTest(TempDirTestCase): """Base class for setting up Cert Manager tests. """ def setUp(self): - self.tempdir = tempfile.mkdtemp() + super(BaseCertManagerTest, self).setUp() os.makedirs(os.path.join(self.tempdir, "renewal")) @@ -68,9 +71,6 @@ class BaseCertManagerTest(unittest.TestCase): config.write() return config - def tearDown(self): - shutil.rmtree(self.tempdir) - class UpdateLiveSymlinksTest(BaseCertManagerTest): """Tests for certbot.cert_manager.update_live_symlinks @@ -384,14 +384,19 @@ class RenameLineageTest(BaseCertManagerTest): @test_util.patch_get_utility() @mock.patch('certbot.cert_manager.lineage_for_certname') def test_no_existing_certname(self, mock_lineage_for_certname, unused_get_utility): - mock_config = mock.Mock(certname="one", new_certname="two") + mock_config = mock.Mock(certname="one", new_certname="two", + renewal_configs_dir="/tmp/etc/letsencrypt/renewal/") mock_lineage_for_certname.return_value = None - self.assertRaises(errors.ConfigurationError, - self._call, mock_config) + self.assertRaises(errors.ConfigurationError, self._call, mock_config) + @mock.patch("certbot.storage.RenewableCert._update_symlinks") @test_util.patch_get_utility() @mock.patch("certbot.storage.RenewableCert._check_symlinks") - def test_rename_cert(self, mock_check, unused_get_utility): + @mock.patch("certbot.storage.relevant_values") + def test_rename_cert(self, mock_rv, mock_check, unused_get_utility, unused_update_symlinks): + # Mock relevant_values() to claim that all values are relevant here + # (to avoid instantiating parser) + mock_rv.side_effect = lambda x: x mock_check.return_value = True mock_config = self.mock_config self._call(mock_config) @@ -400,9 +405,15 @@ class RenameLineageTest(BaseCertManagerTest): self.assertTrue(updated_lineage is not None) self.assertEqual(updated_lineage.lineagename, mock_config.new_certname) + @mock.patch("certbot.storage.RenewableCert._update_symlinks") @test_util.patch_get_utility() @mock.patch("certbot.storage.RenewableCert._check_symlinks") - def test_rename_cert_interactive_certname(self, mock_check, mock_get_utility): + @mock.patch("certbot.storage.relevant_values") + def test_rename_cert_interactive_certname(self, mock_rv, mock_check, mock_get_utility, + unused_update_symlinks): + # python 3.4 and 3.5 order things differently, so remove other.com for this test + os.remove(self.configs["other.com"].filename) + mock_rv.side_effect = lambda x: x mock_check.return_value = True mock_config = self.mock_config mock_config.certname = None @@ -437,9 +448,6 @@ class DuplicativeCertsTest(storage_test.BaseRenewableCertTest): self.config.write() self._write_out_ex_kinds() - def tearDown(self): - shutil.rmtree(self.tempdir) - @mock.patch('certbot.util.make_or_verify_dir') def test_find_duplicative_names(self, unused_makedir): from certbot.cert_manager import find_duplicative_certs diff --git a/certbot/tests/cli_test.py b/certbot/tests/cli_test.py index 5f4a4e2c7..498bd309d 100644 --- a/certbot/tests/cli_test.py +++ b/certbot/tests/cli_test.py @@ -15,17 +15,18 @@ from certbot import constants from certbot import errors from certbot.plugins import disco +from certbot.tests.util import TempDirTestCase + PLUGINS = disco.PluginsRegistry.find_all() -class TestReadFile(unittest.TestCase): +class TestReadFile(TempDirTestCase): '''Test cli.read_file''' _multiprocess_can_split_ = True def test_read_file(self): - tmp_dir = tempfile.mkdtemp() - rel_test_path = os.path.relpath(os.path.join(tmp_dir, 'foo')) + rel_test_path = os.path.relpath(os.path.join(self.tempdir, 'foo')) self.assertRaises( argparse.ArgumentTypeError, cli.read_file, rel_test_path) diff --git a/certbot/tests/client_test.py b/certbot/tests/client_test.py index cc3bb098d..8b72e1df7 100644 --- a/certbot/tests/client_test.py +++ b/certbot/tests/client_test.py @@ -7,6 +7,7 @@ import unittest import OpenSSL import mock +from acme import errors as acme_errors from acme import jose from certbot import account @@ -101,7 +102,7 @@ class RegisterTest(unittest.TestCase): self.config.register_unsafely_without_email = True self.config.dry_run = False self._call() - mock_logger.warning.assert_called_once_with(mock.ANY) + mock_logger.info.assert_called_once_with(mock.ANY) self.assertTrue(mock_handle.called) def test_unsupported_error(self): @@ -170,7 +171,9 @@ class ClientTest(ClientTestCommon): self.acme.fetch_chain.assert_called_once_with(mock.sentinel.certr) @mock.patch("certbot.client.logger") - def test_obtain_certificate_from_csr(self, mock_logger): + @test_util.patch_get_utility() + def test_obtain_certificate_from_csr(self, unused_mock_get_utility, + mock_logger): self._mock_obtain_certificate() test_csr = util.CSR(form="der", file=None, data=CSR_SAN) auth_handler = self.client.auth_handler @@ -203,8 +206,44 @@ class ClientTest(ClientTestCommon): test_csr) mock_logger.warning.assert_called_once_with(mock.ANY) + @test_util.patch_get_utility() + def test_obtain_certificate_from_csr_retry_succeeded( + self, mock_get_utility): + self._mock_obtain_certificate() + self.acme.fetch_chain.side_effect = [acme_errors.Error, + mock.sentinel.chain] + test_csr = util.CSR(form="der", file=None, data=CSR_SAN) + auth_handler = self.client.auth_handler + + authzr = auth_handler.get_authorizations(self.eg_domains, False) + self.assertEqual( + (mock.sentinel.certr, mock.sentinel.chain), + self.client.obtain_certificate_from_csr( + self.eg_domains, + test_csr, + authzr=authzr)) + self.assertEqual(1, mock_get_utility().notification.call_count) + + @test_util.patch_get_utility() + def test_obtain_certificate_from_csr_retry_failed(self, mock_get_utility): + self._mock_obtain_certificate() + self.acme.fetch_chain.side_effect = acme_errors.Error + test_csr = util.CSR(form="der", file=None, data=CSR_SAN) + auth_handler = self.client.auth_handler + + authzr = auth_handler.get_authorizations(self.eg_domains, False) + self.assertRaises( + acme_errors.Error, + self.client.obtain_certificate_from_csr, + self.eg_domains, + test_csr, + authzr=authzr) + self.assertEqual(1, mock_get_utility().notification.call_count) + @mock.patch("certbot.client.crypto_util") - def test_obtain_certificate(self, mock_crypto_util): + @test_util.patch_get_utility() + def test_obtain_certificate(self, unused_mock_get_utility, + mock_crypto_util): self._mock_obtain_certificate() csr = util.CSR(form="der", file=None, data=CSR_SAN) diff --git a/certbot/tests/colored_logging_test.py b/certbot/tests/colored_logging_test.py deleted file mode 100644 index 0a7929561..000000000 --- a/certbot/tests/colored_logging_test.py +++ /dev/null @@ -1,41 +0,0 @@ -"""Tests for certbot.colored_logging.""" -import logging -import unittest - -import six - -from certbot import util - - -class StreamHandlerTest(unittest.TestCase): - """Tests for certbot.colored_logging.""" - - def setUp(self): - from certbot import colored_logging - - self.stream = six.StringIO() - self.stream.isatty = lambda: True - self.handler = colored_logging.StreamHandler(self.stream) - - self.logger = logging.getLogger() - self.logger.setLevel(logging.DEBUG) - self.logger.addHandler(self.handler) - - def test_format(self): - msg = 'I did a thing' - self.logger.debug(msg) - self.assertEqual(self.stream.getvalue(), '{0}\n'.format(msg)) - - def test_format_and_red_level(self): - msg = 'I did another thing' - self.handler.red_level = logging.DEBUG - self.logger.debug(msg) - - self.assertEqual(self.stream.getvalue(), - '{0}{1}{2}\n'.format(util.ANSI_SGR_RED, - msg, - util.ANSI_SGR_RESET)) - - -if __name__ == "__main__": - unittest.main() # pragma: no cover diff --git a/certbot/tests/configuration_test.py b/certbot/tests/configuration_test.py index 3a2f7d291..66a07dddd 100644 --- a/certbot/tests/configuration_test.py +++ b/certbot/tests/configuration_test.py @@ -120,6 +120,12 @@ class NamespaceConfigTest(unittest.TestCase): self.assertTrue(os.path.isabs(config.live_dir)) self.assertTrue(os.path.isabs(config.renewal_configs_dir)) + def test_get_and_set_attr(self): + self.config.foo = 42 + self.assertEqual(self.config.namespace.foo, 42) + self.config.namespace.bar = 1337 + self.assertEqual(self.config.bar, 1337) + if __name__ == '__main__': unittest.main() # pragma: no cover diff --git a/certbot/tests/crypto_util_test.py b/certbot/tests/crypto_util_test.py index 946e772c1..df5d9f0f6 100644 --- a/certbot/tests/crypto_util_test.py +++ b/certbot/tests/crypto_util_test.py @@ -1,7 +1,6 @@ """Tests for certbot.crypto_util.""" import logging -import shutil -import tempfile +import os import unittest import OpenSSL @@ -21,17 +20,20 @@ CERT = test_util.load_vector('cert.pem') SAN_CERT = test_util.load_vector('cert-san.pem') -class InitSaveKeyTest(unittest.TestCase): +class InitSaveKeyTest(test_util.TempDirTestCase): """Tests for certbot.crypto_util.init_save_key.""" def setUp(self): + super(InitSaveKeyTest, self).setUp() + logging.disable(logging.CRITICAL) zope.component.provideUtility( - mock.Mock(strict_permissions=True), interfaces.IConfig) - self.key_dir = tempfile.mkdtemp('key_dir') + mock.Mock(strict_permissions=True, dry_run=False), + interfaces.IConfig) def tearDown(self): + super(InitSaveKeyTest, self).tearDown() + logging.disable(logging.NOTSET) - shutil.rmtree(self.key_dir) @classmethod def _call(cls, key_size, key_dir): @@ -41,41 +43,68 @@ class InitSaveKeyTest(unittest.TestCase): @mock.patch('certbot.crypto_util.make_key') def test_success(self, mock_make): mock_make.return_value = b'key_pem' - key = self._call(1024, self.key_dir) + key = self._call(1024, self.tempdir) self.assertEqual(key.pem, b'key_pem') self.assertTrue('key-certbot.pem' in key.file) + self.assertTrue(os.path.exists(os.path.join(self.tempdir, key.file))) + + @mock.patch('certbot.crypto_util.make_key') + def test_success_dry_run(self, mock_make): + zope.component.provideUtility( + mock.Mock(strict_permissions=True, dry_run=True), + interfaces.IConfig) + mock_make.return_value = b'key_pem' + key = self._call(1024, self.tempdir) + self.assertEqual(key.pem, b'key_pem') + self.assertTrue(key.file is None) @mock.patch('certbot.crypto_util.make_key') def test_key_failure(self, mock_make): mock_make.side_effect = ValueError - self.assertRaises(ValueError, self._call, 431, self.key_dir) + self.assertRaises(ValueError, self._call, 431, self.tempdir) -class InitSaveCSRTest(unittest.TestCase): +class InitSaveCSRTest(test_util.TempDirTestCase): """Tests for certbot.crypto_util.init_save_csr.""" def setUp(self): - zope.component.provideUtility( - mock.Mock(strict_permissions=True), interfaces.IConfig) - self.csr_dir = tempfile.mkdtemp('csr_dir') + super(InitSaveCSRTest, self).setUp() - def tearDown(self): - shutil.rmtree(self.csr_dir) + zope.component.provideUtility( + mock.Mock(strict_permissions=True, dry_run=False), + interfaces.IConfig) @mock.patch('certbot.crypto_util.make_csr') @mock.patch('certbot.crypto_util.util.make_or_verify_dir') - def test_it(self, unused_mock_verify, mock_csr): + def test_success(self, unused_mock_verify, mock_csr): from certbot.crypto_util import init_save_csr mock_csr.return_value = (b'csr_pem', b'csr_der') csr = init_save_csr( - mock.Mock(pem='dummy_key'), 'example.com', self.csr_dir, + mock.Mock(pem='dummy_key'), 'example.com', self.tempdir, 'csr-certbot.pem') self.assertEqual(csr.data, b'csr_der') self.assertTrue('csr-certbot.pem' in csr.file) + @mock.patch('certbot.crypto_util.make_csr') + @mock.patch('certbot.crypto_util.util.make_or_verify_dir') + def test_success_dry_run(self, unused_mock_verify, mock_csr): + from certbot.crypto_util import init_save_csr + + zope.component.provideUtility( + mock.Mock(strict_permissions=True, dry_run=True), + interfaces.IConfig) + mock_csr.return_value = (b'csr_pem', b'csr_der') + + csr = init_save_csr( + mock.Mock(pem='dummy_key'), 'example.com', self.tempdir, + 'csr-certbot.pem') + + self.assertEqual(csr.data, b'csr_der') + self.assertTrue(csr.file is None) + class MakeCSRTest(unittest.TestCase): """Tests for certbot.crypto_util.make_csr.""" diff --git a/certbot/tests/display/completer_test.py b/certbot/tests/display/completer_test.py index 16805314c..333acf2b3 100644 --- a/certbot/tests/display/completer_test.py +++ b/certbot/tests/display/completer_test.py @@ -1,31 +1,30 @@ """Test certbot.display.completer.""" import os import readline -import shutil import string import sys -import tempfile import unittest import mock from six.moves import reload_module # pylint: disable=import-error +from certbot.tests.util import TempDirTestCase -class CompleterTest(unittest.TestCase): +class CompleterTest(TempDirTestCase): """Test certbot.display.completer.Completer.""" def setUp(self): - self.temp_dir = tempfile.mkdtemp() + super(CompleterTest, self).setUp() # directories must end with os.sep for completer to # search inside the directory for possible completions - if self.temp_dir[-1] != os.sep: - self.temp_dir += os.sep + if self.tempdir[-1] != os.sep: + self.tempdir += os.sep self.paths = [] # create some files and directories in temp_dir for c in string.ascii_lowercase: - path = os.path.join(self.temp_dir, c) + path = os.path.join(self.tempdir, c) self.paths.append(path) if ord(c) % 2: os.mkdir(path) @@ -33,21 +32,18 @@ class CompleterTest(unittest.TestCase): with open(path, 'w'): pass - def tearDown(self): - shutil.rmtree(self.temp_dir) - def test_complete(self): from certbot.display import completer my_completer = completer.Completer() num_paths = len(self.paths) for i in range(num_paths): - completion = my_completer.complete(self.temp_dir, i) + completion = my_completer.complete(self.tempdir, i) self.assertTrue(completion in self.paths) self.paths.remove(completion) self.assertFalse(self.paths) - completion = my_completer.complete(self.temp_dir, num_paths) + completion = my_completer.complete(self.tempdir, num_paths) self.assertEqual(completion, None) def test_import_error(self): diff --git a/certbot/tests/display/ops_test.py b/certbot/tests/display/ops_test.py index f2a9b3d07..89cd9e43d 100644 --- a/certbot/tests/display/ops_test.py +++ b/certbot/tests/display/ops_test.py @@ -2,7 +2,6 @@ """Test certbot.display.ops.""" import os import sys -import tempfile import unittest import mock @@ -87,18 +86,19 @@ class GetEmailTest(unittest.TestCase): self.assertTrue(invalid_txt in mock_input.call_args[0][0]) -class ChooseAccountTest(unittest.TestCase): +class ChooseAccountTest(test_util.TempDirTestCase): """Tests for certbot.display.ops.choose_account.""" def setUp(self): + super(ChooseAccountTest, self).setUp() + zope.component.provideUtility(display_util.FileDisplay(sys.stdout, False)) - self.accounts_dir = tempfile.mkdtemp("accounts") - self.account_keys_dir = os.path.join(self.accounts_dir, "keys") + self.account_keys_dir = os.path.join(self.tempdir, "keys") os.makedirs(self.account_keys_dir, 0o700) self.config = mock.MagicMock( - accounts_dir=self.accounts_dir, + accounts_dir=self.tempdir, account_keys_dir=self.account_keys_dir, server="certbot-demo.org") self.key = KEY diff --git a/certbot/tests/log_test.py b/certbot/tests/log_test.py new file mode 100644 index 000000000..745147f0d --- /dev/null +++ b/certbot/tests/log_test.py @@ -0,0 +1,199 @@ +"""Tests for certbot.log.""" +import logging +import traceback +import logging.handlers +import os +import sys +import time +import unittest + +import mock +import six + +from acme import messages + +from certbot import constants +from certbot import errors +from certbot import util +from certbot.tests import util as test_util + + +class PreArgParseSetupTest(unittest.TestCase): + """Tests for certbot.log.pre_arg_parse_setup.""" + + @classmethod + def _call(cls, *args, **kwargs): + from certbot.log import pre_arg_parse_setup + return pre_arg_parse_setup(*args, **kwargs) + + def test_it(self): + with mock.patch('certbot.log.except_hook') as mock_except_hook: + with mock.patch('certbot.log.sys') as mock_sys: + self._call() + + mock_sys.excepthook(1, 2, 3) + mock_except_hook.assert_called_once_with(1, 2, 3, config=None) + + +class PostArgParseSetupTest(test_util.TempDirTestCase): + """Tests for certbot.log.post_arg_parse_setup.""" + + @classmethod + def _call(cls, *args, **kwargs): + from certbot.log import post_arg_parse_setup + return post_arg_parse_setup(*args, **kwargs) + + def setUp(self): + super(PostArgParseSetupTest, self).setUp() + self.config = mock.MagicMock( + logs_dir=self.tempdir, quiet=False, + verbose_count=constants.CLI_DEFAULTS['verbose_count']) + self.root_logger = mock.MagicMock() + + def test_common(self): + with mock.patch('certbot.log.logging.getLogger') as mock_get_logger: + mock_get_logger.return_value = self.root_logger + with mock.patch('certbot.log.except_hook') as mock_except_hook: + with mock.patch('certbot.log.sys') as mock_sys: + mock_sys.version_info = sys.version_info + self._call(self.config) + + self.assertEqual(self.root_logger.addHandler.call_count, 2) + self.assertTrue(os.path.exists(os.path.join( + self.config.logs_dir, 'letsencrypt.log'))) + mock_sys.excepthook(1, 2, 3) + mock_except_hook.assert_called_once_with(1, 2, 3, config=self.config) + + stderr_handler = self.root_logger.addHandler.call_args_list[0][0][0] + level = stderr_handler.level + if self.config.quiet: + self.assertEqual(level, constants.QUIET_LOGGING_LEVEL) + else: + self.assertEqual(level, -self.config.verbose_count * 10) + + def test_quiet(self): + self.config.quiet = True + self.test_common() + + +class SetupLogFileHandlerTest(test_util.TempDirTestCase): + """Tests for certbot.log.setup_log_file_handler.""" + + @classmethod + def _call(cls, *args, **kwargs): + from certbot.log import setup_log_file_handler + return setup_log_file_handler(*args, **kwargs) + + def setUp(self): + super(SetupLogFileHandlerTest, self).setUp() + self.config = mock.MagicMock(logs_dir=self.tempdir) + + @mock.patch('certbot.main.logging.handlers.RotatingFileHandler') + def test_failure(self, mock_handler): + mock_handler.side_effect = IOError + + try: + self._call(self.config, 'test.log', '%(message)s') + except errors.Error as err: + self.assertTrue('--logs-dir' in str(err)) + else: # pragma: no cover + self.fail('Error not raised.') + + def test_success(self): + log_file = 'test.log' + handler, log_path = self._call(self.config, log_file, '%(message)s') + self.assertEqual(handler.level, logging.DEBUG) + self.assertEqual(handler.formatter.converter, time.gmtime) + + expected_path = os.path.join(self.config.logs_dir, log_file) + self.assertEqual(log_path, expected_path) + + +class ColoredStreamHandlerTest(unittest.TestCase): + """Tests for certbot.log.""" + + def setUp(self): + from certbot import log + + self.stream = six.StringIO() + self.stream.isatty = lambda: True + self.handler = log.ColoredStreamHandler(self.stream) + + self.logger = logging.getLogger() + self.logger.setLevel(logging.DEBUG) + self.logger.addHandler(self.handler) + + def test_format(self): + msg = 'I did a thing' + self.logger.debug(msg) + self.assertEqual(self.stream.getvalue(), '{0}\n'.format(msg)) + + def test_format_and_red_level(self): + msg = 'I did another thing' + self.handler.red_level = logging.DEBUG + self.logger.debug(msg) + + self.assertEqual(self.stream.getvalue(), + '{0}{1}{2}\n'.format(util.ANSI_SGR_RED, + msg, + util.ANSI_SGR_RESET)) + + +class ExceptHookTest(unittest.TestCase): + """Tests for certbot.log.except_hook.""" + @classmethod + def _call(cls, *args, **kwargs): + from certbot.log import except_hook + return except_hook(*args, **kwargs) + + @mock.patch('certbot.log.sys') + def test_except_hook(self, mock_sys): + config = mock.MagicMock() + mock_open = mock.mock_open() + + with mock.patch('certbot.log.open', mock_open, create=True): + exception = Exception('detail') + config.verbose_count = 1 + self._call( + Exception, exc_value=exception, trace=None, config=None) + mock_open().write.assert_any_call(''.join( + traceback.format_exception_only(Exception, exception))) + error_msg = mock_sys.exit.call_args_list[0][0][0] + self.assertTrue('unexpected error' in error_msg) + + with mock.patch('certbot.log.open', mock_open, create=True): + mock_open.side_effect = [KeyboardInterrupt] + error = errors.Error('detail') + self._call( + errors.Error, exc_value=error, trace=None, config=None) + # assert_any_call used because sys.exit doesn't exit in cli.py + mock_sys.exit.assert_any_call(''.join( + traceback.format_exception_only(errors.Error, error))) + + bad_typ = messages.ERROR_PREFIX + 'triffid' + exception = messages.Error(detail='alpha', typ=bad_typ, title='beta') + config = mock.MagicMock(debug=False, verbose_count=-3) + self._call( + messages.Error, exc_value=exception, trace=None, config=config) + error_msg = mock_sys.exit.call_args_list[-1][0][0] + self.assertTrue('unexpected error' in error_msg) + self.assertTrue('acme:error' not in error_msg) + self.assertTrue('alpha' in error_msg) + self.assertTrue('beta' in error_msg) + config = mock.MagicMock(debug=False, verbose_count=1) + self._call( + messages.Error, exc_value=exception, trace=None, config=config) + error_msg = mock_sys.exit.call_args_list[-1][0][0] + self.assertTrue('unexpected error' in error_msg) + self.assertTrue('acme:error' in error_msg) + self.assertTrue('alpha' in error_msg) + + interrupt = KeyboardInterrupt('detail') + self._call( + KeyboardInterrupt, exc_value=interrupt, trace=None, config=None) + mock_sys.exit.assert_called_with(''.join( + traceback.format_exception_only(KeyboardInterrupt, interrupt))) + + +if __name__ == "__main__": + unittest.main() # pragma: no cover diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py index 170eceb48..1afe95924 100644 --- a/certbot/tests/main_test.py +++ b/certbot/tests/main_test.py @@ -6,7 +6,6 @@ import itertools import mock import os import shutil -import tempfile import traceback import unittest import datetime @@ -19,7 +18,6 @@ from acme import jose from certbot import account from certbot import cli -from certbot import colored_logging from certbot import constants from certbot import configuration from certbot import crypto_util @@ -219,13 +217,14 @@ class FindDomainsOrCertnameTest(unittest.TestCase): (["one.com", "two.com"], "one.com")) -class RevokeTest(unittest.TestCase): +class RevokeTest(test_util.TempDirTestCase): """Tests for certbot.main.revoke.""" def setUp(self): - self.tempdir_path = tempfile.mkdtemp() - shutil.copy(CERT_PATH, self.tempdir_path) - self.tmp_cert_path = os.path.abspath(os.path.join(self.tempdir_path, + super(RevokeTest, self).setUp() + + shutil.copy(CERT_PATH, self.tempdir) + self.tmp_cert_path = os.path.abspath(os.path.join(self.tempdir, 'cert.pem')) self.patches = [ @@ -251,7 +250,8 @@ class RevokeTest(unittest.TestCase): self.mock_determine_account.return_value = (self.acc, None) def tearDown(self): - shutil.rmtree(self.tempdir_path) + super(RevokeTest, self).tearDown() + for patch in self.patches: patch.stop() @@ -287,89 +287,6 @@ class RevokeTest(unittest.TestCase): self.mock_success_revoke.assert_not_called() -class SetupLogFileHandlerTest(unittest.TestCase): - """Tests for certbot.main.setup_log_file_handler.""" - - def setUp(self): - self.config = mock.Mock(spec_set=['logs_dir'], - logs_dir=tempfile.mkdtemp()) - - def tearDown(self): - shutil.rmtree(self.config.logs_dir) - - def _call(self, *args, **kwargs): - from certbot.main import setup_log_file_handler - return setup_log_file_handler(*args, **kwargs) - - @mock.patch('certbot.main.logging.handlers.RotatingFileHandler') - def test_ioerror(self, mock_handler): - mock_handler.side_effect = IOError - self.assertRaises(errors.Error, self._call, - self.config, "test.log", "%s") - - -class SetupLoggingTest(unittest.TestCase): - """Tests for certbot.main.setup_logging.""" - - def setUp(self): - self.config = mock.Mock( - logs_dir=tempfile.mkdtemp(), - noninteractive_mode=False, quiet=False, - verbose_count=constants.CLI_DEFAULTS['verbose_count']) - - def tearDown(self): - shutil.rmtree(self.config.logs_dir) - - @classmethod - def _call(cls, *args, **kwargs): - from certbot.main import setup_logging - return setup_logging(*args, **kwargs) - - @mock.patch('certbot.main.logging.getLogger') - def test_defaults(self, mock_get_logger): - self._call(self.config) - - cli_handler = mock_get_logger().addHandler.call_args_list[0][0][0] - self.assertEqual(cli_handler.level, -self.config.verbose_count * 10) - self.assertTrue( - isinstance(cli_handler, colored_logging.StreamHandler)) - - @mock.patch('certbot.main.logging.getLogger') - def test_quiet_mode(self, mock_get_logger): - self.config.quiet = self.config.noninteractive_mode = True - self._call(self.config) - - cli_handler = mock_get_logger().addHandler.call_args_list[0][0][0] - self.assertEqual(cli_handler.level, constants.QUIET_LOGGING_LEVEL) - self.assertTrue( - isinstance(cli_handler, colored_logging.StreamHandler)) - - -class MakeOrVerifyCoreDirTest(unittest.TestCase): - """Tests for certbot.main.make_or_verify_core_dir.""" - - def setUp(self): - self.dir = tempfile.mkdtemp() - - def tearDown(self): - shutil.rmtree(self.dir) - - def _call(self, *args, **kwargs): - from certbot.main import make_or_verify_core_dir - return make_or_verify_core_dir(*args, **kwargs) - - def test_success(self): - new_dir = os.path.join(self.dir, 'new') - self._call(new_dir, 0o700, os.geteuid(), False) - self.assertTrue(os.path.exists(new_dir)) - - @mock.patch('certbot.main.util.make_or_verify_dir') - def test_failure(self, mock_make_or_verify): - mock_make_or_verify.side_effect = OSError - self.assertRaises(errors.Error, self._call, - self.dir, 0o700, os.geteuid(), False) - - class DetermineAccountTest(unittest.TestCase): """Tests for certbot.main._determine_account.""" @@ -440,14 +357,15 @@ class DetermineAccountTest(unittest.TestCase): self.assertEqual('other email', self.config.email) -class MainTest(unittest.TestCase): # pylint: disable=too-many-public-methods +class MainTest(test_util.TempDirTestCase): # pylint: disable=too-many-public-methods """Tests for different commands.""" def setUp(self): - self.tmp_dir = tempfile.mkdtemp() - self.config_dir = os.path.join(self.tmp_dir, 'config') - self.work_dir = os.path.join(self.tmp_dir, 'work') - self.logs_dir = os.path.join(self.tmp_dir, 'logs') + super(MainTest, self).setUp() + + self.config_dir = os.path.join(self.tempdir, 'config') + self.work_dir = os.path.join(self.tempdir, 'work') + self.logs_dir = os.path.join(self.tempdir, 'logs') os.mkdir(self.logs_dir) self.standard_args = ['--config-dir', self.config_dir, '--work-dir', self.work_dir, @@ -456,7 +374,8 @@ class MainTest(unittest.TestCase): # pylint: disable=too-many-public-methods def tearDown(self): # Reset globals in cli reload_module(cli) - shutil.rmtree(self.tmp_dir) + + super(MainTest, self).tearDown() def _call(self, args, stdout=None): "Run the cli with output streams and actual client mocked out" @@ -1254,59 +1173,5 @@ class UnregisterTest(unittest.TestCase): self.assertFalse(acme_client.acme.deactivate_registration.called) -class TestHandleException(unittest.TestCase): - """Test main._handle_exception""" - @mock.patch('certbot.main.sys') - def test_handle_exception(self, mock_sys): - # pylint: disable=protected-access - from acme import messages - - config = mock.MagicMock() - mock_open = mock.mock_open() - - with mock.patch('certbot.main.open', mock_open, create=True): - exception = Exception('detail') - config.verbose_count = 1 - main._handle_exception( - Exception, exc_value=exception, trace=None, config=None) - mock_open().write.assert_any_call(''.join( - traceback.format_exception_only(Exception, exception))) - error_msg = mock_sys.exit.call_args_list[0][0][0] - self.assertTrue('unexpected error' in error_msg) - - with mock.patch('certbot.main.open', mock_open, create=True): - mock_open.side_effect = [KeyboardInterrupt] - error = errors.Error('detail') - main._handle_exception( - errors.Error, exc_value=error, trace=None, config=None) - # assert_any_call used because sys.exit doesn't exit in cli.py - mock_sys.exit.assert_any_call(''.join( - traceback.format_exception_only(errors.Error, error))) - - bad_typ = messages.ERROR_PREFIX + 'triffid' - exception = messages.Error(detail='alpha', typ=bad_typ, title='beta') - config = mock.MagicMock(debug=False, verbose_count=-3) - main._handle_exception( - messages.Error, exc_value=exception, trace=None, config=config) - error_msg = mock_sys.exit.call_args_list[-1][0][0] - self.assertTrue('unexpected error' in error_msg) - self.assertTrue('acme:error' not in error_msg) - self.assertTrue('alpha' in error_msg) - self.assertTrue('beta' in error_msg) - config = mock.MagicMock(debug=False, verbose_count=1) - main._handle_exception( - messages.Error, exc_value=exception, trace=None, config=config) - error_msg = mock_sys.exit.call_args_list[-1][0][0] - self.assertTrue('unexpected error' in error_msg) - self.assertTrue('acme:error' in error_msg) - self.assertTrue('alpha' in error_msg) - - interrupt = KeyboardInterrupt('detail') - main._handle_exception( - KeyboardInterrupt, exc_value=interrupt, trace=None, config=None) - mock_sys.exit.assert_called_with(''.join( - traceback.format_exception_only(KeyboardInterrupt, interrupt))) - - if __name__ == '__main__': unittest.main() # pragma: no cover diff --git a/certbot/tests/renewal_test.py b/certbot/tests/renewal_test.py index cd53aa91c..de3efe39c 100644 --- a/certbot/tests/renewal_test.py +++ b/certbot/tests/renewal_test.py @@ -2,8 +2,6 @@ import os import mock import unittest -import shutil -import tempfile from acme import challenges @@ -14,13 +12,11 @@ from certbot import storage from certbot.tests import util -class RenewalTest(unittest.TestCase): +class RenewalTest(util.TempDirTestCase): def setUp(self): - self.tmp_dir = tempfile.mkdtemp() - self.config_dir = os.path.join(self.tmp_dir, 'config') + super(RenewalTest, self).setUp() - def tearDown(self): - shutil.rmtree(self.tmp_dir) + self.config_dir = os.path.join(self.tempdir, 'config') @mock.patch('certbot.cli.set_by_cli') def test_ancient_webroot_renewal_conf(self, mock_set_by_cli): @@ -52,7 +48,7 @@ class RestoreRequiredConfigElementsTest(unittest.TestCase): def test_allow_subset_of_names_success(self, mock_set_by_cli): mock_set_by_cli.return_value = False self._call(self.config, {'allow_subset_of_names': 'True'}) - self.assertTrue(self.config.namespace.allow_subset_of_names is True) + self.assertTrue(self.config.allow_subset_of_names is True) @mock.patch('certbot.renewal.cli.set_by_cli') def test_allow_subset_of_names_failure(self, mock_set_by_cli): @@ -68,7 +64,7 @@ class RestoreRequiredConfigElementsTest(unittest.TestCase): self._call(self.config, renewalparams) expected = [challenges.TLSSNI01.typ, challenges.HTTP01.typ, challenges.DNS01.typ] - self.assertEqual(self.config.namespace.pref_challs, expected) + self.assertEqual(self.config.pref_challs, expected) @mock.patch('certbot.renewal.cli.set_by_cli') def test_pref_challs_str(self, mock_set_by_cli): @@ -76,7 +72,7 @@ class RestoreRequiredConfigElementsTest(unittest.TestCase): renewalparams = {'pref_challs': 'dns'} self._call(self.config, renewalparams) expected = [challenges.DNS01.typ] - self.assertEqual(self.config.namespace.pref_challs, expected) + self.assertEqual(self.config.pref_challs, expected) @mock.patch('certbot.renewal.cli.set_by_cli') def test_pref_challs_failure(self, mock_set_by_cli): @@ -88,7 +84,7 @@ class RestoreRequiredConfigElementsTest(unittest.TestCase): def test_must_staple_success(self, mock_set_by_cli): mock_set_by_cli.return_value = False self._call(self.config, {'must_staple': 'True'}) - self.assertTrue(self.config.namespace.must_staple is True) + self.assertTrue(self.config.must_staple is True) @mock.patch('certbot.renewal.cli.set_by_cli') def test_must_staple_failure(self, mock_set_by_cli): diff --git a/certbot/tests/reporter_test.py b/certbot/tests/reporter_test.py index 0b06cccd7..9ec8dca28 100644 --- a/certbot/tests/reporter_test.py +++ b/certbot/tests/reporter_test.py @@ -38,18 +38,6 @@ class ReporterTest(unittest.TestCase): self.reporter.print_messages() self.assertEqual(sys.stdout.getvalue(), "") - @mock.patch('certbot.reporter.os.getpid') - def test_atexit_print_messages(self, mock_getpid): - self._add_messages() - mock_getpid.return_value = 42 - with mock.patch('certbot.reporter.INITIAL_PID', 42): - self.reporter.atexit_print_messages() - output = sys.stdout.getvalue() - self.assertTrue("IMPORTANT NOTES:" in output) - self.assertTrue("High" in output) - self.assertTrue("Med" in output) - self.assertTrue("Low" in output) - def test_tty_successful_exit(self): sys.stdout.isatty = lambda: True self._successful_exit_common() diff --git a/certbot/tests/storage_test.py b/certbot/tests/storage_test.py index f52f31d3d..6635461aa 100644 --- a/certbot/tests/storage_test.py +++ b/certbot/tests/storage_test.py @@ -3,7 +3,6 @@ import datetime import os import shutil -import tempfile import unittest import configobj @@ -36,7 +35,7 @@ def fill_with_sample_data(rc_object): f.write(kind) -class BaseRenewableCertTest(unittest.TestCase): +class BaseRenewableCertTest(util.TempDirTestCase): """Base class for setting up Renewable Cert tests. .. note:: It may be required to write out self.config for @@ -47,7 +46,8 @@ class BaseRenewableCertTest(unittest.TestCase): def setUp(self): from certbot import storage - self.tempdir = tempfile.mkdtemp() + + super(BaseRenewableCertTest, self).setUp() self.cli_config = configuration.NamespaceConfig( namespace=mock.MagicMock( @@ -91,9 +91,6 @@ class BaseRenewableCertTest(unittest.TestCase): check.return_value = True self.test_rc = storage.RenewableCert(config.filename, self.cli_config) - def tearDown(self): - shutil.rmtree(self.tempdir) - def _write_out_kind(self, kind, ver, value=None): link = getattr(self.test_rc, kind) if os.path.lexists(link): @@ -581,6 +578,15 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertEqual( self._test_relevant_values_common(values), values) + @mock.patch("certbot.cli.set_by_cli") + @mock.patch("certbot.plugins.disco.PluginsRegistry.find_all") + def test_relevant_values_namespace(self, mock_find_all, mock_set_by_cli): + mock_set_by_cli.return_value = True + mock_find_all.return_value = ["certbot-foo:bar"] + values = {"certbot_foo:bar_baz": 42} + self.assertEqual( + self._test_relevant_values_common(values), values) + @mock.patch("certbot.storage.relevant_values") def test_new_lineage(self, mock_rv): """Test for new_lineage() class method.""" @@ -789,6 +795,7 @@ class DeleteFilesTest(BaseRenewableCertTest): """Tests for certbot.storage.delete_files""" def setUp(self): super(DeleteFilesTest, self).setUp() + for kind in ALL_FOUR: kind_path = os.path.join(self.tempdir, "live", "example.org", kind + ".pem") diff --git a/certbot/tests/util.py b/certbot/tests/util.py index 092807b56..d58834335 100644 --- a/certbot/tests/util.py +++ b/certbot/tests/util.py @@ -6,6 +6,7 @@ import os import pkg_resources import shutil +import tempfile import unittest from cryptography.hazmat.backends import default_backend @@ -230,3 +231,13 @@ def _assert_valid_call(*args, **kwargs): # pylint: disable=star-args display_util.assert_valid_call(*assert_args, **assert_kwargs) + + +class TempDirTestCase(unittest.TestCase): + """Base test class which sets up and tears down a temporary directory""" + + def setUp(self): + self.tempdir = tempfile.mkdtemp() + + def tearDown(self): + shutil.rmtree(self.tempdir) diff --git a/certbot/tests/util_test.py b/certbot/tests/util_test.py index 6dc839025..f021c04cf 100644 --- a/certbot/tests/util_test.py +++ b/certbot/tests/util_test.py @@ -2,9 +2,7 @@ import argparse import errno import os -import shutil import stat -import tempfile import unittest import mock @@ -75,7 +73,26 @@ class ExeExistsTest(unittest.TestCase): self.assertFalse(self._call("exe")) -class MakeOrVerifyDirTest(unittest.TestCase): +class MakeOrVerifyCoreDirTest(test_util.TempDirTestCase): + """Tests for certbot.util.make_or_verify_core_dir.""" + + def _call(self, *args, **kwargs): + from certbot.util import make_or_verify_core_dir + return make_or_verify_core_dir(*args, **kwargs) + + def test_success(self): + new_dir = os.path.join(self.tempdir, 'new') + self._call(new_dir, 0o700, os.geteuid(), False) + self.assertTrue(os.path.exists(new_dir)) + + @mock.patch('certbot.main.util.make_or_verify_dir') + def test_failure(self, mock_make_or_verify): + mock_make_or_verify.side_effect = OSError + self.assertRaises(errors.Error, self._call, + self.tempdir, 0o700, os.geteuid(), False) + + +class MakeOrVerifyDirTest(test_util.TempDirTestCase): """Tests for certbot.util.make_or_verify_dir. Note that it is not possible to test for a wrong directory owner, @@ -84,21 +101,19 @@ class MakeOrVerifyDirTest(unittest.TestCase): """ def setUp(self): - self.root_path = tempfile.mkdtemp() - self.path = os.path.join(self.root_path, "foo") + super(MakeOrVerifyDirTest, self).setUp() + + self.path = os.path.join(self.tempdir, "foo") os.mkdir(self.path, 0o400) self.uid = os.getuid() - def tearDown(self): - shutil.rmtree(self.root_path, ignore_errors=True) - def _call(self, directory, mode): from certbot.util import make_or_verify_dir return make_or_verify_dir(directory, mode, self.uid, strict=True) def test_creates_dir_when_missing(self): - path = os.path.join(self.root_path, "bar") + path = os.path.join(self.tempdir, "bar") self._call(path, 0o650) self.assertTrue(os.path.isdir(path)) self.assertEqual(stat.S_IMODE(os.stat(path).st_mode), 0o650) @@ -116,7 +131,7 @@ class MakeOrVerifyDirTest(unittest.TestCase): self.assertRaises(OSError, self._call, "bar", 12312312) -class CheckPermissionsTest(unittest.TestCase): +class CheckPermissionsTest(test_util.TempDirTestCase): """Tests for certbot.util.check_permissions. Note that it is not possible to test for a wrong file owner, @@ -125,34 +140,30 @@ class CheckPermissionsTest(unittest.TestCase): """ def setUp(self): - _, self.path = tempfile.mkstemp() - self.uid = os.getuid() + super(CheckPermissionsTest, self).setUp() - def tearDown(self): - os.remove(self.path) + self.uid = os.getuid() def _call(self, mode): from certbot.util import check_permissions - return check_permissions(self.path, mode, self.uid) + return check_permissions(self.tempdir, mode, self.uid) def test_ok_mode(self): - os.chmod(self.path, 0o600) + os.chmod(self.tempdir, 0o600) self.assertTrue(self._call(0o600)) def test_wrong_mode(self): - os.chmod(self.path, 0o400) + os.chmod(self.tempdir, 0o400) self.assertFalse(self._call(0o600)) -class UniqueFileTest(unittest.TestCase): +class UniqueFileTest(test_util.TempDirTestCase): """Tests for certbot.util.unique_file.""" def setUp(self): - self.root_path = tempfile.mkdtemp() - self.default_name = os.path.join(self.root_path, "foo.txt") + super(UniqueFileTest, self).setUp() - def tearDown(self): - shutil.rmtree(self.root_path, ignore_errors=True) + self.default_name = os.path.join(self.tempdir, "foo.txt") def _call(self, mode=0o600): from certbot.util import unique_file @@ -177,9 +188,9 @@ class UniqueFileTest(unittest.TestCase): self.assertNotEqual(name1, name3) self.assertNotEqual(name2, name3) - self.assertEqual(os.path.dirname(name1), self.root_path) - self.assertEqual(os.path.dirname(name2), self.root_path) - self.assertEqual(os.path.dirname(name3), self.root_path) + self.assertEqual(os.path.dirname(name1), self.tempdir) + self.assertEqual(os.path.dirname(name2), self.tempdir) + self.assertEqual(os.path.dirname(name3), self.tempdir) basename1 = os.path.basename(name2) self.assertTrue(basename1.endswith("foo.txt")) @@ -193,26 +204,20 @@ try: file_type = file except NameError: import io - file_type = io.TextIOWrapper + file_type = io.TextIOWrapper # type: ignore -class UniqueLineageNameTest(unittest.TestCase): +class UniqueLineageNameTest(test_util.TempDirTestCase): """Tests for certbot.util.unique_lineage_name.""" - def setUp(self): - self.root_path = tempfile.mkdtemp() - - def tearDown(self): - shutil.rmtree(self.root_path, ignore_errors=True) - def _call(self, filename, mode=0o777): from certbot.util import unique_lineage_name - return unique_lineage_name(self.root_path, filename, mode) + return unique_lineage_name(self.tempdir, filename, mode) def test_basic(self): f, path = self._call("wow") self.assertTrue(isinstance(f, file_type)) - self.assertEqual(os.path.join(self.root_path, "wow.conf"), path) + self.assertEqual(os.path.join(self.tempdir, "wow.conf"), path) def test_multiple(self): for _ in six.moves.range(10): @@ -237,15 +242,13 @@ class UniqueLineageNameTest(unittest.TestCase): self.assertRaises(OSError, self._call, "wow") -class SafelyRemoveTest(unittest.TestCase): +class SafelyRemoveTest(test_util.TempDirTestCase): """Tests for certbot.util.safely_remove.""" def setUp(self): - self.tmp = tempfile.mkdtemp() - self.path = os.path.join(self.tmp, "foo") + super(SafelyRemoveTest, self).setUp() - def tearDown(self): - shutil.rmtree(self.tmp) + self.path = os.path.join(self.tempdir, "foo") def _call(self): from certbot.util import safely_remove @@ -489,5 +492,37 @@ class OsInfoTest(unittest.TestCase): ("windows", "95")) +class AtexitRegisterTest(unittest.TestCase): + """Tests for certbot.util.atexit_register.""" + def setUp(self): + self.func = mock.MagicMock() + self.args = ('hi',) + self.kwargs = {'answer': 42} + + @classmethod + def _call(cls, *args, **kwargs): + from certbot.util import atexit_register + return atexit_register(*args, **kwargs) + + def test_called(self): + self._test_common(os.getpid()) + self.func.assert_called_with(*self.args, **self.kwargs) + + def test_not_called(self): + self._test_common(initial_pid=-1) + self.assertFalse(self.func.called) + + def _test_common(self, initial_pid): + with mock.patch('certbot.util._INITIAL_PID', initial_pid): + with mock.patch('certbot.util.atexit') as mock_atexit: + self._call(self.func, *self.args, **self.kwargs) + + # _INITAL_PID must be mocked when calling atexit_func + self.assertTrue(mock_atexit.register.called) + args, kwargs = mock_atexit.register.call_args + atexit_func = args[0] + atexit_func(*args[1:], **kwargs) # pylint: disable=star-args + + if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/certbot/util.py b/certbot/util.py index 95c669d0d..55a75097f 100644 --- a/certbot/util.py +++ b/certbot/util.py @@ -1,5 +1,6 @@ """Utilities for all Certbot.""" import argparse +import atexit import collections # distutils.version under virtualenv confuses pylint # For more info, see: https://github.com/PyCQA/pylint/issues/73 @@ -38,6 +39,16 @@ ANSI_SGR_RED = "\033[31m" ANSI_SGR_RESET = "\033[0m" +PERM_ERR_FMT = os.linesep.join(( + "The following error was encountered:", "{0}", + "If running as non-root, set --config-dir, " + "--work-dir, and --logs-dir to writeable paths.")) + + +# Stores importing process ID to be used by atexit_register() +_INITIAL_PID = os.getpid() + + def run_script(params, log=logger.error): """Run the script with the given params. @@ -92,6 +103,23 @@ def exe_exists(exe): return False +def make_or_verify_core_dir(directory, mode, uid, strict): + """Make sure directory exists with proper permissions. + + :param str directory: Path to a directory. + :param int mode: Directory mode. + :param int uid: Directory owner. + :param bool strict: require directory to be owned by current user + + :raises .errors.Error: if the directory cannot be made or verified + + """ + try: + make_or_verify_dir(directory, mode, uid, strict) + except OSError as error: + raise errors.Error(PERM_ERR_FMT.format(error)) + + def make_or_verify_dir(directory, mode=0o755, uid=0, strict=False): """Make sure directory exists with proper permissions. @@ -533,3 +561,20 @@ def is_staging(srv): :rtype bool: """ return srv == constants.STAGING_URI or "staging" in srv + + +def atexit_register(func, *args, **kwargs): + """Sets func to be called before the program exits. + + Special care is taken to ensure func is only called when the process + that first imports this module exits rather than any child processes. + + :param function func: function to be called in case of an error + + """ + atexit.register(_atexit_call, func, *args, **kwargs) + + +def _atexit_call(func, *args, **kwargs): + if _INITIAL_PID == os.getpid(): + func(*args, **kwargs) diff --git a/docs/contributing.rst b/docs/contributing.rst index 5cdf86147..9c7f0636f 100644 --- a/docs/contributing.rst +++ b/docs/contributing.rst @@ -145,13 +145,15 @@ different webservers, other TLS servers, and operating systems. The interfaces available for plugins to implement are defined in `interfaces.py`_ and `plugins/common.py`_. -The most common kind of plugin is a "Configurator", which is likely to -implement the `~certbot.interfaces.IAuthenticator` and -`~certbot.interfaces.IInstaller` interfaces (though some -Configurators may implement just one of those). +The main two plugin interfaces are `~certbot.interfaces.IAuthenticator`, which +implements various ways of proving domain control to a certificate authority, +and `~certbot.interfaces.IInstaller`, which configures a server to use a +certificate once it is issued. Some plugins, like the built-in Apache and Nginx +plugins, implement both interfaces and perform both tasks. Others, like the +built-in Standalone authenticator, implement just one interface. There are also `~certbot.interfaces.IDisplay` plugins, -which implement bindings to alternative UI libraries. +which can change how prompts are displayed to a user. .. _interfaces.py: https://github.com/certbot/certbot/blob/master/certbot/interfaces.py .. _plugins/common.py: https://github.com/certbot/certbot/blob/master/certbot/plugins/common.py#L34 @@ -160,27 +162,20 @@ which implement bindings to alternative UI libraries. Authenticators -------------- -Authenticators are plugins designed to prove that this client deserves a -certificate for some domain name by solving challenges received from -the ACME server. From the protocol, there are essentially two -different types of challenges. Challenges that must be solved by -individual plugins in order to satisfy domain validation (subclasses -of `~.DVChallenge`, i.e. `~.challenges.TLSSNI01`, -`~.challenges.HTTP01`, `~.challenges.DNS`) and continuity specific -challenges (subclasses of `~.ContinuityChallenge`, -i.e. `~.challenges.RecoveryToken`, `~.challenges.RecoveryContact`, -`~.challenges.ProofOfPossession`). Continuity challenges are -always handled by the `~.ContinuityAuthenticator`, while plugins are -expected to handle `~.DVChallenge` types. -Right now, we have two authenticator plugins, the `~.ApacheConfigurator` -and the `~.StandaloneAuthenticator`. The Standalone and Apache -authenticators only solve the `~.challenges.TLSSNI01` challenge currently. -(You can set which challenges your authenticator can handle through the -:meth:`~.IAuthenticator.get_chall_pref`. +Authenticators are plugins that prove control of a domain name by solving a +challenge provided by the ACME server. ACME currently defines three types of +challenges: HTTP, TLS-SNI, and DNS, represented by classes in `acme.challenges`. +An authenticator plugin should implement support for at least one challenge type. -(FYI: We also have a partial implementation for a `~.DNSAuthenticator` -in a separate branch). +An Authenticator indicates which challenges it supports by implementing +get_chall_pref(domain) to return a sorted list of challenge types in preference +order. +An Authenticator must also implement `perform(achalls)`, which "performs" a list +of challenges by, for instance, provisioning a file on an HTTP server, or +setting a TXT record in DNS. Once all challenges have succeeded or failed, +Certbot will call the plugin's `cleanup(achalls)` method to remove any files or +DNS records that were needed only during authentication. Installer --------- @@ -218,16 +213,10 @@ Augeas may still find the `~.Reverter` class helpful in handling configuration checkpoints and rollback. -Display -~~~~~~~ - -We currently only offer a "text" mode for displays. Display plugins -implement the `~certbot.interfaces.IDisplay` interface. - .. _dev-plugin: Writing your own plugin -======================= +~~~~~~~~~~~~~~~~~~~~~~~ Certbot client supports dynamic discovery of plugins through the `setuptools entry points`_. This way you can, for example, create a @@ -236,6 +225,26 @@ the `~certbot.interfaces.IInstaller` without having to merge it with the core upstream source code. An example is provided in ``examples/plugins/`` directory. +While developing, you can install your plugin into a Certbot development +virtualenv like this: + +.. code-block:: shell + . venv/bin/activate + . tests/integration/_common.sh + pip install -e examples/plugins/ + certbot_test plugins + +Your plugin should show up in the output of the last command. If not, +it was not installed properly. + +Once you've finished your plugin and published it, you can have your +users install it system-wide with `pip install`. Note that this will +only work for users who have Certbot installed from OS packages or via +pip. Users who run `certbot-auto` are currently unable to use third-party +plugins. It's technically possible to install third-party plugins into +the virtualenv used by `certbot-auto`, but they will be wiped away when +`certbot-auto` upgrades. + .. warning:: Please be aware though that as this client is still in a developer-preview stage, the API may undergo a few changes. If you believe the plugin will be beneficial to the community, please diff --git a/docs/install.rst b/docs/install.rst index aa59e44ec..e1ec06f16 100644 --- a/docs/install.rst +++ b/docs/install.rst @@ -22,9 +22,8 @@ your system. System Requirements =================== -The Let's Encrypt Client presently only runs on Unix-ish OSes that include -Python 2.6 or 2.7; Python 3.x support will hopefully be added in the future. The -client requires root access in order to write to ``/etc/letsencrypt``, +Certbot currently requires Python 2.6 or 2.7. By default, it requires root +access in order to write to ``/etc/letsencrypt``, ``/var/log/letsencrypt``, ``/var/lib/letsencrypt``; to bind to ports 80 and 443 (if you use the ``standalone`` plugin) and to read and modify webserver configurations (if you use the ``apache`` or ``nginx`` plugins). If none of @@ -33,11 +32,16 @@ but for most users who want to avoid running an ACME client as root, either `letsencrypt-nosudo `_ or `simp_le `_ are more appropriate choices. -The Apache plugin currently requires OS with augeas version 1.0; currently `it +The Apache plugin currently requires an OS with augeas version 1.0; currently `it supports `_ modern OSes based on Debian, Fedora, SUSE, Gentoo and Darwin. +Installing with ``certbot-auto`` requires 512MB of RAM in order to build some +of the dependencies. Installing from pre-built OS packages avoids this +requirement. You can also temporarily set a swap file. See "Problems with +Python virtual environment" below for details. + Alternate installation methods ================================ @@ -76,7 +80,7 @@ For full command line help, you can type:: Problems with Python virtual environment ---------------------------------------- -On a low memory system such as VPS with only 256MB of RAM, the required dependencies of Certbot will failed to build. +On a low memory system such as VPS with less than 512MB of RAM, the required dependencies of Certbot will failed to build. This can be identified if the pip outputs contains something like ``internal compiler error: Killed (program cc1)``. You can workaround this restriction by creating a temporary swapfile:: @@ -120,7 +124,7 @@ to, `install Docker`_, then issue the following command: sudo docker run -it --rm -p 443:443 -p 80:80 --name certbot \ -v "/etc/letsencrypt:/etc/letsencrypt" \ -v "/var/lib/letsencrypt:/var/lib/letsencrypt" \ - quay.io/letsencrypt/letsencrypt:latest certonly + certbot/certbot certonly Running Certbot with the ``certonly`` command will obtain a certificate and place it in the directory ``/etc/letsencrypt/live`` on your system. Because Certonly cannot install the certificate from diff --git a/docs/packaging.rst b/docs/packaging.rst index 1a1b83f50..5b37c2e24 100644 --- a/docs/packaging.rst +++ b/docs/packaging.rst @@ -16,21 +16,19 @@ The following scripts are used in the process: - https://github.com/letsencrypt/letsencrypt/blob/master/tools/release.sh -We currently version with the following scheme: +We use git tags to identify releases, using `Semantic Versioning`_. For +example: `v0.11.1`. -- ``0.1.0`` -- ``0.2.0dev`` for developement in ``master`` -- ``0.2.0`` (only temporarily in ``master``) -- ... +.. _`Semantic Versioning`: http://semver.org/ Notes for package maintainers ============================= -0. Please use our releases, not ``master``! +0. Please use our tagged releases, not ``master``! 1. Do not package ``certbot-compatibility-test`` or ``letshelp-certbot`` - it's only used internally. -2. If you'd like to include automated renewal in your package ``certbot renew -q`` should be added to crontab or systemd timer. +2. If you'd like to include automated renewal in your package ``certbot renew -q`` should be added to crontab or systemd timer. Additionally you should include a random per-machine time offset to avoid having a large number of your clients hit Let's Encrypt's servers simultaneously. 3. ``jws`` is an internal script for ``acme`` module and it doesn't have to be packaged - it's mostly for debugging: you can use it as ``echo foo | jws sign | jws verify``. @@ -44,34 +42,43 @@ Arch ---- From our official releases: + - https://www.archlinux.org/packages/community/any/python2-acme - https://www.archlinux.org/packages/community/any/certbot - https://www.archlinux.org/packages/community/any/certbot-apache - https://www.archlinux.org/packages/community/any/certbot-nginx -- https://www.archlinux.org/packages/community/any/letshelp-certbot From ``master``: https://aur.archlinux.org/packages/certbot-git Debian (and its derivatives, including Ubuntu) ------ -https://packages.debian.org/sid/certbot -https://packages.debian.org/sid/python-certbot -https://packages.debian.org/sid/python-certbot-apache +- https://packages.debian.org/sid/certbot +- https://packages.debian.org/sid/python-certbot +- https://packages.debian.org/sid/python-certbot-apache Fedora ------ In Fedora 23+. -- https://admin.fedoraproject.org/pkgdb/package/letsencrypt/ - https://admin.fedoraproject.org/pkgdb/package/certbot/ - https://admin.fedoraproject.org/pkgdb/package/python-acme/ FreeBSD ------- -https://svnweb.freebsd.org/ports/head/security/py-certbot/ +- https://svnweb.freebsd.org/ports/head/security/py-certbot/ + +Gentoo +------ + +Currently, all ``certbot`` related packages are in the testing branch: + +- https://packages.gentoo.org/packages/app-crypt/certbot +- https://packages.gentoo.org/packages/app-crypt/certbot-apache +- https://packages.gentoo.org/packages/app-crypt/certbot-nginx +- https://packages.gentoo.org/packages/app-crypt/acme GNU Guix -------- diff --git a/docs/using.rst b/docs/using.rst index 7c17796e7..549a3479c 100644 --- a/docs/using.rst +++ b/docs/using.rst @@ -175,15 +175,15 @@ the UI, you can use the plugin to obtain a cert by specifying to copy and paste commands into another terminal session, which may be on a different computer. -The manual plugin can use either the ``http`` or the ``dns`` challenge. You -can use the ``--preferred-challenges`` option to chose the challenge of your +The manual plugin can use either the ``http`` or the ``dns`` challenge. You +can use the ``--preferred-challenges`` option to choose the challenge of your preference. -The ``http`` challenge will ask you to place a file with a specific name and -specific content in the ``/.well-known/acme-challenge/`` directory directly -in the top-level directory (“web root”) containing the files served by your +The ``http`` challenge will ask you to place a file with a specific name and +specific content in the ``/.well-known/acme-challenge/`` directory directly +in the top-level directory (“web root”) containing the files served by your webserver. In essence it's the same as the webroot_ plugin, but not automated. -When using the ``dns`` plugin, ``certbot`` will ask you to place a TXT DNS -record with specific contents under the domain name consisting of the hostname +When using the ``dns`` plugin, ``certbot`` will ask you to place a TXT DNS +record with specific contents under the domain name consisting of the hostname for which you want a certificate issued, prepended by ``_acme-challenge``. For example, for the domain ``example.com``, a zone file entry would look like: diff --git a/examples/cli.ini b/examples/cli.ini index 63af3cc49..dbaa9c599 100644 --- a/examples/cli.ini +++ b/examples/cli.ini @@ -1,6 +1,11 @@ # This is an example of the kind of things you can do in a configuration file. # All flags used by the client can be configured here. Run Certbot with # "--help" to learn more about the available options. +# +# Note that these options apply automatically to all use of Certbot for +# obtaining or renewing certificates, so options specific to a single +# certificate on a system with several certificates should not be placed +# here. # Use a 4096 bit RSA key instead of 2048 rsa-key-size = 4096 @@ -8,13 +13,6 @@ rsa-key-size = 4096 # Uncomment and update to register with the specified e-mail address # email = foo@example.com -# Uncomment and update to generate certificates for the specified -# domains. -# domains = example.com, www.example.com - -# Uncomment to use a text interface instead of ncurses -# text = True - # Uncomment to use the standalone authenticator on port 443 # authenticator = standalone # standalone-supported-challenges = tls-sni-01 diff --git a/letshelp-certbot/letshelp_certbot/apache.py b/letshelp-certbot/letshelp_certbot/apache.py index 5752bdab0..2391a30bb 100755 --- a/letshelp-certbot/letshelp_certbot/apache.py +++ b/letshelp-certbot/letshelp_certbot/apache.py @@ -15,6 +15,7 @@ import tarfile import tempfile import textwrap +import six _DESCRIPTION = """ Let's Help is a simple script you can run to help out the Certbot @@ -69,7 +70,7 @@ def make_and_verify_selection(server_root, temp_dir): sys.stdout.write("\nIs it safe to submit these files? ") while True: - ans = raw_input("(Y)es/(N)o: ").lower() + ans = six.moves.input("(Y)es/(N)o: ").lower() if ans.startswith("y"): return elif ans.startswith("n"): @@ -144,7 +145,8 @@ def safe_config_file(config_file): return False proc = subprocess.Popen(["file", config_file], - stdout=subprocess.PIPE, stderr=subprocess.PIPE) + stdout=subprocess.PIPE, stderr=subprocess.PIPE, + universal_newlines=True) file_output, _ = proc.communicate() if "ASCII" in file_output: diff --git a/letshelp-certbot/letshelp_certbot/apache_test.py b/letshelp-certbot/letshelp_certbot/apache_test.py index 0c1b5f2f6..e0656ae05 100644 --- a/letshelp-certbot/letshelp_certbot/apache_test.py +++ b/letshelp-certbot/letshelp_certbot/apache_test.py @@ -9,6 +9,8 @@ import tempfile import unittest import mock +# six is used in mock.patch() +import six # pylint: disable=unused-import import letshelp_certbot.apache as letshelp_le_apache @@ -63,7 +65,7 @@ class LetsHelpApacheTest(unittest.TestCase): def test_make_and_verify_selection(self, mock_copy_config): mock_copy_config.return_value = (["apache2.conf"], ["apache2"]) - with mock.patch("__builtin__.raw_input") as mock_input: + with mock.patch("six.moves.input") as mock_input: with mock.patch(_MODULE_NAME + ".sys.stdout"): mock_input.side_effect = ["Yes", "No"] letshelp_le_apache.make_and_verify_selection("root", "temp") diff --git a/tests/boulder-fetch.sh b/tests/boulder-fetch.sh index ef61fe3f5..d9a979667 100755 --- a/tests/boulder-fetch.sh +++ b/tests/boulder-fetch.sh @@ -12,5 +12,7 @@ fi cd ${BOULDERPATH} FAKE_DNS=$(ifconfig docker0 | grep "inet addr:" | cut -d: -f2 | awk '{ print $1}') +[ -z "$FAKE_DNS" ] && FAKE_DNS=$(ifconfig docker0 | grep "inet " | xargs | cut -d ' ' -f 2) +[ -z "$FAKE_DNS" ] && echo Unable to find the IP for docker0 && exit 1 sed -i "s/FAKE_DNS: .*/FAKE_DNS: ${FAKE_DNS}/" docker-compose.yml docker-compose up -d diff --git a/tests/boulder-integration.sh b/tests/boulder-integration.sh index ca6f48e60..6612b2e67 100755 --- a/tests/boulder-integration.sh +++ b/tests/boulder-integration.sh @@ -1,4 +1,4 @@ -#!/bin/sh -xe +#!/bin/bash # Simple integration test. Make sure to activate virtualenv beforehand # (source venv/bin/activate) and that you are running Boulder test # instance (see ./boulder-fetch.sh). @@ -8,12 +8,11 @@ # # Note: this script is called by Boulder integration test suite! +set -eux + . ./tests/integration/_common.sh export PATH="$PATH:/usr/sbin" # /usr/sbin/nginx -export GOPATH="${GOPATH:-/tmp/go}" -export PATH="$GOPATH/bin:$PATH" - if [ `uname` = "Darwin" ];then readlink="greadlink" else @@ -27,6 +26,14 @@ cleanup_and_exit() { echo Kill server subprocess, left running by abnormal exit kill $SERVER_STILL_RUNNING fi + # Dump boulder logs in case they contain useful debugging information. + : "------------------ ------------------ ------------------" + : "------------------ begin boulder logs ------------------" + : "------------------ ------------------ ------------------" + docker logs boulder_boulder_1 + : "------------------ ------------------ ------------------" + : "------------------ end boulder logs ------------------" + : "------------------ ------------------ ------------------" exit $EXIT_STATUS } diff --git a/tests/integration/_common.sh b/tests/integration/_common.sh index 9b44631d4..8d4baff95 100755 --- a/tests/integration/_common.sh +++ b/tests/integration/_common.sh @@ -1,12 +1,7 @@ -#!/bin/sh - -if [ "xxx$root" = "xxx" ]; -then - # The -t is required on macOS. It provides a template file path for - # the kernel to use. - root="$(mktemp -d -t leitXXXX)" - echo "Root integration tests directory: $root" -fi +# The -t is required on macOS. It provides a template file path for +# the kernel to use. +root=${root:-$(mktemp -d -t leitXXXX)} +echo "Root integration tests directory: $root" store_flags="--config-dir $root/conf --work-dir $root/work" store_flags="$store_flags --logs-dir $root/logs" tls_sni_01_port=5001 diff --git a/tools/release.sh b/tools/release.sh index 81582cef0..1da11fe2c 100755 --- a/tools/release.sh +++ b/tools/release.sh @@ -109,7 +109,7 @@ do echo "Signing ($pkg_dir)" for x in dist/*.tar.gz dist/*.whl do - gpg -u "$RELEASE_GPG_KEY" --detach-sign --armor --sign $x + gpg2 -u "$RELEASE_GPG_KEY" --detach-sign --armor --sign --digest-algo sha256 $x done cd - @@ -194,7 +194,7 @@ while ! openssl dgst -sha256 -verify $RELEASE_OPENSSL_PUBKEY -signature \ done # This signature is not quite as strong, but easier for people to verify out of band -gpg -u "$RELEASE_GPG_KEY" --detach-sign --armor --sign letsencrypt-auto-source/letsencrypt-auto +gpg2 -u "$RELEASE_GPG_KEY" --detach-sign --armor --sign --digest-algo sha256 letsencrypt-auto-source/letsencrypt-auto # We can't rename the openssl letsencrypt-auto.sig for compatibility reasons, # but we can use the right name for certbot-auto.asc from day one mv letsencrypt-auto-source/letsencrypt-auto.asc letsencrypt-auto-source/certbot-auto.asc @@ -214,7 +214,7 @@ name=${root_without_le%.*} ext="${root_without_le##*.}" rev="$(git rev-parse --short HEAD)" echo tar cJvf $name.$rev.tar.xz $name.$rev -echo gpg -U $RELEASE_GPG_KEY --detach-sign --armor $name.$rev.tar.xz +echo gpg2 -U $RELEASE_GPG_KEY --detach-sign --armor $name.$rev.tar.xz cd ~- echo "New root: $root" diff --git a/tox.ini b/tox.ini index ea1423415..d393bb610 100644 --- a/tox.ini +++ b/tox.ini @@ -40,42 +40,6 @@ deps = py{26,27}-oldest: PyOpenSSL==0.13 py{26,27}-oldest: requests<=2.11.1 -[testenv:py33] -commands = - pip install -e acme[dev] - nosetests -v acme --processes=-1 - pip install -e .[dev] - nosetests -v certbot --processes=-1 --process-timeout=100 - pip install -e certbot-apache - nosetests -v certbot_apache --processes=-1 --process-timeout=80 - -[testenv:py34] -commands = - pip install -e acme[dev] - nosetests -v acme --processes=-1 - pip install -e .[dev] - nosetests -v certbot --processes=-1 --process-timeout=100 - pip install -e certbot-apache - nosetests -v certbot_apache --processes=-1 --process-timeout=80 - -[testenv:py35] -commands = - pip install -e acme[dev] - nosetests -v acme --processes=-1 - pip install -e .[dev] - nosetests -v certbot --processes=-1 --process-timeout=100 - pip install -e certbot-apache - nosetests -v certbot_apache --processes=-1 --process-timeout=80 - -[testenv:py36] -commands = - pip install -e acme[dev] - nosetests -v acme --processes=-1 - pip install -e .[dev] - nosetests -v certbot --processes=-1 --process-timeout=100 - pip install -e certbot-apache - nosetests -v certbot_apache --processes=-1 --process-timeout=80 - [testenv:py27_install] basepython = python2.7 commands = @@ -97,6 +61,13 @@ commands = pip install -q -e acme[dev] -e .[dev] -e certbot-apache -e certbot-nginx -e certbot-compatibility-test -e letshelp-certbot pylint --reports=n --rcfile=.pylintrc acme/acme certbot certbot-apache/certbot_apache certbot-nginx/certbot_nginx certbot-compatibility-test/certbot_compatibility_test letshelp-certbot/letshelp_certbot +[testenv:mypy] +basepython = python3.4 +commands = + pip install mypy + pip install -q -e acme[dev] -e .[dev] -e certbot-apache -e certbot-nginx -e certbot-compatibility-test -e letshelp-certbot + mypy --py2 --ignore-missing-imports acme/acme certbot certbot-apache/certbot_apache certbot-nginx/certbot_nginx certbot-compatibility-test/certbot_compatibility_test letshelp-certbot/letshelp_certbot + [testenv:apacheconftest] #basepython = python2.7 commands =