Merge lp:~cjwatson/launchpad/snap-store-secrets-encrypt into lp:launchpad

[Colin Watson]

Support encrypting snap store discharge macaroons at rest.

We should be encrypting sensitive data such as authentication tokens at rest in the database, and this adds basic general support for doing that and sets it up for the SSO discharge macaroons used for uploading snaps to the store.

These discharge macaroons are refreshed every so often, and that will encrypt the refreshed tokens if a suitable public key is configured.

The JSON-serialised encrypted format includes the public key in order to support future key rotation (by configuring multiple key pairs and using the corresponding private key for decryption), although I haven't actually written the code for that yet.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/lp/services/config/schema-lazr.conf'
--- lib/lp/services/config/schema-lazr.conf	2019-07-11 13:27:09 +0000
+++ lib/lp/services/config/schema-lazr.conf	2019-08-21 09:32:48 +0000
@@ -1896,6 +1896,16 @@
 # The store's search URL endpoint.
 store_search_url: none
 
+# Base64-encoded NaCl private key for decrypting store upload tokens.
+# This should only be set in secret overlays on systems that need to perform
+# store uploads on behalf of users.
+# datatype: string
+store_secrets_private_key: none
+
+# Base64-encoded NaCl public key for encrypting store upload tokens.
+# datatype: string
+store_secrets_public_key: none
+
 [process-job-source-groups]
 # This section is used by cronscripts/process-job-source-groups.py.
 dbuser: process-job-source-groups

=== added directory 'lib/lp/services/crypto'
=== added file 'lib/lp/services/crypto/__init__.py'
=== added file 'lib/lp/services/crypto/interfaces.py'
--- lib/lp/services/crypto/interfaces.py	1970-01-01 00:00:00 +0000
+++ lib/lp/services/crypto/interfaces.py	2019-08-21 09:32:48 +0000
@@ -0,0 +1,56 @@
+# Copyright 2019 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Interface to data encrypted at rest using configured keys."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+    'CryptoError',
+    'IEncryptedContainer',
+    ]
+
+from zope.interface import (
+    Attribute,
+    Interface,
+    )
+
+
+class CryptoError(Exception):
+    pass
+
+
+class IEncryptedContainer(Interface):
+    """Interface to a container that can encrypt and decrypt data."""
+
+    can_encrypt = Attribute(
+        "True iff this container has the configuration it needs to encrypt "
+        "data.")
+
+    def encrypt(data):
+        """Encrypt a blob of data to a JSON-serialisable form.
+
+        This includes the public key to ease future key rotation.
+
+        :param data: An unencrypted byte string to encrypt.
+        :return: A tuple of (base64-encoded public key, base64-encoded
+            encrypted text string).
+        :raises RuntimeError: if no public key is configured for this
+            container.
+        """
+
+    can_decrypt = Attribute(
+        "True iff this container has the configuration it needs to decrypt "
+        "data.")
+
+    def decrypt(data):
+        """Decrypt data that was encrypted by L{encrypt}.
+
+        :param data: A tuple of (base64-encoded public key, base64-encoded
+            encrypted text string) to decrypt.
+        :return: An unencrypted byte string.
+        :raises ValueError: if no private key is configured for this container
+            that corresponds to the requested public key.
+        :raises CryptoError: if decryption failed.
+        """

