diff --git a/external/storm-hdfs-oci/src/main/java/org/apache/storm/container/oci/LocalOrHdfsImageTagToManifestPlugin.java b/external/storm-hdfs-oci/src/main/java/org/apache/storm/container/oci/LocalOrHdfsImageTagToManifestPlugin.java index 949f0e1a7c0..c3244436531 100644 --- a/external/storm-hdfs-oci/src/main/java/org/apache/storm/container/oci/LocalOrHdfsImageTagToManifestPlugin.java +++ b/external/storm-hdfs-oci/src/main/java/org/apache/storm/container/oci/LocalOrHdfsImageTagToManifestPlugin.java @@ -23,6 +23,7 @@ import java.io.File; import java.io.FileReader; import java.io.IOException; +import java.io.UncheckedIOException; import java.io.InputStreamReader; import java.util.HashMap; import java.util.LinkedHashMap; @@ -84,6 +85,16 @@ public class LocalOrHdfsImageTagToManifestPlugin implements OciImageTagToManifes private static final String ALPHA_NUMERIC = "[a-zA-Z0-9]+"; + /** + * Check that a string can be used as an image hash, i.e. it consists of exactly + * {@link #SHA256_HASH_LENGTH} alphanumeric characters. + * @param hash the string to check + * @return true if the string has the shape of an image hash + */ + private static boolean isValidHash(String hash) { + return hash != null && hash.length() == SHA256_HASH_LENGTH && hash.matches(ALPHA_NUMERIC); + } + @Override public void init(Map conf) throws IOException { this.conf = conf; @@ -213,7 +224,7 @@ private static Map readImageToHashFile(BufferedReader br, String String[] imageTagArray = imageTags.split(","); String hash = line.substring(index + 1); - if (!hash.matches(ALPHA_NUMERIC) || hash.length() != SHA256_HASH_LENGTH) { + if (!isValidHash(hash)) { LOG.warn("Malformed image hash: " + hash); continue; } @@ -229,6 +240,10 @@ private static Map readImageToHashFile(BufferedReader br, String @Override public synchronized ImageManifest getManifestFromImageTag(String imageTag) throws IOException { String hash = getHashFromImageTag(imageTag); + if (!isValidHash(hash)) { + throw new IOException("Cannot get manifest for image tag " + imageTag + + ": " + hash + " is not a valid image hash"); + } ImageManifest manifest = manifestCache.get(hash); if (manifest != null) { return manifest; @@ -271,12 +286,16 @@ public synchronized String getHashFromImageTag(String imageTag) { // 1) Go to local file // 2) Go to HDFS - // 3) Use tag as is/Assume tag is the hash + // 3) Use tag as is/Assume tag is the hash; only acceptable if the tag looks like a hash if ((hash = localImageToHashCache.get(imageTag)) != null) { return hash; } else if ((hash = hdfsImageToHashCache.get(imageTag)) != null) { return hash; } else { + if (!isValidHash(imageTag)) { + throw new UncheckedIOException(new IOException("Image tag " + imageTag + + " is not in the image-tag-to-hash files and is not a valid image hash itself")); + } return imageTag; } } diff --git a/external/storm-hdfs-oci/src/test/java/org/apache/storm/container/oci/LocalOrHdfsImageTagToManifestPluginTest.java b/external/storm-hdfs-oci/src/test/java/org/apache/storm/container/oci/LocalOrHdfsImageTagToManifestPluginTest.java new file mode 100644 index 00000000000..2cda2b06378 --- /dev/null +++ b/external/storm-hdfs-oci/src/test/java/org/apache/storm/container/oci/LocalOrHdfsImageTagToManifestPluginTest.java @@ -0,0 +1,85 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.storm.container.oci; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.HashMap; +import java.util.Map; +import org.apache.storm.DaemonConfig; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +public class LocalOrHdfsImageTagToManifestPluginTest { + + private static final String KNOWN_HASH = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; + private static final String UNKNOWN_HASH = "fedcba9876543210fedcba9876543210fedcba9876543210fedcba9876543210"; + //same length as a hash, but made of characters that would still escape the manifest directory + private static final String HASH_LENGTH_PATH = "../../../user/foo/bar/" + "a".repeat(42); + + @TempDir + Path tempDir; + + private LocalOrHdfsImageTagToManifestPlugin createPlugin() throws IOException { + Path hashFile = tempDir.resolve("image-tag-to-hash"); + Files.write(hashFile, ("busybox:latest:" + KNOWN_HASH + "\n").getBytes(StandardCharsets.UTF_8)); + + Map conf = new HashMap<>(); + conf.put("storm.oci.local.or.hdfs.image.tag.to.manifest.plugin.local.hash.file", hashFile.toString()); + conf.put(DaemonConfig.STORM_OCI_IMAGE_HDFS_TOPLEVEL_DIR, "/storm/oci"); + + LocalOrHdfsImageTagToManifestPlugin plugin = new LocalOrHdfsImageTagToManifestPlugin(); + plugin.init(conf); + return plugin; + } + + @Test + public void testKnownImageTagIsMappedToItsHash() throws Exception { + assertEquals(KNOWN_HASH, createPlugin().getHashFromImageTag("busybox:latest")); + } + + @Test + public void testUnmappedImageTagIsUsedAsHashWhenItLooksLikeOne() throws Exception { + assertEquals(UNKNOWN_HASH, createPlugin().getHashFromImageTag(UNKNOWN_HASH)); + } + + @Test + public void testUnmappedImageTagThatIsNotAHashIsRejected() throws Exception { + LocalOrHdfsImageTagToManifestPlugin plugin = createPlugin(); + assertThrows(UncheckedIOException.class, () -> plugin.getHashFromImageTag("../../../user/foo/bar")); + assertThrows(UncheckedIOException.class, () -> plugin.getHashFromImageTag("busybox:unknown")); + assertThrows(UncheckedIOException.class, () -> plugin.getHashFromImageTag("..")); + assertThrows(UncheckedIOException.class, () -> plugin.getHashFromImageTag("/etc/passwd")); + assertThrows(UncheckedIOException.class, () -> plugin.getHashFromImageTag(HASH_LENGTH_PATH)); + assertThrows(UncheckedIOException.class, () -> plugin.getHashFromImageTag(UNKNOWN_HASH + "a")); + } + + @Test + public void testGetManifestFromImageTagRejectsUnmappedNonHashTag() throws Exception { + LocalOrHdfsImageTagToManifestPlugin plugin = createPlugin(); + assertThrows(UncheckedIOException.class, () -> plugin.getManifestFromImageTag("../../../user/foo/bar")); + assertThrows(UncheckedIOException.class, () -> plugin.getManifestFromImageTag(HASH_LENGTH_PATH)); + } +}