Skip to content

Commit f1c1f96

Browse files
committedApr 7, 2023
8278851: Correct signer logic for jars signed with multiple digest algorithms
Reviewed-by: mbalao, andrew Backport-of: cbe497394786ff76a09f9743040e3ba96ee8298f
1 parent 1ee22a7 commit f1c1f96

File tree

3 files changed

+245
-49
lines changed

3 files changed

+245
-49
lines changed
 

‎jdk/src/share/classes/java/util/jar/JarVerifier.java

+8-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 1997, 2022, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -96,6 +96,10 @@ class JarVerifier {
9696
/** collect -DIGEST-MANIFEST values for blacklist */
9797
private List<Object> manifestDigests;
9898

99+
/* A cache mapping code signers to the algorithms used to digest jar
100+
entries, and whether or not the algorithms are permitted. */
101+
private Map<CodeSigner[], Map<String, Boolean>> signersToAlgs;
102+
99103
public JarVerifier(String name, byte rawBytes[]) {
100104
manifestName = name;
101105
manifestRawBytes = rawBytes;
@@ -105,6 +109,7 @@ public JarVerifier(String name, byte rawBytes[]) {
105109
pendingBlocks = new ArrayList<>();
106110
baos = new ByteArrayOutputStream();
107111
manifestDigests = new ArrayList<>();
112+
signersToAlgs = new HashMap<>();
108113
}
109114

110115
/**
@@ -244,7 +249,8 @@ private void processEntry(ManifestEntryVerifier mev)
244249
if (!parsingBlockOrSF) {
245250
JarEntry je = mev.getEntry();
246251
if ((je != null) && (je.signers == null)) {
247-
je.signers = mev.verify(verifiedSigners, sigFileSigners);
252+
je.signers = mev.verify(verifiedSigners, sigFileSigners,
253+
signersToAlgs);
248254
je.certs = mapSignersToCertArray(je.signers);
249255
}
250256
} else {

‎jdk/src/share/classes/sun/security/util/ManifestEntryVerifier.java

+67-47
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 1997, 2020, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 1997, 2022, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -192,7 +192,8 @@ public JarEntry getEntry()
192192
*
193193
*/
194194
public CodeSigner[] verify(Hashtable<String, CodeSigner[]> verifiedSigners,
195-
Hashtable<String, CodeSigner[]> sigFileSigners)
195+
Hashtable<String, CodeSigner[]> sigFileSigners,
196+
Map<CodeSigner[], Map<String, Boolean>> signersToAlgs)
196197
throws JarException
197198
{
198199
if (skip) {
@@ -207,38 +208,60 @@ public CodeSigner[] verify(Hashtable<String, CodeSigner[]> verifiedSigners,
207208
return signers;
208209
}
209210

210-
JarConstraintsParameters params =
211-
getParams(verifiedSigners, sigFileSigners);
211+
CodeSigner[] entrySigners = sigFileSigners.get(name);
212+
Map<String, Boolean> algsPermittedStatus =
213+
algsPermittedStatusForSigners(signersToAlgs, entrySigners);
214+
// Flag that indicates if only disabled algorithms are used and jar
215+
// entry should be treated as unsigned.
216+
boolean disabledAlgs = true;
217+
JarConstraintsParameters params = null;
212218

213219
for (int i=0; i < digests.size(); i++) {
214220
MessageDigest digest = digests.get(i);
215-
if (params != null) {
216-
try {
217-
params.setExtendedExceptionMsg(JarFile.MANIFEST_NAME,
218-
name + " entry");
219-
DisabledAlgorithmConstraints.jarConstraints()
220-
.permits(digest.getAlgorithm(), params, false);
221-
} catch (GeneralSecurityException e) {
222-
if (debug != null) {
223-
debug.println("Digest algorithm is restricted: " + e);
221+
String digestAlg = digest.getAlgorithm();
222+
223+
// Check if this algorithm is permitted, skip if false.
224+
if (algsPermittedStatus != null) {
225+
Boolean permitted = algsPermittedStatus.get(digestAlg);
226+
if (permitted == null) {
227+
if (params == null) {
228+
params = new JarConstraintsParameters(entrySigners);
224229
}
225-
return null;
230+
if (!checkConstraints(digestAlg, params)) {
231+
algsPermittedStatus.put(digestAlg, Boolean.FALSE);
232+
continue;
233+
} else {
234+
algsPermittedStatus.put(digestAlg, Boolean.TRUE);
235+
}
236+
} else if (!permitted) {
237+
continue;
226238
}
227239
}
240+
241+
// A non-disabled algorithm was used.
242+
disabledAlgs = false;
243+
228244
byte [] manHash = manifestHashes.get(i);
229245
byte [] theHash = digest.digest();
230246

231247
if (debug != null) {
232248
debug.println("Manifest Entry: " +
233-
name + " digest=" + digest.getAlgorithm());
249+
name + " digest=" + digestAlg);
234250
debug.println(" manifest " + toHex(manHash));
235251
debug.println(" computed " + toHex(theHash));
236252
debug.println();
237253
}
238254

239-
if (!MessageDigest.isEqual(theHash, manHash))
240-
throw new SecurityException(digest.getAlgorithm()+
255+
if (!MessageDigest.isEqual(theHash, manHash)) {
256+
throw new SecurityException(digestAlg +
241257
" digest error for "+name);
258+
}
259+
}
260+
261+
// If there were only disabled algorithms used, return null and jar
262+
// entry will be treated as unsigned.
263+
if (disabledAlgs) {
264+
return null;
242265
}
243266

244267
// take it out of sigFileSigners and put it in verifiedSigners...
@@ -249,39 +272,36 @@ public CodeSigner[] verify(Hashtable<String, CodeSigner[]> verifiedSigners,
249272
return signers;
250273
}
251274

252-
/**
253-
* Get constraints parameters for JAR. The constraints should be
254-
* checked against all code signers. Returns the parameters,
255-
* or null if the signers for this entry have already been checked
256-
* or there are no signers for this entry.
257-
*/
258-
private JarConstraintsParameters getParams(
259-
Map<String, CodeSigner[]> verifiedSigners,
260-
Map<String, CodeSigner[]> sigFileSigners) {
261-
262-
// verifiedSigners is usually preloaded with the Manifest's signers.
263-
// If verifiedSigners contains the Manifest, then it will have all of
264-
// the signers of the JAR. But if it doesn't then we need to fallback
265-
// and check verifiedSigners to see if the signers of this entry have
266-
// been checked already.
267-
if (verifiedSigners.containsKey(manifestFileName)) {
268-
if (verifiedSigners.size() > 1) {
269-
// this means we already checked it previously
270-
return null;
271-
} else {
272-
return new JarConstraintsParameters(
273-
verifiedSigners.get(manifestFileName));
275+
// Gets the algorithms permitted status for the signers of this entry.
276+
private static Map<String, Boolean> algsPermittedStatusForSigners(
277+
Map<CodeSigner[], Map<String, Boolean>> signersToAlgs,
278+
CodeSigner[] signers) {
279+
if (signers != null) {
280+
Map<String, Boolean> algs = signersToAlgs.get(signers);
281+
// create new HashMap if absent
282+
if (algs == null) {
283+
algs = new HashMap<>();
284+
signersToAlgs.put(signers, algs);
274285
}
275-
} else {
286+
return algs;
287+
}
288+
return null;
289+
}
290+
291+
// Checks the algorithm constraints against the signers of this entry.
292+
private boolean checkConstraints(String algorithm,
293+
JarConstraintsParameters params) {
294+
try {
295+
params.setExtendedExceptionMsg(JarFile.MANIFEST_NAME,
296+
name + " entry");
297+
DisabledAlgorithmConstraints.jarConstraints()
298+
.permits(algorithm, params, false);
299+
return true;
300+
} catch (GeneralSecurityException e) {
276301
if (debug != null) {
277-
debug.println(manifestFileName + " not present in verifiedSigners");
278-
}
279-
CodeSigner[] signers = sigFileSigners.get(name);
280-
if (signers == null || verifiedSigners.containsValue(signers)) {
281-
return null;
282-
} else {
283-
return new JarConstraintsParameters(signers);
302+
debug.println("Digest algorithm is restricted: " + e);
284303
}
304+
return false;
285305
}
286306
}
287307

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
/*
2+
* Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
/*
25+
* @test
26+
* @bug 8278851
27+
* @summary Check that jar entry with at least one non-disabled digest
28+
* algorithm in manifest is treated as signed
29+
* @library /lib/testlibrary /lib/security
30+
* @build jdk.testlibrary.JarUtils
31+
* jdk.testlibrary.Utils
32+
* @run main/othervm JarWithOneNonDisabledDigestAlg
33+
*/
34+
35+
import java.io.InputStream;
36+
import java.io.FileInputStream;
37+
import java.io.FileOutputStream;
38+
import java.nio.file.Files;
39+
import java.nio.file.Path;
40+
import java.nio.file.Paths;
41+
import java.nio.file.StandardCopyOption;
42+
import java.security.CodeSigner;
43+
import java.security.KeyStore;
44+
import java.util.Arrays;
45+
import java.util.Enumeration;
46+
import java.util.List;
47+
import java.util.Locale;
48+
import java.util.jar.JarEntry;
49+
import java.util.jar.JarFile;
50+
import java.util.zip.ZipFile;
51+
52+
import jdk.testlibrary.JarUtils;
53+
54+
public class JarWithOneNonDisabledDigestAlg {
55+
56+
private static final String PASS = "changeit";
57+
private static final String TESTFILE1 = "testfile1";
58+
private static final String TESTFILE2 = "testfile2";
59+
60+
public static void main(String[] args) throws Exception {
61+
SecurityUtils.removeFromDisabledAlgs("jdk.jar.disabledAlgorithms",
62+
Arrays.asList("SHA1"));
63+
Files.write(Paths.get(TESTFILE1), TESTFILE1.getBytes());
64+
JarUtils.createJarFile(Paths.get("unsigned.jar"), Paths.get("."),
65+
Paths.get(TESTFILE1));
66+
67+
genkeypair("-alias SHA1 -sigalg SHA1withRSA");
68+
genkeypair("-alias SHA256 -sigalg SHA256withRSA");
69+
70+
// Sign JAR twice with same signer but different digest algorithms
71+
// so that each entry in manifest file contains two digest values.
72+
signJarFile("SHA1", "MD5", "unsigned.jar", "signed.jar");
73+
signJarFile("SHA1", "SHA1", "signed.jar", "signed2.jar");
74+
checkThatJarIsSigned("signed2.jar", false);
75+
76+
// add another file to the JAR
77+
Files.write(Paths.get(TESTFILE2), "testFile2".getBytes());
78+
JarUtils.updateJar("signed2.jar", "signed3.jar", TESTFILE2);
79+
Files.move(Paths.get("signed3.jar"), Paths.get("signed2.jar"), StandardCopyOption.REPLACE_EXISTING);
80+
81+
// Sign again with different signer (SHA256) and SHA-1 digestalg.
82+
// TESTFILE1 should have two signers and TESTFILE2 should have one
83+
// signer.
84+
signJarFile("SHA256", "SHA1", "signed2.jar", "multi-signed.jar");
85+
86+
checkThatJarIsSigned("multi-signed.jar", true);
87+
}
88+
89+
private static KeyStore.PrivateKeyEntry getEntry(KeyStore ks, String alias)
90+
throws Exception {
91+
92+
return (KeyStore.PrivateKeyEntry)
93+
ks.getEntry(alias,
94+
new KeyStore.PasswordProtection(PASS.toCharArray()));
95+
}
96+
97+
private static void genkeypair(String cmd) throws Exception {
98+
cmd = "-genkeypair -keystore keystore -storepass " + PASS +
99+
" -keypass " + PASS + " -keyalg rsa -dname CN=Duke " + cmd;
100+
sun.security.tools.keytool.Main.main(cmd.split(" "));
101+
}
102+
103+
private static void signJarFile(String alias,
104+
String digestAlg, String inputFile, String outputFile)
105+
throws Exception {
106+
107+
// create a backup file as the signed file will be in-place
108+
Path from = Paths.get(inputFile);
109+
Path backup = Files.createTempFile(inputFile, "jar");
110+
Path output = Paths.get(outputFile);
111+
112+
Files.copy(from, backup, StandardCopyOption.REPLACE_EXISTING);
113+
114+
// sign the jar
115+
sun.security.tools.jarsigner.Main.main(
116+
("-keystore keystore -storepass " + PASS +
117+
" -digestalg " + digestAlg + " " + inputFile + " " +
118+
alias).split(" "));
119+
120+
Files.copy(from, output, StandardCopyOption.REPLACE_EXISTING);
121+
Files.copy(backup, from, StandardCopyOption.REPLACE_EXISTING);
122+
}
123+
124+
private static void checkThatJarIsSigned(String jarFile, boolean multi)
125+
throws Exception {
126+
127+
try (JarFile jf = new JarFile(jarFile, true)) {
128+
Enumeration<JarEntry> entries = jf.entries();
129+
while (entries.hasMoreElements()) {
130+
JarEntry entry = entries.nextElement();
131+
if (entry.isDirectory() || isSigningRelated(entry.getName())) {
132+
continue;
133+
}
134+
InputStream is = jf.getInputStream(entry);
135+
while (is.read() != -1);
136+
CodeSigner[] signers = entry.getCodeSigners();
137+
if (signers == null) {
138+
throw new Exception("JarEntry " + entry.getName() +
139+
" is not signed");
140+
} else if (multi) {
141+
if (entry.getName().equals(TESTFILE1) &&
142+
signers.length != 2) {
143+
throw new Exception("Unexpected number of signers " +
144+
"for " + entry.getName() + ": " + signers.length);
145+
} else if (entry.getName().equals(TESTFILE2) &&
146+
signers.length != 1) {
147+
throw new Exception("Unexpected number of signers " +
148+
"for " + entry.getName() + ": " + signers.length);
149+
}
150+
}
151+
}
152+
}
153+
}
154+
155+
private static boolean isSigningRelated(String name) {
156+
name = name.toUpperCase(Locale.ENGLISH);
157+
if (!name.startsWith("META-INF/")) {
158+
return false;
159+
}
160+
name = name.substring(9);
161+
if (name.indexOf('/') != -1) {
162+
return false;
163+
}
164+
return name.endsWith(".SF")
165+
|| name.endsWith(".DSA")
166+
|| name.endsWith(".RSA")
167+
|| name.endsWith(".EC")
168+
|| name.equals("MANIFEST.MF");
169+
}
170+
}

0 commit comments

Comments
 (0)
Please sign in to comment.