add imgcrypt stream processors to the default config by AkihiroSuda · Pull Request #5135 · containerd/containerd

AkihiroSuda · 2021-03-08T09:10:59Z

Enable the following config by default:

version = 2

[plugins."io.containerd.grpc.v1.cri".image_decryption]
  key_model = "node"

[stream_processors]
  [stream_processors."io.containerd.ocicrypt.decoder.v1.tar.gzip"]
    accepts = ["application/vnd.oci.image.layer.v1.tar+gzip+encrypted"]
    returns = "application/vnd.oci.image.layer.v1.tar+gzip"
    path = "ctd-decoder"
    args = ["--decryption-keys-path", "/etc/containerd/ocicrypt/keys"]
    env = ["OCICRYPT_KEYPROVIDER_CONFIG=/etc/containerd/ocicrypt/ocicrypt_keyprovider.conf"]

  [stream_processors."io.containerd.ocicrypt.decoder.v1.tar"]
    accepts = ["application/vnd.oci.image.layer.v1.tar+encrypted"]
    returns = "application/vnd.oci.image.layer.v1.tar"
    path = "ctd-decoder"
    args = ["--decryption-keys-path", "/etc/containerd/ocicrypt/keys"]
    env = ["OCICRYPT_KEYPROVIDER_CONFIG=/etc/containerd/ocicrypt/ocicrypt_keyprovider.conf"]

Fix #5128

AkihiroSuda · 2021-03-08T09:11:53Z

Bike shedding: should this be "imgcrypt" or "ocicrypt"?

imgcrypt is the project where ctd-decorder is coming from but ocicrypt could be a more generic name for a directory for looking for keys used generally for image encryption. -> "ocicrypt" ?

Changed to ocicrypt

AkihiroSuda · 2021-03-08T13:35:23Z

/test pull-containerd-node-e2e

AkihiroSuda · 2021-03-08T17:34:41Z

@stefanberger Can we have some default value for this?

I would call it /etc/ocicrypt_keyprovider.conf. I would NOT call it ocicrypt.conf, because this is the default name used by an other config file that was introduced earlier: https://github.com/containers/ocicrypt/blob/master/config/pkcs11config/config.go#L35

@lumjjb ?

/etc/ocicrypt_keyprovider.conf sounds fine. We probably want to test this first with ctd-decoder tests to see if it will ignore the case of a file that isn't present.

Added test : containerd/imgcrypt#40

If we are going to place /etc/ocicrypt_keyprovider.conf outside /etc/containerd, do we want to have decryption-keys-path outside /etc/containerd too?

Or can we have everything under /etc/containerd/ocicrypt?

I am fine with putting it in /etc/containerd/ocicrypt/ocicrypt_keyprovider.conf, i like the idea, would probably prevent some conflicts with other tools.

Btw, would the ctd-decoder also be packaged with the installation?

Updated PR to add env = ["OCICRYPT_KEYPROVIDER_CONFIG=/etc/containerd/ocicrypt/ocicrypt_keyprovider.conf"]

Btw, would the ctd-decoder also be packaged with the installation?

SGTM, but out of the scope of this PR.

Opened #5265 for discussing inclusion of ctd-decoder

theopenlab-ci · 2021-03-08T17:40:38Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 5m 47s (non-voting)

Signed-off-by: Akihiro Suda <[email protected]>

`config_linux.go` and `config_windows.go` are identical. `config_unsupported.go` is also almost identical but enables debug logs by default. Signed-off-by: Akihiro Suda <[email protected]>

