diff --git a/image/directory/directory_dest.go b/image/directory/directory_dest.go index fc82c969f1..8c0dc4c5d8 100644 --- a/image/directory/directory_dest.go +++ b/image/directory/directory_dest.go @@ -20,12 +20,21 @@ import ( "go.podman.io/storage/pkg/fileutils" ) -const version = "Directory Transport Version: 1.1\n" +// Write version file based on digest algorithm used +// 1.1 for sha256-only images, 1.2 otherwise. +const ( + versionPrefix = "Directory Transport Version: " + version = versionPrefix + "1.2\n" + version1_1 = versionPrefix + "1.1\n" +) // ErrNotContainerImageDir indicates that the directory doesn't match the expected contents of a directory created // using the 'dir' transport var ErrNotContainerImageDir = errors.New("not a containers image directory, don't want to overwrite important data") +// ErrUnsupportedVersion indicates that the directory uses a version newer than we support +var ErrUnsupportedVersion = errors.New("unsupported directory transport version") + type dirImageDestination struct { impl.Compat impl.PropertyMethodsInitialize @@ -33,7 +42,8 @@ type dirImageDestination struct { stubs.NoPutBlobPartialInitialize stubs.AlwaysSupportsSignatures - ref dirReference + ref dirReference + usesNonSHA256Digest bool } // newImageDestination returns an ImageDestination for writing to a directory. @@ -76,7 +86,11 @@ func newImageDestination(sys *types.SystemContext, ref dirReference) (private.Im return nil, err } // check if contents of version file is what we expect it to be - if string(contents) != version { + versionStr := string(contents) + if versionStr != version && versionStr != version1_1 { + if versionStr > version { + return nil, fmt.Errorf("%w: %q", ErrUnsupportedVersion, versionStr) + } return nil, ErrNotContainerImageDir } } else { @@ -94,11 +108,6 @@ func newImageDestination(sys *types.SystemContext, ref dirReference) (private.Im return nil, fmt.Errorf("unable to create directory %q: %w", ref.resolvedPath, err) } } - // create version file - err = os.WriteFile(ref.versionPath(), []byte(version), 0o644) - if err != nil { - return nil, fmt.Errorf("creating version file %q: %w", ref.versionPath(), err) - } d := &dirImageDestination{ PropertyMethodsInitialize: impl.PropertyMethods(impl.Properties{ @@ -151,7 +160,8 @@ func (d *dirImageDestination) PutBlobWithOptions(ctx context.Context, stream io. } }() - digester, stream := putblobdigest.DigestIfCanonicalUnknown(stream, inputInfo) + digester, stream := putblobdigest.DigestIfUnknown(stream, inputInfo) + // TODO: This can take quite some time, and should ideally be cancellable using ctx.Done(). size, err := io.Copy(blobFile, stream) if err != nil { @@ -165,6 +175,10 @@ func (d *dirImageDestination) PutBlobWithOptions(ctx context.Context, stream io. return private.UploadedBlob{}, err } + if blobDigest.Algorithm() != digest.Canonical { + d.usesNonSHA256Digest = true + } + // On POSIX systems, blobFile was created with mode 0600, so we need to make it readable. // On Windows, the “permissions of newly created files” argument to syscall.Open is // ignored and the file is already readable; besides, blobFile.Chmod, i.e. syscall.Fchmod, @@ -257,6 +271,14 @@ func (d *dirImageDestination) PutSignaturesWithFormat(ctx context.Context, signa // - Uploaded data MAY be visible to others before CommitWithOptions() is called // - Uploaded data MAY be removed or MAY remain around if Close() is called without CommitWithOptions() (i.e. rollback is allowed but not guaranteed) func (d *dirImageDestination) CommitWithOptions(ctx context.Context, options private.CommitOptions) error { + versionToWrite := version1_1 + if d.usesNonSHA256Digest { + versionToWrite = version + } + err := os.WriteFile(d.ref.versionPath(), []byte(versionToWrite), 0o644) + if err != nil { + return fmt.Errorf("writing version file %q: %w", d.ref.versionPath(), err) + } return nil } diff --git a/image/directory/directory_src.go b/image/directory/directory_src.go index b7fb3855c5..18c7899e97 100644 --- a/image/directory/directory_src.go +++ b/image/directory/directory_src.go @@ -26,7 +26,24 @@ type dirImageSource struct { // newImageSource returns an ImageSource reading from an existing directory. // The caller must call .Close() on the returned ImageSource. -func newImageSource(ref dirReference) private.ImageSource { +func newImageSource(ref dirReference) (private.ImageSource, error) { + versionPath := ref.versionPath() + contents, err := os.ReadFile(versionPath) + if err != nil { + if !os.IsNotExist(err) { + return nil, fmt.Errorf("reading version file %q: %w", versionPath, err) + } + } else { + versionStr := string(contents) + if versionStr != version && versionStr != version1_1 { + // Check if it's a future version we don't support + if versionStr > version { + return nil, fmt.Errorf("%w: %q", ErrUnsupportedVersion, versionStr) + } + return nil, fmt.Errorf("invalid version file content: %q", versionStr) + } + } + s := &dirImageSource{ PropertyMethodsInitialize: impl.PropertyMethods(impl.Properties{ HasThreadSafeGetBlob: false, @@ -36,7 +53,7 @@ func newImageSource(ref dirReference) private.ImageSource { ref: ref, } s.Compat = impl.AddCompat(s) - return s + return s, nil } // Reference returns the reference used to set up this source, _as specified by the user_ diff --git a/image/directory/directory_test.go b/image/directory/directory_test.go index 1d5fb96947..dd6e846360 100644 --- a/image/directory/directory_test.go +++ b/image/directory/directory_test.go @@ -6,6 +6,7 @@ import ( "errors" "io" "os" + "path/filepath" "testing" "github.com/opencontainers/go-digest" @@ -201,8 +202,78 @@ func TestGetPutSignatures(t *testing.T) { assert.Equal(t, signatures, sigs) } +func TestVersionAssignment(t *testing.T) { + t.Run("SHA256 gets version 1.1", func(t *testing.T) { + ref, tmpDir := refToTempDir(t) + cache := memory.New() + + dest, err := ref.NewImageDestination(context.Background(), nil) + require.NoError(t, err) + defer dest.Close() + + blob := []byte("test-blob-sha256") + _, err = dest.PutBlob(context.Background(), bytes.NewReader(blob), types.BlobInfo{Digest: "", Size: int64(len(blob))}, cache, false) + require.NoError(t, err) + + err = dest.Commit(context.Background(), nil) + require.NoError(t, err) + + versionBytes, err := os.ReadFile(filepath.Join(tmpDir, "version")) + require.NoError(t, err) + assert.Equal(t, "Directory Transport Version: 1.1\n", string(versionBytes)) + }) + + t.Run("Non-SHA256 gets version 1.2", func(t *testing.T) { + ref, tmpDir := refToTempDir(t) + cache := memory.New() + + dest, err := ref.NewImageDestination(context.Background(), nil) + require.NoError(t, err) + defer dest.Close() + + blob := []byte("test-blob-sha512") + sha512Digest := digest.SHA512.FromBytes(blob) + _, err = dest.PutBlob(context.Background(), bytes.NewReader(blob), types.BlobInfo{Digest: sha512Digest, Size: int64(len(blob))}, cache, false) + require.NoError(t, err) + + err = dest.Commit(context.Background(), nil) + require.NoError(t, err) + + versionBytes, err := os.ReadFile(filepath.Join(tmpDir, "version")) + require.NoError(t, err) + assert.Equal(t, "Directory Transport Version: 1.2\n", string(versionBytes)) + }) + + t.Run("Mixed digests get version 1.2", func(t *testing.T) { + ref, tmpDir := refToTempDir(t) + cache := memory.New() + + dest, err := ref.NewImageDestination(context.Background(), nil) + require.NoError(t, err) + defer dest.Close() + + blob1 := []byte("test-blob-sha256") + _, err = dest.PutBlob(context.Background(), bytes.NewReader(blob1), types.BlobInfo{Digest: "", Size: int64(len(blob1))}, cache, false) + require.NoError(t, err) + + blob2 := []byte("test-blob-sha512") + sha512Digest := digest.SHA512.FromBytes(blob2) + _, err = dest.PutBlob(context.Background(), bytes.NewReader(blob2), types.BlobInfo{Digest: sha512Digest, Size: int64(len(blob2))}, cache, false) + require.NoError(t, err) + + err = dest.Commit(context.Background(), nil) + require.NoError(t, err) + + versionBytes, err := os.ReadFile(filepath.Join(tmpDir, "version")) + require.NoError(t, err) + assert.Equal(t, "Directory Transport Version: 1.2\n", string(versionBytes)) + }) +} + func TestSourceReference(t *testing.T) { ref, tmpDir := refToTempDir(t) + err := os.WriteFile(filepath.Join(tmpDir, "version"), []byte("Directory Transport Version: 1.1\n"), 0o644) + require.NoError(t, err) src, err := ref.NewImageSource(context.Background(), nil) require.NoError(t, err) diff --git a/image/directory/directory_transport.go b/image/directory/directory_transport.go index 77c0e7be57..165159e588 100644 --- a/image/directory/directory_transport.go +++ b/image/directory/directory_transport.go @@ -146,7 +146,7 @@ func (ref dirReference) NewImage(ctx context.Context, sys *types.SystemContext) // NewImageSource returns a types.ImageSource for this reference. // The caller must call .Close() on the returned ImageSource. func (ref dirReference) NewImageSource(ctx context.Context, sys *types.SystemContext) (types.ImageSource, error) { - return newImageSource(ref), nil + return newImageSource(ref) } // NewImageDestination returns a types.ImageDestination for this reference. @@ -172,12 +172,19 @@ func (ref dirReference) manifestPath(instanceDigest *digest.Digest) (string, err } // layerPath returns a path for a layer tarball within a directory using our conventions. -func (ref dirReference) layerPath(digest digest.Digest) (string, error) { - if err := digest.Validate(); err != nil { // digest.Digest.Encoded() panics on failure, and could possibly result in a path with ../, so validate explicitly. +func (ref dirReference) layerPath(d digest.Digest) (string, error) { + if err := d.Validate(); err != nil { // digest.Digest.Encoded() panics on failure, and could possibly result in a path with ../, so validate explicitly. return "", err } - // FIXME: Should we keep the digest identification? - return filepath.Join(ref.path, digest.Encoded()), nil + + var filename string + if d.Algorithm() == digest.Canonical { + filename = d.Encoded() + } else { + filename = d.Algorithm().String() + "-" + d.Encoded() + } + + return filepath.Join(ref.path, filename), nil } // signaturePath returns a path for a signature within a directory using our conventions. diff --git a/image/directory/directory_transport_test.go b/image/directory/directory_transport_test.go index bcafff5352..b9e6ce581c 100644 --- a/image/directory/directory_transport_test.go +++ b/image/directory/directory_transport_test.go @@ -172,7 +172,9 @@ func TestReferenceNewImageNoValidManifest(t *testing.T) { } func TestReferenceNewImageSource(t *testing.T) { - ref, _ := refToTempDir(t) + ref, tmpDir := refToTempDir(t) + err := os.WriteFile(filepath.Join(tmpDir, "version"), []byte("Directory Transport Version: 1.1\n"), 0o644) + require.NoError(t, err) src, err := ref.NewImageSource(context.Background(), nil) assert.NoError(t, err) defer src.Close() @@ -209,14 +211,21 @@ func TestReferenceManifestPath(t *testing.T) { } func TestReferenceLayerPath(t *testing.T) { - const hex = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + const hex256 = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + const hex512 = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" ref, tmpDir := refToTempDir(t) dirRef, ok := ref.(dirReference) require.True(t, ok) - res, err := dirRef.layerPath("sha256:" + hex) + + res, err := dirRef.layerPath("sha256:" + hex256) require.NoError(t, err) - assert.Equal(t, tmpDir+"/"+hex, res) + assert.Equal(t, tmpDir+"/"+hex256, res) + + res, err = dirRef.layerPath("sha512:" + hex512) + require.NoError(t, err) + assert.Equal(t, tmpDir+"/sha512-"+hex512, res) + _, err = dirRef.layerPath(digest.Digest("sha256:../hello")) assert.Error(t, err) } diff --git a/image/internal/manifest/manifest.go b/image/internal/manifest/manifest.go index 687b37fb07..1ef6184f74 100644 --- a/image/internal/manifest/manifest.go +++ b/image/internal/manifest/manifest.go @@ -107,25 +107,45 @@ func GuessMIMEType(manifest []byte) string { return "" } -// Digest returns the a digest of a docker manifest, with any necessary implied transformations like stripping v1s1 signatures. -// This is publicly visible as c/image/manifest.Digest. -func Digest(manifest []byte) (digest.Digest, error) { +// stripManifestSignature strips v1s1 signatures from a manifest if present. +// Returns the manifest bytes (either the original or the unsigned payload). +func stripManifestSignature(manifest []byte) ([]byte, error) { if GuessMIMEType(manifest) == DockerV2Schema1SignedMediaType { sig, err := libtrust.ParsePrettySignature(manifest, "signatures") if err != nil { - return "", err + return nil, err } manifest, err = sig.Payload() if err != nil { // Coverage: This should never happen, libtrust's Payload() can fail only if joseBase64UrlDecode() fails, on a string // that libtrust itself has josebase64UrlEncode()d - return "", err + return nil, err } } + return manifest, nil +} +// Digest returns the a digest of a docker manifest, with any necessary implied transformations like stripping v1s1 signatures. +// This is publicly visible as c/image/manifest.Digest. +func Digest(manifest []byte) (digest.Digest, error) { + manifest, err := stripManifestSignature(manifest) + if err != nil { + return "", err + } return digest.FromBytes(manifest), nil } +// DigestWithAlgorithm returns the digest of a docker manifest using the specified algorithm, +// with any necessary implied transformations like stripping v1s1 signatures. +// This is publicly visible as c/image/manifest.DigestWithAlgorithm. +func DigestWithAlgorithm(manifest []byte, algo digest.Algorithm) (digest.Digest, error) { + manifest, err := stripManifestSignature(manifest) + if err != nil { + return "", err + } + return algo.FromBytes(manifest), nil +} + // MatchesDigest returns true iff the manifest matches expectedDigest. // Error may be set if this returns false. // Note that this is not doing ConstantTimeCompare; by the time we get here, the cryptographic signature must already have been verified, diff --git a/image/internal/manifest/manifest_test.go b/image/internal/manifest/manifest_test.go index da50bf76da..f158010954 100644 --- a/image/internal/manifest/manifest_test.go +++ b/image/internal/manifest/manifest_test.go @@ -73,6 +73,65 @@ func TestDigest(t *testing.T) { assert.Equal(t, digest.Digest(digestSha256EmptyTar), actualDigest) } +func TestDigestWithAlgorithm(t *testing.T) { + sha256Cases := []struct { + path string + expectedDigest digest.Digest + }{ + {"v2s2.manifest.json", TestDockerV2S2ManifestDigest}, + {"v2s1.manifest.json", TestDockerV2S1ManifestDigest}, + {"v2s1-unsigned.manifest.json", TestDockerV2S1UnsignedManifestDigest}, + } + for _, c := range sha256Cases { + manifest, err := os.ReadFile(filepath.Join("testdata", c.path)) + require.NoError(t, err) + actualDigest, err := DigestWithAlgorithm(manifest, digest.SHA256) + require.NoError(t, err) + assert.Equal(t, c.expectedDigest, actualDigest, c.path) + defaultDigest, err := Digest(manifest) + require.NoError(t, err) + assert.Equal(t, defaultDigest, actualDigest, c.path) + } + + // Test with SHA512 + for _, c := range sha256Cases { + manifest, err := os.ReadFile(filepath.Join("testdata", c.path)) + require.NoError(t, err) + actualDigest, err := DigestWithAlgorithm(manifest, digest.SHA512) + require.NoError(t, err) + assert.Equal(t, digest.SHA512, actualDigest.Algorithm()) + sha256Digest, err := DigestWithAlgorithm(manifest, digest.SHA256) + require.NoError(t, err) + assert.NotEqual(t, sha256Digest, actualDigest, c.path) + } + + // Test that v2s1 signed manifest signature stripping works with different algorithms + manifest, err := os.ReadFile("testdata/v2s1.manifest.json") + require.NoError(t, err) + unsignedManifest, err := os.ReadFile("testdata/v2s1-unsigned.manifest.json") + require.NoError(t, err) + + // Both signed and unsigned should produce the same digest for each algorithm + for _, algo := range []digest.Algorithm{digest.SHA256, digest.SHA512} { + signedDigest, err := DigestWithAlgorithm(manifest, algo) + require.NoError(t, err) + unsignedDigest, err := DigestWithAlgorithm(unsignedManifest, algo) + require.NoError(t, err) + assert.Equal(t, unsignedDigest, signedDigest, "algorithm: %s", algo) + } + + manifest, err = os.ReadFile("testdata/v2s1-invalid-signatures.manifest.json") + require.NoError(t, err) + _, err = DigestWithAlgorithm(manifest, digest.SHA256) + assert.Error(t, err) + _, err = DigestWithAlgorithm(manifest, digest.SHA512) + assert.Error(t, err) + + actualDigest, err := DigestWithAlgorithm([]byte{}, digest.SHA256) + require.NoError(t, err) + assert.Equal(t, digest.Digest(digestSha256EmptyTar), actualDigest) +} + func TestMatchesDigest(t *testing.T) { cases := []struct { path string diff --git a/image/manifest/manifest.go b/image/manifest/manifest.go index ed489a5a6c..d54b395d31 100644 --- a/image/manifest/manifest.go +++ b/image/manifest/manifest.go @@ -113,6 +113,12 @@ func Digest(manifestBlob []byte) (digest.Digest, error) { return manifest.Digest(manifestBlob) } +// DigestWithAlgorithm returns the digest of a docker manifest using the specified algorithm, +// with any necessary implied transformations like stripping v1s1 signatures. +func DigestWithAlgorithm(manifestBlob []byte, algo digest.Algorithm) (digest.Digest, error) { + return manifest.DigestWithAlgorithm(manifestBlob, algo) +} + // MatchesDigest returns true iff the manifest matches expectedDigest. // Error may be set if this returns false. // Note that this is not doing ConstantTimeCompare; by the time we get here, the cryptographic signature must already have been verified,