Skip to content

Commit c6d437f

Browse files
lumjjblumjjb
committed
Corrected lease implementation
Signed-off-by: Brandon Lum <[email protected]>
1 parent c00517a commit c6d437f

4 files changed

Lines changed: 34 additions & 108 deletions

File tree

‎cmd/ctr/commands/images/crypt_utils.go‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,10 @@ import (
2424
"os"
2525
"strconv"
2626
"strings"
27-
"time"
2827

2928
"github.com/containerd/containerd"
3029
"github.com/containerd/containerd/images"
3130
imgenc "github.com/containerd/containerd/images/encryption"
32-
"github.com/containerd/containerd/leases"
3331
"github.com/containerd/containerd/pkg/encryption"
3432
encconfig "github.com/containerd/containerd/pkg/encryption/config"
3533
encutils "github.com/containerd/containerd/pkg/encryption/utils"
@@ -254,17 +252,16 @@ func cryptImage(client *containerd.Client, ctx gocontext.Context, name, newName
254252
newSpec ocispec.Descriptor
255253
)
256254

257-
ls := client.LeasesService()
258-
l, err := ls.Create(ctx, leases.WithRandomID(), leases.WithExpiration(5*time.Minute))
255+
ctx, done, err := client.WithLease(ctx)
259256
if err != nil {
260257
return images.Image{}, err
261258
}
262-
defer ls.Delete(ctx, l, leases.SynchronousDelete)
259+
defer done(ctx)
263260

264261
if encrypt {
265-
newSpec, modified, err = imgenc.EncryptImage(ctx, client.ContentStore(), ls, l, image.Target, cc, lf)
262+
newSpec, modified, err = imgenc.EncryptImage(ctx, client.ContentStore(), image.Target, cc, lf)
266263
} else {
267-
newSpec, modified, err = imgenc.DecryptImage(ctx, client.ContentStore(), ls, l, image.Target, cc, lf)
264+
newSpec, modified, err = imgenc.DecryptImage(ctx, client.ContentStore(), image.Target, cc, lf)
268265
}
269266
if err != nil {
270267
return image, err

‎docs/encryption.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,10 @@ The current draft of exposed interfaces we believe will be used by consumers are
7575
/* Functions */
7676
7777
// EncryptImage encrypts an image; it accepts either an OCI descriptor representing a manifest list or a single manifest
78-
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)
78+
func EncryptImage(ctx context.Context, cs content.Store, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf *LayerFilter) (ocispec.Descriptor, bool, error)
7979
8080
// DecryptImage decrypts an image; it accepts either an OCI descriptor representing a manifest list or a single manifest
81-
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)
81+
func DecryptImage(ctx context.Context, cs content.Store, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf *LayerFilter) (ocispec.Descriptor, bool, error)
8282
8383
// CheckAuthorization checks whether a user has the right keys to be allowed to access an image (every layer)
8484
// It takes decrypting of the layers only as far as decrypting the asymmetrically encrypted data

