8339280: jarsigner -verify performs cross-checking between CEN and LOC

franferrax · jerboaa · commit 3e41a78da726 · 2025-09-23T14:23:29.000Z
8353299: VerifyJarEntryName.java test fails
8367782: VerifyJarEntryName.java: Fix modifyJarEntryName to operate on bytes and re-introduce verifySignatureEntryName

Reviewed-by: sgehwolf, abakhtin
Backport-of: bbd5b174c50346152a624317b6bd76ec48f7e551
diff --git a/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java b/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java
@@ -28,6 +28,8 @@
 import java.io.*;
 import java.net.UnknownHostException;
 import java.net.URLClassLoader;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.security.cert.CertPathValidatorException;
 import java.security.cert.PKIXBuilderParameters;
 import java.security.interfaces.ECKey;
@@ -228,6 +230,8 @@ public static void main(String args[]) throws Exception {
     private Throwable chainNotValidatedReason = null;
     private Throwable tsaChainNotValidatedReason = null;
 
+    private List<String> crossChkWarnings = new ArrayList<>();
+
     PKIXBuilderParameters pkixParameters;
     Set<X509Certificate> trustedCerts = new HashSet<>();
 
@@ -1099,6 +1103,7 @@ void verifyJar(String jarName)
                 }
             }
             System.out.println();
