Hi Max,
You are right. An explicit manifest is not necessary. I don't remember
why I added it in the first place. It works also without it.
Regards,
Philipp
On Thu, 2019-05-09 at 16:44 +0800, Weijun Wang wrote:
> Hi Philipp,
>
> I've posted your patch at
>
> http://cr.openjdk.java.net/~weijun/8221719/webrev.00/
>
> Everyone please take a review.
>
> I think it looks fine. Just one question: why do you need to create
> the Manifest in the test. Can you just create a jar without
> MANIFEST.MF and let jarsigner add it?
>
> Thanks,
> Max
>
> > On May 1, 2019, at 10:14 PM, Philipp Kunz <[email protected]>
> > wrote:
> >
> > Hi Max and everyone,
> >
> > With respect to the previous patch, parentheses moved from
> > storeHash to printCert, bug number added, and some comments added
> > and clarified.
> >
> > Regards,
> > Philipp
> >
> >
>
>
diff -r c2551d161358 src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java
--- a/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java Fri May 10 17:47:42 2019 -0700
+++ b/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java Sat May 11 09:31:41 2019 +0200
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2019, 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
@@ -1300,7 +1300,7 @@
String alias = storeHash.get(c);
if (alias != null) {
- certStr.append(space).append(alias);
+ certStr.append(space).append("(").append(alias).append(")");
}
if (x509Cert != null) {
@@ -1425,37 +1425,43 @@
}
int result = 0;
- List<? extends Certificate> certs = signer.getSignerCertPath().getCertificates();
- for (Certificate c : certs) {
- String alias = storeHash.get(c);
- if (alias != null) {
- if (alias.startsWith("(")) {
- result |= IN_KEYSTORE;
- }
- if (ckaliases.contains(alias.substring(1, alias.length() - 1))) {
- result |= SIGNED_BY_ALIAS;
- }
- } else {
- if (store != null) {
- try {
+ if (store != null) {
+ try {
+ List<? extends Certificate> certs =
+ signer.getSignerCertPath().getCertificates();
+ for (Certificate c : certs) {
+ String alias = storeHash.get(c);
+ if (alias == null) {
alias = store.getCertificateAlias(c);
- } catch (KeyStoreException kse) {
- // never happens, because keystore has been loaded
+ if (alias != null) {
+ storeHash.put(c, alias);
+ }
}
if (alias != null) {
- storeHash.put(c, "(" + alias + ")");
result |= IN_KEYSTORE;
}
+ for (String ckalias : ckaliases) {
+ if (c.equals(store.getCertificate(ckalias))) {
+ result |= SIGNED_BY_ALIAS;
+ // must continue with next certificate c and cannot
+ // return or break outer loop because has to fill
+ // storeHash for printCert
+ break;
+ }
+ }
}
- if (ckaliases.contains(alias)) {
- result |= SIGNED_BY_ALIAS;
- }
+ } catch (KeyStoreException kse) {
+ // never happens, because keystore has been loaded
}
}
cacheForInKS.put(signer, result);
return result;
}
+ /**
+ * Maps certificates (as keys) to alias names associated in the keystore
+ * {@link #store} (as values).
+ */
Hashtable<Certificate, String> storeHash = new Hashtable<>();
int inKeyStore(CodeSigner[] signers) {
diff -r c2551d161358 test/jdk/sun/security/tools/jarsigner/JavaKeyStoreAliasCaseInsensitive.java
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/sun/security/tools/jarsigner/JavaKeyStoreAliasCaseInsensitive.java Sat May 11 09:31:41 2019 +0200
@@ -0,0 +1,136 @@
+/*
+ * Copyright (c) 2019, 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.
+ */
+
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.util.jar.Manifest;
+import java.util.jar.Attributes.Name;
+import jdk.test.lib.util.JarUtils;
+import jdk.test.lib.SecurityTools;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.*;
+
+/**
+ * @test
+ * @bug 8221719
+ * @library /test/lib
+ * @run testng JavaKeyStoreAliasCaseInsensitive
+ * @summary Checks that jarsigner verifies a signed jar with the same alias as
+ * was specified for signing, particularly regarding upper and lower case and
+ * its conversion to lower case by JKS
+ * ({@link sun.security.provider.JavaKeyStore.JKS#convertAlias(String)}).
+ */
+public class JavaKeyStoreAliasCaseInsensitive {
+
+ /**
+ * Alias for certificates in the keystore with letters in different
+ * (upper and lower) cases.
+ */
+ static final String ALIAS = "AlIaS";
+
+ @Test
+ public void testAliasCase() throws Exception {
+ final String KEYSTORE_OPTIONS = "-storetype JKS -keystore "
+ + "test-alias-case.jks -storepass changeit";
+ SecurityTools.keytool(KEYSTORE_OPTIONS + " -genkeypair"
+ + " -keypass changeit -alias " + ALIAS + " -dname CN=" + ALIAS)
+ .shouldHaveExitValue(0);
+ String jarFilename = "test-alias-case.jar";
+ JarUtils.createJarFile(Paths.get(jarFilename), Paths.get("."),
+ Files.write(Paths.get("aFile"), new byte[1]));
+
+ SecurityTools.jarsigner(KEYSTORE_OPTIONS + " -verbose -debug " +
+ jarFilename + " " + ALIAS).shouldHaveExitValue(0);
+
+ SecurityTools.jarsigner("-verify -strict " + KEYSTORE_OPTIONS +
+ " -debug -verbose " + jarFilename + " " + ALIAS)
+ .shouldHaveExitValue(0).shouldNotContain(
+ "This jar contains signed entries which are not "
+ + "signed by the specified alias(es).");
+ }
+
+ /**
+ * This test essentially covers compatibility with the previous version of
+ * {@link sun.security.tools.jarsigner.Main#inKeyStoreForOneSigner} in case
+ * a certificate and alias entry is already in
+ * {@link sun.security.tools.jarsigner.Main#storeHash} when
+ * {@link sun.security.tools.jarsigner.Main#inKeyStoreForOneSigner} is
+ * invoked from a previous invocation of it.
+ * It passed with the previous {@code jarsigner} version with a lowercase
+ * alias {@link #ALIAS} and basically covers the duplicated portions of
+ * code in {@link sun.security.tools.jarsigner.Main#inKeyStoreForOneSigner}
+ * near {@code IN_KEYSTORE} and {@code SIGNED_BY_ALIAS} before having
+ * refactored and re-unified them in order to demonstrate identical
+ * behavior.
+ */
+ @Test
+ public void testAliasCaseStoreHash() throws Exception {
+ // Create a keystore with a certificate associated with ALIAS + "2"
+ // signed by another certificate associated with ALIAS + "1".
+ final String KEYSTORE_OPTIONS = "-storetype JKS -keystore"
+ + " test-alias-storeHash-case.jks -storepass changeit";
+ SecurityTools.keytool(KEYSTORE_OPTIONS + " -genkeypair"
+ + " -keypass changeit -alias " + ALIAS + "1 -dname CN=" +
+ ALIAS + "1").shouldHaveExitValue(0);
+ SecurityTools.keytool(KEYSTORE_OPTIONS + " -genkeypair"
+ + " -keypass changeit -alias " + ALIAS + "2 -dname CN="
+ + ALIAS + "2").shouldHaveExitValue(0);
+ String certReq = SecurityTools.keytool(KEYSTORE_OPTIONS +
+ " -certreq -keypass changeit -alias " + ALIAS + "2")
+ .shouldHaveExitValue(0).getStdout();
+ SecurityTools.setResponse(certReq);
+ String cert = SecurityTools.keytool(KEYSTORE_OPTIONS +
+ " -gencert -rfc -keypass changeit -alias " + ALIAS + "1")
+ .shouldHaveExitValue(0).getOutput();
+ SecurityTools.setResponse(cert);
+ SecurityTools.keytool(KEYSTORE_OPTIONS +
+ " -importcert -keypass changeit -alias " + ALIAS + "2")
+ .shouldHaveExitValue(0);
+
+ // Create a jar file signed by ALIAS + "2" and then add another file to
+ // that same jar and sign it by ALIAS + "1", ALIAS + "1" being an alias
+ // for a certificate which is part of the certificate chain of ALIAS +
+ // "2" but not the certificate ALIAS + "2" points to directly.
+ String jarFilename = "test-alias-storeHash-case.jar";
+ JarUtils.createJarFile(Paths.get(jarFilename), Paths.get("."));
+ SecurityTools.jarsigner(KEYSTORE_OPTIONS + " -verbose -debug " +
+ jarFilename + " " + ALIAS + "2").shouldHaveExitValue(0);
+ JarUtils.updateJarFile(Paths.get(jarFilename), Paths.get("."),
+ Files.write(Paths.get("added-file"), new byte[1]));
+ SecurityTools.jarsigner(KEYSTORE_OPTIONS + " -verbose -debug " +
+ jarFilename + " " + ALIAS + "1").shouldHaveExitValue(0);
+
+ // The later added file "added-file" is signed by the certificate
+ // associated with alias ALIAS + "1" directly while the other jarfile
+ // contents is signed by a certificate associated with alias ALIAS + "2"
+ // which includes the certificate associated with alias ALIAS + "1" in
+ // its certification path.
+ SecurityTools.jarsigner("-verify -strict " + KEYSTORE_OPTIONS +
+ " -debug -verbose " + jarFilename + " " + ALIAS + "1")
+ .shouldHaveExitValue(0).shouldNotContain(
+ "This jar contains signed entries which are not "
+ + "signed by the specified alias(es).");
+ }
+
+}
