Implement simultaneous decryption and conversion from OCI to other fo…

…rmats Previously this would try first converting to the other format, and that would fail because the other format can't represent encrypted layers. So, do the layer edits for decryption first. Signed-off-by: Miloslav Trmač <[email protected]>
containers · Sep 7, 2023 · afe409a · afe409a
1 parent 3743a56
commit afe409a
Show file tree

Hide file tree

Showing 2 changed files with 148 additions and 4 deletions.
diff --git a/internal/image/oci.go b/internal/image/oci.go
@@ -15,6 +15,7 @@ import (
 	ociencspec "github.com/containers/ocicrypt/spec"
 	"github.com/opencontainers/go-digest"
 	imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1"
+	"golang.org/x/exp/slices"
 )
 
 type manifestOCI1 struct {
@@ -195,26 +196,72 @@ func (m *manifestOCI1) convertToManifestSchema2Generic(ctx context.Context, opti
 	return m.convertToManifestSchema2(ctx, options)
 }
 
+// prepareLayerDecryptEditsIfNecessary checks if options requires layer decryptions.
+// If not, it returns (nil, nil).
+// If decryption is required, it returns a set of edits to provide to OCI1.UpdateLayerInfos,
+// and edits *options to not try decryption again.
+func (m *manifestOCI1) prepareLayerDecryptEditsIfNecessary(options *types.ManifestUpdateOptions) ([]types.BlobInfo, error) {
+	if options == nil || !slices.ContainsFunc(options.LayerInfos, func(info types.BlobInfo) bool {
+		return info.CryptoOperation == types.Decrypt
+	}) {
+		return nil, nil
+	}
+
+	originalInfos := m.LayerInfos()
+	if len(originalInfos) != len(options.LayerInfos) {
+		return nil, fmt.Errorf("preparing to decrypt before conversion: %d layers vs. %d layer edits", len(originalInfos), len(options.LayerInfos))
+	}
+
+	res := slices.Clone(originalInfos) // Start with a full copy so that we don't forget to copy anything: use the current data in full unless we intentionaly deviate.
+	updatedEdits := slices.Clone(options.LayerInfos)
+	for i, info := range options.LayerInfos {
+		if info.CryptoOperation == types.Decrypt {
+			res[i].CryptoOperation = types.Decrypt
+			updatedEdits[i].CryptoOperation = types.PreserveOriginalCrypto // Don't try to decrypt in a schema[12] manifest later, that would fail.
+		}
+		// Don't do any compression-related MIME type conversions. m.LayerInfos() should not set these edit instructions, but be explicit.
+		res[i].CompressionOperation = types.PreserveOriginal
+		res[i].CompressionAlgorithm = nil
+	}
+	options.LayerInfos = updatedEdits
+	return res, nil
+}
+
 // convertToManifestSchema2 returns a genericManifest implementation converted to manifest.DockerV2Schema2MediaType.
 // It may use options.InformationOnly and also adjust *options to be appropriate for editing the returned
 // value.
 // This does not change the state of the original manifestOCI1 object.
-func (m *manifestOCI1) convertToManifestSchema2(_ context.Context, _ *types.ManifestUpdateOptions) (*manifestSchema2, error) {
+func (m *manifestOCI1) convertToManifestSchema2(_ context.Context, options *types.ManifestUpdateOptions) (*manifestSchema2, error) {
 	if m.m.Config.MediaType != imgspecv1.MediaTypeImageConfig {
 		return nil, internalManifest.NewNonImageArtifactError(&m.m.Manifest)
 	}
 
+	// Mostly we first make a format conversion, and _afterwards_ do layer edits. But first we need to do the layer edits
+	// which remove OCI-specific features, because trying to convert those layers would fail.
+	// So, do the layer updates for decryption.
+	ociManifest := m.m
+	layerDecryptEdits, err := m.prepareLayerDecryptEditsIfNecessary(options)
+	if err != nil {
+		return nil, err
+	}
+	if layerDecryptEdits != nil {
+		ociManifest = manifest.OCI1Clone(ociManifest)
+		if err := ociManifest.UpdateLayerInfos(layerDecryptEdits); err != nil {
+			return nil, err
+		}
+	}
+
 	// Create a copy of the descriptor.
-	config := schema2DescriptorFromOCI1Descriptor(m.m.Config)
+	config := schema2DescriptorFromOCI1Descriptor(ociManifest.Config)
 
 	// Above, we have already checked that this manifest refers to an image, not an OCI artifact,
 	// so the only difference between OCI and DockerSchema2 is the mediatypes. The
 	// media type of the manifest is handled by manifestSchema2FromComponents.
 	config.MediaType = manifest.DockerV2Schema2ConfigMediaType
 
-	layers := make([]manifest.Schema2Descriptor, len(m.m.Layers))
+	layers := make([]manifest.Schema2Descriptor, len(ociManifest.Layers))
 	for idx := range layers {
-		layers[idx] = schema2DescriptorFromOCI1Descriptor(m.m.Layers[idx])
+		layers[idx] = schema2DescriptorFromOCI1Descriptor(ociManifest.Layers[idx])
 		switch layers[idx].MediaType {
 		case imgspecv1.MediaTypeImageLayerNonDistributable: //nolint:staticcheck // NonDistributable layers are deprecated, but we want to continue to support manipulating pre-existing images.
 			layers[idx].MediaType = manifest.DockerV2Schema2ForeignLayerMediaType

diff --git a/internal/image/oci_test.go b/internal/image/oci_test.go
@@ -19,6 +19,7 @@ import (
 	imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"golang.org/x/exp/slices"
 )
 
 func manifestOCI1FromFixture(t *testing.T, src types.ImageSource, fixture string) genericManifest {
@@ -558,6 +559,49 @@ func TestManifestOCI1ConvertToManifestSchema1(t *testing.T) {
 	})
 	assert.Error(t, err)
 
+	// Conversion to schema1 with simultaneous decryption is possible
+	updatedLayers = layerInfosWithCryptoOperation(encrypted.LayerInfos(), types.Decrypt)
+	updatedLayersCopy = slices.Clone(updatedLayers)
+	res, err = encrypted.UpdatedImage(context.Background(), types.ManifestUpdateOptions{
+		LayerInfos:       updatedLayers,
+		ManifestMIMEType: manifest.DockerV2Schema1SignedMediaType,
+		InformationOnly: types.ManifestUpdateInformation{
+			Destination: memoryDest,
+		},
+	})
+	require.NoError(t, err)
+	assert.Equal(t, updatedLayersCopy, updatedLayers) // updatedLayers have not been modified in place
+	convertedJSON, mt, err = res.Manifest(context.Background())
+	require.NoError(t, err)
+	assert.Equal(t, manifest.DockerV2Schema1SignedMediaType, mt)
+	// Layers have been updated as expected
+	s1Manifest, err = manifestSchema1FromManifest(convertedJSON)
+	require.NoError(t, err)
+	assert.Equal(t, []types.BlobInfo{
+		{Digest: "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", Size: -1},
+		{Digest: GzippedEmptyLayerDigest, Size: -1},
+		{Digest: GzippedEmptyLayerDigest, Size: -1},
+		{Digest: GzippedEmptyLayerDigest, Size: -1},
+		{Digest: "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", Size: -1},
+		{Digest: GzippedEmptyLayerDigest, Size: -1},
+		{Digest: "sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc", Size: -1},
+		{Digest: GzippedEmptyLayerDigest, Size: -1},
+		{Digest: GzippedEmptyLayerDigest, Size: -1},
+		{Digest: GzippedEmptyLayerDigest, Size: -1},
+		{Digest: GzippedEmptyLayerDigest, Size: -1},
+		{Digest: "sha256:dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd", Size: -1},
+		{Digest: "sha256:eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee", Size: -1},
+		{Digest: GzippedEmptyLayerDigest, Size: -1},
+		{Digest: GzippedEmptyLayerDigest, Size: -1},
+	}, s1Manifest.LayerInfos())
+	// encrypted = the source Image implementation hasn’t been changed by the edits involved in the decrypt+convert+update
+	encrypted2 := manifestOCI1FromFixture(t, originalSrc, "oci1.encrypted.json")
+	typedEncrypted, ok := encrypted.(*manifestOCI1)
+	require.True(t, ok)
+	typedEncrypted2, ok := encrypted2.(*manifestOCI1)
+	require.True(t, ok)
+	assert.Equal(t, *typedEncrypted2, *typedEncrypted)
+
 	// FIXME? Test also the other failure cases, if only to see that we don't crash?
 }
 
@@ -600,6 +644,59 @@ func TestConvertToManifestSchema2(t *testing.T) {
 	})
 	assert.Error(t, err)
 
+	// Conversion to schema2 with simultaneous decryption is possible
+	updatedLayers := layerInfosWithCryptoOperation(encrypted.LayerInfos(), types.Decrypt)
+	updatedLayersCopy := slices.Clone(updatedLayers)
+	res, err = encrypted.UpdatedImage(context.Background(), types.ManifestUpdateOptions{
+		LayerInfos:       updatedLayers,
+		ManifestMIMEType: manifest.DockerV2Schema2MediaType,
+	})
+	require.NoError(t, err)
+	assert.Equal(t, updatedLayersCopy, updatedLayers) // updatedLayers have not been modified in place
+	convertedJSON, mt, err = res.Manifest(context.Background())
+	require.NoError(t, err)
+	assert.Equal(t, manifest.DockerV2Schema2MediaType, mt)
+	s2Manifest, err := manifestSchema2FromManifest(originalSrc, convertedJSON)
+	require.NoError(t, err)
+	assert.Equal(t, []types.BlobInfo{
+		{
+			Digest:    "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+			Size:      51354364,
+			MediaType: "application/vnd.docker.image.rootfs.diff.tar.gzip",
+		},
+		{
+			Digest:    "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
+			Size:      150,
+			MediaType: "application/vnd.docker.image.rootfs.diff.tar.gzip",
+		},
+		{
+			Digest:    "sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc",
+			Size:      11739507,
+			MediaType: "application/vnd.docker.image.rootfs.diff.tar.gzip",
+			URLs:      []string{"https://layer.url"},
+		},
+		{
+			Digest:    "sha256:dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd",
+			Size:      8841833,
+			MediaType: "application/vnd.docker.image.rootfs.diff.tar.gzip",
+		},
+		{
+			Digest:    "sha256:eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee",
+			Size:      291,
+			MediaType: "application/vnd.docker.image.rootfs.diff.tar.gzip",
+		},
+	}, s2Manifest.LayerInfos())
+	convertedConfig, err = res.ConfigBlob(context.Background())
+	require.NoError(t, err)
+	assertJSONEqualsFixture(t, convertedConfig, "oci1-to-schema2-config.json")
+	// encrypted = the source Image implementation hasn’t been changed by the edits involved in the decrypt+convert+update
+	encrypted2 := manifestOCI1FromFixture(t, originalSrc, "oci1.encrypted.json")
+	typedEncrypted, ok := encrypted.(*manifestOCI1)
+	require.True(t, ok)
+	typedEncrypted2, ok := encrypted2.(*manifestOCI1)
+	require.True(t, ok)
+	assert.Equal(t, *typedEncrypted2, *typedEncrypted)
+
 	// FIXME? Test also the other failure cases, if only to see that we don't crash?
 }