Skip to content

Commit 334c8cd

Browse files
committed
Workaround for issue mentioned in #25529 - PKCS12/JKS vs private key passwords
Signed-off-by: David Matějček <[email protected]>
1 parent c34bbc2 commit 334c8cd

File tree

3 files changed

+174
-101
lines changed
  • nucleus

3 files changed

+174
-101
lines changed

nucleus/admin/server-mgmt/src/main/java/com/sun/enterprise/admin/servermgmt/KeystoreManager.java

Lines changed: 1 addition & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -25,20 +25,15 @@
2525
import java.io.File;
2626
import java.io.IOException;
2727
import java.lang.System.Logger;
28-
import java.security.Key;
29-
import java.security.KeyStore;
30-
import java.security.UnrecoverableKeyException;
3128
import java.util.ArrayList;
3229
import java.util.Arrays;
33-
import java.util.Collections;
3430
import java.util.List;
3531
import java.util.regex.Pattern;
3632

3733
import org.glassfish.main.jdke.security.KeyTool;
3834

3935
import static com.sun.enterprise.admin.servermgmt.domain.DomainConstants.KEYSTORE_FILE;
4036
import static com.sun.enterprise.admin.servermgmt.domain.DomainConstants.TRUSTSTORE_FILE;
41-
import static java.lang.System.Logger.Level.WARNING;
4237

4338
/**
4439
* @author kebbs
@@ -138,53 +133,6 @@ protected void changeKeystorePassword(String oldPassword, String newPassword, Fi
138133
}
139134
}
140135

141-
/**
142-
* Changes the key password for the default cert whose alias is s1as. The assumption here is that the keystore password
143-
* is not the same as the key password. This is due to the fact that the keystore password should first be changed
144-
* followed next by the key password. The end result is that the keystore and s1as key both have the same passwords.
145-
* This function will tolerate deletion of the s1as alias, but it will not tolerate changing the s1as key from something
146-
* other than the database password.
147-
*
148-
* @param config
149-
* @param storePassword the keystore password
150-
* @param oldKeyPassword the old password for the s1as alias
151-
* @param newKeyPassword the new password for the s1as alias
152-
* @throws DomainException
153-
*/
154-
protected void changeKeyPasswords(RepositoryConfig config, String storePassword, String oldKeyPassword,
155-
String newKeyPassword) throws DomainException {
156-
if (storePassword.equals(oldKeyPassword) || oldKeyPassword.equals(newKeyPassword)) {
157-
return;
158-
}
159-
final PEFileLayout layout = getFileLayout(config);
160-
final File keystore = layout.getKeyStore();
161-
try {
162-
KeyStore keyStore = KeyStore.getInstance(keystore, storePassword.toCharArray());
163-
List<String> aliases = Collections.list(keyStore.aliases());
164-
List<String> keyAliases = new ArrayList<>();
165-
for (String alias : aliases) {
166-
Key key;
167-
try {
168-
key = keyStore.getKey(alias, oldKeyPassword.toCharArray());
169-
} catch (UnrecoverableKeyException e) {
170-
LOG.log(WARNING,
171-
"Key entry with alias {0} in a key store {1} could not be recovered with provided key password.",
172-
alias, keystore);
173-
continue;
174-
}
175-
if (key != null) {
176-
keyAliases.add(alias);
177-
}
178-
}
179-
KeyTool keyTool = new KeyTool(keystore, storePassword.toCharArray());
180-
for (String alias : keyAliases) {
181-
keyTool.changeKeyPassword(alias, oldKeyPassword.toCharArray(), newKeyPassword.toCharArray());
182-
}
183-
} catch (Exception e) {
184-
throw new DomainException(_strMgr.getString("s1asKeyPasswordNotChanged", keystore), e);
185-
}
186-
}
187-
188136
/**
189137
* Changes the password of the keystore, truststore and the key password of the s1as alias. It is expected that the key
190138
* / truststores may not exist. This is due to the fact that the user may have deleted them and wishes to set up their
@@ -197,16 +145,11 @@ protected void changeKeyPasswords(RepositoryConfig config, String storePassword,
197145
protected void changeSSLCertificateDatabasePassword(RepositoryConfig config, String oldPassword, String newPassword) throws DomainException {
198146
final PEFileLayout layout = getFileLayout(config);
199147
File keystore = layout.getKeyStore();
200-
File truststore = layout.getTrustStore();
201-
202148
if (keystore.exists()) {
203-
// Change the password on the keystore
204149
changeKeystorePassword(oldPassword, newPassword, keystore);
205-
changeKeyPasswords(config, newPassword, oldPassword, newPassword);
206150
}
207-
151+
File truststore = layout.getTrustStore();
208152
if (truststore.exists()) {
209-
// Change the password on the truststore
210153
changeKeystorePassword(oldPassword, newPassword, truststore);
211154
}
212155
}

nucleus/common/glassfish-jdk-extensions/src/main/java/org/glassfish/main/jdke/security/KeyTool.java

Lines changed: 113 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,18 @@
2828
import java.security.KeyStore;
2929
import java.security.KeyStoreException;
3030
import java.security.NoSuchAlgorithmException;
31+
import java.security.PrivateKey;
32+
import java.security.cert.Certificate;
3133
import java.security.cert.CertificateException;
34+
import java.util.Collections;
3235
import java.util.List;
3336
import java.util.concurrent.TimeUnit;
3437
import java.util.stream.Collectors;
3538

3639
import static java.lang.System.Logger.Level.DEBUG;
3740
import static java.lang.System.Logger.Level.ERROR;
3841
import static java.lang.System.Logger.Level.INFO;
42+
import static java.lang.System.Logger.Level.WARNING;
3943

4044
/**
4145
* Java adapter to call the keytool command.
@@ -63,18 +67,34 @@ public class KeyTool {
6367
}
6468

6569
private final File keyStore;
70+
private final String keyStoreType;
6671
private char[] password;
6772

6873
/**
6974
* Creates a new instance of KeyTool managing the keystore file.
7075
* The file may not exist yet.
76+
* The type is detected automatically from the file extension.
7177
*
7278
* @param keyStore the file representing the keystore
7379
* @param password keystore and key password, must have at least 6 characters
7480
*/
7581
public KeyTool(File keyStore, char[] password) {
82+
this(keyStore, guessKeyStoreType(keyStore), password);
83+
}
84+
85+
86+
/**
87+
* Creates a new instance of KeyTool managing the keystore file.
88+
* The file may not exist yet.
89+
*
90+
* @param keyStore the file representing the keystore
91+
* @param keyStoreType the type of the keystore, e.g. "PKCS12", "JKS"
92+
* @param password keystore and key password, must have at least 6 characters
93+
*/
94+
public KeyTool(File keyStore, String keyStoreType, char[] password) {
7695
this.keyStore = keyStore;
7796
this.password = password;
97+
this.keyStoreType = keyStoreType;
7898
}
7999

