launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Approve code



Diff comments:

> 
> === 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-06-27 21:46:50 +0000
> @@ -0,0 +1,122 @@
> +# 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,
> +    )
> +
> +
> +def _make_public_key(public_key_bytes):
> +    pass

This seems to be unused.

> +
> +
> +@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)
> 
> === modified file 'lib/lp/snappy/model/snapstoreclient.py'
> --- lib/lp/snappy/model/snapstoreclient.py	2019-06-20 16:33:19 +0000
> +++ lib/lp/snappy/model/snapstoreclient.py	2019-06-27 21:46:50 +0000
> @@ -320,7 +349,15 @@
>                  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"]
> +            container = getUtility(IEncryptedContainer, "snap-store-secrets")
> +            if container.can_encrypt:
> +                new_secrets["discharge_encrypted"] = (
> +                    removeSecurityProxy(container.encrypt(
> +                        response_data["discharge_macaroon"].encode("UTF-8"))))
> +                new_secrets.pop("discharge", None)
> +            else:
> +                new_secrets["discharge"] = response_data["discharge_macaroon"]
> +                new_secrets.pop("discharge_encrypted", None)

It probably makes sense to factor this out.

>              snap.store_secrets = new_secrets
>          except requests.HTTPError as e:
>              raise cls._makeSnapStoreError(BadRefreshResponse, e)


-- 
https://code.launchpad.net/~cjwatson/launchpad/snap-store-secrets-encrypt/+merge/369428
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.