Skip to content
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

Support for image encryption and decryption #634

Closed
harche opened this issue May 22, 2019 · 16 comments
Closed

Support for image encryption and decryption #634

harche opened this issue May 22, 2019 · 16 comments

Comments

@harche
Copy link

harche commented May 22, 2019

Hello,

We are working on getting the OCI spec extended to support encrypted images. opencontainers/image-spec#747

Once the OCI spec supports image encryption, the tools around container lifecycle will have to support the encrypting as well as decrypting a container image.

Right now, there is a PR with containerd to add this support, containerd/containerd#3134

Meanwhile, we have also added a KEP (Kubernetes Enhancement Proposal) to add support for container image decryption using kubernetes secretes, https://github.com/kubernetes/enhancements/blob/f63942200e733cd1e099df1ef9628eabeaffd11e/keps/sig-node/20190517-image-decryption.md

The tracking issue for the KEP work is, kubernetes/enhancements#1067

We believe along with the other tools in the ecosystem, containers/image should also support this upcoming image encryption in the OCI-spec. This way, tools like buildah, podman will inherit those capabilities.

@rhatdan Let me know what do you think.

@harche
Copy link
Author

harche commented May 22, 2019

CRI-O will need to support this too, but I will wait for the KEP doc to get accepted first so that we could file a more concrete issue with CRI-O.

@mtrmac
Copy link
Collaborator

mtrmac commented May 22, 2019

Thanks for the pointer.

Copying encrypted images without decrypting should be easy enough.

As for decrypting them (e.g. to implement podman pull / CRI-O image pull), that would involve an actual implementation, and, sure, the existing proposal seems possible to implement in principle.

I do have one comment on the proposed spec in opencontainers/image-spec#775 , though:
I feel rather strongly that existing formats, and existing implementations, for encapsulation should be used, i.e. that LayerBlockCipherOptions should not exist. Implementing this when existing implementations exist and are maintained is an extra risk, and extra code calling the cryptographic primitives makes crypto review/certification (e.g. possible FIPS-140 compliance) more costly — for no benefit that I can see.

Reading the existing discussion, it’s not obvious to me why the encrypted layer is not simply a big OpenPGP- or PKCS#7-formatted object directly; why is there a separate OpenPGP-/PKCS#7-encrypted LayerBlockCipherOptions object wrapping symmetric keys in a newly-invented format (i.e. two symmetric encryption layers, ultimately), and a custom-formatted raw layer, when OpenPGP/PKCS#7 were from the start designed to support this, including multiple public key recipient and single-shot streaming support, natively?

Where is the best place to comment on this? Is the comment above sufficient, or should I speak up in opencontainers/image-spec#747 ? Or is there some reason for this design that I have missed?

@stefanberger
Copy link

stefanberger commented May 22, 2019

Reading the existing discussion, it’s not obvious to me why the encrypted layer is not simply a big OpenPGP- or PKCS#7-formatted object directly; why is there a separate OpenPGP-/PKCS#7-encrypted LayerBlockCipherOptions object wrapping symmetric keys in a newly-invented format (i.e. two symmetric encryption layers, ultimately), and a custom-formatted raw layer, when OpenPGP/PKCS#7 were from the start designed to support this, including multiple public key recipient and single-shot streaming support, natively?

We initially had an OpenPGP implementation that encrypted the layer right in its payload. We moved away from this so that we can support OpenPGP recipients as well as PKCS#7 recipients and JWK recipients of an image through this level of indirection. This way we only need to encrypt a layer once with one symmetric key that all recipients can get to by decrypting their payload. A benefit is also that one can then add recipients to the image.

To support OpenPGP was basically the first feedback we got but the crypto used by OpenPGP will not be FIPS compliant, so we added the other ones also for being able to support different types of keys. PKCS#7 implementation in golang needs public key certificates for the encryption keys, while JWK works with just plain public RSA keys and symmetric and elliptic curve keys as well.

A further issue is the encryption of large layers using an AEAD symmetric key cipher. The golang APIs for AEAD take byte arrays as input parameters, but this doesn't work well for large layers (GB scale size) while keeping the memory footprint of the encrypting process low. That's why we use the separate stream cipher from miscreant to be able to chunk up the input data while we don't have to play around with IVs for the chunks but let miscreant handle it.

