Skip to content
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

8309841: Jarsigner should print a warning if an entry is removed #3007

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ public static void main(String args[]) throws Exception {
private boolean hasExpiringCert = false;
private boolean hasExpiringTsaCert = false;
private boolean noTimestamp = true;
private boolean hasNonexistentEntries = false;

// Expiration date. The value could be null if signed by a trusted cert.
private Date expireDate = null;
Expand Down Expand Up @@ -695,6 +696,7 @@ void verifyJar(String jarName)
Map<String,PKCS7> sigMap = new HashMap<>();
Map<String,String> sigNameMap = new HashMap<>();
Map<String,String> unparsableSignatures = new HashMap<>();
Map<String,Set<String>> entriesInSF = new HashMap<>();

try {
jf = new JarFile(jarName, true);
Expand Down Expand Up @@ -731,6 +733,7 @@ void verifyJar(String jarName)
break;
}
}
entriesInSF.put(alias, sf.getEntries().keySet());
if (!found) {
unparsableSignatures.putIfAbsent(alias,
String.format(
Expand Down Expand Up @@ -829,6 +832,9 @@ void verifyJar(String jarName)
sb.append('\n');
}
}
for (var signed : entriesInSF.values()) {
signed.remove(name);
}
} else if (showcerts && !verbose.equals("all")) {
// Print no info for unsigned entries when -verbose:all,
// to be consistent with old behavior.
Expand Down Expand Up @@ -1020,6 +1026,13 @@ void verifyJar(String jarName)
if (verbose != null) {
System.out.println(history);
}
var signed = entriesInSF.get(s);
if (!signed.isEmpty()) {
if (verbose != null) {
System.out.println(rb.getString("history.nonexistent.entries") + signed);
}
hasNonexistentEntries = true;
}
} else {
unparsableSignatures.putIfAbsent(s, String.format(
rb.getString("history.nobk"), s));
Expand Down Expand Up @@ -1243,6 +1256,7 @@ private void displayMessagesAndResult(boolean isSigning) {
(hasExpiringTsaCert && expireDate != null) ||
(noTimestamp && expireDate != null) ||
(hasExpiredTsaCert && signerNotExpired) ||
hasNonexistentEntries ||
extraAttrsDetected) {

if (hasExpiredTsaCert && signerNotExpired) {
Expand Down Expand Up @@ -1280,6 +1294,9 @@ private void displayMessagesAndResult(boolean isSigning) {
: "no.timestamp.verifying"), expireDate));
}
}
if (hasNonexistentEntries) {
warnings.add(rb.getString("nonexistent.entries.found"));
}
if (extraAttrsDetected) {
warnings.add(rb.getString("extra.attributes.detected"));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ public class Resources extends java.util.ListResourceBundle {

{"history.with.ts", "- Signed by \"%1$s\"\n Digest algorithm: %2$s\n Signature algorithm: %3$s, %4$s\n Timestamped by \"%6$s\" on %5$tc\n Timestamp digest algorithm: %7$s\n Timestamp signature algorithm: %8$s, %9$s"},
{"history.without.ts", "- Signed by \"%1$s\"\n Digest algorithm: %2$s\n Signature algorithm: %3$s, %4$s"},
{"history.nonexistent.entries", " Warning: nonexistent signed entries: "},
{"history.unparsable", "- Unparsable signature-related file %s"},
{"history.nosf", "- Missing signature-related file META-INF/%s.SF"},
{"history.nobk", "- Missing block file for signature-related file META-INF/%s.SF"},
Expand All @@ -174,6 +175,7 @@ public class Resources extends java.util.ListResourceBundle {
{"key.bit.weak", "%d-bit key (weak)"},
{"key.bit.disabled", "%d-bit key (disabled)"},
{"unknown.size", "unknown size"},
{"nonexistent.entries.found", "This jar contains signed entries for files that do not exist. See the -verbose output for more details."},
{"extra.attributes.detected", "POSIX file permission and/or symlink attributes detected. These attributes are ignored when signing and are not protected by the signature."},

{"jarsigner.", "jarsigner: "},
Expand Down
94 changes: 94 additions & 0 deletions test/jdk/sun/security/tools/jarsigner/RemovedFiles.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
/*
* Copyright (c) 2024, 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 8309841
* @summary Jarsigner should print a warning if an entry is removed
* @library /test/lib
*/

import jdk.test.lib.SecurityTools;
import jdk.test.lib.util.JarUtils;

import java.nio.file.Files;
import java.nio.file.Path;
import java.util.jar.Attributes;
import java.util.jar.Manifest;

public class RemovedFiles {

private static final String NONEXISTENT_ENTRIES_FOUND
= "This jar contains signed entries for files that do not exist. See the -verbose output for more details.";

public static void main(String[] args) throws Exception {
JarUtils.createJarFile(
Path.of("a.jar"),
Path.of("."),
Files.writeString(Path.of("a"), "a"),
Files.writeString(Path.of("b"), "b"));
SecurityTools.keytool("-genkeypair -storepass changeit -keystore ks -alias x -dname CN=x -keyalg RSA");
SecurityTools.jarsigner("-storepass changeit -keystore ks a.jar x");

// All is fine at the beginning.
SecurityTools.jarsigner("-verify a.jar")
.shouldNotContain(NONEXISTENT_ENTRIES_FOUND);

// Remove an entry after signing. There will be a warning.
JarUtils.deleteEntries(Path.of("a.jar"), "a");
SecurityTools.jarsigner("-verify a.jar")
.shouldContain(NONEXISTENT_ENTRIES_FOUND);
SecurityTools.jarsigner("-verify -verbose a.jar")
.shouldContain(NONEXISTENT_ENTRIES_FOUND)
.shouldContain("Warning: nonexistent signed entries: [a]");

// Remove one more entry.
JarUtils.deleteEntries(Path.of("a.jar"), "b");
SecurityTools.jarsigner("-verify a.jar")
.shouldContain(NONEXISTENT_ENTRIES_FOUND);
SecurityTools.jarsigner("-verify -verbose a.jar")
.shouldContain(NONEXISTENT_ENTRIES_FOUND)
.shouldContain("Warning: nonexistent signed entries: [a, b]");

// Re-sign will not clear the warning.
SecurityTools.jarsigner("-storepass changeit -keystore ks a.jar x");
SecurityTools.jarsigner("-verify a.jar")
.shouldContain(NONEXISTENT_ENTRIES_FOUND);

// Unfortunately, if there is a non-file entry in manifest, there will be
// a false alarm. See https://bugs.openjdk.org/browse/JDK-8334261.
var man = new Manifest();
man.getMainAttributes().putValue("Manifest-Version", "1.0");
man.getEntries().computeIfAbsent("Hello", key -> new Attributes())
.putValue("Foo", "Bar");
JarUtils.createJarFile(Path.of("b.jar"),
man,
Path.of("."),
Path.of("a"));
SecurityTools.jarsigner("-storepass changeit -keystore ks b.jar x");
SecurityTools.jarsigner("-verbose -verify b.jar")
.shouldContain("Warning: nonexistent signed entries: [Hello]")
.shouldContain(NONEXISTENT_ENTRIES_FOUND);

}
}
77 changes: 77 additions & 0 deletions test/lib-test/jdk/test/lib/util/JarUtilsTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/*
* Copyright (c) 2024, 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 8309841
* @summary Unit Test for a common Test API in jdk.test.lib.util.JarUtils
* @library /test/lib
*/

import jdk.test.lib.Asserts;
import jdk.test.lib.util.JarUtils;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Set;
import java.util.jar.JarEntry;
import java.util.jar.JarFile;
import java.util.stream.Collectors;

public class JarUtilsTest {
public static void main(String[] args) throws Exception {
Files.createDirectory(Path.of("bx"));
JarUtils.createJarFile(Path.of("a.jar"),
Path.of("."),
Files.writeString(Path.of("a"), ""),
Files.writeString(Path.of("b1"), ""),
Files.writeString(Path.of("b2"), ""),
Files.writeString(Path.of("bx/x"), ""),
Files.writeString(Path.of("c"), ""),
Files.writeString(Path.of("e1"), ""),
Files.writeString(Path.of("e2"), ""));
checkContent("a", "b1", "b2", "bx/x", "c", "e1", "e2");

JarUtils.deleteEntries(Path.of("a.jar"), "a");
checkContent("b1", "b2", "bx/x", "c", "e1", "e2");

// Note: b* covers everything starting with b, even bx/x
JarUtils.deleteEntries(Path.of("a.jar"), "b*");
checkContent("c", "e1", "e2");

// d* does not match
JarUtils.deleteEntries(Path.of("a.jar"), "d*");
checkContent("c", "e1", "e2");

// multiple patterns
JarUtils.deleteEntries(Path.of("a.jar"), "d*", "e*");
checkContent("c");
}

static void checkContent(String... expected) throws IOException {
try (var jf = new JarFile("a.jar")) {
Asserts.assertEquals(Set.of(expected),
jf.stream().map(JarEntry::getName).collect(Collectors.toSet()));
}
}
}
53 changes: 52 additions & 1 deletion test/lib/jdk/test/lib/util/JarUtils.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2015, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2015, 2024, 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
Expand Down Expand Up @@ -318,6 +318,57 @@ public static void updateManifest(String src, String dest, Manifest man)
updateJar(src, dest, Map.of(JarFile.MANIFEST_NAME, bout.toByteArray()));
}

/**
* Remove entries from a ZIP file.
*
* Each entry can be a name or a name ending with "*".
*
* @return number of removed entries
* @throws IOException if there is any I/O error
*/
public static int deleteEntries(Path jarfile, String... patterns)
throws IOException {
Path tmpfile = Files.createTempFile("jar", "jar");
int count = 0;

try (OutputStream out = Files.newOutputStream(tmpfile);
JarOutputStream jos = new JarOutputStream(out)) {
try (JarFile jf = new JarFile(jarfile.toString())) {
Enumeration<JarEntry> jentries = jf.entries();
top: while (jentries.hasMoreElements()) {
JarEntry jentry = jentries.nextElement();
String name = jentry.getName();
for (String pattern : patterns) {
if (pattern.endsWith("*")) {
if (name.startsWith(pattern.substring(
0, pattern.length() - 1))) {
// Go directly to next entry. This
// one is not written into `jos` and
// therefore removed.
count++;
continue top;
}
} else {
if (name.equals(pattern)) {
// Same as above
count++;
continue top;
}
}
}
// No pattern matched, file retained
jos.putNextEntry(copyEntry(jentry));
jf.getInputStream(jentry).transferTo(jos);
}
}
}

// replace the original JAR file
Files.move(tmpfile, jarfile, StandardCopyOption.REPLACE_EXISTING);

return count;
}

private static void updateEntry(JarOutputStream jos, String name, Object content)
throws IOException {
if (content instanceof Boolean) {
Expand Down