-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Extend containerd and ctr with support for (extensible) image encryption (OpenPGP, JWE, ...) #2532
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
2bc97e3 to
3247f81
Compare
api/services/images/v1/images.proto
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't make sense in the containerd model. The decryption/encryption should occur client-side, as part of unpacking/packing an image.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me explain this a little more clearly: unpacking of images isn't really performed in this service. This service is only dealing with metadata.
There may be some integration with the diff/applier service. I'll let @dmcgowan expand on that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though retrieving information about the layers should be done by containerd then for existing images? So it's not like containerd 'owns' the /var/lib/containerd/ directory and its subdirectories but ctr can also touch it for writing to? Also that bolt database is shared between containerd and ctr ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bolt is only for containerd daemon is accessed via the higher level interfaces, content, snapshot, etc....
The client does not need to touch /var/lib/containerd, but it is allowed to read content using the content store interface and mount snapshots it got from the snapshotter interface to do client side unpacking. As @stevvooe mentioned, we have a diff/applier service which allows doing this in the daemon to avoid roundtripping the entire content blobs, this interface is designed to be extensible via the content type, however we have not yet implemented plugins. The best solution maybe to add diff and apply plugins which this PoC could leverage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I must say I have had my challenges understanding the code at the beginning. Now I guess I may need some help here for seeing how things are done and probably also which code paths to go into. I guess the 'bootstrapping' has to start with creating a first encyrpted image. Would the encryption go into 'ctr images push' then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the 'bootstrapping' has to start with creating a first encyrpted image. Would the encryption go into 'ctr images push' then?
ctr images push does not build images. Doing this would look like...
- Read and parse the image manifest
- Read each content blob, encrypt, and write back to the content store
- Update the image manifest with the updated descriptors
- Write the image manifest back to the content store
- Create the image pointing at the new manifest descriptor
ctr images pushthe new image
It would be the expectation that a tool like buildkit would be responsible for creating content, or maybe a helper tool in ctr which did steps 1-5 above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what I can tell, we are doing steps 1-5 in images encrypt now, inside containerd though. The command also allows encryption of individual layers and for each platform separately, along with management of the recipients. Removing the RPC layer may be more or less straight forward if we can still write into the content store and bolt directly from the client, which I haven't tried. Though which command would this code go into if "ctr images push does not build images"? Either way, it would be good if it still allows management of recipients then as well as control over the encryption of individual layers.
3247f81 to
5843306
Compare
Codecov Report
@@ Coverage Diff @@
## master #2532 +/- ##
=========================================
- Coverage 43.96% 43.2% -0.77%
=========================================
Files 102 113 +11
Lines 10881 11904 +1023
=========================================
+ Hits 4784 5143 +359
- Misses 5362 5956 +594
- Partials 735 805 +70
Continue to review full report at Codecov.
|
f72ff86 to
b3b8511
Compare
|
The current implementation requires OpenPGP keys and results in an OpenPGP formatted blob for the wrapped keys and the bulk data. This could be abstracted to only require a public key for encryption as input and result in a common format for the wrapped session key (wrapped once for each recipient) and the symmetrically encrypted bulk data. However, I think this may require the definition of a new format since no standard exists that can accomodate any type of public key. I just want to mention this here. So, for OpenPGP RFC 4880 defines the wrapped session key as follows : For PKCS#7 the wrapped session key is defined like this: I would argue one could come up with a data structure like this here in exemplary JSON format that could accommodate any of these wrapped keys: For OpenPGP this would look like this: For PKCS#7 this would look like this: The image encryption process would create a list of the above two examples, one such map for each recipient and specific to the recipient's key. The JSON format is just an example here. The bulk data, which are the layer data, could then be written out in a format like this one that can be handled easily: The advantage of this type of format would be that this should allow us to handle any type of public key as input (OpenPGP, OpenSSL, x509, JWK, TPM1.2, TPM2, SmartCards, HSMs) and we can define the 'specifics' for the individual key type. In some cases 'specifics' could point to some sort of config file where the IP address is located to reach the device to handle the decryption. The specific technology would be required to unwrap the symmetric key using the private key. The symmetric key could then generically be used to decrypt the bulk data. This would avoid possibly different formats for the encrypted layer. The current OpenPGP implementation produces one such very specific format. |
|
The keyid in OpenPGP is used to look up the private key for decrypting the data (rather trying all the private keys one after another). Similarly all other technologies would have to provide a keyid of some format serving as a lookup index to quickly find the private key. Sometimes a password may be needed to access the key, which would be stored in some sort of config file. |
b3b8511 to
857edad
Compare
|
We could also use JWK to represent some of the keys. Some types of keys, like for hardware security modules or TPMs, could be provided as JWKs to the person encrypting the image and that person could then pass multiple of those on the command line. The JWK format for hardware devices' keys is not standardized in JWK specs afaics, though, but using JWK provides a path to support it in the future. Some other keys, like in the more common PEM format or for PGP recipients could still be passed directly. The client tool may need to detect the format: Internally we should probably convert the pem and pgp type of keys into a map[string]string like JWK and could then use JWK fields to represent the 'specifics' map above when storing metadata for the wrapped key. Though this is all outside some standard and would have to be agreed upon. |
|
JWE would probably be the best (unifying) standard to follow, though we probably couldn't follow it to the letter. The encrypted bulk data is part of the JSON there and in the layer case it would be a file. Also for the standards route I haven't seen OpenPGP type of keys representation in JWK format. |
857edad to
deac7e5
Compare
|
Next version supports recipients with different key types (so far: PEM,DER,OpenPGP) and supports JWE (PEM, DER) and OpenPGP (OpenPGP keys) for the encryption of the symmetric key that in turn handles the layer encryption: |
|
Encryption to PGP, JWE, and PKCS7 recipients: |
deac7e5 to
6bcaa39
Compare
91546d4 to
6efdf5b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't seem to find the definition of this function but I would prefer not to have this be a bool. There should be discrete encrypt/decrypt functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed this now so that images now exports EncryptImage and DecryptImage but then funnels right into the previous cryptImage function. The reason is that encryption and decryption share a lot of code. There are quite a few functions that start with crypt in the name but are not exported from images.
image.go
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These interface methods are way to specific for the image. Notice the other methods aren't specific to an implementation or specific image format. These should probably be set through the client or as part of an option set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think by now this could be a map[string]string where we pass the different types of decryption keys (gpg, pem/der keys) and other material (x509s).
images/encryption/blockcipher.go
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This package should be broken up such that the encryption package defines the interfaces primitives and the sub packages have the implementations. For example, you'd have image/encryption, image/encryption/gpg and image/encryption/pkcs7 and so on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes for the next version.
Allow encryption of images with elliptic curve keys using JWE. Signed-off-by: Stefan Berger <[email protected]>
Signed-off-by: Stefan Berger <[email protected]>
Signed-off-by: Stefan Berger <[email protected]>
Set ContentEncryptionAlgorithm = EncryptionAlgorithmAES128GCM to activate AES128GCM for encryption. Signed-off-by: Stefan Berger <[email protected]>
Signed-off-by: Harshal Patil <[email protected]>
errdefs/errors.go primarily wraps grpc related errors, so do not wrap the wrong passsord error there. Signed-off-by: Stefan Berger <[email protected]>
Use HasEncryptedLayer() to determine whether an image is encrypted. For this to work we also need to add the missing MediaType to the LayerInfos. Have the ctr image decrypt tool use this function rather than relying on the number of Annotations, which is the wrong indicator. Signed-off-by: Stefan Berger <[email protected]>
Roll back the extension of the Diff plugin's Apply() API call with the dcparameters and use a key in the Annotations map to pass a JSON representation of the dcparameters map ([string][][]bytes) in a temporary key "_dcparameters" that we insert before the grpc call and remove on the server side. Signed-off-by: Harshal Patil <[email protected]> Signed-off-by: Stefan Berger <[email protected]>
Properly clean up the decrypted image and do this synchronously so it is gone afterwards. Without this the ImageIsUnpacked test case would occasionally fail since the image wasn't entirely gone by the time that test case checked whether it is UnPacked(). Signed-off-by: Stefan Berger <[email protected]>
Signed-off-by: Stefan Berger <[email protected]>
This is a first step in the conversion of using the notion of message streams in the block cipher implementation. In this patch we convert the code to pass a content.ReadAt to the encryption function so that later on we can replace the symmetric cipher with an implementation that works on blocks read from this reader. Signed-off-by: Stefan Berger <[email protected]>
The block cipher functions now return a Reader that is wrapping an io.Reader and the Hash and size of the resulting layer. We need the latter two to be able to store the layer along with its size and the filename derived from the hash. Signed-off-by: Stefan Berger <[email protected]>
Implement support for ocispec.Descriptor.Size = 0 when writing the blob to the content store. We introduce a new function WriteLayer() that returns the size of the encrypted layer. Signed-off-by: Stefan Berger <[email protected]>
Signed-off-by: Stefan Berger <[email protected]>
Remove the ocispec.Descriptor from the WriteBlobN() parameter list since it holds no valid content that is needed for writing a layer blob. Signed-off-by: Stefan Berger <[email protected]>
Refactor the GCM block cipher to do all the work in the Reader's Read() function. This changes the semantics of the Size() function a bit, which will only return the size of the encrypted or decrypted data after Read() has been called for all the data. So the test cases need to be adjusted for this. Also errors related to decrypting the data with the wrong key only become apparent when calling Read(), which also requires test cases to be adapted. Signed-off-by: Stefan Berger <[email protected]>
Signed-off-by: Stefan Berger <[email protected]>
Use AES-SIV-CMAC-512 rather than AEAD GCM and get rid of the latter. The AES-SIV-CMAC-512 is used by reading chunks from the input stream, encrypting or decrypting the chunk, and then returning the data through the buffer passed to the Read() function. When encrypting a chunk the size of the encrypted chunk is a certain amount larger than the plain one. We have to make sure that the encrypted chunks passed back to the caller are given to us by the caller exactly the same when decrypting them. We make the chunksize of an encrypted chunk 1MB but only read plain data of the size 1MB - Overhead. Signed-off-by: Stefan Berger <[email protected]>
Signed-off-by: Stefan Berger <[email protected]>
Signed-off-by: Brandon Lum <[email protected]>
224a5a6 to
77a1977
Compare
|
Any reason to keep this PR open? The code seems out of date compared to the much more recent PRs which are in-flight. |
|
I think the only thing here is the pull path code. But I think at this point, with all the changes, we would have to end up rewriting a chunk of it anyway. I'm in favor of closing this and focusing on the other PRs. |
|
Per @lumjjb closing this PR as we have existing PRs for review and potential merge. All the discussion, code commits, and initial opening info describing the capabilities are still available, but the PR won't be in the open list. |
diff: opencontainers/runc@v1.1.14...v1.1.15 Release notes: - The -ENOSYS seccomp stub is now always generated for the native architecture that runc is running on. This is needed to work around some arguably specification-incompliant behaviour from Docker on architectures such as ppc64le, where the allowed architecture list is set to null. This ensures that we always generate at least one -ENOSYS stub for the native architecture even with these weird configs. (containerd#4391) - On a system with older kernel, reading /proc/self/mountinfo may skip some entries, as a consequence runc may not properly set mount propagation, causing container mounts leak onto the host mount namespace. (containerd#2404, containerd#4425) - In order to fix performance issues in the "lightweight" bindfd protection against [CVE-2019-5736], the temporary ro bind-mount of /proc/self/exe has been removed. runc now creates a binary copy in all cases. (containerd#4392, containerd#2532) Signed-off-by: Samuel Karp <[email protected]>
diff: opencontainers/runc@v1.1.14...v1.1.15 Release notes: - The -ENOSYS seccomp stub is now always generated for the native architecture that runc is running on. This is needed to work around some arguably specification-incompliant behaviour from Docker on architectures such as ppc64le, where the allowed architecture list is set to null. This ensures that we always generate at least one -ENOSYS stub for the native architecture even with these weird configs. (containerd#4391) - On a system with older kernel, reading /proc/self/mountinfo may skip some entries, as a consequence runc may not properly set mount propagation, causing container mounts leak onto the host mount namespace. (containerd#2404, containerd#4425) - In order to fix performance issues in the "lightweight" bindfd protection against [CVE-2019-5736], the temporary ro bind-mount of /proc/self/exe has been removed. runc now creates a binary copy in all cases. (containerd#4392, containerd#2532) Signed-off-by: Samuel Karp <[email protected]>
diff: opencontainers/runc@v1.1.14...v1.1.15 Release notes: - The -ENOSYS seccomp stub is now always generated for the native architecture that runc is running on. This is needed to work around some arguably specification-incompliant behaviour from Docker on architectures such as ppc64le, where the allowed architecture list is set to null. This ensures that we always generate at least one -ENOSYS stub for the native architecture even with these weird configs. (containerd#4391) - On a system with older kernel, reading /proc/self/mountinfo may skip some entries, as a consequence runc may not properly set mount propagation, causing container mounts leak onto the host mount namespace. (containerd#2404, containerd#4425) - In order to fix performance issues in the "lightweight" bindfd protection against [CVE-2019-5736], the temporary ro bind-mount of /proc/self/exe has been removed. runc now creates a binary copy in all cases. (containerd#4392, containerd#2532) Signed-off-by: Samuel Karp <[email protected]>
This series of patches implements support for image encryption in containerd and ctr based on OpenPGP (GnuPGP). It is a followup to an issue raised for the opencontainers image specification to support encrypted containers. This pull request is mean to start a discussion based on an implementation.
The following provides example steps for how to encrypt and decrypt images as well as manage the list of recipients that can decrypt an image. It is currently not possible to start an encrypted image right away but one has to go through the step of decryption first. Support for starting an encrypted image could be added following acceptance of an initial set of patches. Also, there are currently no test cases covering this code.
Regards,
Stefan, Brandon, and Harshal
Note: The demo below encrypts an image for
[email protected]for which the demonstrator also has the private key. You may want to create your own gpg private key, associate it with your email, and repeat the demo with your new key.Encrypted Container Images Containerd Demo
Overview
As in this issue, we propose a new media type for encrypted layers of a container image. This addition would facilitate the ecosystem for encrypted container images. This allows users with stricter trust requirements to be able ensure end-to-end encryption from build to runtime. In addition, it allows users to use a centralized managed repository (i.e. Docker Hub) without any risk of their images being compromised.
Today, we have implemented an encrypt/decrypt interface in containerd to perform the cryptographic operations on existing images based on the
openpgpprotocol RFC4880. The client,ctr, initiating the procedures manages the keys via gpg/gpg2, and is able to encrypt specific layers, as well as add/remove recipients from images.The current implementation can be found here:
Basic Functions Demo
We will demonstrate the ability to encrypt the public
alpine:latestimage, and push the encrypted image to a docker registry. We will then show the pulling of the encrypted image from the repository and decryption of the image for use.Containerd client encrypt
We use existing
ctr images pullto get an existing OCI image.The encryption scheme is based off OpenPGP. Thus, we allow the use of
gpgto manage keys. In the event that we want to perform encryption for a trusted party, we want to retrieve their public key so that we can securely share encryption keys with them.First, we use the
gpgclient to retrieve the public key of a recipient we want to create an encrypted image for. In this case, we want to create an encrypted image forBrandon Lum <[email protected]>.We then signify that we want to encrypt the image
docker.io/library/alpine:latestfor user[email protected]and tag the new encrypted image asdocker.io/lumjjb/alpine:enc.We then push the encrypted image to the Docker Hub registry.
Containerd client decrypt
As user,
Brandon Lum <[email protected]>, we will pull, decrypt and use the encryptedalpine:latestimage.We first pull the encrypted image we pushed earlier:
We then verify that we have the private key necessary to decrypt the wrapped keys, and Proceed with performing a decryption. We note that our private key is passphrase protected. And thus we are asked to enter our passphrase to decrypt the encrypted image. In the command we specify that we would like to decrypt the image
docker.io/lumjjb/alpine:encand save the decrypted image asdocker.io/lumjjb/alpine:plain. We note that the decryption process will automatically derive what keys arerequired from the annotations within the layers' manifests.
Additional functionality
We have implemented additional functionality to provide convenience and support for various usecases:
Show layer info.
We allow users to show the layer information of an image - as well as the keys associated with the image. This will allow the user to check what keys are required or who is allowed to decrypt the image.
Encrypt specific layer
Ability to encrypt specific layers have also been added. This will allow the user to benefit from deduplication of layers of non-secret code/data. I.e. Building a secret application on top of an
ubuntuimage, theubuntupart of the image is not secret. Layers are indexed bottom up, with negative layers pointing to top layers. In this example, we specify the encryption of the topmost layer by passing in the--layer -1flag.Add recipients
By issuing the encrypt command on an already existing image, it allows the addition of recipients. This allows the adding of recipient wrapped keys to the image without re-encryption of the image. Thus allowing the addition of recipients to benefit from re-uploading the large encrypted blob. Because we need the symmetric key needs to be re-wrapped, the adding of recipients requires access to at least one of the existing wrapped keys of the image.
Remove recipients
This allows the removal of a recipient of an encrypted image. This is done by issuing the
--removeflag.Next steps
Implement handling of encrypted container mediatype by containerd
Currently, the way decryption of an image is done is via the use of an explicit
ctrclient. We would like for runtimes to be able to decrypt the images directly when running them. i.e. containerd daemon should know how to handle the mediaType of+pgpand decrypt the image as part of the image unpacking process.Passing of keys to runtime
We note that when having containerd handle the encrypted mediatype, it would have to be able to access keys for the decryption process. However, we need to be able to properly handle permissions of access of keys.