@mtrmac
Copy link
Collaborator

mtrmac commented May 22, 2019

Thanks for the explanation.

Reading the existing discussion, it’s not obvious to me why the encrypted layer is not simply a big OpenPGP- or PKCS#7-formatted object directly; why is there a separate OpenPGP-/PKCS#7-encrypted LayerBlockCipherOptions object wrapping symmetric keys in a newly-invented format (i.e. two symmetric encryption layers, ultimately), and a custom-formatted raw layer, when OpenPGP/PKCS#7 were from the start designed to support this, including multiple public key recipient and single-shot streaming support, natively?

We initially had an OpenPGP implementation that encrypted the layer right in its payload. We moved away from this so that we can support OpenPGP recipients as well as PKCS#7 recipients and JWK recipients of an image through this level of indirection.

(This seems an entirely artificial goal to me; a single recipient of a specific image is likely to have a single key standard (whether internally decided or imposed by the image creator).)

A benefit is also that one can then add recipients to the image.

OK, this is a good point (although, as discussed in opencontainers/image-spec#747, sharing a single version of the encrypted layer across recipients discloses that this layer is the same for all recipients, which is a violation of confidentiality, at least in principle).

Conceding that the indirection is valuable, I’d still prefer the layer encryption to use an established format and rely on existing implementations widely shared across applications. Also, the “algorithm identifier” metadata is logically a part of the shared layer, not of the per-recipient metadata as in LayerBlockCipherOptions.type.

… hum, but there seems to only exist a draft RFC for AEAD in OpenPGP. That’s awkward. Still, there’s PKCS#7 with RFC 5084.

A further issue is the encryption of large layers using an AEAD symmetric key cipher. The golang APIs for AEAD take byte arrays as input parameters, but this doesn't work well for large layers (GB scale size) while keeping the memory footprint of the encrypting process low.

That’s a valid implementation concern, but not something that should influence the format decision I think. (FWIW, talking of implementation concerns: I expect serious difficulties with FIPS compliance of a pure-Golang algorithm implementation like the one in miscreant-go, at least because it’s not possible to reliably zeroize secret key material after use.)

@stefanberger
Copy link

stefanberger commented May 22, 2019

(This seems an entirely artificial goal to me; a single recipient of a specific image is likely to have a single key standard (whether internally decided or imposed by the image creator).)

Our assumption is that a single image may have different recipients with possibly different key types.

That’s a valid implementation concern, but not something that should influence the format decision I think. (FWIW, talking of implementation concerns: I expect serious difficulties with FIPS compliance of a pure-Golang algorithm implementation like the one in miscreant-go, at least because it’s not possible to reliably zeroize secret key material after use.)

The PKCS#7 implementation uses a byte array as input as well: https://godoc.org/github.com/fullsailor/pkcs7#Encrypt
JWE does so as well: https://github.com/square/go-jose/blob/v2/crypter.go#L304

I do expect issues along the FIPs compliancy lines as well. The FIPS compliant golang based on boringssl is unlikely to help with the API limitations of the golang libraries for AEAD's and also not for 3rd pary libraries like JWE and PKCS#7. To me it looks like any FIPS compliant solution for the layer encryption likely cannot be done in golang.

@mtrmac
Copy link
Collaborator

mtrmac commented May 22, 2019

To me it looks like any FIPS compliant solution for the layer encryption likely cannot be done in golang.

Yes, which is one of the reasons I am advocating for using an existing industry standard, if possible with an already FIPS-validated implementation.

@stefanberger
Copy link

FYI: https://github.com/miscreant/miscreant/wiki/STREAM

Yes, which is one of the reasons I am advocating for using an existing industry standard, if possible with an already FIPS-validated implementation.

I think we would need to agree on the spec first and let this then drive changes to the implementation. I guess it would fall out naturally that we cannot implement it in golang while having to meet memory footprint restrictions but have to call into a crypto library, possibly via golang-to-C interface.

@stefanberger
Copy link

OK, this is a good point (although, as discussed in opencontainers/image-spec#747, sharing a single version of the encrypted layer across recipients discloses that this layer is the same for all recipients, which is a violation of confidentiality, at least in principle).

OpenPGP, PKCS#7, and JWE are all multi-recipient capable and encrypt a single payload with the same symmetric key, violating confidentiality by design... we just take it a step further here and share a single encryption key across these technologies.

@lumjjb
Copy link
Contributor

lumjjb commented May 28, 2019

I think @stefanberger and @harche have articulated a lot of the main discussion points. I'd just like to add on a little more.

Standards JWE, OpenPGP, PKCS7

We initially wanted to support just 1, which had well defined semantics to be able to separate the key wrapping distribution and block encryption to help with the deduplication and recipient adding. This would give us a lot of freedom to benefit from some of your suggestions. In the end, it seemed like there wasn't a clear winner of the standards (considering security, usability and adoption) - and the OCI spec community considered the ability to choose different protocol options as an important factor - so as not to favor particular vendors, etc.

For multiple key recipients

One of the scenarios that we considered the use of multiple key recipients in an enterprise setting is where a recipient would be a cluster, boostrapped with the appropriate key that represents it. We also envision that wrapped keys may be propagated by other key management mechanisms as well. Our initial proposal had a separate protocol to perform key management - kind of like how the notary handles the signatures for images. Per suggestion for completeness from the OCI community, we included it as part of the image.

FIPS & special usecases

@mtrmac you are indeed right about FIPS compliance. It is fairly tricky issue, and there are parties with differing views on the matter - i.e. some think STREAM and experimental algorithms will be FIPS. Ultimately, we hope the discussion around the spec will provide the right balance, and/or customization for majority of the usecases.

One idea we've been floating around is the ability to allow the spec to define a custom protocol, and have plugin extensions within the runtimes that are able to call out to a daemon/library. This would also help in scenarios where company X only has FIPS compliant approved C++ library, but not one in go-lang, or require handling of some special hardware encryption usecases, or have their KMS perform key wrapping service (Like talking directly to vault to perform keywrapping or something non-KMIP, like the current KeyProtect wrapping interface).

@lumjjb
Copy link
Contributor

lumjjb commented Jul 12, 2019

@mtrmac We are in the process of coming up with a POC to show the functionality.

We have a question about garbage collection. Is there a gc mechanism that we need to be aware of in terms of managing content? Thanks!

@mtrmac
Copy link
Collaborator

mtrmac commented Jul 12, 2019

FYI: https://github.com/miscreant/miscreant/wiki/STREAM

Sorry, I haven’t noticed that before… That may well be a good design, but I’m not a cryptographer and I can’t review or responsibly make decisions about using non-mainstream cryptosystems.

Practically speaking, getting consensus both on a cryptosystem and on an interoperability file format seems much harder than only defining an interoperable a file format (which will need to have some kind of crypto agility in any case.)

@mtrmac
Copy link
Collaborator

mtrmac commented Jul 12, 2019

One idea we've been floating around is the ability to allow the spec to define a custom protocol, and have plugin extensions within the runtimes that are able to call out to a daemon/library.

That, to me, rather misses the point. Of course, with a plugin mechanism anything is possible to achieve in every single case, but that also automatically makes reliable interoperability across systems impossible. Of course, some flexibility is inevitable (if only to be able to move from defeated cryptosystems to new ones), but overall interoperability specifications should try hard to minimize it.

This would also help in scenarios where company X only has FIPS compliant approved C++ library, but not one in go-lang

That’s an implementation concern (which may, possibly, lead to use of plugins in the implementation) that should not in any way manifest in the image format.

, or require handling of some special hardware encryption usecases, or have their KMS perform key wrapping service (Like talking directly to vault to perform keywrapping or something non-KMIP, like the current KeyProtect wrapping interface).

I would expect these to be again implementation concerns for “how do I get a key”, or, to the extent the current proposal in opencontainers/image-spec#775 is already flexible WRT key encryption, com.private-company.my-special-key-server-wrapping could be an alternative to the specified org.opencontainers.image.org.opencontainers.image.enc.keys.* annotations.

@mtrmac
Copy link
Collaborator

mtrmac commented Jul 12, 2019

We have a question about garbage collection. Is there a gc mechanism that we need to be aware of in terms of managing content? Thanks!

c/image copies images around and converts it, it is not directly responsible for any of the image storage mechanisms it works with, nor for managing their backing storage.

I guess the design does need to take at least the GitHub.com/docker/distribution servers, and the GitHub.com/containers/storage local storage, into account.

@lumjjb
Copy link
Contributor

lumjjb commented Jul 12, 2019

Thanks for all the feedback. Let me try to make sure that I don't miss anything. Btw, @harche and I are working on a prototype in the image library together with cri-o as a consumer for a POC. This should be ready soon, so hopefully we can have some discussions around that in the near future :).

For miscreant STREAM

TL; DR; We will have the main certified crypto algorithms + a few other experimental one (with warnings) as part of the spec.

Yup - currently we defer to the IANA registry of cryptographic algorithms. We had a chat with the OCI community and we agreed that we need to be able to focus on a few crypto algorithms, for the sake of needing to support implementations of the entire list of crypto algorithms. Our reference would still be based off the IANA registry. So the traditional AEAD GCM, etc. algorithms would be used. We also will include several other crypto options like STREAM, but it will be part of a list with some warning disclosures for users to know about the certification of the standard (i.e. no NIST certification yet). We have STREAM on the list because it currently fills up a gap which isn't filled in the golang crypto land - i.e. an authenticated block cipher that streams instead of requiring memory residency of the entire blob. Technically AEAD algorithms can also be streamed... but the golang crypto libraries chose the route of making the interface safer and simpler for users, which requires full residency.

I would expect these to be again implementation concerns for “how do I get a key”, or, to the extent the current proposal in opencontainers/image-spec#775 is already flexible WRT key encryption, com.private-company.my-special-key-server-wrapping could be an alternative to the specified org.opencontainers.image.org.opencontainers.image.enc.keys.* annotations.

I agree with you on the ideals of this... unfortunately, the world of KMS standards adoption isn't where we'd like it yet... So we've deferred on this discussion for now. This is a more niche user persona (which happens to be one we care about though). We are hoping that everyone will agree on KMIP and implement it across the board. If this happens, it would be super awesome :).

That, to me, rather misses the point. Of course, with a plugin mechanism anything is possible to achieve in every single case, but that also automatically makes reliable interoperability across systems impossible. Of course, some flexibility is inevitable (if only to be able to move from defeated cryptosystems to new ones), but overall interoperability specifications should try hard to minimize it.

I think I am in agreement here w.r.t. standardizing the use. Yes, perhaps this should be specific to implementation rather than spec. My desire for the ability to add a custom scheme stems from the ability to support the above usecases which are not really at the point of standardization. Perhaps there's a better way to go about this.

GC stuff

That's great - I think we shouldn't have an issue there then. I think we generally prescribe to the way an image is specified so we don't have issues with distribution. The only thing we've seen so far is in the process of decrypting an image because unlike pulling, in performing decryption, the hash of the blob is not present before decrypting it. We've considered adding the hash to the unwrapped package, but there is no real security concern as long as we use an AEAD cipher. Implementation wise, we've not run into any issues yet.

However, now that I think about it, perhaps we can drop AEAD in place of storing the hash... Probably warrants a more in-depth discussion!

@stefanberger
Copy link

stefanberger commented Jul 13, 2019

However, now that I think about it, perhaps we can drop AEAD in place of storing the hash... Probably warrants a more in-depth discussion!

Either encrypting the hash for each image recipient as part of the JSON or using Encrypt-then-MAC with an HMAC(key, HASH(encrypted layer)). The HMAC could be stored in public.

@lumjjb
Copy link
Contributor

lumjjb commented Dec 17, 2019

@mtrmac @mrunalp @rhatdan @vrothberg I think we can close this issue for now :) .

#673
containers/skopeo#732
cri-o/cri-o#2813

@rhatdan rhatdan closed this as completed Dec 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants