stan/jdk-24
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

8182879: Add warnings to keytool when using JKS and JCEKS

Browse Source 
Reviewed-by: vinnie, ahgross, mullan
This commit is contained in:
Weijun Wang 2017-07-12 10:55:40 +08:00
parent bbfbe9af71
commit 7cf3c0ff14
3 changed files with 319 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

187
src/java.base/share/classes/sun/security/tools/keytool/Main.java
View File

@ -26,6 +26,8 @@
package sun.security.tools.keytool;
import java.io.*;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.security.CodeSigner;
import java.security.CryptoPrimitive;
import java.security.KeyStore;
@ -168,7 +170,12 @@ public final class Main {
private List<String> ids = new ArrayList<>(); // used in GENCRL
private List<String> v3ext = new ArrayList<>();
// Warnings on weak algorithms
// In-place importkeystore is special.
// A backup is needed, and no need to prompt for deststorepass.
private boolean inplaceImport = false;
private String inplaceBackupName = null;
// Warnings on weak algorithms etc
private List<String> weakWarnings = new ArrayList<>();
private static final DisabledAlgorithmConstraints DISABLED_CHECK =
@ -846,37 +853,52 @@ public final class Main {
("New.password.must.be.at.least.6.characters"));
}
// Set this before inplaceImport check so we can compare name.
if (ksfname == null) {
ksfname = System.getProperty("user.home") + File.separator
+ ".keystore";
}
KeyStore srcKeyStore = null;
if (command == IMPORTKEYSTORE) {
inplaceImport = inplaceImportCheck();
if (inplaceImport) {
// We load srckeystore first so we have srcstorePass that
// can be assigned to storePass
srcKeyStore = loadSourceKeyStore();
if (storePass == null) {
storePass = srcstorePass;
}
}
}
// Check if keystore exists.
// If no keystore has been specified at the command line, try to use
// the default, which is located in $HOME/.keystore.
// If the command is "genkey", "identitydb", "import", or "printcert",
// it is OK not to have a keystore.
if (isKeyStoreRelated(command)) {
if (ksfname == null) {
ksfname = System.getProperty("user.home") + File.separator
+ ".keystore";
}
if (!nullStream) {
try {
ksfile = new File(ksfname);
// Check if keystore file is empty
if (ksfile.exists() && ksfile.length() == 0) {
throw new Exception(rb.getString
("Keystore.file.exists.but.is.empty.") + ksfname);
}
ksStream = new FileInputStream(ksfile);
} catch (FileNotFoundException e) {
if (command != GENKEYPAIR &&
// DO NOT open the existing keystore if this is an in-place import.
// The keystore should be created as brand new.
if (isKeyStoreRelated(command) && !nullStream && !inplaceImport) {
try {
ksfile = new File(ksfname);
// Check if keystore file is empty
if (ksfile.exists() && ksfile.length() == 0) {
throw new Exception(rb.getString
("Keystore.file.exists.but.is.empty.") + ksfname);
}
ksStream = new FileInputStream(ksfile);
} catch (FileNotFoundException e) {
if (command != GENKEYPAIR &&
command != GENSECKEY &&
command != IDENTITYDB &&
command != IMPORTCERT &&
command != IMPORTPASS &&
command != IMPORTKEYSTORE &&
command != PRINTCRL) {
throw new Exception(rb.getString
("Keystore.file.does.not.exist.") + ksfname);
}
throw new Exception(rb.getString
("Keystore.file.does.not.exist.") + ksfname);
}
}
}
@ -900,7 +922,7 @@ public final class Main {
// Create new keystore
// Probe for keystore type when filename is available
if (ksfile != null && ksStream != null && providerName == null &&
hasStoretypeOption == false) {
hasStoretypeOption == false && !inplaceImport) {
keyStore = KeyStore.getInstance(ksfile, storePass);
} else {
if (providerName == null) {
@ -930,7 +952,11 @@ public final class Main {
* Null stream keystores are loaded later.
*/
if (!nullStream) {
keyStore.load(ksStream, storePass);
if (inplaceImport) {
keyStore.load(null, storePass);
} else {
keyStore.load(ksStream, storePass);
}
if (ksStream != null) {
ksStream.close();
}
@ -1167,7 +1193,11 @@ public final class Main {
}
}
} else if (command == IMPORTKEYSTORE) {
doImportKeyStore();
// When not in-place import, srcKeyStore is not loaded yet.
if (srcKeyStore == null) {
srcKeyStore = loadSourceKeyStore();
}
doImportKeyStore(srcKeyStore);
kssave = true;
} else if (command == KEYCLONE) {
keyPassNew = newPass;
@ -1298,6 +1328,51 @@ public final class Main {
}
}
}
if (isKeyStoreRelated(command)
&& !token && !nullStream && ksfname != null) {
// JKS storetype warning on the final result keystore
File f = new File(ksfname);
char[] pass = (storePassNew!=null) ? storePassNew : storePass;
if (f.exists()) {
// Probe for real type. A JKS can be loaded as PKCS12 because
// DualFormat support, vice versa.
keyStore = KeyStore.getInstance(f, pass);
String realType = keyStore.getType();
if (realType.equalsIgnoreCase("JKS")
|| realType.equalsIgnoreCase("JCEKS")) {
boolean allCerts = true;
for (String a : Collections.list(keyStore.aliases())) {
if (!keyStore.entryInstanceOf(
a, TrustedCertificateEntry.class)) {
allCerts = false;
break;
}
}
// Don't warn for "cacerts" style keystore.
if (!allCerts) {
weakWarnings.add(String.format(
rb.getString("jks.storetype.warning"),
realType, ksfname));
}
}
if (inplaceImport) {
String realSourceStoreType = KeyStore.getInstance(
new File(inplaceBackupName), srcstorePass).getType();
String format =
realType.equalsIgnoreCase(realSourceStoreType) ?
rb.getString("backup.keystore.warning") :
rb.getString("migrate.keystore.warning");
weakWarnings.add(
String.format(format,
srcksfname,
realSourceStoreType,
inplaceBackupName,
realType));
}
}
}
}
/**
@ -1989,12 +2064,40 @@ public final class Main {
}
}
boolean inplaceImportCheck() throws Exception {
if (P11KEYSTORE.equalsIgnoreCase(srcstoretype) ||
KeyStoreUtil.isWindowsKeyStore(srcstoretype)) {
return false;
}
if (srcksfname != null) {
File srcksfile = new File(srcksfname);
if (srcksfile.exists() && srcksfile.length() == 0) {
throw new Exception(rb.getString
("Source.keystore.file.exists.but.is.empty.") +
srcksfname);
}
if (srcksfile.getCanonicalFile()
.equals(new File(ksfname).getCanonicalFile())) {
return true;
} else {
// Informational, especially if destkeystore is not
// provided, which default to ~/.keystore.
System.err.println(String.format(rb.getString(
"importing.keystore.status"), srcksfname, ksfname));
return false;
}
} else {
throw new Exception(rb.getString
("Please.specify.srckeystore"));
}
}
/**
* Load the srckeystore from a stream, used in -importkeystore
* @return the src KeyStore
*/
KeyStore loadSourceKeyStore() throws Exception {
boolean isPkcs11 = false;
InputStream is = null;
File srcksfile = null;
@ -2007,20 +2110,9 @@ public final class Main {
System.err.println();
tinyHelp();
}
isPkcs11 = true;
} else {
if (srcksfname != null) {
srcksfile = new File(srcksfname);
if (srcksfile.exists() && srcksfile.length() == 0) {
throw new Exception(rb.getString
("Source.keystore.file.exists.but.is.empty.") +
srcksfname);
}
is = new FileInputStream(srcksfile);
} else {
throw new Exception(rb.getString
("Please.specify.srckeystore"));
}
srcksfile = new File(srcksfname);
is = new FileInputStream(srcksfile);
}
KeyStore store;
@ -2087,17 +2179,32 @@ public final class Main {
* keep alias unchanged if no name conflict, otherwise, prompt.
* keep keypass unchanged for keys
*/
private void doImportKeyStore() throws Exception {
private void doImportKeyStore(KeyStore srcKS) throws Exception {
if (alias != null) {
doImportKeyStoreSingle(loadSourceKeyStore(), alias);
doImportKeyStoreSingle(srcKS, alias);
} else {
if (dest != null || srckeyPass != null) {
throw new Exception(rb.getString(
"if.alias.not.specified.destalias.and.srckeypass.must.not.be.specified"));
}
doImportKeyStoreAll(loadSourceKeyStore());
doImportKeyStoreAll(srcKS);
}
if (inplaceImport) {
// Backup to file.old or file.old2...
// The keystore is not rewritten yet now.
for (int n = 1; /* forever */; n++) {
inplaceBackupName = srcksfname + ".old" + (n == 1 ? "" : n);
File bkFile = new File(inplaceBackupName);
if (!bkFile.exists()) {
Files.copy(Paths.get(srcksfname), bkFile.toPath());
break;
}
}
}
/*
* Information display rule of -importkeystore
* 1. inside single, shows failure

6
src/java.base/share/classes/sun/security/tools/keytool/Resources.java
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2000, 2014, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2000, 2017, 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
@ -471,6 +471,10 @@ public class Resources extends java.util.ListResourceBundle {
{"verified.by.s.in.s.weak", "Verified by %s in %s with a %s"},
{"whose.sigalg.risk", "%s uses the %s signature algorithm which is considered a security risk."},
{"whose.key.risk", "%s uses a %s which is considered a security risk."},
{"jks.storetype.warning", "The %1$s keystore uses a proprietary format. It is recommended to migrate to PKCS12 which is an industry standard format using \"keytool -importkeystore -srckeystore %2$s -destkeystore %2$s -deststoretype pkcs12\"."},
{"migrate.keystore.warning", "Migrated \"%1$s\" to %4$s. The %2$s keystore is backed up as \"%3$s\"."},
{"backup.keystore.warning", "The original keystore \"%1$s\" is backed up as \"%3$s\"..."},
{"importing.keystore.status", "Importing keystore %1$s to %2$s..."},
};

181
test/jdk/sun/security/tools/keytool/WeakAlg.java
View File

@ -23,7 +23,7 @@
/*
* @test
* @bug 8171319 8177569
* @bug 8171319 8177569 8182879
* @summary keytool should print out warnings when reading or generating
* cert/cert req using weak algorithms
* @library /test/lib
@ -40,6 +40,7 @@
* @run main/othervm/timeout=600 -Duser.language=en -Duser.country=US WeakAlg
*/
import jdk.test.lib.Asserts;
import jdk.test.lib.SecurityTools;
import jdk.test.lib.process.OutputAnalyzer;
import sun.security.tools.KeyStoreUtil;
@ -47,6 +48,7 @@ import sun.security.util.DisabledAlgorithmConstraints;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.PrintStream;
@ -141,25 +143,164 @@ public class WeakAlg {
kt("-delete -alias b");
kt("-printcrl -file b.crl")
.shouldContain("WARNING: not verified");
jksTypeCheck();
checkInplaceImportKeyStore();
}
static void jksTypeCheck() throws Exception {
// No warning for cacerts, all certs
kt0("-cacerts -list -storepass changeit")
.shouldNotContain("Warning:");
rm("ks");
rm("ks2");
kt("-genkeypair -alias a -dname CN=A")
.shouldNotContain("Warning:");
kt("-list")
.shouldNotContain("Warning:");
kt("-list -storetype jks") // no warning if PKCS12 used as JKS
.shouldNotContain("Warning:");
kt("-exportcert -alias a -file a.crt")
.shouldNotContain("Warning:");
// warn if migrating to JKS
importkeystore("ks", "ks2", "-deststoretype jks")
.shouldContain("JKS keystore uses a proprietary format");
rm("ks");
rm("ks2");
rm("ks3");
// no warning if all certs
kt("-importcert -alias b -file a.crt -storetype jks -noprompt")
.shouldNotContain("Warning:");
kt("-genkeypair -alias a -dname CN=A")
.shouldContain("JKS keystore uses a proprietary format");
kt("-list")
.shouldContain("JKS keystore uses a proprietary format");
kt("-list -storetype pkcs12") // warn if JKS used as PKCS12
.shouldContain("JKS keystore uses a proprietary format");
kt("-exportcert -alias a -file a.crt")
.shouldContain("JKS keystore uses a proprietary format");
kt("-printcert -file a.crt") // no warning if keystore not touched
.shouldNotContain("Warning:");
kt("-certreq -alias a -file a.req")
.shouldContain("JKS keystore uses a proprietary format");
kt("-printcertreq -file a.req") // no warning if keystore not touched
.shouldNotContain("Warning:");
// No warning if migrating from JKS
importkeystore("ks", "ks2", "")
.shouldNotContain("Warning:");
importkeystore("ks", "ks3", "-deststoretype pkcs12")
.shouldNotContain("Warning:");
rm("ks");
kt("-genkeypair -alias a -dname CN=A -storetype jceks")
.shouldContain("JCEKS keystore uses a proprietary format");
kt("-list")
.shouldContain("JCEKS keystore uses a proprietary format");
kt("-importcert -alias b -file a.crt -noprompt")
.shouldContain("JCEKS keystore uses a proprietary format");
kt("-exportcert -alias a -file a.crt")
.shouldContain("JCEKS keystore uses a proprietary format");
kt("-printcert -file a.crt")
.shouldNotContain("Warning:");
kt("-certreq -alias a -file a.req")
.shouldContain("JCEKS keystore uses a proprietary format");
kt("-printcertreq -file a.req")
.shouldNotContain("Warning:");
kt("-genseckey -alias c -keyalg AES -keysize 128")
.shouldContain("JCEKS keystore uses a proprietary format");
}
static void checkImportKeyStore() throws Exception {
saveStore();
rm("ks2");
rm("ks3");
rm("ks");
kt("-importkeystore -srckeystore ks2 -srcstorepass changeit")
importkeystore("ks", "ks2", "")
.shouldContain("3 entries successfully imported")
.shouldContain("Warning")
.shouldMatch("<b>.*512-bit RSA key.*risk")
.shouldMatch("<a>.*MD5withRSA.*risk");
rm("ks");
kt("-importkeystore -srckeystore ks2 -srcstorepass changeit -srcalias a")
importkeystore("ks", "ks3", "-srcalias a")
.shouldContain("Warning")
.shouldMatch("<a>.*MD5withRSA.*risk");
}
reStore();
static void checkInplaceImportKeyStore() throws Exception {
rm("ks");
genkeypair("a", "");
// Same type backup
importkeystore("ks", "ks", "")
.shouldContain("Warning:")
.shouldMatch("original.*ks.old");
importkeystore("ks", "ks", "")
.shouldContain("Warning:")
.shouldMatch("original.*ks.old2");
importkeystore("ks", "ks", "-srcstoretype jks") // it knows real type
.shouldContain("Warning:")
.shouldMatch("original.*ks.old3");
String cPath = new File("ks").getCanonicalPath();
importkeystore("ks", cPath, "")
.shouldContain("Warning:")
.shouldMatch("original.*ks.old4");
// Migration
importkeystore("ks", "ks", "-deststoretype jks")
.shouldContain("Warning:")
.shouldContain("JKS keystore uses a proprietary format")
.shouldMatch("Migrated.*JKS.*PKCS12.*ks.old5");
Asserts.assertEQ(
KeyStore.getInstance(
new File("ks"), "changeit".toCharArray()).getType(),
"JKS");
importkeystore("ks", "ks", "-srcstoretype PKCS12")
.shouldContain("Warning:")
.shouldNotContain("proprietary format")
.shouldMatch("Migrated.*PKCS12.*JKS.*ks.old6");
Asserts.assertEQ(
KeyStore.getInstance(
new File("ks"), "changeit".toCharArray()).getType(),
"PKCS12");
Asserts.assertEQ(
KeyStore.getInstance(
new File("ks.old6"), "changeit".toCharArray()).getType(),
"JKS");
// One password prompt is enough for migration
kt0("-importkeystore -srckeystore ks -destkeystore ks", "changeit")
.shouldMatch("original.*ks.old7");
// But three if importing to a different keystore
rm("ks2");
kt0("-importkeystore -srckeystore ks -destkeystore ks2",
"changeit")
.shouldContain("Keystore password is too short");
kt0("-importkeystore -srckeystore ks -destkeystore ks2",
"changeit", "changeit", "changeit")
.shouldContain("Importing keystore ks to ks2...")
.shouldNotContain("original")
.shouldNotContain("Migrated");
}
static void checkImport() throws Exception {
@ -525,17 +666,22 @@ public class WeakAlg {
}
}
// Fast keytool execution by directly calling its main() method
static OutputAnalyzer kt(String cmd, String... input) {
return kt0("-keystore ks -storepass changeit " +
"-keypass changeit " + cmd, input);
}
// Fast keytool execution by directly calling its main() method
static OutputAnalyzer kt0(String cmd, String... input) {
PrintStream out = System.out;
PrintStream err = System.err;
InputStream ins = System.in;
ByteArrayOutputStream bout = new ByteArrayOutputStream();
ByteArrayOutputStream berr = new ByteArrayOutputStream();
boolean succeed = true;
String sout;
String serr;
try {
cmd = "-keystore ks -storepass changeit " +
"-keypass changeit " + cmd;
System.out.println("---------------------------------------------");
System.out.println("$ keytool " + cmd);
System.out.println();
@ -559,19 +705,26 @@ public class WeakAlg {
System.setOut(out);
System.setErr(err);
System.setIn(ins);
sout = new String(bout.toByteArray());
serr = new String(berr.toByteArray());
System.out.println("STDOUT:\n" + sout + "\nSTDERR:\n" + serr);
}
String sout = new String(bout.toByteArray());
String serr = new String(berr.toByteArray());
System.out.println("STDOUT:\n" + sout + "\nSTDERR:\n" + serr);
if (!succeed) {
throw new RuntimeException();
}
return new OutputAnalyzer(sout, serr);
}
static OutputAnalyzer importkeystore(String src, String dest,
String options) {
return kt0("-importkeystore "
+ "-srckeystore " + src + " -destkeystore " + dest
+ " -srcstorepass changeit -deststorepass changeit " + options);
}
static OutputAnalyzer genkeypair(String alias, String options) {
return kt("-genkeypair -alias " + alias + " -dname CN=" + alias
+ " -storetype JKS " + options);
+ " -storetype PKCS12 " + options);
}
static OutputAnalyzer certreq(String alias, String options) {