=== added file 'lib/lp/services/crypto/model.py'
--- lib/lp/services/crypto/model.py	1970-01-01 00:00:00 +0000
+++ lib/lp/services/crypto/model.py	2019-08-21 09:32:48 +0000
@@ -0,0 +1,118 @@
+# Copyright 2019 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""A container for data encrypted at rest using configured keys."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+    'NaClEncryptedContainerBase',
+    ]
+
+import base64
+
+from nacl.exceptions import CryptoError as NaClCryptoError
+from nacl.public import (
+    PrivateKey,
+    PublicKey,
+    SealedBox,
+    )
+import six
+from zope.interface import implementer
+
+from lp.services.crypto.interfaces import (
+    CryptoError,
+    IEncryptedContainer,
+    )
+
+
+@implementer(IEncryptedContainer)
+class NaClEncryptedContainerBase:
+    """A container that can encrypt and decrypt data using NaCl.
+
+    See `IEncryptedContainer`.
+    """
+
+    @property
+    def public_key_bytes(self):
+        """The serialised public key as a byte string.
+
+        Concrete implementations must provide this.
+        """
+        raise NotImplementedError
+
+    @property
+    def public_key(self):
+        """The public key as a L{nacl.public.PublicKey}."""
+        if self.public_key_bytes is not None:
+            try:
+                return PublicKey(self.public_key_bytes)
+            except NaClCryptoError as e:
+                six.raise_from(CryptoError(str(e)), e)
+        else:
+            return None
+
+    @property
+    def can_encrypt(self):
+        try:
+            return self.public_key is not None
+        except CryptoError:
+            return False
+
+    def encrypt(self, data):
+        """See `IEncryptedContainer`."""
+        if self.public_key is None:
+            raise RuntimeError("No public key configured")
+        try:
+            data_encrypted = SealedBox(self.public_key).encrypt(data)
+        except NaClCryptoError as e:
+            six.raise_from(CryptoError(str(e)), e)
+        return (
+            base64.b64encode(self.public_key_bytes).decode("UTF-8"),
+            base64.b64encode(data_encrypted).decode("UTF-8"))
+
+    @property
+    def private_key_bytes(self):
+        """The serialised private key as a byte string.
+
+        Concrete implementations must provide this.
+        """
+        raise NotImplementedError
+
+    @property
+    def private_key(self):
+        """The private key as a L{nacl.public.PrivateKey}."""
+        if self.private_key_bytes is not None:
+            try:
+                return PrivateKey(self.private_key_bytes)
+            except NaClCryptoError as e:
+                six.raise_from(CryptoError(str(e)), e)
+        else:
+            return None
+
+    @property
+    def can_decrypt(self):
+        try:
+            return self.private_key is not None
+        except CryptoError:
+            return False
+
+    def decrypt(self, data):
+        """See `IEncryptedContainer`."""
+        public_key, encrypted = data
+        try:
+            public_key_bytes = base64.b64decode(public_key.encode("UTF-8"))
+            encrypted_bytes = base64.b64decode(encrypted.encode("UTF-8"))
+        except TypeError as e:
+            six.raise_from(CryptoError(str(e)), e)
+        if public_key_bytes != self.public_key_bytes:
+            raise ValueError(
+                "Public key %r does not match configured public key %r" %
+                (public_key_bytes, self.public_key_bytes))
+        if self.private_key is None:
+            raise ValueError("No private key configured")
+        try:
+            return SealedBox(self.private_key).decrypt(encrypted_bytes)
+        except NaClCryptoError as e:
+            six.raise_from(CryptoError(str(e)), e)

