From 3a9cdf773260cee34c5975bbeef2c88b232eb389 Mon Sep 17 00:00:00 2001 From: Sam Freund Date: Thu, 19 Feb 2026 16:19:07 -0600 Subject: [PATCH] Verify native library integrity during extraction/load (#2368) ## Description We've hit a problem where the `CombinedRuntimeLoader` extracts native files, but gets interrupted in the middle. This is bad, cause all `CombinedRuntimeLoader` used to check a file was its existence. This change uses the hash of the file to verify that it's correct. This will be checked at runtime, everytime, if the file is extant. If this check fails, we will delete the extant file and attempt to reextract. We also check a newly extracted file, if that hash does not match we error. Note that this is reliant on https://github.com/PhotonVision/wpilib-tool-plugin/pull/8 and should follow #2367 ## Meta Merge checklist: - [x] Pull Request title is [short, imperative summary](https://cbea.ms/git-commit/) of proposed changes - [x] The description documents the _what_ and _why_, including events that led to this PR - [ ] If this PR changes behavior or adds a feature, user documentation is updated - [ ] If this PR touches photon-serde, all messages have been regenerated and hashes have not changed unexpectedly - [ ] If this PR touches configuration, this is backwards compatible with all settings going back to the previous seasons's last release (seasons end after champs ends) - [ ] If this PR touches pipeline settings or anything related to data exchange, the frontend typing is updated - [ ] If this PR addresses a bug, a regression test for it is added --------- Co-authored-by: Matt M --- .github/workflows/build.yml | 27 ++++- build.gradle | 2 +- .../common/hardware/Platform.java | 2 +- .../jni/CombinedRuntimeLoader.java | 113 ++++++++++++++---- 4 files changed, 117 insertions(+), 27 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 3acc279ae..38f64f77c 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -443,9 +443,32 @@ jobs: - uses: actions/download-artifact@v7 with: name: ${{ matrix.artifact-name }} - - run: java -jar ${{ matrix.extraOpts }} *.jar --smoketest + # The jar is run twice to exercise different code paths. + - run: | + echo "=== First run ===" + java -jar ${{ matrix.extraOpts }} *.jar --smoketest + echo "=== Checking for files to corrupt ===" + find ~ -type f \( -name "*.so" -o -name "*.dylib" \) | head -20 + if [ -d ~/.wpilib ]; then + echo "~/.wpilib directory exists" + echo "Contents of ~/.wpilib:" + find ~/.wpilib -type f \( -name "*.so" -o -name "*.dylib" \) | head -10 + RANDOM_FILE=$(find ~/.wpilib -type f \( -name "*.so" -o -name "*.dylib" \) | sort -R | head -n 1) + if [ ! -z "$RANDOM_FILE" ]; then + echo "Corrupting file: $RANDOM_FILE" + echo "corrupted data" > "$RANDOM_FILE" + else + echo "No .so or .dylib files found in ~/.wpilib" + fi + else + echo "~/.wpilib directory does not exist" + fi + echo "=== Second run ===" + java -jar ${{ matrix.extraOpts }} *.jar --smoketest if: ${{ (matrix.os) != 'windows-latest' }} - - run: ls *.jar | %{ Write-Host "Running $($_.Name)"; Start-Process "java" -ArgumentList "-jar `"$($_.FullName)`" --smoketest" -NoNewWindow -Wait; break } + - run: | + ls *.jar | %{ Write-Host "Running $($_.Name)"; Start-Process "java" -ArgumentList "-jar `"$($_.FullName)`" --smoketest" -NoNewWindow -Wait; break } + ls *.jar | %{ Write-Host "Running $($_.Name)"; Start-Process "java" -ArgumentList "-jar `"$($_.FullName)`" --smoketest" -NoNewWindow -Wait; break } if: ${{ (matrix.os) == 'windows-latest' }} build-image: diff --git a/build.gradle b/build.gradle index e42347284..b176711f5 100644 --- a/build.gradle +++ b/build.gradle @@ -5,7 +5,7 @@ plugins { id "com.diffplug.spotless" version "8.1.0" id "edu.wpi.first.wpilib.repositories.WPILibRepositoriesPlugin" version "2020.2" id "edu.wpi.first.GradleRIO" version "2026.2.1" - id 'org.photonvision.tools.WpilibTools' version '2.3.3-photon' + id 'org.photonvision.tools.WpilibTools' version '2.4.1-photon' id 'com.google.protobuf' version '0.9.3' apply false id 'edu.wpi.first.GradleJni' version '1.1.0' id "org.ysb33r.doxygen" version "2.0.0" apply false diff --git a/photon-targeting/src/main/java/org/photonvision/common/hardware/Platform.java b/photon-targeting/src/main/java/org/photonvision/common/hardware/Platform.java index 01f359eb8..340eed92a 100644 --- a/photon-targeting/src/main/java/org/photonvision/common/hardware/Platform.java +++ b/photon-targeting/src/main/java/org/photonvision/common/hardware/Platform.java @@ -17,13 +17,13 @@ package org.photonvision.common.hardware; -import edu.wpi.first.util.CombinedRuntimeLoader; import java.io.BufferedReader; import java.io.File; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Paths; import java.util.function.Supplier; +import org.photonvision.jni.CombinedRuntimeLoader; @SuppressWarnings({"unused", "doclint"}) public enum Platform { diff --git a/photon-targeting/src/main/java/org/photonvision/jni/CombinedRuntimeLoader.java b/photon-targeting/src/main/java/org/photonvision/jni/CombinedRuntimeLoader.java index 332cf986d..b97027b9f 100644 --- a/photon-targeting/src/main/java/org/photonvision/jni/CombinedRuntimeLoader.java +++ b/photon-targeting/src/main/java/org/photonvision/jni/CombinedRuntimeLoader.java @@ -18,17 +18,23 @@ package org.photonvision.jni; -import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; +import java.io.BufferedInputStream; import java.io.File; +import java.io.FileInputStream; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Paths; +import java.nio.file.StandardCopyOption; +import java.security.DigestInputStream; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; import java.util.ArrayList; -import java.util.HashMap; +import java.util.HexFormat; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.concurrent.CopyOnWriteArrayList; /** Loads dynamic libraries for all platforms. */ public final class CombinedRuntimeLoader { @@ -36,6 +42,10 @@ public final class CombinedRuntimeLoader { private static String extractionDirectory; + private static final Object extractCompleteLock = new Object(); + private static boolean extractAndVerifyComplete = false; + private static List filesAlreadyExtracted = new CopyOnWriteArrayList<>(); + /** * Returns library extraction directory. * @@ -124,6 +134,27 @@ public final class CombinedRuntimeLoader { return msg.toString(); } + /** + * Architecture-specific information containing file hashes for a specific CPU architecture (e.g., + * x86-64, arm64). + */ + public record ArchInfo(Map fileHashes) {} + + /** + * Platform-specific information containing architectures for a specific OS platform (e.g., linux, + * windows). + */ + public record PlatformInfo(Map architectures) {} + + /** Overall resource information to be serialized */ + public record ResourceInformation( + // Combined MD5 hash of all native resource files + String hash, + // Platform-specific native libraries organized by platform then architecture + Map platforms, + // List of supported versions for these native resources + List versions) {} + /** * Extract a list of native libraries. * @@ -133,31 +164,39 @@ public final class CombinedRuntimeLoader { * @return List of all libraries that were extracted * @throws IOException Thrown if resource not found or file could not be extracted */ - @SuppressWarnings("unchecked") public static List extractLibraries(Class clazz, String resourceName) throws IOException { - TypeReference> typeRef = new TypeReference<>() {}; ObjectMapper mapper = new ObjectMapper(); - Map map; + ResourceInformation resourceInfo; try (var stream = clazz.getResourceAsStream(resourceName)) { - map = mapper.readValue(stream, typeRef); + resourceInfo = mapper.readValue(stream, ResourceInformation.class); } var platformPath = Paths.get(getPlatformPath()); var platform = platformPath.getName(0).toString(); var arch = platformPath.getName(1).toString(); - var platformMap = (Map>) map.get(platform); + var platformInfo = resourceInfo.platforms().get(platform); + if (platformInfo == null) { + throw new IOException("Platform " + platform + " not found in resource information"); + } - var fileList = platformMap.get(arch); + var archInfo = platformInfo.architectures().get(arch); + if (archInfo == null) { + throw new IOException("Architecture " + arch + " not found for platform " + platform); + } + + // Map of to + Map filenameToHash = archInfo.fileHashes(); var extractionPathString = getExtractionDirectory(); if (extractionPathString == null) { - String hash = (String) map.get("hash"); + // Folder to extract to derived from overall hash + String combinedHash = resourceInfo.hash(); var defaultExtractionRoot = getDefaultExtractionRoot(); - var extractionPath = Paths.get(defaultExtractionRoot, platform, arch, hash); + var extractionPath = Paths.get(defaultExtractionRoot, platform, arch, combinedHash); extractionPathString = extractionPath.toString(); setExtractionDirectory(extractionPathString); @@ -165,16 +204,25 @@ public final class CombinedRuntimeLoader { List extractedFiles = new ArrayList<>(); - byte[] buffer = new byte[0x10000]; // 64K copy buffer - - for (var file : fileList) { + for (String file : filenameToHash.keySet()) { try (var stream = clazz.getResourceAsStream(file)) { Objects.requireNonNull(stream); var outputFile = Paths.get(extractionPathString, new File(file).getName()); + + String fileHash = filenameToHash.get(file); + extractedFiles.add(outputFile.toString()); if (outputFile.toFile().exists()) { - continue; + if (hashEm(outputFile.toFile()).equals(fileHash)) { + continue; + } else { + // Hashes don't match, delete and re-extract + System.err.println( + outputFile.toAbsolutePath().toString() + + " failed validation - deleting and re-extracting"); + outputFile.toFile().delete(); + } } var parent = outputFile.getParent(); if (parent == null) { @@ -183,10 +231,11 @@ public final class CombinedRuntimeLoader { parent.toFile().mkdirs(); try (var os = Files.newOutputStream(outputFile)) { - int readBytes; - while ((readBytes = stream.read(buffer)) != -1) { // NOPMD - os.write(buffer, 0, readBytes); - } + Files.copy(stream, outputFile, StandardCopyOption.REPLACE_EXISTING); + } + + if (!hashEm(outputFile.toFile()).equals(fileHash)) { + throw new IOException("Hash of extracted file does not match expected hash"); } } } @@ -194,6 +243,20 @@ public final class CombinedRuntimeLoader { return extractedFiles; } + private static String hashEm(File f) throws IOException { + try { + MessageDigest fileHash = MessageDigest.getInstance("MD5"); + try (var dis = + new DigestInputStream(new BufferedInputStream(new FileInputStream(f)), fileHash)) { + dis.readAllBytes(); + } + var ret = HexFormat.of().formatHex(fileHash.digest()); + return ret; + } catch (NoSuchAlgorithmException e) { + throw new IOException("Unable to verify extracted native files", e); + } + } + /** * Load a single library from a list of extracted files. * @@ -229,12 +292,16 @@ public final class CombinedRuntimeLoader { */ public static void loadLibraries(Class clazz, String... librariesToLoad) throws IOException { - // Extract everything + synchronized (extractCompleteLock) { + if (extractAndVerifyComplete == false) { + // Extract everything + filesAlreadyExtracted = extractLibraries(clazz, "/ResourceInformation.json"); + extractAndVerifyComplete = true; + } - var extractedFiles = extractLibraries(clazz, "/ResourceInformation.json"); - - for (var library : librariesToLoad) { - loadLibrary(library, extractedFiles); + for (var library : librariesToLoad) { + loadLibrary(library, filesAlreadyExtracted); + } } } }