From f065a0a81cbbcb9b54927dda8289a1d48ecadc5c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= <mitr@redhat.com>
Date: Wed, 12 Jun 2024 16:10:58 +0200
Subject: [PATCH] Decide on tar-split usage based on trusted data in TOC
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Don't ignore the tar-split when the TOC requires one,
otherwise we could deduplicate a layer without tar-split
with a layer with tar-split.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
---
 pkg/chunked/compression_linux.go | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/pkg/chunked/compression_linux.go b/pkg/chunked/compression_linux.go
index 155a963e8..05b4ba11d 100644
--- a/pkg/chunked/compression_linux.go
+++ b/pkg/chunked/compression_linux.go
@@ -209,18 +209,26 @@ func readZstdChunkedManifest(blobStream ImageSourceSeekable, tocDigest digest.Di
 	}
 
 	decodedTarSplit := []byte{}
-	if tarSplitChunk.Offset > 0 {
-		// we must consume the data to not block the producer
+	if toc.TarSplitDigest != "" {
+		if tarSplitChunk.Offset <= 0 {
+			return nil, nil, nil, 0, fmt.Errorf("TOC requires a tar-split, but the %s annotation does not describe a position", internal.TarSplitInfoKey)
+		}
 		tarSplit, err := readBlob(tarSplitChunk.Length)
 		if err != nil {
 			return nil, nil, nil, 0, err
 		}
-		// but ignore it when the digest is not present, because we can’t authenticate it against tocDigest
-		if toc.TarSplitDigest != "" {
-			decodedTarSplit, err = decodeAndValidateBlob(tarSplit, tarSplitLengthUncompressed, toc.TarSplitDigest.String())
-			if err != nil {
-				return nil, nil, nil, 0, fmt.Errorf("validating and decompressing tar-split: %w", err)
-			}
+		decodedTarSplit, err = decodeAndValidateBlob(tarSplit, tarSplitLengthUncompressed, toc.TarSplitDigest.String())
+		if err != nil {
+			return nil, nil, nil, 0, fmt.Errorf("validating and decompressing tar-split: %w", err)
+		}
+	} else if tarSplitChunk.Offset > 0 {
+		// We must ignore the tar-split when the digest is not present in the TOC, because we can’t authenticate it.
+		//
+		// But if we asked for the chunk, now we must consume the data to not block the producer.
+		// Ideally the GetBlobAt API should be changed so that this is not necessary.
+		_, err := readBlob(tarSplitChunk.Length)
+		if err != nil {
+			return nil, nil, nil, 0, err
 		}
 	}
 	return decodedBlob, toc, decodedTarSplit, int64(manifestChunk.Offset), err