80100

@@ -113,7 +133,7 @@ public void generateKeyPair(String alias, String dn, String keyAlgorithm, int ce
113133
"-keyalg", keyAlgorithm,
114134
"-validity", Integer.toString(certValidity),
115135
"-keystore", keyStore.getAbsolutePath(),
116-
"-storetype", "JKS"
136+
"-storetype", keyStoreType
117137
);
118138
if (keyStore.getParentFile().mkdirs()) {
119139
// The directory must exist, keytool will not create it
@@ -196,6 +216,7 @@ public void exportCertificate(String alias, final File outputFile) throws IOExce
196216

197217
/**
198218
* Changes the key store password and remembers it.
219+
* Changes also passwords of all keys in the key store which use the same password.
199220
*
200221
* @param newPassword the new key store password
201222
* @throws IOException
@@ -209,29 +230,61 @@ public void changeKeyStorePassword(char[] newPassword) throws IOException {
209230
"-keystore", this.keyStore.getAbsolutePath()
210231
);
211232
execute(command, password, newPassword, newPassword, newPassword);
233+
final char[] oldPassword = password;
212234
this.password = newPassword;
235+
if ("PKCS12".equals(this.keyStoreType)) {
236+
// PKCS12 key store type changes passwords of keys together with the key store password
237+
// JKS and JCEKS key store types require changing passwords of keys separately
238+
return;
239+
}
240+
KeyStore ks = loadKeyStore();
241+
final List<String> aliases;
242+
try {
243+
aliases = Collections.list(ks.aliases());
244+
} catch (KeyStoreException e) {
245+
throw new IOException("Could not list aliases in keystore: " + keyStore, e);
246+
}
247+
for (String alias : aliases) {
248+
try {
249+
if (ks.isKeyEntry(alias)) {
250+
changeKeyPassword(ks, alias, oldPassword, newPassword);
251+
}
252+
} catch (IOException | KeyStoreException e) {
253+
LOG.log(WARNING, "Could not change key password for alias: {0}, it may use different password.", alias);
254+
}
255+
}
256+
try (FileOutputStream output = new FileOutputStream(keyStore)) {
257+
ks.store(output, password);
258+
} catch (GeneralSecurityException e) {
259+
throw new IOException(
260+
"Keystore password successfuly changed, however failed changing key passwords: " + keyStore, e);
261+
}
213262
}
214263

215264

216265
/**
217266
* Changes the key password
267+
* <p>
268+
* WARNING: This is not required for the PKCS12 key store type, as it changes passwords of keys
269+
* together with the key store password.
218270
*
219271
* @param alias the alias of the key whose password should be changed
220272
* @param oldPassword the current key entry password
221273
* @param newPassword the new key entry password
222274
* @throws IOException
223275
*/
224276
public void changeKeyPassword(String alias, char[] oldPassword, char[] newPassword) throws IOException {
225-
List<String> command = List.of(
226-
KEYTOOL,
227-
"-J-Duser.language=en",
228-
"-noprompt",
229-
"-keypasswd",
230-
"-alias", alias,
231-
"-keystore", this.keyStore.getAbsolutePath()
232-
);
233-
234-
execute(command, password, newPassword, newPassword);
277+
try {
278+
KeyStore sourceStore = loadKeyStore();
279+
Certificate[] chain = sourceStore.getCertificateChain(alias);
280+
PrivateKey key = (PrivateKey) sourceStore.getKey(alias, oldPassword);
281+
sourceStore.setKeyEntry(alias, key, newPassword, chain);
282+
try (FileOutputStream output = new FileOutputStream(keyStore)) {
283+
sourceStore.store(output, password);
284+
}
285+
} catch (GeneralSecurityException e) {
286+
throw new IOException("Could not change key password for alias: " + alias, e);
287+
}
235288
}
236289

237290

@@ -240,6 +293,20 @@ private void execute(final List<String> command, char[]... stdinLines) throws IO
240293
}
241294

