-
-
Notifications
You must be signed in to change notification settings - Fork 25
harden: verify dependency integrity and add download timeouts #1381
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| package com.eternalcode.core.loader.dependency; | ||
|
|
||
| import java.security.MessageDigest; | ||
| import java.security.NoSuchAlgorithmException; | ||
|
|
||
| /** | ||
| * Verifies the integrity of downloaded artifacts against the checksum files published alongside them in a Maven | ||
| * repository (e.g. {@code artifact-1.0.jar.sha256}). | ||
| * | ||
| * <p>Ordered strongest-first so callers can prefer the most secure digest a repository publishes. SHA-1 is retained | ||
| * only as a last-resort fallback because it is the single digest Maven repositories are guaranteed to serve; it is | ||
| * cryptographically weak and should not be relied upon on its own. | ||
| */ | ||
| public enum Checksum { | ||
|
|
||
| SHA512("sha512", "SHA-512"), | ||
| SHA256("sha256", "SHA-256"), | ||
| SHA1("sha1", "SHA-1"); | ||
|
|
||
| private final String extension; | ||
| private final String algorithm; | ||
|
|
||
| Checksum(String extension, String algorithm) { | ||
| this.extension = extension; | ||
| this.algorithm = algorithm; | ||
| } | ||
|
|
||
| /** | ||
| * The file extension appended to the artifact name to locate this checksum (without a leading dot). | ||
| */ | ||
| public String extension() { | ||
| return this.extension; | ||
| } | ||
|
|
||
| /** | ||
| * Computes the lower-case hex digest of the given data using this algorithm. | ||
| */ | ||
| public String hash(byte[] data) { | ||
| MessageDigest digest; | ||
| try { | ||
| digest = MessageDigest.getInstance(this.algorithm); | ||
| } | ||
| catch (NoSuchAlgorithmException exception) { | ||
| throw new DependencyException("Missing digest algorithm: " + this.algorithm, exception); | ||
| } | ||
|
|
||
| return toHex(digest.digest(data)); | ||
| } | ||
|
|
||
| /** | ||
| * Returns {@code true} if the digest of {@code data} equals the published checksum. | ||
| * | ||
| * <p>Published checksum files sometimes contain trailing content such as {@code "<hash> <filename>"}; | ||
| * only the leading token is compared, and comparison is case-insensitive. | ||
| */ | ||
| public boolean matches(byte[] data, String publishedChecksum) { | ||
| if (publishedChecksum == null) { | ||
| return false; | ||
| } | ||
|
|
||
| String expected = normalize(publishedChecksum); | ||
| if (expected.isEmpty()) { | ||
| return false; | ||
| } | ||
|
|
||
| return expected.equalsIgnoreCase(this.hash(data)); | ||
| } | ||
|
|
||
| static String normalize(String rawChecksum) { | ||
| String trimmed = rawChecksum.trim(); | ||
|
|
||
| for (int index = 0; index < trimmed.length(); index++) { | ||
| if (Character.isWhitespace(trimmed.charAt(index))) { | ||
| return trimmed.substring(0, index); | ||
| } | ||
| } | ||
|
|
||
| return trimmed; | ||
| } | ||
|
|
||
| private static String toHex(byte[] bytes) { | ||
| StringBuilder builder = new StringBuilder(bytes.length * 2); | ||
|
|
||
| for (byte value : bytes) { | ||
| builder.append(Character.forDigit((value >> 4) & 0xF, 16)); | ||
| builder.append(Character.forDigit(value & 0xF, 16)); | ||
| } | ||
|
|
||
| return builder.toString(); | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -8,6 +8,7 @@ | |||||||||
| import java.io.InputStream; | ||||||||||
| import java.net.URISyntaxException; | ||||||||||
| import java.net.URLConnection; | ||||||||||
| import java.nio.charset.StandardCharsets; | ||||||||||
| import java.nio.file.Files; | ||||||||||
| import java.nio.file.NoSuchFileException; | ||||||||||
| import java.nio.file.Path; | ||||||||||
|
|
@@ -18,6 +19,9 @@ | |||||||||
|
|
||||||||||
| public class DependencyDownloader { | ||||||||||
|
|
||||||||||
| private static final int CONNECT_TIMEOUT_MILLIS = 15_000; | ||||||||||
| private static final int READ_TIMEOUT_MILLIS = 30_000; | ||||||||||
|
|
||||||||||
| private final Logger logger; | ||||||||||
| private final Repository localRepository; | ||||||||||
| private final List<Repository> repositories; | ||||||||||
|
|
@@ -70,6 +74,8 @@ private Path tryDownloadDependency(Dependency dependency) throws URISyntaxExcept | |||||||||
| private Path downloadJarAndSave(Repository repository, Dependency dependency, Path file) { | ||||||||||
| try { | ||||||||||
| byte[] bytes = this.downloadJar(repository, dependency); | ||||||||||
| this.verifyChecksum(repository, dependency, bytes); | ||||||||||
|
|
||||||||||
| Path parent = file.getParent(); | ||||||||||
|
|
||||||||||
| Files.createDirectories(parent); | ||||||||||
|
|
@@ -86,9 +92,7 @@ private Path downloadJarAndSave(Repository repository, Dependency dependency, Pa | |||||||||
| } | ||||||||||
|
|
||||||||||
| private byte[] downloadJar(Repository repository, Dependency dependency) throws IOException { | ||||||||||
| URLConnection connection = dependency.toMavenJar(repository).toURL().openConnection(); | ||||||||||
|
|
||||||||||
| try (InputStream in = connection.getInputStream()) { | ||||||||||
| try (InputStream in = openStream(dependency.toMavenJar(repository).toURL().openConnection())) { | ||||||||||
| byte[] bytes = ByteStreams.toByteArray(in); | ||||||||||
|
|
||||||||||
| if (bytes.length == 0) { | ||||||||||
|
|
@@ -99,4 +103,53 @@ private byte[] downloadJar(Repository repository, Dependency dependency) throws | |||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Verifies the downloaded artifact against the strongest checksum the repository publishes for it. Fails closed: | ||||||||||
| * a mismatch, or the total absence of any published checksum, rejects this repository so the caller can fall back | ||||||||||
| * to another one (and ultimately fail if none can vouch for the artifact). This prevents a tampered or corrupted | ||||||||||
| * jar from being written to the local cache and loaded into the JVM. | ||||||||||
| */ | ||||||||||
| private void verifyChecksum(Repository repository, Dependency dependency, byte[] jarBytes) { | ||||||||||
| for (Checksum checksum : Checksum.values()) { | ||||||||||
| String publishedChecksum = this.downloadChecksum(repository, dependency, checksum); | ||||||||||
|
|
||||||||||
| if (publishedChecksum == null) { | ||||||||||
| continue; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| if (!checksum.matches(jarBytes, publishedChecksum)) { | ||||||||||
| throw new DependencyException( | ||||||||||
| "Checksum mismatch (" + checksum.extension() + ") for " + dependency + " from " + repository | ||||||||||
| + " - refusing to load a potentially tampered dependency"); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| return; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| throw new DependencyException( | ||||||||||
| "No published checksum (sha512/sha256/sha1) available to verify " + dependency + " from " + repository); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| private String downloadChecksum(Repository repository, Dependency dependency, Checksum checksum) { | ||||||||||
| try (InputStream in = openStream(dependency.toMavenJarChecksum(repository, checksum.extension()).toURL().openConnection())) { | ||||||||||
| byte[] bytes = ByteStreams.toByteArray(in); | ||||||||||
|
Comment on lines
+134
to
+135
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To prevent potential
Suggested change
|
||||||||||
|
|
||||||||||
| if (bytes.length == 0) { | ||||||||||
| return null; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| return new String(bytes, StandardCharsets.UTF_8); | ||||||||||
| } | ||||||||||
| catch (IOException exception) { | ||||||||||
| // Checksum not served by this repository; the caller treats this as "cannot verify here". | ||||||||||
| return null; | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| private static InputStream openStream(URLConnection connection) throws IOException { | ||||||||||
| connection.setConnectTimeout(CONNECT_TIMEOUT_MILLIS); | ||||||||||
| connection.setReadTimeout(READ_TIMEOUT_MILLIS); | ||||||||||
| return connection.getInputStream(); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| } | ||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| package com.eternalcode.core.loader.dependency; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||
| import static org.junit.jupiter.api.Assertions.assertFalse; | ||
| import static org.junit.jupiter.api.Assertions.assertTrue; | ||
|
|
||
| import java.nio.charset.StandardCharsets; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| class ChecksumTest { | ||
|
|
||
| // Known digests of the ASCII string "abc". | ||
| private static final byte[] ABC = "abc".getBytes(StandardCharsets.UTF_8); | ||
| private static final String ABC_SHA1 = "a9993e364706816aba3e25717850c26c9cd0d89d"; | ||
| private static final String ABC_SHA256 = "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad"; | ||
| private static final String ABC_SHA512 = | ||
| "ddaf35a193617abacc417349ae20413112e6fa4e89a97ea20a9eeee64b55d39a" | ||
| + "2192992a274fc1a836ba3c23a3feebbd454d4423643ce80e2a9ac94fa54ca49f"; | ||
|
|
||
| @Test | ||
| void computesKnownDigests() { | ||
| assertEquals(ABC_SHA1, Checksum.SHA1.hash(ABC)); | ||
| assertEquals(ABC_SHA256, Checksum.SHA256.hash(ABC)); | ||
| assertEquals(ABC_SHA512, Checksum.SHA512.hash(ABC)); | ||
| } | ||
|
|
||
| @Test | ||
| void matchesIgnoringCase() { | ||
| assertTrue(Checksum.SHA256.matches(ABC, ABC_SHA256)); | ||
| assertTrue(Checksum.SHA256.matches(ABC, ABC_SHA256.toUpperCase())); | ||
| } | ||
|
|
||
| @Test | ||
| void rejectsMismatchedChecksum() { | ||
| assertFalse(Checksum.SHA256.matches(ABC, ABC_SHA1)); | ||
| assertFalse(Checksum.SHA256.matches(ABC, "not-a-hash")); | ||
| } | ||
|
|
||
| @Test | ||
| void rejectsNullOrBlankChecksum() { | ||
| assertFalse(Checksum.SHA256.matches(ABC, null)); | ||
| assertFalse(Checksum.SHA256.matches(ABC, " ")); | ||
| } | ||
|
|
||
| @Test | ||
| void ignoresTrailingFilenameInPublishedChecksum() { | ||
| // Maven repositories occasionally publish "<hash> <filename>". | ||
| assertTrue(Checksum.SHA256.matches(ABC, ABC_SHA256 + " artifact-1.0.jar")); | ||
| assertEquals(ABC_SHA256, Checksum.normalize(ABC_SHA256 + " artifact-1.0.jar")); | ||
| } | ||
|
|
||
| @Test | ||
| void exposesExpectedExtensions() { | ||
| assertEquals("sha512", Checksum.SHA512.extension()); | ||
| assertEquals("sha256", Checksum.SHA256.extension()); | ||
| assertEquals("sha1", Checksum.SHA1.extension()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the project is using Java 25, we can leverage the standard
java.util.HexFormatutility introduced in Java 17 instead of implementing a custom hex conversion method. This simplifies the codebase, improves maintainability, and reduces the risk of bugs in custom bit-manipulation logic.