=== added directory 'lib/lp/services/crypto/tests'
=== added file 'lib/lp/services/crypto/tests/__init__.py'
=== added file 'lib/lp/services/crypto/tests/test_model.py'
--- lib/lp/services/crypto/tests/test_model.py	1970-01-01 00:00:00 +0000
+++ lib/lp/services/crypto/tests/test_model.py	2019-08-21 09:32:48 +0000
@@ -0,0 +1,83 @@
+# Copyright 2019 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for encrypted data containers."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+
+from nacl.public import PrivateKey
+
+from lp.services.crypto.interfaces import CryptoError
+from lp.services.crypto.model import NaClEncryptedContainerBase
+from lp.testing import TestCase
+from lp.testing.layers import ZopelessLayer
+
+
+class FakeEncryptedContainer(NaClEncryptedContainerBase):
+
+    def __init__(self, public_key_bytes, private_key_bytes=None):
+        self._public_key_bytes = public_key_bytes
+        self._private_key_bytes = private_key_bytes
+
+    @property
+    def public_key_bytes(self):
+        return self._public_key_bytes
+
+    @property
+    def private_key_bytes(self):
+        return self._private_key_bytes
+
+
+class TestNaClEncryptedContainerBase(TestCase):
+
+    layer = ZopelessLayer
+
+    def test_public_key_valid(self):
+        public_key = PrivateKey.generate().public_key
+        container = FakeEncryptedContainer(bytes(public_key))
+        self.assertEqual(public_key, container.public_key)
+        self.assertTrue(container.can_encrypt)
+
+    def test_public_key_invalid(self):
+        container = FakeEncryptedContainer(b"nonsense")
+        self.assertRaises(CryptoError, getattr, container, "public_key")
+        self.assertFalse(container.can_encrypt)
+
+    def test_public_key_unset(self):
+        container = FakeEncryptedContainer(None)
+        self.assertIsNone(container.public_key)
+        self.assertFalse(container.can_encrypt)
+
+    def test_encrypt_without_private_key(self):
+        # Encryption only requires the public key, not the private key.
+        public_key = PrivateKey.generate().public_key
+        container = FakeEncryptedContainer(bytes(public_key))
+        self.assertIsNotNone(container.encrypt(b"plaintext"))
+
+    def test_private_key_valid(self):
+        private_key = PrivateKey.generate()
+        container = FakeEncryptedContainer(
+            bytes(private_key.public_key), bytes(private_key))
+        self.assertEqual(private_key, container.private_key)
+        self.assertTrue(container.can_decrypt)
+
+    def test_private_key_invalid(self):
+        public_key = PrivateKey.generate().public_key
+        container = FakeEncryptedContainer(bytes(public_key), b"nonsense")
+        self.assertRaises(CryptoError, getattr, container, "private_key")
+        self.assertFalse(container.can_decrypt)
+
+    def test_private_key_unset(self):
+        public_key = PrivateKey.generate().public_key
+        container = FakeEncryptedContainer(bytes(public_key), None)
+        self.assertIsNone(container.private_key)
+        self.assertFalse(container.can_decrypt)
+
+    def test_encrypt_decrypt(self):
+        private_key = PrivateKey.generate()
+        container = FakeEncryptedContainer(
+            bytes(private_key.public_key), bytes(private_key))
+        self.assertEqual(
+            b"plaintext", container.decrypt(container.encrypt(b"plaintext")))

=== modified file 'lib/lp/snappy/configure.zcml'
--- lib/lp/snappy/configure.zcml	2019-03-12 19:18:19 +0000
+++ lib/lp/snappy/configure.zcml	2019-08-21 09:32:48 +0000
@@ -49,6 +49,14 @@
             interface="lp.snappy.interfaces.snap.ISnapBuildRequest" />
     </class>
 
+    <!-- SnapStoreSecretsEncryptedContainer -->
+    <securedutility
+        class="lp.snappy.model.snap.SnapStoreSecretsEncryptedContainer"
+        provides="lp.services.crypto.interfaces.IEncryptedContainer"
+        name="snap-store-secrets">
+        <allow interface="lp.services.crypto.interfaces.IEncryptedContainer"/>
+    </securedutility>
+
     <!-- SnapBuild -->
     <class class="lp.snappy.model.snapbuild.SnapBuild">
         <require

=== modified file 'lib/lp/snappy/model/snap.py'
--- lib/lp/snappy/model/snap.py	2019-07-15 17:15:25 +0000
+++ lib/lp/snappy/model/snap.py	2019-08-21 09:32:48 +0000
@@ -6,6 +6,7 @@
     'Snap',
     ]
 
+import base64
 from collections import OrderedDict
 from datetime import (
     datetime,
@@ -100,6 +101,8 @@
 from lp.registry.model.series import ACTIVE_STATUSES
 from lp.registry.model.teammembership import TeamParticipation
 from lp.services.config import config
+from lp.services.crypto.interfaces import IEncryptedContainer
+from lp.services.crypto.model import NaClEncryptedContainerBase
 from lp.services.database.bulk import load_related
 from lp.services.database.constants import (
     DEFAULT,
@@ -602,9 +605,18 @@
                     "beginAuthorization must be called before "
                     "completeAuthorization.")
         if discharge_macaroon is not None:
-            self.store_secrets["discharge"] = discharge_macaroon
+            container = getUtility(IEncryptedContainer, "snap-store-secrets")
+            if container.can_encrypt:
+                self.store_secrets["discharge_encrypted"] = (
+                    removeSecurityProxy(container.encrypt(
+                        discharge_macaroon.encode("UTF-8"))))
+                self.store_secrets.pop("discharge", None)
+            else:
+                self.store_secrets["discharge"] = discharge_macaroon
+                self.store_secrets.pop("discharge_encrypted", None)
         else:
             self.store_secrets.pop("discharge", None)
+            self.store_secrets.pop("discharge_encrypted", None)
 
     @property
     def can_upload_to_store(self):
@@ -617,7 +629,8 @@
             return False
         root_macaroon = Macaroon.deserialize(self.store_secrets["root"])
         if (self.extractSSOCaveats(root_macaroon) and
-                "discharge" not in self.store_secrets):
+                "discharge" not in self.store_secrets and
+                "discharge_encrypted" not in self.store_secrets):
             return False
         return True
 