Enable the following config by default: ```toml version = 2 [plugins."io.containerd.grpc.v1.cri".image_decryption] key_model = "node" [stream_processors] [stream_processors."io.containerd.ocicrypt.decoder.v1.tar.gzip"] accepts = ["application/vnd.oci.image.layer.v1.tar+gzip+encrypted"] returns = "application/vnd.oci.image.layer.v1.tar+gzip" path = "ctd-decoder" args = ["--decryption-keys-path", "/etc/containerd/ocicrypt/keys"] env = ["OCICRYPT_KEYPROVIDER_CONFIG=/etc/containerd/ocicrypt/ocicrypt_keyprovider.conf"] [stream_processors."io.containerd.ocicrypt.decoder.v1.tar"] accepts = ["application/vnd.oci.image.layer.v1.tar+encrypted"] returns = "application/vnd.oci.image.layer.v1.tar" path = "ctd-decoder" args = ["--decryption-keys-path", "/etc/containerd/ocicrypt/keys"] env = ["OCICRYPT_KEYPROVIDER_CONFIG=/etc/containerd/ocicrypt/ocicrypt_keyprovider.conf"] ``` Fix issue 5128 Signed-off-by: Akihiro Suda <[email protected]>

theopenlab-ci · 2021-03-15T04:35:55Z

Build succeeded.

containerd-build-arm64 : SUCCESS in 5m 43s (non-voting)

lumjjb

LGTM!

lumjjb · 2021-03-23T12:52:30Z

 	MediaTypeDockerSchema1Manifest = "application/vnd.docker.distribution.manifest.v1+prettyjws"
+	// Encypted media types
+	MediaTypeImageLayerEncrypted     = ocispec.MediaTypeImageLayer + "+encrypted"
+	MediaTypeImageLayerGzipEncrypted = ocispec.MediaTypeImageLayerGzip + "+encrypted"


FYI. There is a definition of the media type in https://github.com/containers/ocicrypt/blob/master/spec/spec.go. I'm guessing it may cause more dependencies for building certain builds though, so not sure what the implications there are if imported.

I'd prefer not to add Go dependency on ocicrypt here

Mark it as followup ~

fuweid

LGTM

AkihiroSuda added impact/changelog kind/enhancement labels Mar 8, 2021

AkihiroSuda mentioned this pull request Mar 8, 2021

Add ocicrypt stream processor to the default config #5128

Closed

AkihiroSuda commented Mar 8, 2021

View reviewed changes

AkihiroSuda force-pushed the default-config-crypt branch from d9b56da to f44f652 Compare March 8, 2021 13:06

containerd deleted a comment from theopenlab-ci Bot Mar 8, 2021

AkihiroSuda force-pushed the default-config-crypt branch 3 times, most recently from 5fb3841 to 0d2be78 Compare March 8, 2021 17:33

AkihiroSuda commented Mar 8, 2021

View reviewed changes

AkihiroSuda added 3 commits March 15, 2021 13:27

defaults: add DefaultConfigDir

9a7ca39

Signed-off-by: Akihiro Suda <[email protected]>

cmd/containerd: deduplicate config*.go

ac2726e

`config_linux.go` and `config_windows.go` are identical. `config_unsupported.go` is also almost identical but enables debug logs by default. Signed-off-by: Akihiro Suda <[email protected]>

AkihiroSuda force-pushed the default-config-crypt branch from 0d2be78 to ecb881e Compare March 15, 2021 04:28

AkihiroSuda added this to the 1.5 milestone Mar 15, 2021

AkihiroSuda requested review from dmcgowan, lumjjb and stefanberger March 23, 2021 01:52

lumjjb approved these changes Mar 23, 2021

View reviewed changes

kzys approved these changes Mar 23, 2021

View reviewed changes

fuweid approved these changes Mar 25, 2021

View reviewed changes

fuweid merged commit 80fa9fe into containerd:master Mar 25, 2021

AkihiroSuda mentioned this pull request Mar 25, 2021

Add ctd-decoder to cri-containerd-cni-<VERSION>-<OS>-<ARCH>.tar.gz #5265

Closed

AkihiroSuda mentioned this pull request Apr 9, 2021

Prepare v1.5.0-rc.0 #5257

Merged

AkihiroSuda mentioned this pull request Aug 20, 2021

docs/ocicrypt.md: update for containerd 1.5 containerd/nerdctl#320

Merged

Conversation

AkihiroSuda commented Mar 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AkihiroSuda commented Mar 8, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theopenlab-ci Bot commented Mar 8, 2021

Uh oh!

theopenlab-ci Bot commented Mar 15, 2021

Uh oh!

lumjjb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fuweid left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

AkihiroSuda commented Mar 8, 2021 •

edited

Loading