diff --git a/AUTHORS.md b/AUTHORS.md index 0cedcbd19..56fe2a3d8 100644 --- a/AUTHORS.md +++ b/AUTHORS.md @@ -61,6 +61,7 @@ Authors * [Daniel Albers](https://github.com/AID) * [Daniel Aleksandersen](https://github.com/da2x) * [Daniel Convissor](https://github.com/convissor) +* [Daniel "Drex" Drexler](https://github.com/aeturnum) * [Daniel Huang](https://github.com/dhuang) * [Dave Guarino](https://github.com/daguar) * [David cz](https://github.com/dave-cz) diff --git a/acme/acme/messages.py b/acme/acme/messages.py index 030e62d6c..8c9bf9701 100644 --- a/acme/acme/messages.py +++ b/acme/acme/messages.py @@ -315,6 +315,9 @@ class Registration(ResourceBody): # on new-reg key server ignores 'key' and populates it based on # JWS.signature.combined.jwk key = jose.Field('key', omitempty=True, decoder=jose.JWK.from_json) + # Contact field implements special behavior to allow messages that clear existing + # contacts while not expecting the `contact` field when loading from json. + # This is implemented in the constructor and *_json methods. contact = jose.Field('contact', omitempty=True, default=()) agreement = jose.Field('agreement', omitempty=True) status = jose.Field('status', omitempty=True) @@ -327,24 +330,73 @@ class Registration(ResourceBody): @classmethod def from_data(cls, phone=None, email=None, external_account_binding=None, **kwargs): - """Create registration resource from contact details.""" + """ + Create registration resource from contact details. + + The `contact` keyword being passed to a Registration object is meaningful, so + this function represents empty iterables in its kwargs by passing on an empty + `tuple`. + """ + + # Note if `contact` was in kwargs. + contact_provided = 'contact' in kwargs + + # Pop `contact` from kwargs and add formatted email or phone numbers details = list(kwargs.pop('contact', ())) if phone is not None: details.append(cls.phone_prefix + phone) if email is not None: details.extend([cls.email_prefix + mail for mail in email.split(',')]) - kwargs['contact'] = tuple(details) + + # Insert formatted contact information back into kwargs + # or insert an empty tuple if `contact` provided. + if details or contact_provided: + kwargs['contact'] = tuple(details) if external_account_binding: kwargs['external_account_binding'] = external_account_binding return cls(**kwargs) + def __init__(self, **kwargs): + """Note if the user provides a value for the `contact` member.""" + if 'contact' in kwargs: + # Avoid the __setattr__ used by jose.TypedJSONObjectWithFields + object.__setattr__(self, '_add_contact', True) + super(Registration, self).__init__(**kwargs) + def _filter_contact(self, prefix): return tuple( detail[len(prefix):] for detail in self.contact # pylint: disable=not-an-iterable if detail.startswith(prefix)) + def _add_contact_if_appropriate(self, jobj): + """ + The `contact` member of Registration objects should not be required when + de-serializing (as it would be if the Fields' `omitempty` flag were `False`), but + it should be included in serializations if it was provided. + + :param jobj: Dictionary containing this Registrations' data + :type jobj: dict + + :returns: Dictionary containing Registrations data to transmit to the server + :rtype: dict + """ + if getattr(self, '_add_contact', False): + jobj['contact'] = self.encode('contact') + + return jobj + + def to_partial_json(self): + """Modify josepy.JSONDeserializable.to_partial_json()""" + jobj = super(Registration, self).to_partial_json() + return self._add_contact_if_appropriate(jobj) + + def fields_to_partial_json(self): + """Modify josepy.JSONObjectWithFields.fields_to_partial_json()""" + jobj = super(Registration, self).fields_to_partial_json() + return self._add_contact_if_appropriate(jobj) + @property def phones(self): """All phones found in the ``contact`` field.""" diff --git a/acme/tests/messages_test.py b/acme/tests/messages_test.py index 890a5f413..3458105b2 100644 --- a/acme/tests/messages_test.py +++ b/acme/tests/messages_test.py @@ -254,6 +254,19 @@ class RegistrationTest(unittest.TestCase): from acme.messages import Registration hash(Registration.from_json(self.jobj_from)) + def test_default_not_transmitted(self): + from acme.messages import NewRegistration + empty_new_reg = NewRegistration() + new_reg_with_contact = NewRegistration(contact=()) + + self.assertEqual(empty_new_reg.contact, ()) + self.assertEqual(new_reg_with_contact.contact, ()) + + self.assertTrue('contact' not in empty_new_reg.to_partial_json()) + self.assertTrue('contact' not in empty_new_reg.fields_to_partial_json()) + self.assertTrue('contact' in new_reg_with_contact.to_partial_json()) + self.assertTrue('contact' in new_reg_with_contact.fields_to_partial_json()) + class UpdateRegistrationTest(unittest.TestCase): """Tests for acme.messages.UpdateRegistration.""" diff --git a/certbot.wrapper b/certbot.wrapper index c779e9939..b7ce120bd 100755 --- a/certbot.wrapper +++ b/certbot.wrapper @@ -1,16 +1,35 @@ -#!/bin/bash +#!/bin/sh set -e -join() { - sep=$1 - first=$2 - if [ "$first" != "" ]; then - shift 2 - echo -n "${first}" - for item in "$@"; do echo -n "${sep}${item}"; done - echo - fi -} -paths=$(for plugin_snap in $(snap connections certbot|sed -n '2,$p'|awk '$1=="content[certbot-1]"{print $3}'|cut -d: -f1); do echo /snap/$plugin_snap/current/lib/python3.8/site-packages; done) -export CERTBOT_PLUGIN_PATH=$(join : $paths) +# This code is based on snapcraft's own patch to work around this problem at +# https://github.com/snapcore/snapcraft/blob/a97fb5c7ea553a1bd20f4887a7c3393e75761890/patches/ctypes_init.diff. +# We may not build the Certbot snap for all of these architectures (and as of +# writing this we do not), but we keep the code for them to avoid having to +# solve this problem again in the future if we add support for new +# architectures. +case "${SNAP_ARCH}" in + 'arm64') + ARCH_TRIPLET='aarch64-linux-gnu';; + 'armhf') + ARCH_TRIPLET='arm-linux-gnueabihf';; + 'i386') + ARCH_TRIPLET='i386-linux-gnu';; + 'ppc64el') + ARCH_TRIPLET='powerpc64le-linux-gnu';; + 'powerpc') + ARCH_TRIPLET='powerpc-linux-gnu';; + 'amd64') + ARCH_TRIPLET='x86_64-linux-gnu';; + 's390x') + ARCH_TRIPLET='s390x-linux-gnu';; + *) + echo "Unrecongized value of SNAP_ARCH: ${SNAP_ARCH}" >&2 + exit 1 +esac + +export CERTBOT_AUGEAS_PATH="${SNAP}/usr/lib/${ARCH_TRIPLET}/libaugeas.so.0" + +CERTBOT_PLUGIN_PATH="$(snap connections certbot | gawk 'BEGIN {ORS=""} NR>1 { if ($1 == "content[certbot-1]") { split($3,a,":"); PLUGINS=PLUGINS":/snap/"a[1]"/current/lib/python3.8/site-packages/"; next; } } END { print substr(PLUGINS, 2) }')" +export CERTBOT_PLUGIN_PATH + exec certbot "$@" diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index 5ef2769a9..78495ceda 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -6,7 +6,8 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). ### Added -* +* Added the ability to remove email and phone contact information from an account + using `update_account --register-unsafely-without-email` ### Changed @@ -14,7 +15,11 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). ### Fixed -* +* The problem causing the Apache plugin in the Certbot snap on ARM systems to + fail to load the Augeas library it depends on has been fixed. +* The `acme` library can now tell the ACME server to clear contact information by passing an empty + `tuple` to the `contact` field of a `Registration` message. +* Fixed the `*** stack smashing detected ***` error in the Certbot snap on some systems. More details about these changes can be found on our GitHub repo. @@ -36,7 +41,6 @@ More details about these changes can be found on our GitHub repo. ### Fixed -* More details about these changes can be found on our GitHub repo. diff --git a/certbot/certbot/_internal/cli/__init__.py b/certbot/certbot/_internal/cli/__init__.py index 54485e113..45165054d 100644 --- a/certbot/certbot/_internal/cli/__init__.py +++ b/certbot/certbot/_internal/cli/__init__.py @@ -171,13 +171,10 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): ["register", "automation"], "--register-unsafely-without-email", action="store_true", default=flag_default("register_unsafely_without_email"), help="Specifying this flag enables registering an account with no " - "email address. This is strongly discouraged, because in the " - "event of key loss or account compromise you will irrevocably " - "lose access to your account. You will also be unable to receive " - "notice about impending expiration or revocation of your " - "certificates. Updates to the Subscriber Agreement will still " - "affect you, and will be effective 14 days after posting an " - "update to the web site.") + "email address. This is strongly discouraged, because you will be " + "unable to receive notice about impending expiration or " + "revocation of your certificates or problems with your Certbot " + "installation that will lead to failure to renew.") helpful.add( ["register", "update_account", "unregister", "automation"], "-m", "--email", default=flag_default("email"), diff --git a/certbot/certbot/_internal/log.py b/certbot/certbot/_internal/log.py index 3b87f9bca..3d417c3a5 100644 --- a/certbot/certbot/_internal/log.py +++ b/certbot/certbot/_internal/log.py @@ -319,6 +319,9 @@ def post_arg_parse_except_hook(exc_type, exc_value, trace, debug, log_path): # logger.DEBUG should be used if debug or not issubclass(exc_type, Exception): assert constants.QUIET_LOGGING_LEVEL <= logging.ERROR + if exc_type is KeyboardInterrupt: + logger.error('Exiting due to user request.') + sys.exit(1) logger.error('Exiting abnormally:', exc_info=exc_info) else: logger.debug('Exiting abnormally:', exc_info=exc_info) diff --git a/certbot/certbot/_internal/main.py b/certbot/certbot/_internal/main.py index 30f4dd0a2..d60e6edba 100644 --- a/certbot/certbot/_internal/main.py +++ b/certbot/certbot/_internal/main.py @@ -11,7 +11,7 @@ import josepy as jose import zope.component from acme import errors as acme_errors -from acme.magic_typing import Union +from acme.magic_typing import Union, Iterable, Optional # pylint: disable=unused-import import certbot from certbot import crypto_util from certbot import errors @@ -590,7 +590,7 @@ def _init_le_client(config, authenticator, installer): :type config: interfaces.IConfig :param authenticator: Acme authentication handler - :type authenticator: interfaces.IAuthenticator + :type authenticator: Optional[interfaces.IAuthenticator] :param installer: Installer object :type installer: interfaces.IInstaller @@ -703,17 +703,17 @@ def update_account(config, unused_plugins): if not accounts: return "Could not find an existing account to update." - if config.email is None: - if config.register_unsafely_without_email: - return ("--register-unsafely-without-email provided, however, a " - "new e-mail address must\ncurrently be provided when " - "updating a registration.") + if config.email is None and not config.register_unsafely_without_email: config.email = display_ops.get_email(optional=False) acc, acme = _determine_account(config) cb_client = client.Client(config, acc, None, None, acme=acme) + # Empty list of contacts in case the user is removing all emails + + acc_contacts = () # type: Iterable[str] + if config.email: + acc_contacts = ['mailto:' + email for email in config.email.split(',')] # We rely on an exception to interrupt this process if it didn't work. - acc_contacts = ['mailto:' + email for email in config.email.split(',')] prev_regr_uri = acc.regr.uri acc.regr = cb_client.acme.update_registration(acc.regr.update( body=acc.regr.body.update(contact=acc_contacts))) @@ -722,8 +722,13 @@ def update_account(config, unused_plugins): # so that we can also continue to use the account object with acmev1. acc.regr = acc.regr.update(uri=prev_regr_uri) account_storage.update_regr(acc, cb_client.acme) - eff.prepare_subscription(config, acc) - add_msg("Your e-mail address was updated to {0}.".format(config.email)) + + if config.email is None: + add_msg("Any contact information associated with this account has been removed.") + else: + eff.prepare_subscription(config, acc) + add_msg("Your e-mail address was updated to {0}.".format(config.email)) + return None diff --git a/certbot/certbot/display/ops.py b/certbot/certbot/display/ops.py index 1c4ee47d4..fc369176b 100644 --- a/certbot/certbot/display/ops.py +++ b/certbot/certbot/display/ops.py @@ -6,7 +6,6 @@ import zope.component from certbot import errors from certbot import interfaces from certbot import util -from certbot.compat import misc from certbot.compat import os from certbot.display import util as display_util @@ -33,9 +32,10 @@ def get_email(invalid=False, optional=True): msg = "Enter email address (used for urgent renewal and security notices)\n" unsafe_suggestion = ("\n\nIf you really want to skip this, you can run " "the client with --register-unsafely-without-email " - "but make sure you then backup your account key from " - "{0}\n\n".format(os.path.join( - misc.get_default_folder('config'), 'accounts'))) + "but you will then be unable to receive notice about " + "impending expiration or revocation of your " + "certificates or problems with your Certbot " + "installation that will lead to failure to renew.\n\n") if optional: if invalid: msg += unsafe_suggestion diff --git a/certbot/tests/log_test.py b/certbot/tests/log_test.py index 5cd287c2e..76764e61b 100644 --- a/certbot/tests/log_test.py +++ b/certbot/tests/log_test.py @@ -306,7 +306,7 @@ class PostArgParseExceptHookTest(unittest.TestCase): self.log_path = 'foo.log' def test_base_exception(self): - exc_type = KeyboardInterrupt + exc_type = BaseException mock_logger, output = self._test_common(exc_type, debug=False) self._assert_exception_logged(mock_logger.error, exc_type) self._assert_logfile_output(output) @@ -342,6 +342,11 @@ class PostArgParseExceptHookTest(unittest.TestCase): self._assert_exception_logged(mock_logger.debug, exc_type) self._assert_quiet_output(mock_logger, output) + def test_keyboardinterrupt(self): + exc_type = KeyboardInterrupt + mock_logger, output = self._test_common(exc_type, debug=False) + mock_logger.error.assert_called_once_with('Exiting due to user request.') + def _test_common(self, error_type, debug): """Returns the mocked logger and stderr output.""" mock_err = six.StringIO() diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py index d2d7ccf79..52d8c24d0 100644 --- a/certbot/tests/main_test.py +++ b/certbot/tests/main_test.py @@ -1404,6 +1404,43 @@ class MainTest(test_util.ConfigTestCase): "user@example.org"]) self.assertTrue("Could not find an existing account" in x[0]) + @mock.patch('certbot._internal.main._determine_account') + @mock.patch('certbot._internal.eff.prepare_subscription') + @mock.patch('certbot._internal.main.account') + def test_update_account_remove_email(self, mocked_account_module, mock_prepare, mock_det_acc): + # Mock account storage and the account object returned + mocked_storage = mock.MagicMock() + mocked_account = mock.MagicMock() + + mocked_account_module.AccountFileStorage.return_value = mocked_storage + mocked_storage.find_all.return_value = [mocked_account] + mock_det_acc.return_value = (mocked_account, "foo") + + # Mock registration body to verify calls are made + mock_regr_body = mock.MagicMock() + + # mocked_account.regr is overwritten in update, requiring an odd mock setup + mocked_account.regr.body = mock_regr_body + + x = self._call( + ["update_account", "--register-unsafely-without-email"]) + + + # When update succeeds, the return value of update_account() is None + self.assertTrue(x[0] is None) + # and we got supposedly did update the registration from + # the server + client_mock = x[3] + self.assertTrue(client_mock.Client().acme.update_registration.called) + + self.assertTrue(mock_regr_body.update.called) + self.assertTrue('contact' in mock_regr_body.update.call_args[1]) + self.assertEqual(mock_regr_body.update.call_args[1]['contact'], ()) + # and we saved the updated registration on disk + self.assertTrue(mocked_storage.update_regr.called) + # ensure we didn't try to subscribe (no email to subscribe with) + self.assertFalse(mock_prepare.called) + @mock.patch('certbot._internal.main.display_ops.get_email') @test_util.patch_get_utility() def test_update_account_with_email(self, mock_utility, mock_email): diff --git a/snap/hooks/configure b/snap/hooks/configure index 2678c47b2..9545f8b8a 100644 --- a/snap/hooks/configure +++ b/snap/hooks/configure @@ -1,3 +1,3 @@ -#!/bin/bash -e +#!/bin/sh -e exit 0 diff --git a/snap/hooks/prepare-plug-plugin b/snap/hooks/prepare-plug-plugin index ee309addf..f2f7ff86b 100644 --- a/snap/hooks/prepare-plug-plugin +++ b/snap/hooks/prepare-plug-plugin @@ -1,4 +1,4 @@ -#!/bin/bash -e +#!/bin/sh -e if [ "$(snapctl get trust-plugin-with-root)" = "ok" ]; then # allow the connection, but reset config to allow for other slots to go through this auth flow diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml index c2dab9180..2cd5271c4 100644 --- a/snap/snapcraft.yaml +++ b/snap/snapcraft.yaml @@ -24,7 +24,6 @@ apps: environment: PATH: "$SNAP/bin:$SNAP/usr/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games" AUGEAS_LENS_LIB: "$SNAP/usr/share/augeas/lenses/dist" - LD_LIBRARY_PATH: "$SNAP/usr/lib/x86_64-linux-gnu/:$LD_LIBRARY_PATH" CERTBOT_SNAPPED: "True" renew: command: certbot.wrapper -q renew @@ -32,7 +31,6 @@ apps: environment: PATH: "$SNAP/bin:$SNAP/usr/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games" AUGEAS_LENS_LIB: $SNAP/usr/share/augeas/lenses/dist - LD_LIBRARY_PATH: "$SNAP/usr/lib/x86_64-linux-gnu/:$LD_LIBRARY_PATH" CERTBOT_SNAPPED: "True" # Run approximately twice a day with randomization timer: 00:00~24:00/2 @@ -44,7 +42,7 @@ parts: source: . constraints: [$SNAPCRAFT_PART_SRC/snap-constraints.txt] python-packages: - - git+https://github.com/basak/python-augeas.git@snap + - git+https://github.com/certbot/python-augeas.git@certbot-patched - ./acme - ./certbot - ./certbot-apache @@ -54,7 +52,7 @@ parts: # Old versions of this file used to unstage # lib/python3.8/site-packages/augeas.py to avoid conflicts between # python-augeas 0.5.0 which was pinned in snap-constraints.txt and - # Robie's python-augeas fork which creates an auto-generated cffi file at + # our python-augeas fork which creates an auto-generated cffi file at # the same path. Since we've combined things in one part and removed the # python-augeas pinning, unstaging this file had a different, unintended # effect so we now stage the file to keep the auto-generated cffi file. @@ -73,10 +71,13 @@ parts: - python3-distutils - python3-pkg-resources - python3.8-minimal + # added for certbot.wrapper script: + - gawk # To build cryptography and cffi if needed build-packages: [gcc, libffi-dev, libssl-dev, git, libaugeas-dev, python3-dev] build-environment: - SNAPCRAFT_PYTHON_VENV_ARGS: --system-site-packages + - PIP_NO_BUILD_ISOLATION: "no" override-pull: | snapcraftctl pull cd $SNAPCRAFT_PART_SRC diff --git a/tests/letstest/apache2_targets.yaml b/tests/letstest/apache2_targets.yaml index d584705ca..2fa64568c 100644 --- a/tests/letstest/apache2_targets.yaml +++ b/tests/letstest/apache2_targets.yaml @@ -8,12 +8,6 @@ targets: type: ubuntu virt: hvm user: ubuntu - - ami: ami-008680ee60f23c94b - name: ubuntu20.04_arm64 - type: ubuntu - virt: hvm - user: ubuntu - machine_type: a1.medium - ami: ami-0545f7036167eb3aa name: ubuntu19.10 type: ubuntu @@ -36,6 +30,12 @@ targets: type: ubuntu virt: hvm user: admin + - ami: ami-0dcd54b7d2fff584f + name: debian10_arm64 + type: ubuntu + virt: hvm + user: admin + machine_type: a1.medium - ami: ami-003f19e0e687de1cd name: debian9 type: ubuntu diff --git a/tests/letstest/scripts/test_apache2.sh b/tests/letstest/scripts/test_apache2.sh index e39cee9ef..ba3d94379 100755 --- a/tests/letstest/scripts/test_apache2.sh +++ b/tests/letstest/scripts/test_apache2.sh @@ -12,7 +12,7 @@ then # For apache 2.4, set up ServerName sudo sed -i '/ServerName/ s/#ServerName/ServerName/' $CONFFILE sudo sed -i '/ServerName/ s/www.example.com/'$PUBLIC_HOSTNAME'/' $CONFFILE - if [ $(python3 -V 2>&1 | cut -d" " -f 2 | cut -d. -f1,2 | sed 's/\.//') -ne 38 ] + if [ $(python3 -V 2>&1 | cut -d" " -f 2 | cut -d. -f1,2 | sed 's/\.//') -lt 36 ] then # Upgrade python version using pyenv because py3.5 is deprecated # Don't upgrade if it's already 3.8 because pyenv doesn't work great on arm, and diff --git a/tests/letstest/targets.yaml b/tests/letstest/targets.yaml index 711053469..f6d3dd42f 100644 --- a/tests/letstest/targets.yaml +++ b/tests/letstest/targets.yaml @@ -8,12 +8,6 @@ targets: type: ubuntu virt: hvm user: ubuntu - - ami: ami-008680ee60f23c94b - name: ubuntu20.04_arm64 - type: ubuntu - virt: hvm - user: ubuntu - machine_type: a1.medium - ami: ami-0545f7036167eb3aa name: ubuntu19.10 type: ubuntu @@ -31,6 +25,12 @@ targets: type: ubuntu virt: hvm user: admin + - ami: ami-0dcd54b7d2fff584f + name: debian10_arm64 + type: ubuntu + virt: hvm + user: admin + machine_type: a1.medium #----------------------------------------------------------------------------- # Other Redhat Distros - ami: ami-0916c408cb02e310b