From c6d437fd70c54d34ef87da117ded797c21420b2d Mon Sep 17 00:00:00 2001 From: Brandon Lum Date: Thu, 18 Jul 2019 15:06:25 -0400 Subject: [PATCH] Corrected lease implementation Signed-off-by: Brandon Lum --- cmd/ctr/commands/images/crypt_utils.go | 11 +-- docs/encryption.md | 4 +- image_enc_test.go | 26 ++----- images/encryption/encryption.go | 101 +++++-------------------- 4 files changed, 34 insertions(+), 108 deletions(-) diff --git a/cmd/ctr/commands/images/crypt_utils.go b/cmd/ctr/commands/images/crypt_utils.go index 09b9f3b94..2f93c88fb 100644 --- a/cmd/ctr/commands/images/crypt_utils.go +++ b/cmd/ctr/commands/images/crypt_utils.go @@ -24,12 +24,10 @@ import ( "os" "strconv" "strings" - "time" "github.com/containerd/containerd" "github.com/containerd/containerd/images" imgenc "github.com/containerd/containerd/images/encryption" - "github.com/containerd/containerd/leases" "github.com/containerd/containerd/pkg/encryption" encconfig "github.com/containerd/containerd/pkg/encryption/config" encutils "github.com/containerd/containerd/pkg/encryption/utils" @@ -254,17 +252,16 @@ func cryptImage(client *containerd.Client, ctx gocontext.Context, name, newName newSpec ocispec.Descriptor ) - ls := client.LeasesService() - l, err := ls.Create(ctx, leases.WithRandomID(), leases.WithExpiration(5*time.Minute)) + ctx, done, err := client.WithLease(ctx) if err != nil { return images.Image{}, err } - defer ls.Delete(ctx, l, leases.SynchronousDelete) + defer done(ctx) if encrypt { - newSpec, modified, err = imgenc.EncryptImage(ctx, client.ContentStore(), ls, l, image.Target, cc, lf) + newSpec, modified, err = imgenc.EncryptImage(ctx, client.ContentStore(), image.Target, cc, lf) } else { - newSpec, modified, err = imgenc.DecryptImage(ctx, client.ContentStore(), ls, l, image.Target, cc, lf) + newSpec, modified, err = imgenc.DecryptImage(ctx, client.ContentStore(), image.Target, cc, lf) } if err != nil { return image, err diff --git a/docs/encryption.md b/docs/encryption.md index d0abbd425..64731d798 100644 --- a/docs/encryption.md +++ b/docs/encryption.md @@ -75,10 +75,10 @@ The current draft of exposed interfaces we believe will be used by consumers are /* Functions */ // EncryptImage encrypts an image; it accepts either an OCI descriptor representing a manifest list or a single manifest -func EncryptImage(ctx context.Context, cs content.Store, ls leases.Manager, l leases.Lease, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf *LayerFilter) (ocispec.Descriptor, bool, error) +func EncryptImage(ctx context.Context, cs content.Store, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf *LayerFilter) (ocispec.Descriptor, bool, error) // DecryptImage decrypts an image; it accepts either an OCI descriptor representing a manifest list or a single manifest -func DecryptImage(ctx context.Context, cs content.Store, ls leases.Manager, l leases.Lease, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf *LayerFilter) (ocispec.Descriptor, bool, error) +func DecryptImage(ctx context.Context, cs content.Store, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf *LayerFilter) (ocispec.Descriptor, bool, error) // CheckAuthorization checks whether a user has the right keys to be allowed to access an image (every layer) // It takes decrypting of the layers only as far as decrypting the asymmetrically encrypted data diff --git a/image_enc_test.go b/image_enc_test.go index bce5dad4a..97499715c 100644 --- a/image_enc_test.go +++ b/image_enc_test.go @@ -20,13 +20,11 @@ import ( "context" "runtime" "testing" - "time" "github.com/containerd/containerd/content" "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/images" imgenc "github.com/containerd/containerd/images/encryption" - "github.com/containerd/containerd/leases" encconfig "github.com/containerd/containerd/pkg/encryption/config" "github.com/containerd/containerd/pkg/encryption/utils" "github.com/containerd/containerd/platforms" @@ -86,7 +84,11 @@ func TestImageEncryption(t *testing.T) { defer client.Close() s := client.ImageService() - ls := client.LeasesService() + ctx, done, err := client.WithLease(ctx) + if err != nil { + t.Fatal(err) + } + defer done(ctx) image, err := s.Get(ctx, imageName) if err != nil { @@ -136,14 +138,8 @@ func TestImageEncryption(t *testing.T) { }, } - l, err := ls.Create(ctx, leases.WithRandomID(), leases.WithExpiration(5*time.Minute)) - if err != nil { - t.Fatal("Unable to create lease for encryption") - } - defer ls.Delete(ctx, l, leases.SynchronousDelete) - // Perform encryption of image - encSpec, modified, err := imgenc.EncryptImage(ctx, client.ContentStore(), ls, l, image.Target, cc, lf) + encSpec, modified, err := imgenc.EncryptImage(ctx, client.ContentStore(), image.Target, cc, lf) if err != nil { t.Fatal(err) } @@ -159,8 +155,6 @@ func TestImageEncryption(t *testing.T) { if _, err := s.Create(ctx, image); err != nil { t.Fatalf("Unable to create image: %v", err) } - // Force deletion of lease early to check for proper referencing - ls.Delete(ctx, l, leases.SynchronousDelete) cc = &encconfig.CryptoConfig{ DecryptConfig: &encconfig.DecryptConfig{ @@ -175,13 +169,7 @@ func TestImageEncryption(t *testing.T) { return true } - l, err = ls.Create(ctx, leases.WithRandomID(), leases.WithExpiration(5*time.Minute)) - if err != nil { - t.Fatal("Unable to create lease for decryption") - } - defer ls.Delete(ctx, l, leases.SynchronousDelete) - - decSpec, modified, err := imgenc.DecryptImage(ctx, client.ContentStore(), ls, l, encSpec, cc, lf) + decSpec, modified, err := imgenc.DecryptImage(ctx, client.ContentStore(), encSpec, cc, lf) if err != nil { t.Fatal(err) } diff --git a/images/encryption/encryption.go b/images/encryption/encryption.go index 3e3a628f6..687fa5983 100644 --- a/images/encryption/encryption.go +++ b/images/encryption/encryption.go @@ -30,7 +30,6 @@ import ( "github.com/containerd/containerd/content" "github.com/containerd/containerd/errdefs" - "github.com/containerd/containerd/leases" "github.com/containerd/containerd/platforms" digest "github.com/opencontainers/go-digest" specs "github.com/opencontainers/image-spec/specs-go" @@ -149,7 +148,7 @@ func decryptLayer(cc *encconfig.CryptoConfig, dataReader content.ReaderAt, desc } // cryptLayer handles the changes due to encryption or decryption of a layer -func cryptLayer(ctx context.Context, cs content.Store, ls leases.Manager, l leases.Lease, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, cryptoOp cryptoOp) (ocispec.Descriptor, error) { +func cryptLayer(ctx context.Context, cs content.Store, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, cryptoOp cryptoOp) (ocispec.Descriptor, error) { var ( resultReader io.Reader newDesc ocispec.Descriptor @@ -171,33 +170,13 @@ func cryptLayer(ctx context.Context, cs content.Store, ls leases.Manager, l leas } // some operations, such as changing recipients, may not touch the layer at all if resultReader != nil { - if ls == nil { - return ocispec.Descriptor{}, errors.New("Unexpected write to object without lease") - } - - var rsrc leases.Resource var ref string - // If we have the digest, write blob with checks haveDigest := newDesc.Digest.String() != "" if haveDigest { ref = fmt.Sprintf("layer-%s", newDesc.Digest.String()) - rsrc = leases.Resource{ - ID: newDesc.Digest.String(), - Type: "content", - } } else { ref = fmt.Sprintf("blob-%d-%d", rand.Int(), rand.Int()) - rsrc = leases.Resource{ - ID: ref, - Type: "ingests", - } - - } - - // Add resource to lease and write blob - if err := ls.AddResource(ctx, l, rsrc); err != nil { - return ocispec.Descriptor{}, errors.Wrap(err, "Unable to add resource to lease") } if haveDigest { @@ -205,20 +184,16 @@ func cryptLayer(ctx context.Context, cs content.Store, ls leases.Manager, l leas return ocispec.Descriptor{}, errors.Wrap(err, "failed to write config") } } else { - newDesc.Digest, newDesc.Size, err = ingestReaderLeaseContent(ctx, cs, ls, l, ref, resultReader) + newDesc.Digest, newDesc.Size, err = ingestReader(ctx, cs, ref, resultReader) if err != nil { return ocispec.Descriptor{}, err } - // Delete the ingest lease since the blob is protected now by the content lease - if err := ls.DeleteResource(ctx, l, rsrc); err != nil { - return ocispec.Descriptor{}, errors.Wrap(err, "Unable to delete resource from lease") - } } } return newDesc, err } -func ingestReaderLeaseContent(ctx context.Context, cs content.Ingester, ls leases.Manager, l leases.Lease, ref string, r io.Reader) (digest.Digest, int64, error) { +func ingestReader(ctx context.Context, cs content.Ingester, ref string, r io.Reader) (digest.Digest, int64, error) { cw, err := content.OpenWriter(ctx, cs, content.WithRef(ref)) if err != nil { return "", 0, errors.Wrap(err, "failed to open writer") @@ -234,16 +209,7 @@ func ingestReaderLeaseContent(ctx context.Context, cs content.Ingester, ls lease return "", 0, errors.Wrap(err, "failed to get state") } - rsrc := leases.Resource{ - ID: cw.Digest().String(), - Type: "content", - } - - if err := ls.AddResource(ctx, l, rsrc); err != nil { - return "", 0, errors.Wrapf(err, "Unable to add resource to lease") - } - - if err := cw.Commit(ctx, st.Offset, cw.Digest()); err != nil { + if err := cw.Commit(ctx, st.Offset, ""); err != nil { if !errdefs.IsAlreadyExists(err) { return "", 0, errors.Wrapf(err, "failed commit on ref %q", ref) } @@ -253,7 +219,7 @@ func ingestReaderLeaseContent(ctx context.Context, cs content.Ingester, ls lease } // Encrypt or decrypt all the Children of a given descriptor -func cryptChildren(ctx context.Context, cs content.Store, ls leases.Manager, l leases.Lease, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf LayerFilter, cryptoOp cryptoOp, thisPlatform *ocispec.Platform) (ocispec.Descriptor, bool, error) { +func cryptChildren(ctx context.Context, cs content.Store, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf LayerFilter, cryptoOp cryptoOp, thisPlatform *ocispec.Platform) (ocispec.Descriptor, bool, error) { children, err := images.Children(ctx, cs, desc) if err != nil { if errdefs.IsNotFound(err) { @@ -274,7 +240,7 @@ func cryptChildren(ctx context.Context, cs content.Store, ls leases.Manager, l l case images.MediaTypeDockerSchema2LayerGzip, images.MediaTypeDockerSchema2Layer, ocispec.MediaTypeImageLayerGzip, ocispec.MediaTypeImageLayer: if cryptoOp == cryptoOpEncrypt && lf(child) { - nl, err := cryptLayer(ctx, cs, ls, l, child, cc, cryptoOp) + nl, err := cryptLayer(ctx, cs, child, cc, cryptoOp) if err != nil { return ocispec.Descriptor{}, false, err } @@ -286,7 +252,7 @@ func cryptChildren(ctx context.Context, cs content.Store, ls leases.Manager, l l case images.MediaTypeDockerSchema2LayerGzipEnc, images.MediaTypeDockerSchema2LayerEnc: // this one can be decrypted but also its recipients list changed if lf(child) { - nl, err := cryptLayer(ctx, cs, ls, l, child, cc, cryptoOp) + nl, err := cryptLayer(ctx, cs, child, cc, cryptoOp) if err != nil || cryptoOp == cryptoOpUnwrapOnly { return ocispec.Descriptor{}, false, err } @@ -332,19 +298,6 @@ func cryptChildren(ctx context.Context, cs content.Store, ls leases.Manager, l l ref := fmt.Sprintf("manifest-%s", newDesc.Digest.String()) - if ls == nil { - return ocispec.Descriptor{}, false, errors.New("Unexpected write to object without lease") - } - - rsrc := leases.Resource{ - ID: newDesc.Digest.String(), - Type: "content", - } - - if err := ls.AddResource(ctx, l, rsrc); err != nil { - return ocispec.Descriptor{}, false, errors.Wrap(err, "Unable to add resource to lease") - } - if err := content.WriteBlob(ctx, cs, ref, bytes.NewReader(mb), newDesc, content.WithLabels(labels)); err != nil { return ocispec.Descriptor{}, false, errors.Wrap(err, "failed to write config") } @@ -355,7 +308,7 @@ func cryptChildren(ctx context.Context, cs content.Store, ls leases.Manager, l l } // cryptManifest encrypts or decrypts the children of a top level manifest -func cryptManifest(ctx context.Context, cs content.Store, ls leases.Manager, l leases.Lease, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf LayerFilter, cryptoOp cryptoOp) (ocispec.Descriptor, bool, error) { +func cryptManifest(ctx context.Context, cs content.Store, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf LayerFilter, cryptoOp cryptoOp) (ocispec.Descriptor, bool, error) { p, err := content.ReadBlob(ctx, cs, desc) if err != nil { return ocispec.Descriptor{}, false, err @@ -365,7 +318,7 @@ func cryptManifest(ctx context.Context, cs content.Store, ls leases.Manager, l l return ocispec.Descriptor{}, false, err } platform := platforms.DefaultSpec() - newDesc, modified, err := cryptChildren(ctx, cs, ls, l, desc, cc, lf, cryptoOp, &platform) + newDesc, modified, err := cryptChildren(ctx, cs, desc, cc, lf, cryptoOp, &platform) if err != nil || cryptoOp == cryptoOpUnwrapOnly { return ocispec.Descriptor{}, false, err } @@ -373,7 +326,7 @@ func cryptManifest(ctx context.Context, cs content.Store, ls leases.Manager, l l } // cryptManifestList encrypts or decrypts the children of a top level manifest list -func cryptManifestList(ctx context.Context, cs content.Store, ls leases.Manager, l leases.Lease, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf LayerFilter, cryptoOp cryptoOp) (ocispec.Descriptor, bool, error) { +func cryptManifestList(ctx context.Context, cs content.Store, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf LayerFilter, cryptoOp cryptoOp) (ocispec.Descriptor, bool, error) { // read the index; if any layer is encrypted and any manifests change we will need to rewrite it b, err := content.ReadBlob(ctx, cs, desc) if err != nil { @@ -388,7 +341,7 @@ func cryptManifestList(ctx context.Context, cs content.Store, ls leases.Manager, var newManifests []ocispec.Descriptor modified := false for _, manifest := range index.Manifests { - newManifest, m, err := cryptChildren(ctx, cs, ls, l, manifest, cc, lf, cryptoOp, manifest.Platform) + newManifest, m, err := cryptChildren(ctx, cs, manifest, cc, lf, cryptoOp, manifest.Platform) if err != nil || cryptoOp == cryptoOpUnwrapOnly { return ocispec.Descriptor{}, false, err } @@ -423,19 +376,6 @@ func cryptManifestList(ctx context.Context, cs content.Store, ls leases.Manager, ref := fmt.Sprintf("index-%s", newDesc.Digest.String()) - if ls == nil { - return ocispec.Descriptor{}, false, errors.New("Unexpected write to object without lease") - } - - rsrc := leases.Resource{ - ID: newDesc.Digest.String(), - Type: "content", - } - - if err := ls.AddResource(ctx, l, rsrc); err != nil { - return ocispec.Descriptor{}, false, errors.Wrap(err, "Unable to add resource to lease") - } - if err = content.WriteBlob(ctx, cs, ref, bytes.NewReader(mb), newDesc, content.WithLabels(labels)); err != nil { return ocispec.Descriptor{}, false, errors.Wrap(err, "failed to write index") } @@ -447,28 +387,28 @@ func cryptManifestList(ctx context.Context, cs content.Store, ls leases.Manager, // cryptImage is the dispatcher to encrypt/decrypt an image; it accepts either an OCI descriptor // representing a manifest list or a single manifest -func cryptImage(ctx context.Context, cs content.Store, ls leases.Manager, l leases.Lease, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf LayerFilter, cryptoOp cryptoOp) (ocispec.Descriptor, bool, error) { +func cryptImage(ctx context.Context, cs content.Store, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf LayerFilter, cryptoOp cryptoOp) (ocispec.Descriptor, bool, error) { if cc == nil { return ocispec.Descriptor{}, false, errors.Wrapf(errdefs.ErrInvalidArgument, "CryptoConfig must not be nil") } switch desc.MediaType { case ocispec.MediaTypeImageIndex, images.MediaTypeDockerSchema2ManifestList: - return cryptManifestList(ctx, cs, ls, l, desc, cc, lf, cryptoOp) + return cryptManifestList(ctx, cs, desc, cc, lf, cryptoOp) case ocispec.MediaTypeImageManifest, images.MediaTypeDockerSchema2Manifest: - return cryptManifest(ctx, cs, ls, l, desc, cc, lf, cryptoOp) + return cryptManifest(ctx, cs, desc, cc, lf, cryptoOp) default: return ocispec.Descriptor{}, false, errors.Errorf("CryptImage: Unhandled media type: %s", desc.MediaType) } } // EncryptImage encrypts an image; it accepts either an OCI descriptor representing a manifest list or a single manifest -func EncryptImage(ctx context.Context, cs content.Store, ls leases.Manager, l leases.Lease, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf LayerFilter) (ocispec.Descriptor, bool, error) { - return cryptImage(ctx, cs, ls, l, desc, cc, lf, cryptoOpEncrypt) +func EncryptImage(ctx context.Context, cs content.Store, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf LayerFilter) (ocispec.Descriptor, bool, error) { + return cryptImage(ctx, cs, desc, cc, lf, cryptoOpEncrypt) } // DecryptImage decrypts an image; it accepts either an OCI descriptor representing a manifest list or a single manifest -func DecryptImage(ctx context.Context, cs content.Store, ls leases.Manager, l leases.Lease, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf LayerFilter) (ocispec.Descriptor, bool, error) { - return cryptImage(ctx, cs, ls, l, desc, cc, lf, cryptoOpDecrypt) +func DecryptImage(ctx context.Context, cs content.Store, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf LayerFilter) (ocispec.Descriptor, bool, error) { + return cryptImage(ctx, cs, desc, cc, lf, cryptoOpDecrypt) } // CheckAuthorization checks whether a user has the right keys to be allowed to access an image (every layer) @@ -478,11 +418,12 @@ func CheckAuthorization(ctx context.Context, cs content.Store, desc ocispec.Desc cc := encconfig.CryptoConfig{ DecryptConfig: dc, } + lf := func(desc ocispec.Descriptor) bool { return true } - // We shouldn't need to create any objects in CheckAuthorization, so no lease required. - _, _, err := cryptImage(ctx, cs, nil, leases.Lease{}, desc, &cc, lf, cryptoOpUnwrapOnly) + + _, _, err := cryptImage(ctx, cs, desc, &cc, lf, cryptoOpUnwrapOnly) if err != nil { return errors.Wrapf(err, "you are not authorized to use this image") }