‎image_enc_test.go‎

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,11 @@ import (
2020
"context"
2121
"runtime"
2222
"testing"
23-
"time"
2423

2524
"github.com/containerd/containerd/content"
2625
"github.com/containerd/containerd/errdefs"
2726
"github.com/containerd/containerd/images"
2827
imgenc "github.com/containerd/containerd/images/encryption"
29-
"github.com/containerd/containerd/leases"
3028
encconfig "github.com/containerd/containerd/pkg/encryption/config"
3129
"github.com/containerd/containerd/pkg/encryption/utils"
3230
"github.com/containerd/containerd/platforms"
@@ -86,7 +84,11 @@ func TestImageEncryption(t *testing.T) {
8684
defer client.Close()
8785

8886
s := client.ImageService()
89-
ls := client.LeasesService()
87+
ctx, done, err := client.WithLease(ctx)
88+
if err != nil {
89+
t.Fatal(err)
90+
}
91+
defer done(ctx)
9092

9193
image, err := s.Get(ctx, imageName)
9294
if err != nil {
@@ -136,14 +138,8 @@ func TestImageEncryption(t *testing.T) {
136138
},
137139
}
138140

139-
l, err := ls.Create(ctx, leases.WithRandomID(), leases.WithExpiration(5*time.Minute))
140-
if err != nil {
141-
t.Fatal("Unable to create lease for encryption")
142-
}
143-
defer ls.Delete(ctx, l, leases.SynchronousDelete)
144-
145141
// Perform encryption of image
146-
encSpec, modified, err := imgenc.EncryptImage(ctx, client.ContentStore(), ls, l, image.Target, cc, lf)
142+
encSpec, modified, err := imgenc.EncryptImage(ctx, client.ContentStore(), image.Target, cc, lf)
147143
if err != nil {
148144
t.Fatal(err)
149145
}
@@ -159,8 +155,6 @@ func TestImageEncryption(t *testing.T) {
159155
if _, err := s.Create(ctx, image); err != nil {
160156
t.Fatalf("Unable to create image: %v", err)
161157
}
162-
// Force deletion of lease early to check for proper referencing
163-
ls.Delete(ctx, l, leases.SynchronousDelete)
164158

165159
cc = &encconfig.CryptoConfig{
166160
DecryptConfig: &encconfig.DecryptConfig{
@@ -175,13 +169,7 @@ func TestImageEncryption(t *testing.T) {
175169
return true
176170
}
177171

178-
l, err = ls.Create(ctx, leases.WithRandomID(), leases.WithExpiration(5*time.Minute))
179-
if err != nil {
180-
t.Fatal("Unable to create lease for decryption")
181-
}
182-
defer ls.Delete(ctx, l, leases.SynchronousDelete)
183-
184-
decSpec, modified, err := imgenc.DecryptImage(ctx, client.ContentStore(), ls, l, encSpec, cc, lf)
172+
decSpec, modified, err := imgenc.DecryptImage(ctx, client.ContentStore(), encSpec, cc, lf)
185173
if err != nil {
186174
t.Fatal(err)
187175
}

‎images/encryption/encryption.go‎

Lines changed: 21 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import (
3030

3131
"github.com/containerd/containerd/content"
3232
"github.com/containerd/containerd/errdefs"
33-
"github.com/containerd/containerd/leases"
3433
"github.com/containerd/containerd/platforms"
3534
digest "github.com/opencontainers/go-digest"
3635
specs "github.com/opencontainers/image-spec/specs-go"
@@ -149,7 +148,7 @@ func decryptLayer(cc *encconfig.CryptoConfig, dataReader content.ReaderAt, desc
149148
}
150149

151150
// cryptLayer handles the changes due to encryption or decryption of a layer
152-
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) {
151+
func cryptLayer(ctx context.Context, cs content.Store, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, cryptoOp cryptoOp) (ocispec.Descriptor, error) {
153152
var (
154153
resultReader io.Reader
155154
newDesc ocispec.Descriptor
@@ -171,54 +170,30 @@ func cryptLayer(ctx context.Context, cs content.Store, ls leases.Manager, l leas
171170
}
172171
// some operations, such as changing recipients, may not touch the layer at all
173172
if resultReader != nil {
174-
if ls == nil {
175-
return ocispec.Descriptor{}, errors.New("Unexpected write to object without lease")
176-
}
177-
178-
var rsrc leases.Resource
179173
var ref string
180-
181174
// If we have the digest, write blob with checks
182175
haveDigest := newDesc.Digest.String() != ""
183176
if haveDigest {
184177
ref = fmt.Sprintf("layer-%s", newDesc.Digest.String())
185-
rsrc = leases.Resource{
186-
ID: newDesc.Digest.String(),
187-
Type: "content",
188-
}
189178
} else {
190179
ref = fmt.Sprintf("blob-%d-%d", rand.Int(), rand.Int())
191-
rsrc = leases.Resource{
192-
ID: ref,
193-
Type: "ingests",
194-
}
195-
196-
}
197-
198-
// Add resource to lease and write blob
199-
if err := ls.AddResource(ctx, l, rsrc); err != nil {
200-
return ocispec.Descriptor{}, errors.Wrap(err, "Unable to add resource to lease")
201180
}
202181

203182
if haveDigest {
204183
if err := content.WriteBlob(ctx, cs, ref, resultReader, newDesc); err != nil {
205184
return ocispec.Descriptor{}, errors.Wrap(err, "failed to write config")
206185
}
207186
} else {
208-
newDesc.Digest, newDesc.Size, err = ingestReaderLeaseContent(ctx, cs, ls, l, ref, resultReader)
187+
newDesc.Digest, newDesc.Size, err = ingestReader(ctx, cs, ref, resultReader)
209188
if err != nil {
210189
return ocispec.Descriptor{}, err
211190
}
212-
// Delete the ingest lease since the blob is protected now by the content lease
213-
if err := ls.DeleteResource(ctx, l, rsrc); err != nil {
214-
return ocispec.Descriptor{}, errors.Wrap(err, "Unable to delete resource from lease")
215-
}
216191
}
217192
}
218193
return newDesc, err
219194
}
220195

221-
func ingestReaderLeaseContent(ctx context.Context, cs content.Ingester, ls leases.Manager, l leases.Lease, ref string, r io.Reader) (digest.Digest, int64, error) {
196+
func ingestReader(ctx context.Context, cs content.Ingester, ref string, r io.Reader) (digest.Digest, int64, error) {
222197
cw, err := content.OpenWriter(ctx, cs, content.WithRef(ref))
223198
if err != nil {
224199
return "", 0, errors.Wrap(err, "failed to open writer")
@@ -234,16 +209,7 @@ func ingestReaderLeaseContent(ctx context.Context, cs content.Ingester, ls lease
234209
return "", 0, errors.Wrap(err, "failed to get state")
235210
}
236211

237-
rsrc := leases.Resource{
238-
ID: cw.Digest().String(),
239-
Type: "content",
240-
}
241-
242-
if err := ls.AddResource(ctx, l, rsrc); err != nil {
243-
return "", 0, errors.Wrapf(err, "Unable to add resource to lease")
244-
}
245-
246-
if err := cw.Commit(ctx, st.Offset, cw.Digest()); err != nil {
212+
if err := cw.Commit(ctx, st.Offset, ""); err != nil {
247213
if !errdefs.IsAlreadyExists(err) {
248214
return "", 0, errors.Wrapf(err, "failed commit on ref %q", ref)
249215
}
@@ -253,7 +219,7 @@ func ingestReaderLeaseContent(ctx context.Context, cs content.Ingester, ls lease
253219
}
254220

255221
// Encrypt or decrypt all the Children of a given descriptor
256-
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) {
222+
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) {
257223
children, err := images.Children(ctx, cs, desc)
258224
if err != nil {
259225
if errdefs.IsNotFound(err) {
@@ -274,7 +240,7 @@ func cryptChildren(ctx context.Context, cs content.Store, ls leases.Manager, l l
274240
case images.MediaTypeDockerSchema2LayerGzip, images.MediaTypeDockerSchema2Layer,
275241
ocispec.MediaTypeImageLayerGzip, ocispec.MediaTypeImageLayer:
276242
if cryptoOp == cryptoOpEncrypt && lf(child) {
277-
nl, err := cryptLayer(ctx, cs, ls, l, child, cc, cryptoOp)
243+
nl, err := cryptLayer(ctx, cs, child, cc, cryptoOp)
278244
if err != nil {
279245
return ocispec.Descriptor{}, false, err
280246
}
@@ -286,7 +252,7 @@ func cryptChildren(ctx context.Context, cs content.Store, ls leases.Manager, l l
286252
case images.MediaTypeDockerSchema2LayerGzipEnc, images.MediaTypeDockerSchema2LayerEnc:
287253
// this one can be decrypted but also its recipients list changed
288254
if lf(child) {
289-
nl, err := cryptLayer(ctx, cs, ls, l, child, cc, cryptoOp)
255+
nl, err := cryptLayer(ctx, cs, child, cc, cryptoOp)
290256
if err != nil || cryptoOp == cryptoOpUnwrapOnly {
291257
return ocispec.Descriptor{}, false, err
292258
}
@@ -332,19 +298,6 @@ func cryptChildren(ctx context.Context, cs content.Store, ls leases.Manager, l l
332298

333299
ref := fmt.Sprintf("manifest-%s", newDesc.Digest.String())
334300

335-
if ls == nil {
336-
return ocispec.Descriptor{}, false, errors.New("Unexpected write to object without lease")
337-
}
338-
339-
rsrc := leases.Resource{
340-
ID: newDesc.Digest.String(),
341-
Type: "content",
342-
}
343-
344-
if err := ls.AddResource(ctx, l, rsrc); err != nil {
345-
return ocispec.Descriptor{}, false, errors.Wrap(err, "Unable to add resource to lease")
346-
}
347-
348301
if err := content.WriteBlob(ctx, cs, ref, bytes.NewReader(mb), newDesc, content.WithLabels(labels)); err != nil {
349302
return ocispec.Descriptor{}, false, errors.Wrap(err, "failed to write config")
350303
}
@@ -355,7 +308,7 @@ func cryptChildren(ctx context.Context, cs content.Store, ls leases.Manager, l l
355308
}
356309

357310
// cryptManifest encrypts or decrypts the children of a top level manifest
358-
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) {
311+
func cryptManifest(ctx context.Context, cs content.Store, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf LayerFilter, cryptoOp cryptoOp) (ocispec.Descriptor, bool, error) {
359312
p, err := content.ReadBlob(ctx, cs, desc)
360313
if err != nil {
361314
return ocispec.Descriptor{}, false, err
@@ -365,15 +318,15 @@ func cryptManifest(ctx context.Context, cs content.Store, ls leases.Manager, l l
365318
return ocispec.Descriptor{}, false, err
366319
}
367320
platform := platforms.DefaultSpec()
368-
newDesc, modified, err := cryptChildren(ctx, cs, ls, l, desc, cc, lf, cryptoOp, &platform)
321+
newDesc, modified, err := cryptChildren(ctx, cs, desc, cc, lf, cryptoOp, &platform)
369322
if err != nil || cryptoOp == cryptoOpUnwrapOnly {
370323
return ocispec.Descriptor{}, false, err
371324
}
372325
return newDesc, modified, nil
373326
}
374327

375328
// cryptManifestList encrypts or decrypts the children of a top level manifest list
376-
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) {
329+
func cryptManifestList(ctx context.Context, cs content.Store, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf LayerFilter, cryptoOp cryptoOp) (ocispec.Descriptor, bool, error) {
377330
// read the index; if any layer is encrypted and any manifests change we will need to rewrite it
378331
b, err := content.ReadBlob(ctx, cs, desc)
379332
if err != nil {
@@ -388,7 +341,7 @@ func cryptManifestList(ctx context.Context, cs content.Store, ls leases.Manager,
388341
var newManifests []ocispec.Descriptor
389342
modified := false
390343
for _, manifest := range index.Manifests {
391-
newManifest, m, err := cryptChildren(ctx, cs, ls, l, manifest, cc, lf, cryptoOp, manifest.Platform)
344+
newManifest, m, err := cryptChildren(ctx, cs, manifest, cc, lf, cryptoOp, manifest.Platform)
392345
if err != nil || cryptoOp == cryptoOpUnwrapOnly {
393346
return ocispec.Descriptor{}, false, err
394347
}
@@ -423,19 +376,6 @@ func cryptManifestList(ctx context.Context, cs content.Store, ls leases.Manager,
423376

424377
ref := fmt.Sprintf("index-%s", newDesc.Digest.String())
425378

426-
if ls == nil {
427-
return ocispec.Descriptor{}, false, errors.New("Unexpected write to object without lease")
428-
}
429-
430-
rsrc := leases.Resource{
431-
ID: newDesc.Digest.String(),
432-
Type: "content",
433-
}
434-
435-
if err := ls.AddResource(ctx, l, rsrc); err != nil {
436-
return ocispec.Descriptor{}, false, errors.Wrap(err, "Unable to add resource to lease")
437-
}
438-
439379
if err = content.WriteBlob(ctx, cs, ref, bytes.NewReader(mb), newDesc, content.WithLabels(labels)); err != nil {
440380
return ocispec.Descriptor{}, false, errors.Wrap(err, "failed to write index")
441381
}
@@ -447,28 +387,28 @@ func cryptManifestList(ctx context.Context, cs content.Store, ls leases.Manager,
447387

448388
// cryptImage is the dispatcher to encrypt/decrypt an image; it accepts either an OCI descriptor
449389
// representing a manifest list or a single manifest
450-
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) {
390+
func cryptImage(ctx context.Context, cs content.Store, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf LayerFilter, cryptoOp cryptoOp) (ocispec.Descriptor, bool, error) {
451391
if cc == nil {
452392
return ocispec.Descriptor{}, false, errors.Wrapf(errdefs.ErrInvalidArgument, "CryptoConfig must not be nil")
453393
}
454394
switch desc.MediaType {
455395
case ocispec.MediaTypeImageIndex, images.MediaTypeDockerSchema2ManifestList:
456-
return cryptManifestList(ctx, cs, ls, l, desc, cc, lf, cryptoOp)
396+
return cryptManifestList(ctx, cs, desc, cc, lf, cryptoOp)
457397
case ocispec.MediaTypeImageManifest, images.MediaTypeDockerSchema2Manifest:
458-
return cryptManifest(ctx, cs, ls, l, desc, cc, lf, cryptoOp)
398+
return cryptManifest(ctx, cs, desc, cc, lf, cryptoOp)
459399
default:
460400
return ocispec.Descriptor{}, false, errors.Errorf("CryptImage: Unhandled media type: %s", desc.MediaType)
461401
}
462402
}
463403

464404
// EncryptImage encrypts an image; it accepts either an OCI descriptor representing a manifest list or a single manifest
465-
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) {
466-
return cryptImage(ctx, cs, ls, l, desc, cc, lf, cryptoOpEncrypt)
405+
func EncryptImage(ctx context.Context, cs content.Store, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf LayerFilter) (ocispec.Descriptor, bool, error) {
406+
return cryptImage(ctx, cs, desc, cc, lf, cryptoOpEncrypt)
467407
}
468408

469409
// DecryptImage decrypts an image; it accepts either an OCI descriptor representing a manifest list or a single manifest
470-
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) {
471-
return cryptImage(ctx, cs, ls, l, desc, cc, lf, cryptoOpDecrypt)
410+
func DecryptImage(ctx context.Context, cs content.Store, desc ocispec.Descriptor, cc *encconfig.CryptoConfig, lf LayerFilter) (ocispec.Descriptor, bool, error) {
411+
return cryptImage(ctx, cs, desc, cc, lf, cryptoOpDecrypt)
472412
}
473413

474414
// 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
478418
cc := encconfig.CryptoConfig{
479419
DecryptConfig: dc,
480420
}
421+
481422
lf := func(desc ocispec.Descriptor) bool {
482423
return true
483424
}
484-
// We shouldn't need to create any objects in CheckAuthorization, so no lease required.
485-
_, _, err := cryptImage(ctx, cs, nil, leases.Lease{}, desc, &cc, lf, cryptoOpUnwrapOnly)
425+
426+
_, _, err := cryptImage(ctx, cs, desc, &cc, lf, cryptoOpUnwrapOnly)
486427
if err != nil {
487428
return errors.Wrapf(err, "you are not authorized to use this image")
488429
}

0 commit comments

Comments
 (0)