+            crossCheckEntries(jarName);
 
             if (!anySigned) {
                 if (disabledAlgFound) {
@@ -1133,6 +1138,143 @@ void verifyJar(String jarName)
         System.exit(1);
     }
 
+    private void crossCheckEntries(String jarName) throws Exception {
+        Set<String> locEntries = new HashSet<>();
+
+        try (JarFile jarFile = new JarFile(jarName);
+             JarInputStream jis = new JarInputStream(
+                     Files.newInputStream(Path.of(jarName)))) {
+
+            Manifest cenManifest = jarFile.getManifest();
+            Manifest locManifest = jis.getManifest();
+            compareManifest(cenManifest, locManifest);
+
+            JarEntry locEntry;
+            while ((locEntry = jis.getNextJarEntry()) != null) {
+                String entryName = locEntry.getName();
+                locEntries.add(entryName);
+
+                JarEntry cenEntry = jarFile.getJarEntry(entryName);
+                if (cenEntry == null) {
+                    crossChkWarnings.add(String.format(rb.getString(
+                            "entry.1.present.when.reading.jarinputstream.but.missing.via.jarfile"),
+                            entryName));
+                    continue;
+                }
+
+                try {
+                    readEntry(jis);
+                } catch (SecurityException e) {
+                    crossChkWarnings.add(String.format(rb.getString(
+                            "signature.verification.failed.on.entry.1.when.reading.via.jarinputstream"),
+                            entryName));
+                    continue;
+                }
+
+                try (InputStream cenInputStream = jarFile.getInputStream(cenEntry)) {
+                    if (cenInputStream == null) {
+                        crossChkWarnings.add(String.format(rb.getString(
+                                "entry.1.present.in.jarfile.but.unreadable"),
+                                entryName));
+                        continue;
+                    } else {
+                        try {
+                            readEntry(cenInputStream);
+                        } catch (SecurityException e) {
+                            crossChkWarnings.add(String.format(rb.getString(
+                                    "signature.verification.failed.on.entry.1.when.reading.via.jarfile"),
+                                    entryName));
+                            continue;
+                        }
+                    }
+                }
+
+                compareSigners(cenEntry, locEntry);
+            }
+
+            jarFile.stream()
+                    .map(JarEntry::getName)
+                    .filter(n -> !locEntries.contains(n) && !n.equals(JarFile.MANIFEST_NAME))
+                    .forEach(n -> crossChkWarnings.add(String.format(rb.getString(
+                            "entry.1.present.when.reading.jarfile.but.missing.via.jarinputstream"), n)));
+        }
+    }
+
+    private void readEntry(InputStream is) throws IOException {
+        is.transferTo(OutputStream.nullOutputStream());
+    }
+
+    private void compareManifest(Manifest cenManifest, Manifest locManifest) {
+        if (cenManifest == null) {
+            crossChkWarnings.add(rb.getString(
+                    "manifest.missing.when.reading.jarfile"));
+            return;
+        }
+        if (locManifest == null) {
+            crossChkWarnings.add(rb.getString(
+                    "manifest.missing.when.reading.jarinputstream"));
+            return;
+        }
+
+        Attributes cenMainAttrs = cenManifest.getMainAttributes();
+        Attributes locMainAttrs = locManifest.getMainAttributes();
+
+        for (Object key : cenMainAttrs.keySet()) {
+            Object cenValue = cenMainAttrs.get(key);
+            Object locValue = locMainAttrs.get(key);
+
+            if (locValue == null) {
+                crossChkWarnings.add(String.format(rb.getString(
+                        "manifest.attribute.1.present.when.reading.jarfile.but.missing.via.jarinputstream"),
+                        key));
+            } else if (!cenValue.equals(locValue)) {
+                crossChkWarnings.add(String.format(rb.getString(
+                        "manifest.attribute.1.differs.jarfile.value.2.jarinputstream.value.3"),
+                        key, cenValue, locValue));
+            }
+        }
+
+        for (Object key : locMainAttrs.keySet()) {
+            if (!cenMainAttrs.containsKey(key)) {
+                crossChkWarnings.add(String.format(rb.getString(
+                        "manifest.attribute.1.present.when.reading.jarinputstream.but.missing.via.jarfile"),
+                        key));
+            }
+        }
+    }
+
+    private void compareSigners(JarEntry cenEntry, JarEntry locEntry) {
+        CodeSigner[] cenSigners = cenEntry.getCodeSigners();
+        CodeSigner[] locSigners = locEntry.getCodeSigners();
+
+        boolean cenHasSigners = cenSigners != null;
+        boolean locHasSigners = locSigners != null;
+
+        if (cenHasSigners && locHasSigners) {
+            if (!Arrays.equals(cenSigners, locSigners)) {
+                crossChkWarnings.add(String.format(rb.getString(
+                        "codesigners.different.for.entry.1.when.reading.jarfile.and.jarinputstream"),
+                        cenEntry.getName()));
+            }
+        } else if (cenHasSigners) {
+            crossChkWarnings.add(String.format(rb.getString(
+                    "entry.1.is.signed.in.jarfile.but.is.not.signed.in.jarinputstream"),
+                    cenEntry.getName()));
+        } else if (locHasSigners) {
+            crossChkWarnings.add(String.format(rb.getString(
+                    "entry.1.is.signed.in.jarinputstream.but.is.not.signed.in.jarfile"),
+                    locEntry.getName()));
+        }
+    }
+
+    private void displayCrossChkWarnings() {
+        System.out.println();
+        // First is a summary warning
+        System.out.println(rb.getString("jar.contains.internal.inconsistencies.result.in.different.contents.via.jarfile.and.jarinputstream"));
+        // each warning message with prefix "- "
+        crossChkWarnings.forEach(warning -> System.out.println("- " + warning));
+    }
+
     private void displayMessagesAndResult(boolean isSigning) {
         String result;
         List<String> errors = new ArrayList<>();
@@ -1359,13 +1501,19 @@ private void displayMessagesAndResult(boolean isSigning) {
                 System.out.println(rb.getString("Warning."));
                 warnings.forEach(System.out::println);
             }
+            if (!crossChkWarnings.isEmpty()) {
+                displayCrossChkWarnings();
+            }
         } else {
             if (!errors.isEmpty() || !warnings.isEmpty()) {
                 System.out.println();
                 System.out.println(rb.getString("Warning."));
                 errors.forEach(System.out::println);
                 warnings.forEach(System.out::println);
             }
+            if (!crossChkWarnings.isEmpty()) {
+                displayCrossChkWarnings();
+            }
         }
 
         if (!isSigning && (!errors.isEmpty() || !warnings.isEmpty())) {
diff --git a/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Resources.java b/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Resources.java
@@ -325,6 +325,34 @@ public class Resources extends java.util.ListResourceBundle {
         {"Cannot.find.file.", "Cannot find file: "},
         {"event.ocsp.check", "Contacting OCSP server at %s ..."},
         {"event.crl.check", "Downloading CRL from %s ..."},
+        {"manifest.missing.when.reading.jarfile",
+                "Manifest is missing when reading via JarFile"},
+        {"manifest.missing.when.reading.jarinputstream",
+                "Manifest is missing when reading via JarInputStream"},
+        {"manifest.attribute.1.present.when.reading.jarfile.but.missing.via.jarinputstream",
+                "Manifest main attribute %s is present when reading via JarFile but missing when reading via JarInputStream"},
+        {"manifest.attribute.1.present.when.reading.jarinputstream.but.missing.via.jarfile",
+                "Manifest main attribute %s is present when reading via JarInputStream but missing when reading via JarFile"},
+        {"manifest.attribute.1.differs.jarfile.value.2.jarinputstream.value.3",
+                "Manifest main attribute %1$s differs: JarFile value = %2$s, JarInputStream value = %3$s"},
+        {"entry.1.present.when.reading.jarinputstream.but.missing.via.jarfile",
+                "Entry %s is present when reading via JarInputStream but missing when reading via JarFile"},
+        {"entry.1.present.when.reading.jarfile.but.missing.via.jarinputstream",
+                "Entry %s is present when reading via JarFile but missing when reading via JarInputStream"},
+        {"entry.1.present.in.jarfile.but.unreadable",
+                "Entry %s is present in JarFile but unreadable"},
+        {"codesigners.different.for.entry.1.when.reading.jarfile.and.jarinputstream",
+                "Code signers are different for entry %s when reading from JarFile and JarInputStream"},
+        {"entry.1.is.signed.in.jarfile.but.is.not.signed.in.jarinputstream",
+                "Entry %s is signed in JarFile but is not signed in JarInputStream"},
+        {"entry.1.is.signed.in.jarinputstream.but.is.not.signed.in.jarfile",
+                "Entry %s is signed in JarInputStream but is not signed in JarFile"},
+        {"jar.contains.internal.inconsistencies.result.in.different.contents.via.jarfile.and.jarinputstream",
+                "This JAR file contains internal inconsistencies that may result in different contents when reading via JarFile and JarInputStream:"},
+        {"signature.verification.failed.on.entry.1.when.reading.via.jarinputstream",
+                "Signature verification failed on entry %s when reading via JarInputStream"},
+        {"signature.verification.failed.on.entry.1.when.reading.via.jarfile",
+                "Signature verification failed on entry %s when reading via JarFile"},
     };
 
     /**
diff --git a/src/jdk.jartool/share/man/jarsigner.1 b/src/jdk.jartool/share/man/jarsigner.1
@@ -1208,6 +1208,11 @@ months.
 hasExpiringTsaCert
 The timestamp will expire within one year on \f[V]YYYY-MM-DD\f[R].
 .TP
+internalInconsistenciesDetected
+This JAR contains internal inconsistencies detected during verification
+that may result in different contents when reading via JarFile
+and JarInputStream.
+.TP
 legacyAlg
 An algorithm used is considered a security risk but not disabled.
 .TP
diff --git a/test/jdk/sun/security/tools/jarsigner/VerifyJarEntryName.java b/test/jdk/sun/security/tools/jarsigner/VerifyJarEntryName.java
@@ -0,0 +1,142 @@
+/*
+ * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8339280
+ * @summary Test that jarsigner -verify emits a warning when the filename of
+ *     an entry in the LOC is changed
+ * @library /test/lib
+ * @run junit VerifyJarEntryName
+ */
+
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.io.FileOutputStream;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Arrays;
+import java.util.jar.JarFile;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipOutputStream;
+
+import jdk.test.lib.SecurityTools;
+import static org.junit.jupiter.api.Assertions.fail;
+
+public class VerifyJarEntryName {
+
+    private static final Path ORIGINAL_JAR = Path.of("test.jar");
+    private static final Path MODIFIED_JAR = Path.of("modified_test.jar");
+
+    @BeforeAll
+    static void setup() throws Exception {
+        try (FileOutputStream fos = new FileOutputStream(ORIGINAL_JAR.toFile());
+             ZipOutputStream zos = new ZipOutputStream(fos)) {
+            zos.putNextEntry(new ZipEntry(JarFile.MANIFEST_NAME));
+            zos.write("Manifest-Version: 1.0\nCreated-By: Test\n".
+                    getBytes(StandardCharsets.UTF_8));
+            zos.closeEntry();
+
+            // Add hello.txt file
+            ZipEntry textEntry = new ZipEntry("hello.txt");
+            zos.putNextEntry(textEntry);
+            zos.write("hello".getBytes(StandardCharsets.UTF_8));
+            zos.closeEntry();
+        }
+
+        SecurityTools.keytool("-genkeypair -keystore ks -storepass changeit "
+                + "-alias mykey -keyalg rsa -dname CN=me ");
+
+        SecurityTools.jarsigner("-keystore ks -storepass changeit "
+                        + ORIGINAL_JAR + " mykey")
+                .shouldHaveExitValue(0);
+    }
+
+    @BeforeEach
+    void cleanup() throws Exception {
+        Files.deleteIfExists(MODIFIED_JAR);
+    }
+
+    /*
+     * Modify a single byte in "MANIFEST.MF" filename in LOC, and
+     * validate that jarsigner -verify emits a warning message.
+     */
+    @Test
+    void verifyManifestEntryName() throws Exception {
+        modifyJarEntryName(ORIGINAL_JAR, MODIFIED_JAR, "META-INF/MANIFEST.MF");
+        SecurityTools.jarsigner("-verify -verbose " + MODIFIED_JAR)
+                .shouldContain("This JAR file contains internal " +
+                        "inconsistencies that may result in different " +
+                        "contents when reading via JarFile and JarInputStream:")
+                .shouldContain("- Manifest is missing when " +
+                        "reading via JarInputStream")
+                .shouldHaveExitValue(0);
+    }
+
+    /*
+     * Modify a single byte in signature filename in LOC, and
+     * validate that jarsigner -verify emits a warning message.
+     */
+    @Test
+    void verifySignatureEntryName() throws Exception {
+        modifyJarEntryName(ORIGINAL_JAR, MODIFIED_JAR, "META-INF/MYKEY.SF");
+        SecurityTools.jarsigner("-verify -verbose " + MODIFIED_JAR)
+                .shouldContain("This JAR file contains internal " +
+                        "inconsistencies that may result in different " +
+                        "contents when reading via JarFile and JarInputStream:")
+                .shouldContain("- Entry XETA-INF/MYKEY.SF is present when reading " +
+                        "via JarInputStream but missing when reading via JarFile")
+                .shouldHaveExitValue(0);
+    }
+
+    /*
+     * Validate that jarsigner -verify on a valid JAR works without
+     * emitting warnings about internal inconsistencies.
+     */
+    @Test
+    void verifyOriginalJar() throws Exception {
+        SecurityTools.jarsigner("-verify -verbose " + ORIGINAL_JAR)
+                .shouldNotContain("This JAR file contains internal " +
+                        "inconsistencies that may result in different contents when " +
+                        "reading via JarFile and JarInputStream:")
+                .shouldHaveExitValue(0);
+    }
+
+    private void modifyJarEntryName(Path origJar, Path modifiedJar,
+            String entryName) throws Exception {
+        byte[] jarBytes = Files.readAllBytes(origJar);
+        byte[] entryNameBytes = entryName.getBytes(StandardCharsets.UTF_8);
+        int pos = 0;
+        try {
+            while (!Arrays.equals(jarBytes, pos, pos + entryNameBytes.length,
+                    entryNameBytes, 0, entryNameBytes.length)) pos++;
+        } catch (ArrayIndexOutOfBoundsException ignore) {
+            fail(entryName + " is not present in the JAR");
+        }
+        jarBytes[pos] = 'X';
+        Files.write(modifiedJar, jarBytes);
+    }
+}
