Merge pull request #2202 from mtrmac/oci-dest-tar-privacy

Don't expose local account details in oci-archive tar files

go.mod

@@ -7,7 +7,7 @@ require (
 	github.com/BurntSushi/toml v1.3.2
 	github.com/containers/libtrust v0.0.0-20230121012942-c1716e8a8d01
 	github.com/containers/ocicrypt v1.1.9
-	github.com/containers/storage v1.51.1-0.20231120144510-2cf61989a5bc
+	github.com/containers/storage v1.51.1-0.20231129011200-e9f9f6605078
 	github.com/cyberphone/json-canonicalization v0.0.0-20231011164504-785e29786b46
 	github.com/distribution/reference v0.5.0
 	github.com/docker/cli v24.0.7+incompatible

go.sum

@@ -51,8 +51,8 @@ github.com/containers/libtrust v0.0.0-20230121012942-c1716e8a8d01 h1:Qzk5C6cYgle
 github.com/containers/libtrust v0.0.0-20230121012942-c1716e8a8d01/go.mod h1:9rfv8iPl1ZP7aqh9YA68wnZv2NUDbXdcdPHVz0pFbPY=
 github.com/containers/ocicrypt v1.1.9 h1:2Csfba4jse85Raxk5HIyEk8OwZNjRvfkhEGijOjIdEM=
 github.com/containers/ocicrypt v1.1.9/go.mod h1:dTKx1918d8TDkxXvarscpNVY+lyPakPNFN4jwA9GBys=
-github.com/containers/storage v1.51.1-0.20231120144510-2cf61989a5bc h1:K+fKkKkqwwY3YYM+RejJ6OcbCRZfDRZLoKsMMBAT2Bw=
-github.com/containers/storage v1.51.1-0.20231120144510-2cf61989a5bc/go.mod h1:oz9n9uia9xtxDQhw7nnlpMID5YKbHmMZsVFy4rR+5+s=
+github.com/containers/storage v1.51.1-0.20231129011200-e9f9f6605078 h1:b9vDfBHAbKKWotUoInxAP/rUf6KmqGRZBND6lSzUcbo=
+github.com/containers/storage v1.51.1-0.20231129011200-e9f9f6605078/go.mod h1:FHXkEBvKRmsTeB1JQIFfXnSyXCp+wVrt172O2ZlSzM4=
 github.com/coreos/go-oidc/v3 v3.7.0 h1:FTdj0uexT4diYIPlF4yoFVI5MRO1r5+SEcIpEw9vC0o=
 github.com/coreos/go-oidc/v3 v3.7.0/go.mod h1:yQzSCqBnK3e6Fs5l+f5i0F8Kwf0zpH9bPEsbY00KanM=
 github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=

oci/archive/oci_dest.go

@@ -13,6 +13,7 @@ import (
 	"github.com/containers/image/v5/internal/signature"
 	"github.com/containers/image/v5/types"
 	"github.com/containers/storage/pkg/archive"
+	"github.com/containers/storage/pkg/idtools"
 	digest "github.com/opencontainers/go-digest"
 	"github.com/sirupsen/logrus"
 )
@@ -169,10 +170,15 @@ func (d *ociArchiveImageDestination) Commit(ctx context.Context, unparsedTopleve
 // tar converts the directory at src and saves it to dst
 func tarDirectory(src, dst string) error {
 	// input is a stream of bytes from the archive of the directory at path
-	input, err := archive.Tar(src, archive.Uncompressed)
+	input, err := archive.TarWithOptions(src, &archive.TarOptions{
+		Compression: archive.Uncompressed,
+		// Don’t include the data about the user account this code is running under.
+		ChownOpts: &idtools.IDPair{UID: 0, GID: 0},
+	})
 	if err != nil {
 		return fmt.Errorf("retrieving stream of bytes from %q: %w", src, err)
 	}
+	defer input.Close()
 
 	// creates the tar file
 	outFile, err := os.Create(dst)

oci/archive/oci_dest_test.go

@@ -1,5 +1,45 @@
 package archive
 
-import "github.com/containers/image/v5/internal/private"
+import (
+	"archive/tar"
+	"io"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/containers/image/v5/internal/private"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
 
 var _ private.ImageDestination = (*ociArchiveImageDestination)(nil)
+
+func TestTarDirectory(t *testing.T) {
+	srcDir := t.TempDir()
+	err := os.WriteFile(filepath.Join(srcDir, "regular"), []byte("contents"), 0o600)
+	require.NoError(t, err)
+
+	dest := filepath.Join(t.TempDir(), "file.tar")
+	err = tarDirectory(srcDir, dest)
+	require.NoError(t, err)
+
+	f, err := os.Open(dest)
+	require.NoError(t, err)
+	defer f.Close()
+	reader := tar.NewReader(f)
+	numItems := 0
+	for {
+		hdr, err := reader.Next()
+		if err == io.EOF {
+			break
+		}
+		require.NoError(t, err)
+		// Test that the header does not expose data about the local account
+		assert.Equal(t, 0, hdr.Uid)
+		assert.Equal(t, 0, hdr.Gid)
+		assert.Empty(t, hdr.Uname)
+		assert.Empty(t, hdr.Gname)
+		numItems++
+	}
+	assert.Equal(t, 1, numItems)
+}