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

Reviewed-by: mullan, weijun, lancea

Author: Hai-May Chao

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.spec.ECParameterSpec;
@@ -242,6 +244,8 @@ public ExitException(int errorCode) {
     private Throwable chainNotValidatedReason = null;
     private Throwable tsaChainNotValidatedReason = null;
 
+    private List<String> crossChkWarnings = new ArrayList<>();
+
     PKIXBuilderParameters pkixParameters;
     Set<X509Certificate> trustedCerts = new HashSet<>();
 
@@ -1103,6 +1107,7 @@ void verifyJar(String jarName)
                 }
             }
             System.out.println();
+            crossCheckEntries(jarName);
 
             if (!anySigned) {
                 if (disabledAlgFound) {
@@ -1129,6 +1134,143 @@ void verifyJar(String jarName)
         }
     }
 
+    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<>();
@@ -1360,13 +1502,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())) {

src/jdk.jartool/share/classes/sun/security/tools/jarsigner/resources/jarsigner.properties

@@ -208,3 +208,17 @@ Cannot.find.environment.variable.=Cannot find environment variable:\u0020
 Cannot.find.file.=Cannot find file:\u0020
 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

src/jdk.jartool/share/man/jarsigner.md

@@ -929,6 +929,11 @@ was selected, and the others were discarded.
 hasNonexistentEntries
 :   This JAR contains signed entries for files that do not exist.
 
+internalInconsistenciesDetected
+:   This JAR contains internal inconsistencies detected during verification
+    that may result in different contents when reading via JarFile
+    and JarInputStream.
+
 legacyAlg
 :   An algorithm used is considered a security risk but not disabled.
 

test/jdk/sun/security/tools/jarsigner/VerifyJarEntryName.java

@@ -0,0 +1,136 @@
+/*
+ * 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.jar.JarFile;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipOutputStream;
+
+import jdk.test.lib.SecurityTools;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+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, "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, "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);
+        var jarString = new String(jarBytes, StandardCharsets.UTF_8);
+        var pos = jarString.indexOf(entryName);
+        assertTrue(pos != -1, entryName + " is not present in the JAR");
+        jarBytes[pos] = 'X';
+        Files.write(modifiedJar, jarBytes);
+    }
+}