242295

296+
/**
297+
* Creates an empty key store file with the specified password.
298+
* The type is detected from the file extension.
299+
*
300+
* @param file
301+
* @param password
302+
* @return KeyTool suitable to manage the newly created key store
303+
* @throws IOException
304+
*/
305+
public static KeyTool createEmptyKeyStore(File file, char[] password) throws IOException {
306+
return createEmptyKeyStore(file, guessKeyStoreType(file), password);
307+
}
308+
309+
243310
/**
244311
* Creates an empty key store file with the specified type and password.
245312
*
@@ -268,6 +335,41 @@ public static KeyTool createEmptyKeyStore(File file, String keyStoreType, char[]
268335
}
269336

270337

338+
private static String guessKeyStoreType(File keyStore) {
339+
String filename = keyStore.getName();
340+
int lastDot = filename.lastIndexOf('.');
341+
if (lastDot < 0) {
342+
throw new IllegalArgumentException(
343+
"Key store file name must have an extension to guess the key store type: " + keyStore);
344+
}
345+
String suffix = filename.substring(lastDot + 1).toUpperCase();
346+
switch (suffix) {
347+
case "JKS":
348+
return "JKS";
349+
case "P12":
350+
case "PFX":
351+
return "PKCS12";
352+
case "JCEKS":
353+
return "JCEKS";
354+
default:
355+
LOG.log(WARNING, "Unknown key store type for file {0}, using its suffix as a keystore type.", keyStore);
356+
return suffix;
357+
}
358+
}
359+
360+
361+
private static void changeKeyPassword(KeyStore keyStore, String alias, char[] oldPassword, char[] newPassword)
362+
throws IOException {
363+
try {
364+
Certificate[] chain = keyStore.getCertificateChain(alias);
365+
PrivateKey key = (PrivateKey) keyStore.getKey(alias, oldPassword);
366+
keyStore.setKeyEntry(alias, key, newPassword, chain);
367+
} catch (GeneralSecurityException e) {
368+
throw new IOException("Could not change key password for alias: " + alias, e);
369+
}
370+
}
371+
372+
271373
private static void execute(final File keyStore, final List<String> command, final char[]... stdinLines) throws IOException {
272374
LOG.log(INFO, () -> "Executing command: " + command.stream().collect(Collectors.joining(" ")));
273375
final ProcessBuilder builder = new ProcessBuilder(command).directory(keyStore.getParentFile());

0 commit comments

Comments
 (0)