Hi,
When signing a jarfile the specified alias is converted by JavaKeyStore
(storetype JKS) to lower case.
When verifying the jarfile with the same alias, it is currently not
converted by JKS to lower case and jarsigner reports that the jar is
not signed by the alias even after having just signed it with that same
alias if not all characters are in lower case.
I found no statement in the documentation that an alias would support
only lower case or which characters could be used. It could, however,
be worth noting in the documentation of JavaKeyStore or the keytool
that the alias can be changed by the keystore in certain circumstances.
In my opinion, it feels like a bug that aliases are treated differently
when signing and verifying with respect to their upper and lower case.
Find a patch attached. Some explanations, just in case not too obvious
anyway:
- The jarsigner never changed the upper and lower cases of aliases
itself and neither does with the patch. That is now delegated to the
keystore implementation not only for signing but in addition also for
verifying. The fix is therefore not JSK-specific.
- I assume it is correct that the if (store != null) check and the try
catch KeyStoreException can both be moved outside the for loop over the
certificates because before as well as now with the patch the result
was alway 0 if store was null and KeyStoreException can happen only
when loading the keystore, which is certainly true for JKS.
- storeHash is used only by printCert and inKeyStoreForOneSigner and
contains the aliases as values enclosed in parentheses "(" and ")"
which is how it is printed by printCert. It would have probably been
easier to add the parentheses only later in printCert but then I tried
to change as little as possible.
- Two pairs of "result |= " statements were duplicate. Therefore there
are the two methods in the test which show that both actually failed
with "ckaliases.contains" being the offending piece of code. In the
patch I refactored to recombine them.
- I really wonder how "result |= SIGNED_BY_ALIAS;" should be set for
any certificate c and not only the one an alias in ckaliases points at
but I guess that is another story if one at all and might have come
from filling storeHash with all certificates and not only the ones in
ckaliases or so.
- At first glance it might look like a mistake that "alias" is not used
inside the loop over ckaliases but instead of comparing "alias" in
lower case to ckaliases with unspecified case, the certificate c is now
compared to the one gotten from the keystore handling the alias case in
its own way.
Would someone sponsor this fix or can/should it be improved?
Regards,
Philippdiff -r 31dde2f0ddf1 src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java
--- a/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java Wed Feb 20 10:48:36 2019 +0100
+++ b/src/jdk.jartool/share/classes/sun/security/tools/jarsigner/Main.java Wed Feb 20 13:33:07 2019 +0100
@@ -1425,31 +1425,35 @@
}
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.startsWith("(")) {
+ alias = alias.substring(1, alias.length() - 1);
+ } else {
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);
diff -r 31dde2f0ddf1 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 Wed Feb 20 13:33:07 2019 +0100
@@ -0,0 +1,121 @@
+/*
+ * 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
+ * @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
+ * ({@link sun.security.provider.JavaKeyStore.JKS#convertAlias(String)}).
+ */
+public class JavaKeyStoreAliasCaseInsensitive {
+
+ /**
+ * Alias for keystore with letters in different 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";
+ Manifest mf = new Manifest();
+ mf.getMainAttributes().put(Name.MANIFEST_VERSION, "1.0");
+ JarUtils.createJarFile(Paths.get(jarFilename), mf, 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).");
+ }
+
+ @Test
+ public void testAliasCaseStoreHash() throws Exception {
+ // Create a keystore with ALIAS + "2" signed by 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";
+ Manifest mf = new Manifest();
+ mf.getMainAttributes().put(Name.MANIFEST_VERSION, "1.0");
+ JarUtils.createJarFile(Paths.get(jarFilename), mf, 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 both ALIAS + "1"
+ // and ALIAS + "2" aliases and should verify. The manifest file is
+ // additionally signed by ALIAS + "2".
+ 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).");
+ }
+
+}
