Merge branch 'master' into remove-setuptools-test

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)

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."""

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."""

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 "$@"

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.
 

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"),

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)

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
 
 

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

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()

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):

snap/hooks/configure

@@ -1,3 +1,3 @@
-#!/bin/bash -e
+#!/bin/sh -e
 
 exit 0

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

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

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

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

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