@@ -1415,3 +1428,23 @@
     def empty_list(self):
         """See `ISnapSet`."""
         return []
+
+
+@implementer(IEncryptedContainer)
+class SnapStoreSecretsEncryptedContainer(NaClEncryptedContainerBase):
+
+    @property
+    def public_key_bytes(self):
+        if config.snappy.store_secrets_public_key is not None:
+            return base64.b64decode(
+                config.snappy.store_secrets_public_key.encode("UTF-8"))
+        else:
+            return None
+
+    @property
+    def private_key_bytes(self):
+        if config.snappy.store_secrets_private_key is not None:
+            return base64.b64decode(
+                config.snappy.store_secrets_private_key.encode("UTF-8"))
+        else:
+            return None

=== modified file 'lib/lp/snappy/model/snapstoreclient.py'
--- lib/lp/snappy/model/snapstoreclient.py	2019-07-29 10:52:35 +0000
+++ lib/lp/snappy/model/snapstoreclient.py	2019-08-21 09:32:48 +0000
@@ -30,6 +30,10 @@
 from zope.security.proxy import removeSecurityProxy
 
 from lp.services.config import config
+from lp.services.crypto.interfaces import (
+    CryptoError,
+    IEncryptedContainer,
+    )
 from lp.services.features import getFeatureFlag
 from lp.services.memcache.interfaces import IMemcacheClient
 from lp.services.scripts import log
@@ -154,6 +158,45 @@
         return r
 
 
+def _get_discharge_macaroon_raw(snap):
+    """Get the serialised discharge macaroon for a snap, if any.
+
+    This copes with either unencrypted (the historical default) or encrypted
+    macaroons.
+    """
+    if snap.store_secrets is None:
+        raise AssertionError("snap.store_secrets is None")
+    if "discharge_encrypted" in snap.store_secrets:
+        container = getUtility(IEncryptedContainer, "snap-store-secrets")
+        try:
+            return container.decrypt(
+                snap.store_secrets["discharge_encrypted"]).decode("UTF-8")
+        except CryptoError as e:
+            raise UnauthorizedUploadResponse(
+                "Failed to decrypt discharge macaroon: %s" % e)
+    else:
+        return snap.store_secrets.get("discharge")
+
+
+def _set_discharge_macaroon_raw(snap, discharge_macaroon_raw):
+    """Set the serialised discharge macaroon for a snap.
+
+    The macaroon is encrypted if possible.
+    """
+    # Set a new dict here to avoid problems with security proxies.
+    new_secrets = dict(snap.store_secrets)
+    container = getUtility(IEncryptedContainer, "snap-store-secrets")
+    if container.can_encrypt:
+        new_secrets["discharge_encrypted"] = (
+            removeSecurityProxy(container.encrypt(
+                discharge_macaroon_raw.encode("UTF-8"))))
+        new_secrets.pop("discharge", None)
+    else:
+        new_secrets["discharge"] = discharge_macaroon_raw
+        new_secrets.pop("discharge_encrypted", None)
+    snap.store_secrets = new_secrets
+
+
 # Hardcoded fallback.
 _default_store_channels = [
     {"name": "candidate", "display_name": "Candidate"},
@@ -257,6 +300,7 @@
         snap = snapbuild.snap
         assert config.snappy.store_url is not None
         assert snap.store_name is not None
+        assert snap.store_secrets is not None
         assert snapbuild.date_started is not None
         upload_url = urlappend(config.snappy.store_url, "dev/api/snap-push/")
         data = {
@@ -276,12 +320,11 @@
         # XXX cjwatson 2016-05-09: This should add timeline information, but
         # that's currently difficult in jobs.
         try:
-            assert snap.store_secrets is not None
             response = urlfetch(
                 upload_url, method="POST", json=data,
                 auth=MacaroonAuth(
                     snap.store_secrets["root"],
-                    snap.store_secrets.get("discharge")))
+                    _get_discharge_macaroon_raw(snap)))
             response_data = response.json()
             return response_data["status_details_url"]
         except requests.HTTPError as e:
@@ -312,18 +355,20 @@
         assert snap.store_secrets is not None
         refresh_url = urlappend(
             config.launchpad.openid_provider_root, "api/v2/tokens/refresh")
-        data = {"discharge_macaroon": snap.store_secrets["discharge"]}
+        discharge_macaroon_raw = _get_discharge_macaroon_raw(snap)
+        if discharge_macaroon_raw is None:
+            raise UnauthorizedUploadResponse(
+                "Tried to refresh discharge for snap with no discharge "
+                "macaroon")
+        data = {"discharge_macaroon": discharge_macaroon_raw}
         try:
             response = urlfetch(refresh_url, method="POST", json=data)
-            response_data = response.json()
-            if "discharge_macaroon" not in response_data:
-                raise BadRefreshResponse(response.text)
-            # Set a new dict here to avoid problems with security proxies.
-            new_secrets = dict(snap.store_secrets)
-            new_secrets["discharge"] = response_data["discharge_macaroon"]
-            snap.store_secrets = new_secrets
         except requests.HTTPError as e:
             raise cls._makeSnapStoreError(BadRefreshResponse, e)
+        response_data = response.json()
+        if "discharge_macaroon" not in response_data:
+            raise BadRefreshResponse(response.text)
+        _set_discharge_macaroon_raw(snap, response_data["discharge_macaroon"])
 
     @classmethod
     def refreshIfNecessary(cls, snap, f, *args, **kwargs):

=== modified file 'lib/lp/snappy/tests/test_snap.py'
--- lib/lp/snappy/tests/test_snap.py	2019-07-15 17:15:25 +0000
+++ lib/lp/snappy/tests/test_snap.py	2019-08-21 09:32:48 +0000
@@ -7,6 +7,7 @@
 
 __metaclass__ = type
 
+import base64
 from datetime import (
     datetime,
     timedelta,
@@ -21,6 +22,7 @@
     MockPatch,
     )
 import iso8601
+from nacl.public import PrivateKey
 from pymacaroons import Macaroon
 import pytz
 import responses
@@ -67,6 +69,7 @@
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.registry.interfaces.series import SeriesStatus
 from lp.services.config import config
