diff --git a/certbot-apache/certbot_apache/_internal/override_centos.py b/certbot-apache/certbot_apache/_internal/override_centos.py index de5c31268..9883bb1f1 100644 --- a/certbot-apache/certbot_apache/_internal/override_centos.py +++ b/certbot-apache/certbot_apache/_internal/override_centos.py @@ -58,8 +58,13 @@ class CentOSConfigurator(configurator.ApacheConfigurator): "rhel", "redhatenterpriseserver", "red hat enterprise linux server", "scientific", "scientific linux", ] + # It is important that the loose version comparison below is not made + # if the OS is not RHEL derived. See + # https://github.com/certbot/certbot/issues/9481. + if not rhel_derived: + return False at_least_v9 = util.parse_loose_version(os_version) >= util.parse_loose_version('9') - return rhel_derived and at_least_v9 + return at_least_v9 def _override_cmds(self) -> None: super()._override_cmds() diff --git a/certbot-nginx/certbot_nginx/_internal/configurator.py b/certbot-nginx/certbot_nginx/_internal/configurator.py index 508f50ab7..b91b11a53 100644 --- a/certbot-nginx/certbot_nginx/_internal/configurator.py +++ b/certbot-nginx/certbot_nginx/_internal/configurator.py @@ -1060,8 +1060,8 @@ class NginxConfigurator(common.Configurator): product_name, product_version = version_matches[0] if product_name != 'nginx': - logger.warning("NGINX derivative %s is not officially supported by" - " certbot", product_name) + logger.warning("nginx derivative %s is not officially supported by " + "Certbot.", product_name) nginx_version = tuple(int(i) for i in product_version.split(".")) diff --git a/certbot-nginx/certbot_nginx/_internal/nginxparser.py b/certbot-nginx/certbot_nginx/_internal/nginxparser.py index 1c74cd367..99955447a 100644 --- a/certbot-nginx/certbot_nginx/_internal/nginxparser.py +++ b/certbot-nginx/certbot_nginx/_internal/nginxparser.py @@ -33,6 +33,18 @@ if TYPE_CHECKING: logger = logging.getLogger(__name__) +class UnsupportedDirectiveException(RuntimeError): + """Exception when encountering an nginx directive which is not supported + by this parser.""" + + directive_name: str + line_no: int + + def __init__(self, directive_name: str, line_no: int) -> None: + self.directive_name = directive_name + self.line_no = line_no + + class RawNginxParser: # pylint: disable=pointless-statement """A class that parses nginx configuration with pyparsing.""" @@ -74,6 +86,7 @@ class RawNginxParser: def __init__(self, source: str) -> None: self.source = source + self.whitespace_token_group.addParseAction(self._check_disallowed_directive) def parse(self) -> ParseResults: """Returns the parsed tree.""" @@ -83,6 +96,12 @@ class RawNginxParser: """Returns the parsed tree as a list.""" return self.parse().asList() + def _check_disallowed_directive(self, _source: str, line: int, results: ParseResults) -> None: + # *_by_lua_block might be first or second result, due to optional leading whitespace + toks = [t for t in results[0:2] if isinstance(t, str) and t.endswith("_by_lua_block")] + if toks: + raise UnsupportedDirectiveException(toks[0], line) + class RawNginxDumper: """A class that dumps nginx configuration from the provided tree.""" diff --git a/certbot-nginx/certbot_nginx/_internal/parser.py b/certbot-nginx/certbot_nginx/_internal/parser.py index e60d66eb2..bc1643426 100644 --- a/certbot-nginx/certbot_nginx/_internal/parser.py +++ b/certbot-nginx/certbot_nginx/_internal/parser.py @@ -223,6 +223,12 @@ class NginxParser: "supported.", item) except pyparsing.ParseException as err: logger.warning("Could not parse file: %s due to %s", item, err) + except nginxparser.UnsupportedDirectiveException as e: + logger.warning( + "%s:%d contained the '%s' directive, which is not supported by Certbot. The " + "file has been ignored, which may prevent Certbot from functioning properly. " + "Consider using the --webroot plugin and manually installing the certificate.", + item, e.line_no, e.directive_name) return trees def _find_config_root(self) -> str: diff --git a/certbot-nginx/tests/nginxparser_test.py b/certbot-nginx/tests/nginxparser_test.py index d8f0a5909..3713f16e4 100644 --- a/certbot-nginx/tests/nginxparser_test.py +++ b/certbot-nginx/tests/nginxparser_test.py @@ -12,6 +12,7 @@ from certbot_nginx._internal.nginxparser import load from certbot_nginx._internal.nginxparser import loads from certbot_nginx._internal.nginxparser import RawNginxParser from certbot_nginx._internal.nginxparser import UnspacedList +from certbot_nginx._internal.nginxparser import UnsupportedDirectiveException import test_util as util FIRST = operator.itemgetter(0) @@ -354,6 +355,69 @@ class TestRawNginxParser(unittest.TestCase): parsed = loads("") self.assertEqual(parsed, []) + def test_lua(self): + # https://github.com/certbot/certbot/issues/9066 + self.assertRaises(UnsupportedDirectiveException, loads, """ + location /foo { + content_by_lua_block { + ngx.say('Hello World') + } + } + """) + + # Without leading whitespace + self.assertRaises(UnsupportedDirectiveException, loads, """ + location /foo {content_by_lua_block { + ngx.say('Hello World') + } + } + """) + + # Doesn't trigger if it's commented or not in the right position. + parsed = loads(""" + location /foo {server_name content_by_lua_block; + #content_by_lua_block { + # ngx.say('Hello World') + # } + } + """) + self.assertEqual( + parsed, + [ + [['location', '/foo'], + [['server_name', 'content_by_lua_block'], + ['#', 'content_by_lua_block {'], + ['#', " ngx.say('Hello World')"], + ['#', ' }'] + ]] + ]) + + # *_by_lua should parse successfully. + parsed = loads(""" + location / { + set $a 32; + set $b 56; + set_by_lua $sum + 'return tonumber(ngx.arg[1]) + tonumber(ngx.arg[2])' + $a $b; + content_by_lua ' + ngx.say("foo"); + '; + } + """) + self.assertEqual( + parsed, + [ + [['location', '/'], + [['set', '$a', '32'], + ['set', '$b', '56'], + ['set_by_lua', '$sum', + "'return tonumber(ngx.arg[1]) + tonumber(ngx.arg[2])'", '$a', '$b' + ], + ['content_by_lua', '\'\n ngx.say("foo");\n \''] + ]] + ] + ) class TestUnspacedList(unittest.TestCase): """Test the UnspacedList data structure""" diff --git a/certbot-nginx/tests/parser_test.py b/certbot-nginx/tests/parser_test.py index 7d5b5e669..93a8dcedc 100644 --- a/certbot-nginx/tests/parser_test.py +++ b/certbot-nginx/tests/parser_test.py @@ -4,6 +4,7 @@ import re import shutil import unittest from typing import List +from unittest import mock from certbot import errors from certbot.compat import os @@ -532,6 +533,14 @@ class NginxParserTest(util.NginxTest): for output in log.output )) + @mock.patch('certbot_nginx._internal.parser.logger.warning') + def test_load_unsupported_directive_logged(self, mock_warn): + nparser = parser.NginxParser(self.config_path) + nparser.config_root = nparser.abs_path('unsupported_directives.conf') + nparser.load() + self.assertEqual(mock_warn.call_count, 1) + self.assertIn("which is not supported by Certbot", mock_warn.call_args[0][0]) + if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/certbot-nginx/tests/testdata/etc_nginx/unsupported_directives.conf b/certbot-nginx/tests/testdata/etc_nginx/unsupported_directives.conf new file mode 100644 index 000000000..d071c68c1 --- /dev/null +++ b/certbot-nginx/tests/testdata/etc_nginx/unsupported_directives.conf @@ -0,0 +1,11 @@ +# This configuration file contains unsupported direcives. + +server { + listen 80; + + location / { + foobar_by_lua_block { + ngx.say("Hello World") + } + } +} diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index 8ab011f50..177ab0926 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -14,7 +14,14 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). ### Fixed -* +* Interfaces which plugins register themselves as implementing without inheriting from them now show up in `certbot plugins` output. +* `IPluginFactory`, `IPlugin`, `IAuthenticator` and `IInstaller` have been re-added to + `certbot.interfaces`. + - This is to fix compatibility with a number of third-party DNS plugins which may + have started erroring with `AttributeError` in Certbot v2.0.0. + - Plugin authors can find more information about Certbot 2.x compatibility + [here](https://github.com/certbot/certbot/wiki/Certbot-v2.x-Plugin-Compatibility). +* A bug causing our certbot-apache tests to crash on some systems has been resolved. More details about these changes can be found on our GitHub repo. diff --git a/certbot/certbot/_internal/plugins/disco.py b/certbot/certbot/_internal/plugins/disco.py index 62e75ead1..5e767ae6d 100644 --- a/certbot/certbot/_internal/plugins/disco.py +++ b/certbot/certbot/_internal/plugins/disco.py @@ -24,25 +24,9 @@ from certbot.errors import Error logger = logging.getLogger(__name__) -PREFIX_FREE_DISTRIBUTIONS = [ - "certbot", - "certbot-apache", - "certbot-dns-cloudflare", - "certbot-dns-digitalocean", - "certbot-dns-dnsimple", - "certbot-dns-dnsmadeeasy", - "certbot-dns-gehirn", - "certbot-dns-google", - "certbot-dns-linode", - "certbot-dns-luadns", - "certbot-dns-nsone", - "certbot-dns-ovh", - "certbot-dns-rfc2136", - "certbot-dns-route53", - "certbot-dns-sakuracloud", - "certbot-nginx", -] -"""Distributions for which prefix will be omitted.""" + +PLUGIN_INTERFACES = [interfaces.Authenticator, interfaces.Installer, interfaces.Plugin] +"""Interfaces that should be listed in `certbot plugins` output""" class PluginEntryPoint: @@ -165,8 +149,8 @@ class PluginEntryPoint: "* {0}".format(self.name), "Description: {0}".format(self.plugin_cls.description), "Interfaces: {0}".format(", ".join( - cls.__name__ for cls in self.plugin_cls.mro() - if cls.__module__ == 'certbot.interfaces' + iface.__name__ for iface in PLUGIN_INTERFACES + if issubclass(self.plugin_cls, iface) )), "Entry point: {0}".format(self.entry_point), ] diff --git a/certbot/certbot/interfaces.py b/certbot/certbot/interfaces.py index 84223b6c0..176f1a21c 100644 --- a/certbot/certbot/interfaces.py +++ b/certbot/certbot/interfaces.py @@ -16,6 +16,11 @@ from acme.client import ClientV2 from certbot import configuration from certbot.achallenges import AnnotatedChallenge +try: + from zope.interface import Interface as ZopeInterface +except ImportError: + ZopeInterface = object + if TYPE_CHECKING: from certbot._internal.account import Account @@ -465,3 +470,16 @@ class RenewDeployer(metaclass=ABCMeta): :type lineage: RenewableCert """ + + +class IPluginFactory(ZopeInterface): + """Compatibility shim for plugins that still use Certbot's old zope.interface classes.""" + +class IPlugin(ZopeInterface): + """Compatibility shim for plugins that still use Certbot's old zope.interface classes.""" + +class IAuthenticator(IPlugin): + """Compatibility shim for plugins that still use Certbot's old zope.interface classes.""" + +class IInstaller(IPlugin): + """Compatibility shim for plugins that still use Certbot's old zope.interface classes.""" diff --git a/certbot/tests/plugins/disco_test.py b/certbot/tests/plugins/disco_test.py index 17677792f..673a8d04b 100644 --- a/certbot/tests/plugins/disco_test.py +++ b/certbot/tests/plugins/disco_test.py @@ -152,6 +152,12 @@ class PluginEntryPointTest(unittest.TestCase): self.assertIs(self.plugin_ep.misconfigured, False) self.assertIs(self.plugin_ep.available, False) + def test_str(self): + output = str(self.plugin_ep) + self.assertIn("Authenticator", output) + self.assertNotIn("Installer", output) + self.assertIn("Plugin", output) + def test_repr(self): self.assertEqual("PluginEntryPoint#sa", repr(self.plugin_ep))