--- a/jdk/src/java.base/share/classes/sun/security/tools/keytool/Main.java Wed Mar 29 10:50:45 2017 -0700
+++ b/jdk/src/java.base/share/classes/sun/security/tools/keytool/Main.java Thu Mar 30 07:29:58 2017 +0800
@@ -1025,6 +1025,13 @@
cf = CertificateFactory.getInstance("X509");
}
+ // -trustcacerts can only be specified on -importcert.
+ // Reset it so that warnings on CA cert will remain for
+ // -printcert, etc.
+ if (command != IMPORTCERT) {
+ trustcacerts = false;
+ }
+
if (trustcacerts) {
caks = KeyStoreUtil.getCacertsKeyStore();
}
@@ -1758,9 +1765,8 @@
if (keyPass == null) {
keyPass = promptForKeyPass(alias, null, storePass);
}
+ checkWeak(rb.getString("the.generated.certificate"), chain[0]);
keyStore.setKeyEntry(alias, privKey, keyPass, chain);
-
- checkWeak(rb.getString("the.generated.certificate"), chain[0]);
}
/**
@@ -2118,6 +2124,10 @@
}
try {
+ Certificate c = srckeystore.getCertificate(alias);
+ if (c != null) {
+ checkWeak("<" + newAlias + ">", c);
+ }
keyStore.setEntry(newAlias, entry, pp);
// Place the check so that only successful imports are blocked.
// For example, we don't block a failed SecretEntry import.
@@ -2127,10 +2137,6 @@
"The.destination.pkcs12.keystore.has.different.storepass.and.keypass.Please.retry.with.destkeypass.specified."));
}
}
- Certificate c = srckeystore.getCertificate(alias);
- if (c != null) {
- checkWeak("<" + newAlias + ">", c);
- }
return 1;
} catch (KeyStoreException kse) {
Object[] source2 = {alias, kse.toString()};
@@ -2814,8 +2820,8 @@
}
if (noprompt) {
+ checkWeak(rb.getString("the.input"), cert);
keyStore.setCertificateEntry(alias, cert);
- checkWeak(rb.getString("the.input"), cert);
return true;
}
@@ -3049,6 +3055,11 @@
MessageFormat form = new MessageFormat
(rb.getString(".PATTERN.printX509Cert.with.weak"));
PublicKey pkey = cert.getPublicKey();
+ String sigName = cert.getSigAlgName();
+ // No need to warn about sigalg of a trust anchor
+ if (!isTrustedCert(cert)) {
+ sigName = withWeak(sigName);
+ }
Object[] source = {cert.getSubjectDN().toString(),
cert.getIssuerDN().toString(),
cert.getSerialNumber().toString(16),
@@ -3056,7 +3067,7 @@
cert.getNotAfter().toString(),
getCertFingerPrint("SHA-1", cert),
getCertFingerPrint("SHA-256", cert),
- withWeak(cert.getSigAlgName()),
+ sigName,
withWeak(pkey),
cert.getVersion()
};
@@ -3111,7 +3122,7 @@
* or null otherwise. A label is added.
*/
private static Pair<String,Certificate>
- getTrustedSigner(Certificate cert, KeyStore ks) throws Exception {
+ getSigner(Certificate cert, KeyStore ks) throws Exception {
if (ks.getCertificateAlias(cert) != null) {
return new Pair<>("", cert);
}
@@ -3467,9 +3478,9 @@
// do we trust the cert at the top?
Certificate topCert = replyCerts[replyCerts.length-1];
boolean fromKeyStore = true;
- Pair<String,Certificate> root = getTrustedSigner(topCert, keyStore);
+ Pair<String,Certificate> root = getSigner(topCert, keyStore);
if (root == null && trustcacerts && caks != null) {
- root = getTrustedSigner(topCert, caks);
+ root = getSigner(topCert, caks);
fromKeyStore = false;
}
if (root == null) {
@@ -4301,9 +4312,19 @@
return result;
}
+ private boolean isTrustedCert(Certificate cert) throws KeyStoreException {
+ if (caks != null && caks.getCertificateAlias(cert) != null) {
+ return true;
+ } else {
+ String inKS = keyStore.getCertificateAlias(cert);
+ return inKS != null && keyStore.isCertificateEntry(inKS);
+ }
+ }
+
private void checkWeak(String label, String sigAlg, Key key) {
- if (!DISABLED_CHECK.permits(SIG_PRIMITIVE_SET, sigAlg, null)) {
+ if (sigAlg != null && !DISABLED_CHECK.permits(
+ SIG_PRIMITIVE_SET, sigAlg, null)) {
weakWarnings.add(String.format(
rb.getString("whose.sigalg.risk"), label, sigAlg));
}
@@ -4316,7 +4337,8 @@
}
}
- private void checkWeak(String label, Certificate[] certs) {
+ private void checkWeak(String label, Certificate[] certs)
+ throws KeyStoreException {
for (int i = 0; i < certs.length; i++) {
Certificate cert = certs[i];
if (cert instanceof X509Certificate) {
@@ -4325,15 +4347,18 @@
if (certs.length > 1) {
fullLabel = oneInMany(label, i, certs.length);
}
- checkWeak(fullLabel, xc.getSigAlgName(), xc.getPublicKey());
+ checkWeak(fullLabel, xc);
}
}
}
- private void checkWeak(String label, Certificate cert) {
+ private void checkWeak(String label, Certificate cert)
+ throws KeyStoreException {
if (cert instanceof X509Certificate) {
X509Certificate xc = (X509Certificate)cert;
- checkWeak(label, xc.getSigAlgName(), xc.getPublicKey());
+ // No need to check the sigalg of a trust anchor
+ String sigAlg = isTrustedCert(cert) ? null : xc.getSigAlgName();
+ checkWeak(label, sigAlg, xc.getPublicKey());
}
}
--- a/jdk/test/sun/security/tools/keytool/WeakAlg.java Wed Mar 29 10:50:45 2017 -0700
+++ b/jdk/test/sun/security/tools/keytool/WeakAlg.java Thu Mar 30 07:29:58 2017 +0800
@@ -23,7 +23,7 @@
/*
* @test
- * @bug 8171319
+ * @bug 8171319 8177569
* @summary keytool should print out warnings when reading or generating
* cert/cert req using weak algorithms
* @library /test/lib
@@ -78,7 +78,8 @@
.shouldMatch("<b>.*512-bit RSA key.*risk")
.shouldContain("512-bit RSA key (weak)");
- // Multiple warnings for multiple cert in -printcert or -list or -exportcert
+ // Multiple warnings for multiple cert in -printcert
+ // or -list or -exportcert
// -certreq, -printcertreq, -gencert
checkCertReq("a", "", null);
@@ -184,7 +185,7 @@
.shouldMatch("The input.*MD5withRSA.*risk")
.shouldNotContain("[no]");
- // cert is self-signed cacerts
+ // JDK-8177569: no warning for sigalg of trusted cert
String weakSigAlgCA = null;
KeyStore ks = KeyStoreUtil.getCacertsKeyStore();
if (ks != null) {
@@ -208,12 +209,40 @@
}
}
if (weakSigAlgCA != null) {
+ // The following 2 commands still have a warning on why not using
+ // the -cacerts option directly.
+ kt("-list -keystore " + KeyStoreUtil.getCacerts())
+ .shouldNotContain("risk");
+ kt("-list -v -keystore " + KeyStoreUtil.getCacerts())
+ .shouldNotContain("risk");
+
+ // -printcert will always show warnings
+ kt("-printcert -file ca.cert")
+ .shouldContain("name: " + weakSigAlgCA + " (weak)")
+ .shouldContain("Warning")
+ .shouldMatch("The certificate.*" + weakSigAlgCA + ".*risk");
+ kt("-printcert -file ca.cert -trustcacerts") // -trustcacerts useless
+ .shouldContain("name: " + weakSigAlgCA + " (weak)")
+ .shouldContain("Warning")
+ .shouldMatch("The certificate.*" + weakSigAlgCA + ".*risk");
+
+ // Importing with -trustcacerts ignore CA cert's sig alg
kt("-delete -alias d");
kt("-importcert -alias d -trustcacerts -file ca.cert", "no")
.shouldContain("Certificate already exists in system-wide CA")
+ .shouldNotContain("risk")
+ .shouldContain("Do you still want to add it to your own keystore?");
+ kt("-importcert -alias d -trustcacerts -file ca.cert -noprompt")
+ .shouldNotContain("risk")
+ .shouldNotContain("[no]");
+
+ // but not without -trustcacerts
+ kt("-delete -alias d");
+ kt("-importcert -alias d -file ca.cert", "no")
+ .shouldContain("name: " + weakSigAlgCA + " (weak)")
.shouldContain("Warning")
.shouldMatch("The input.*" + weakSigAlgCA + ".*risk")
- .shouldContain("Do you still want to add it to your own keystore?");
+ .shouldContain("Trust this certificate?");
kt("-importcert -alias d -file ca.cert -noprompt")
.shouldContain("Warning")
.shouldMatch("The input.*" + weakSigAlgCA + ".*risk")
@@ -266,6 +295,26 @@
// install reply
reStore();
+ certreq("c", "");
+ gencert("a-c", "");
+ kt("-importcert -alias c -file a-c.cert")
+ .shouldContain("Warning")
+ .shouldMatch("Issuer <a>.*MD5withRSA.*risk");
+
+ // JDK-8177569: no warning for sigalg of trusted cert
+ reStore();
+ // Change a into a TrustedCertEntry
+ kt("-exportcert -alias a -file a.cert");
+ kt("-delete -alias a");
+ kt("-importcert -alias a -file a.cert -noprompt");
+ kt("-list -alias a -v")
+ .shouldNotContain("weak")
+ .shouldNotContain("Warning");
+ // This time a is trusted and no warning on its weak sig alg
+ kt("-importcert -alias c -file a-c.cert")
+ .shouldNotContain("Warning");
+
+ reStore();
gencert("a-b", "");
gencert("b-c", "");