From 15cfbaed59edddf60530edd767dcdc286324818d Mon Sep 17 00:00:00 2001 From: Valentin Delaye Date: Sun, 9 Mar 2025 09:45:51 +0100 Subject: [PATCH] Validate digest when pushing blobs Signed-off-by: Valentin Delaye --- pom.xml | 2 +- src/main/java/land/oras/OCILayout.java | 34 ++++++++------ src/main/java/land/oras/Registry.java | 33 ++++++++------ src/test/java/land/oras/OCILayoutTest.java | 44 +++++++++++++------ src/test/java/land/oras/RegistryTest.java | 18 +++++++- .../java/land/oras/RegistryWireMockTest.java | 25 +++++++---- 6 files changed, 103 insertions(+), 53 deletions(-) diff --git a/pom.xml b/pom.xml index d0656ef0..e7d91652 100644 --- a/pom.xml +++ b/pom.xml @@ -4,7 +4,7 @@ land.oras oras-java-sdk - 0.3.0-SNAPSHOT + 0.2.1-SNAPSHOT jar ${project.groupId}:${project.artifactId} ORAS Java SDK diff --git a/src/main/java/land/oras/OCILayout.java b/src/main/java/land/oras/OCILayout.java index 2d1929cd..df1fdf4f 100644 --- a/src/main/java/land/oras/OCILayout.java +++ b/src/main/java/land/oras/OCILayout.java @@ -98,6 +98,7 @@ public void fetchBlob(LayoutRef ref, Path path) { InputStream is = fetchBlob(ref); try { Files.copy(is, path); + LOG.info("Downloaded: {}", ref.getTag()); } catch (IOException e) { throw new OrasException("Failed to fetch blob", e); } @@ -115,14 +116,14 @@ public InputStream fetchBlob(LayoutRef ref) { @Override public Layer pushBlob(LayoutRef ref, Path blob, Map annotations) { - ensureDigest(ref); + ensureDigest(ref, blob); Path blobPath = getBlobPath(ref); String digest = ref.getAlgorithm().digest(blob); ensureAlgorithmPath(digest); LOG.debug("Digest: {}", digest); try { if (Files.exists(blobPath)) { - LOG.debug("Blob already exists: {}", blobPath); + LOG.info("Blob already exists: {}", digest); return Layer.fromFile(blobPath, ref.getAlgorithm()).withAnnotations(annotations); } Files.copy(blob, blobPath); @@ -134,12 +135,12 @@ public Layer pushBlob(LayoutRef ref, Path blob, Map annotations) @Override public Layer pushBlob(LayoutRef ref, byte[] data) { - ensureDigest(ref); - String digest = ref.getAlgorithm().digest(data); - ensureAlgorithmPath(digest); try { Path path = Files.createTempFile("oras", "blob"); Files.write(path, data); + ensureDigest(ref, path); + String digest = ref.getAlgorithm().digest(data); + ensureAlgorithmPath(digest); return pushBlob(ref, path, Map.of()); } catch (IOException e) { throw new OrasException("Failed to push blob to OCI layout", e); @@ -296,15 +297,6 @@ else if (registry.isIndexMediaType(contentType)) { } } - private void ensureDigest(LayoutRef ref) { - if (ref.getTag() == null) { - throw new OrasException("Missing ref"); - } - if (!SupportedAlgorithm.isSupported(ref.getTag())) { - throw new OrasException("Unsupported digest: %s".formatted(ref.getTag())); - } - } - private Path getOciLayoutPath() { return path.resolve(Const.OCI_LAYOUT_FILE); } @@ -431,6 +423,20 @@ private void writeConfig(Registry registry, ContainerRef containerRef, Config co } } + private void ensureDigest(LayoutRef ref, Path path) { + if (ref.getTag() == null) { + throw new OrasException("Missing ref"); + } + if (!SupportedAlgorithm.isSupported(ref.getTag())) { + throw new OrasException("Unsupported digest: %s".formatted(ref.getTag())); + } + SupportedAlgorithm algorithm = SupportedAlgorithm.fromDigest(ref.getTag()); + String pathDigest = algorithm.digest(path); + if (!ref.getTag().equals(pathDigest)) { + throw new OrasException("Digest mismatch: %s != %s".formatted(ref.getTag(), pathDigest)); + } + } + /** * Builder for the registry */ diff --git a/src/main/java/land/oras/Registry.java b/src/main/java/land/oras/Registry.java index 04895971..d9bc516f 100644 --- a/src/main/java/land/oras/Registry.java +++ b/src/main/java/land/oras/Registry.java @@ -445,6 +445,9 @@ public Layer pushBlob(ContainerRef containerRef, Path blob, Map @Override public Layer pushBlob(ContainerRef containerRef, byte[] data) { String digest = containerRef.getAlgorithm().digest(data); + if (containerRef.getDigest() != null) { + ensureDigest(containerRef, data); + } if (hasBlob(containerRef.withDigest(digest))) { LOG.info("Blob already exists: {}", digest); return Layer.fromData(containerRef, data); @@ -517,21 +520,11 @@ public boolean hasBlob(ContainerRef containerRef) { */ @Override public byte[] getBlob(ContainerRef containerRef) { - if (!hasBlob(containerRef)) { - throw new OrasException(new OrasHttpClient.ResponseWrapper<>("", 404, Map.of())); - } - URI uri = URI.create("%s://%s".formatted(getScheme(), containerRef.getBlobsPath())); - OrasHttpClient.ResponseWrapper response = - client.get(uri, Map.of(Const.ACCEPT_HEADER, Const.APPLICATION_OCTET_STREAM_HEADER_VALUE)); - logResponse(response); - - // Switch to bearer auth if needed and retry first request - if (switchTokenAuth(containerRef, response)) { - response = client.get(uri, Map.of(Const.ACCEPT_HEADER, Const.APPLICATION_OCTET_STREAM_HEADER_VALUE)); - logResponse(response); + try (InputStream is = fetchBlob(containerRef)) { + return ensureDigest(containerRef, is.readAllBytes()); + } catch (IOException e) { + throw new OrasException("Failed to get blob", e); } - handleError(response); - return response.response().getBytes(); } @Override @@ -620,6 +613,18 @@ private OrasHttpClient.ResponseWrapper getManifestResponse(ContainerRef return client.get(uri, Map.of("Accept", Const.MANIFEST_ACCEPT_TYPE)); } + private byte[] ensureDigest(ContainerRef ref, byte[] data) { + if (ref.getDigest() == null) { + throw new OrasException("Missing digest"); + } + SupportedAlgorithm algorithm = SupportedAlgorithm.fromDigest(ref.getDigest()); + String dataDigest = algorithm.digest(data); + if (!ref.getDigest().equals(dataDigest)) { + throw new OrasException("Digest mismatch: %s != %s".formatted(ref.getTag(), dataDigest)); + } + return data; + } + /** * Switch the current authentication to token auth * @param response The response diff --git a/src/test/java/land/oras/OCILayoutTest.java b/src/test/java/land/oras/OCILayoutTest.java index f2bf5cd3..8bf92afe 100644 --- a/src/test/java/land/oras/OCILayoutTest.java +++ b/src/test/java/land/oras/OCILayoutTest.java @@ -122,9 +122,10 @@ void shouldPushArtifact() throws IOException { Path path = layoutPath.resolve("shouldPushArtifact"); - LayoutRef layoutRef = - LayoutRef.parse("%s@sha256:98ea6e4f216f2fb4b69fff9b3a44842c38686ca685f3f55dc48c5d3fb1107be4" - .formatted(path.toString())); + byte[] content = "hi".getBytes(StandardCharsets.UTF_8); + String digest = SupportedAlgorithm.SHA256.digest(content); + + LayoutRef layoutRef = LayoutRef.parse("%s@%s".formatted(path.toString(), digest)); OCILayout ociLayout = OCILayout.Builder.builder().defaults(path).build(); // Not implemented @@ -136,14 +137,14 @@ void shouldPushArtifact() throws IOException { ociLayout.pushBlob(layoutRef, "hi".getBytes(StandardCharsets.UTF_8)); // Assert file exists - assertBlobExists(path, "sha256:98ea6e4f216f2fb4b69fff9b3a44842c38686ca685f3f55dc48c5d3fb1107be4"); - assertBlobContent(path, "sha256:98ea6e4f216f2fb4b69fff9b3a44842c38686ca685f3f55dc48c5d3fb1107be4", "hi"); + assertBlobExists(path, digest); + assertBlobContent(path, digest, "hi"); // Push again ociLayout.pushBlob(layoutRef, "hi".getBytes(StandardCharsets.UTF_8)); - assertBlobExists(path, "sha256:98ea6e4f216f2fb4b69fff9b3a44842c38686ca685f3f55dc48c5d3fb1107be4"); - assertBlobContent(path, "sha256:98ea6e4f216f2fb4b69fff9b3a44842c38686ca685f3f55dc48c5d3fb1107be4", "hi"); + assertBlobExists(path, digest); + assertBlobContent(path, digest, "hi"); } @Test @@ -165,6 +166,27 @@ void cannotPushBlobWithoutTagOrDigest() throws IOException { }); } + @Test + void cannotPushWithInvalidDigest() { + Path invalidBlobPushDir = layoutPath.resolve("cannotPushWithInvalidDigest"); + + LayoutRef wrongDigest1 = LayoutRef.parse("%s@sha234:1234".formatted(invalidBlobPushDir.toString())); + OCILayout ociLayout = + OCILayout.Builder.builder().defaults(invalidBlobPushDir).build(); + + // Push more blobs + assertThrows(OrasException.class, () -> { + ociLayout.pushBlob(wrongDigest1, "hi".getBytes(StandardCharsets.UTF_8)); + }); + + LayoutRef wrongDigest2 = LayoutRef.parse("%s@sha256:1234".formatted(invalidBlobPushDir.toString())); + + // Push more blobs + assertThrows(OrasException.class, () -> { + ociLayout.pushBlob(wrongDigest2, "hi".getBytes(StandardCharsets.UTF_8)); + }); + } + @Test void testShouldCopyArtifactFromRegistryIntoOciLayout() throws IOException { @@ -273,10 +295,7 @@ void testShouldCopyImageIntoOciLayoutWithoutIndex() { Manifest emptyManifest = Manifest.empty() .withLayers(List.of(Layer.fromDigest(layer1.getDigest(), 2), Layer.fromDigest(layer2.getDigest(), 6))); - String manifestDigest = - SupportedAlgorithm.SHA256.digest(emptyManifest.toJson().getBytes(StandardCharsets.UTF_8)); - String configDigest = - SupportedAlgorithm.SHA256.digest(Config.empty().toJson().getBytes(StandardCharsets.UTF_8)); + String configDigest = Config.empty().getDigest(); // Push config and manifest registry.pushConfig(containerRef.withDigest(configDigest), Config.empty()); @@ -344,8 +363,7 @@ void testShouldCopyImageIntoOciLayoutWithIndex() throws IOException { .withLayers(List.of(Layer.fromDigest(layer1.getDigest(), 2), Layer.fromDigest(layer2.getDigest(), 6))); String manifestDigest = SupportedAlgorithm.SHA256.digest(emptyManifest.toJson().getBytes(StandardCharsets.UTF_8)); - String configDigest = - SupportedAlgorithm.SHA256.digest(Config.empty().toJson().getBytes(StandardCharsets.UTF_8)); + String configDigest = Config.empty().getDigest(); // Push config and manifest registry.pushConfig(containerRef.withDigest(configDigest), Config.empty()); diff --git a/src/test/java/land/oras/RegistryTest.java b/src/test/java/land/oras/RegistryTest.java index 26ab8d38..970dd13a 100644 --- a/src/test/java/land/oras/RegistryTest.java +++ b/src/test/java/land/oras/RegistryTest.java @@ -71,6 +71,21 @@ void before() { registry.withFollowOutput(); } + @Test + void shouldFailToPushBlobForInvalidDigest() { + Registry registry = Registry.Builder.builder() + .defaults("myuser", "mypass") + .withInsecure(true) + .build(); + ContainerRef containerRef1 = ContainerRef.parse( + "%s/library/artifact-text@sha256:2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824" + .formatted(this.registry.getRegistry())); + // Ensure the blob is deleted + assertThrows(OrasException.class, () -> { + registry.pushBlob(containerRef1, "invalid".getBytes()); + }); + } + @Test void shouldPushAndGetBlobThenDeleteWithSha256() { Registry registry = Registry.Builder.builder() @@ -515,8 +530,7 @@ void testNotFailToPullArtifactFromImage() { Manifest emptyManifest = Manifest.empty().withLayers(List.of(Layer.fromDigest(emptyLayer.getDigest(), 2))); String manifestDigest = SupportedAlgorithm.SHA256.digest(emptyManifest.toJson().getBytes(StandardCharsets.UTF_8)); - String configDigest = - SupportedAlgorithm.SHA256.digest(Config.empty().toJson().getBytes(StandardCharsets.UTF_8)); + String configDigest = Config.empty().getDigest(); // Push config and manifest registry.pushConfig(containerRef.withDigest(configDigest), Config.empty()); diff --git a/src/test/java/land/oras/RegistryWireMockTest.java b/src/test/java/land/oras/RegistryWireMockTest.java index ae06e6f3..07276b5e 100644 --- a/src/test/java/land/oras/RegistryWireMockTest.java +++ b/src/test/java/land/oras/RegistryWireMockTest.java @@ -42,6 +42,7 @@ import land.oras.exception.OrasException; import land.oras.utils.Const; import land.oras.utils.JsonUtils; +import land.oras.utils.SupportedAlgorithm; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -64,14 +65,16 @@ public class RegistryWireMockTest { @Test void shouldRedirectWhenDownloadingBlob(WireMockRuntimeInfo wmRuntimeInfo) { + String digest = SupportedAlgorithm.SHA256.digest("blob-data".getBytes()); + // Return data from wiremock WireMock wireMock = wmRuntimeInfo.getWireMock(); - wireMock.register(WireMock.any(WireMock.urlEqualTo("/v2/library/artifact-text/blobs/sha256:one")) + wireMock.register(WireMock.any(WireMock.urlEqualTo("/v2/library/artifact-text/blobs/%s".formatted(digest))) .willReturn(WireMock.temporaryRedirect("http://localhost:%d/v2/library/artifact-text/blobs/sha256:other" .formatted(wmRuntimeInfo.getHttpPort())))); // Return blob on new location - wireMock.register(WireMock.head(WireMock.urlEqualTo("/v2/library/artifact-text/blobs/sha256:other")) + wireMock.register(WireMock.head(WireMock.urlEqualTo("/v2/library/artifact-text/blobs/%s".formatted(digest))) .willReturn(WireMock.ok())); wireMock.register(WireMock.get(WireMock.urlEqualTo("/v2/library/artifact-text/blobs/sha256:other")) .willReturn(WireMock.ok().withBody("blob-data"))); @@ -84,7 +87,7 @@ void shouldRedirectWhenDownloadingBlob(WireMockRuntimeInfo wmRuntimeInfo) { ContainerRef containerRef = ContainerRef.parse("localhost:%d/library/artifact-text".formatted(wmRuntimeInfo.getHttpPort())); - byte[] blob = registry.getBlob(containerRef.withDigest("sha256:one")); + byte[] blob = registry.getBlob(containerRef.withDigest(digest)); assertEquals("blob-data", new String(blob)); } @@ -270,9 +273,11 @@ void shouldRetryBlobUpload(WireMockRuntimeInfo wmRuntimeInfo) throws IOException @Test void shouldGetToken(WireMockRuntimeInfo wmRuntimeInfo) { + String digest = SupportedAlgorithm.SHA256.digest("blob-data".getBytes()); + // Return data from wiremock WireMock wireMock = wmRuntimeInfo.getWireMock(); - wireMock.register(WireMock.any(WireMock.urlEqualTo("/v2/library/get-token/blobs/sha256:one")) + wireMock.register(WireMock.any(WireMock.urlEqualTo("/v2/library/get-token/blobs/%s".formatted(digest))) .inScenario("get token") .willReturn(WireMock.unauthorized() .withHeader( @@ -289,7 +294,7 @@ void shouldGetToken(WireMockRuntimeInfo wmRuntimeInfo) { "fake-token", "access-token", 300, ZonedDateTime.now()))))); // On the second call we return ok - wireMock.register(WireMock.any(WireMock.urlEqualTo("/v2/library/get-token/blobs/sha256:one")) + wireMock.register(WireMock.any(WireMock.urlEqualTo("/v2/library/get-token/blobs/%s".formatted(digest))) .inScenario("get token") .whenScenarioStateIs("get") .willReturn(WireMock.ok().withBody("blob-data"))); @@ -302,16 +307,18 @@ void shouldGetToken(WireMockRuntimeInfo wmRuntimeInfo) { ContainerRef containerRef = ContainerRef.parse("localhost:%d/library/get-token".formatted(wmRuntimeInfo.getHttpPort())); - byte[] blob = registry.getBlob(containerRef.withDigest("sha256:one")); + byte[] blob = registry.getBlob(containerRef.withDigest(digest)); assertEquals("blob-data", new String(blob)); } @Test void shouldRefreshExpiredToken(WireMockRuntimeInfo wmRuntimeInfo) { + String digest = SupportedAlgorithm.SHA256.digest("blob-data".getBytes()); + // Return data from wiremock WireMock wireMock = wmRuntimeInfo.getWireMock(); - wireMock.register(WireMock.any(WireMock.urlEqualTo("/v2/library/refresh-token/blobs/sha256:one")) + wireMock.register(WireMock.any(WireMock.urlEqualTo("/v2/library/refresh-token/blobs/%s".formatted(digest))) .inScenario("get token") .willReturn(WireMock.forbidden() .withHeader( @@ -328,7 +335,7 @@ void shouldRefreshExpiredToken(WireMockRuntimeInfo wmRuntimeInfo) { "fake-token", "access-token", 300, ZonedDateTime.now()))))); // On the second call we return ok - wireMock.register(WireMock.any(WireMock.urlEqualTo("/v2/library/refresh-token/blobs/sha256:one")) + wireMock.register(WireMock.any(WireMock.urlEqualTo("/v2/library/refresh-token/blobs/%s".formatted(digest))) .inScenario("get token") .whenScenarioStateIs("get") .willReturn(WireMock.ok().withBody("blob-data"))); @@ -341,7 +348,7 @@ void shouldRefreshExpiredToken(WireMockRuntimeInfo wmRuntimeInfo) { ContainerRef containerRef = ContainerRef.parse("localhost:%d/library/refresh-token".formatted(wmRuntimeInfo.getHttpPort())); - byte[] blob = registry.getBlob(containerRef.withDigest("sha256:one")); + byte[] blob = registry.getBlob(containerRef.withDigest(digest)); assertEquals("blob-data", new String(blob)); } }