+from lp.services.crypto.interfaces import IEncryptedContainer
 from lp.services.database.constants import (
     ONE_DAY_AGO,
     UTC_NOW,
@@ -3232,6 +3235,37 @@
         with person_logged_in(self.person):
             self.assertEqual({"root": "dummy-root"}, snap.store_secrets)
 
+    def test_completeAuthorization_encrypted(self):
+        private_key = PrivateKey.generate()
+        self.pushConfig(
+            "snappy",
+            store_secrets_public_key=base64.b64encode(
+                bytes(private_key.public_key)).decode("UTF-8"))
+        with admin_logged_in():
+            snappy_series = self.factory.makeSnappySeries()
+        snap = self.factory.makeSnap(
+            registrant=self.person, store_upload=True,
+            store_series=snappy_series,
+            store_name=self.factory.getUniqueUnicode(),
+            store_secrets={"root": "dummy-root"})
+        snap_url = api_url(snap)
+        logout()
+        response = self.webservice.named_post(
+            snap_url, "completeAuthorization",
+            discharge_macaroon="dummy-discharge")
+        self.assertEqual(200, response.status)
+        self.pushConfig(
+            "snappy",
+            store_secrets_private_key=base64.b64encode(
+                bytes(private_key)).decode("UTF-8"))
+        container = getUtility(IEncryptedContainer, "snap-store-secrets")
+        with person_logged_in(self.person):
+            self.assertThat(snap.store_secrets, MatchesDict({
+                "root": Equals("dummy-root"),
+                "discharge_encrypted": AfterPreprocessing(
+                    container.decrypt, Equals("dummy-discharge")),
+                }))
+
     def makeBuildableDistroArchSeries(self, **kwargs):
         das = self.factory.makeDistroArchSeries(**kwargs)
         fake_chroot = self.factory.makeLibraryFileAlias(

=== modified file 'lib/lp/snappy/tests/test_snapstoreclient.py'
--- lib/lp/snappy/tests/test_snapstoreclient.py	2019-07-29 10:52:35 +0000
+++ lib/lp/snappy/tests/test_snapstoreclient.py	2019-08-21 09:32:48 +0000
@@ -14,6 +14,7 @@
 import json
 
 from lazr.restful.utils import get_current_browser_request
+from nacl.public import PrivateKey
 from pymacaroons import (
     Macaroon,
     Verifier,
@@ -34,9 +35,11 @@
     )
 import transaction
 from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
 
 from lp.buildmaster.enums import BuildStatus
 from lp.services.config import config
+from lp.services.crypto.interfaces import IEncryptedContainer
 from lp.services.features.testing import FeatureFixture
 from lp.services.log.logger import BufferLogger
 from lp.services.memcache.interfaces import IMemcacheClient
@@ -229,7 +232,7 @@
             ]
         self.channels_memcache_key = "search.example:channels".encode("UTF-8")
 
-    def _make_store_secrets(self):
+    def _make_store_secrets(self, encrypted=False):
         self.root_key = hashlib.sha256(
             self.factory.getUniqueString()).hexdigest()
         root_macaroon = Macaroon(key=self.root_key)
@@ -241,10 +244,15 @@
         unbound_discharge_macaroon = Macaroon(
             location="sso.example", key=self.discharge_key,
             identifier=self.discharge_caveat_id)
-        return {
-            "root": root_macaroon.serialize(),
-            "discharge": unbound_discharge_macaroon.serialize(),
-            }
+        secrets = {"root": root_macaroon.serialize()}
+        if encrypted:
+            container = getUtility(IEncryptedContainer, "snap-store-secrets")
+            secrets["discharge_encrypted"] = (
+                removeSecurityProxy(container.encrypt(
+                    unbound_discharge_macaroon.serialize().encode("UTF-8"))))
+        else:
+            secrets["discharge"] = unbound_discharge_macaroon.serialize()
+        return secrets
 
     def _addUnscannedUploadResponse(self):
         responses.add(
@@ -334,9 +342,9 @@
             self.client.requestPackageUploadPermission,
             snappy_series, "test-snap")
 
-    def makeUploadableSnapBuild(self, store_secrets=None):
+    def makeUploadableSnapBuild(self, store_secrets=None, encrypted=False):
         if store_secrets is None:
-            store_secrets = self._make_store_secrets()
+            store_secrets = self._make_store_secrets(encrypted=encrypted)
         snap = self.factory.makeSnap(
             store_upload=True,
             store_series=self.factory.makeSnappySeries(name="rolling"),
@@ -458,6 +466,46 @@
             ]))
 
     @responses.activate
+    def test_upload_encrypted_discharge(self):
+        private_key = PrivateKey.generate()
+        self.pushConfig(
+            "snappy",
+            store_secrets_public_key=base64.b64encode(
+                bytes(private_key.public_key)).decode("UTF-8"),
+            store_secrets_private_key=base64.b64encode(
+                bytes(private_key)).decode("UTF-8"))
+        snapbuild = self.makeUploadableSnapBuild(encrypted=True)
+        transaction.commit()
+        self._addUnscannedUploadResponse()
+        self._addSnapPushResponse()
+        with dbuser(config.ISnapStoreUploadJobSource.dbuser):
+            self.assertEqual(
+                "http://sca.example/dev/api/snaps/1/builds/1/status",
+                self.client.upload(snapbuild))
+        requests = [call.request for call in responses.calls]
+        self.assertThat(requests, MatchesListwise([
+            RequestMatches(
+                url=Equals("http://updown.example/unscanned-upload/"),
+                method=Equals("POST"),
+                form_data={
+                    "binary": MatchesStructure.byEquality(
+                        name="binary", filename="test-snap.snap",
+                        value="dummy snap content",
+                        type="application/octet-stream",
+                        )}),
+            RequestMatches(
+                url=Equals("http://sca.example/dev/api/snap-push/"),
+                method=Equals("POST"),
+                headers=ContainsDict(
+                    {"Content-Type": Equals("application/json")}),
+                auth=("Macaroon", MacaroonsVerify(self.root_key)),
+                json_data={
+                    "name": "test-snap", "updown_id": 1, "series": "rolling",
+                    "built_at": snapbuild.date_started.isoformat(),
+                    }),
+            ]))
+
+    @responses.activate
     def test_upload_unauthorized(self):
         store_secrets = self._make_store_secrets()
         snapbuild = self.makeUploadableSnapBuild(store_secrets=store_secrets)
@@ -504,6 +552,39 @@
             snapbuild.snap.store_secrets["discharge"])
 
     @responses.activate
+    def test_upload_needs_encrypted_discharge_macaroon_refresh(self):
+        private_key = PrivateKey.generate()
+        self.pushConfig(
+            "snappy",
+            store_secrets_public_key=base64.b64encode(
+                bytes(private_key.public_key)).decode("UTF-8"),
+            store_secrets_private_key=base64.b64encode(
+                bytes(private_key)).decode("UTF-8"))
+        store_secrets = self._make_store_secrets(encrypted=True)
+        snapbuild = self.makeUploadableSnapBuild(store_secrets=store_secrets)
+        transaction.commit()
+        self._addUnscannedUploadResponse()
+        responses.add(
+            "POST", "http://sca.example/dev/api/snap-push/", status=401,
+            headers={"WWW-Authenticate": "Macaroon needs_refresh=1"})
+        self._addMacaroonRefreshResponse()
+        self._addSnapPushResponse()
+        with dbuser(config.ISnapStoreUploadJobSource.dbuser):
+            self.assertEqual(
+                "http://sca.example/dev/api/snaps/1/builds/1/status",
+                self.client.upload(snapbuild))
+        requests = [call.request for call in responses.calls]
+        self.assertThat(requests, MatchesListwise([
+            MatchesStructure.byEquality(path_url="/unscanned-upload/"),
+            MatchesStructure.byEquality(path_url="/dev/api/snap-push/"),
+            MatchesStructure.byEquality(path_url="/api/v2/tokens/refresh"),
+            MatchesStructure.byEquality(path_url="/dev/api/snap-push/"),
+            ]))
+        self.assertNotEqual(
+            store_secrets["discharge_encrypted"],
+            snapbuild.snap.store_secrets["discharge_encrypted"])
+
+    @responses.activate
     def test_upload_unsigned_agreement(self):
         store_secrets = self._make_store_secrets()
         snapbuild = self.makeUploadableSnapBuild(store_secrets=store_secrets)
@@ -553,6 +634,36 @@
             store_secrets["discharge"], snap.store_secrets["discharge"])
 
     @responses.activate
+    def test_refresh_encrypted_discharge_macaroon(self):
+        private_key = PrivateKey.generate()
+        self.pushConfig(
+            "snappy",
+            store_secrets_public_key=base64.b64encode(
+                bytes(private_key.public_key)).decode("UTF-8"),
+            store_secrets_private_key=base64.b64encode(
+                bytes(private_key)).decode("UTF-8"))
+        store_secrets = self._make_store_secrets(encrypted=True)
+        snap = self.factory.makeSnap(
+            store_upload=True,
+            store_series=self.factory.makeSnappySeries(name="rolling"),
+            store_name="test-snap", store_secrets=store_secrets)
+        self._addMacaroonRefreshResponse()
+        with dbuser(config.ISnapStoreUploadJobSource.dbuser):
+            self.client.refreshDischargeMacaroon(snap)
+        container = getUtility(IEncryptedContainer, "snap-store-secrets")
+        self.assertThat(responses.calls[-1].request, RequestMatches(
+            url=Equals("http://sso.example/api/v2/tokens/refresh"),
+            method=Equals("POST"),
+            headers=ContainsDict({"Content-Type": Equals("application/json")}),
+            json_data={
+                "discharge_macaroon": container.decrypt(
+                    store_secrets["discharge_encrypted"]).decode("UTF-8"),
+                }))
+        self.assertNotEqual(
+            store_secrets["discharge_encrypted"],
+            snap.store_secrets["discharge_encrypted"])
+
+    @responses.activate
     def test_checkStatus_pending(self):
         status_url = "http://sca.example/dev/api/snaps/1/builds/1/status"
         responses.add(