Workaround for issue mentioned in #25529 - PKCS12/JKS vs private key passwords

Signed-off-by: David Matějček <[email protected]>

Author: dmatej

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

@@ -25,20 +25,15 @@
 import java.io.File;
 import java.io.IOException;
 import java.lang.System.Logger;
-import java.security.Key;
-import java.security.KeyStore;
-import java.security.UnrecoverableKeyException;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.List;
 import java.util.regex.Pattern;
 
 import org.glassfish.main.jdke.security.KeyTool;
 
 import static com.sun.enterprise.admin.servermgmt.domain.DomainConstants.KEYSTORE_FILE;
 import static com.sun.enterprise.admin.servermgmt.domain.DomainConstants.TRUSTSTORE_FILE;
-import static java.lang.System.Logger.Level.WARNING;
 
 /**
  * @author kebbs
@@ -138,53 +133,6 @@ protected void changeKeystorePassword(String oldPassword, String newPassword, Fi
         }
     }
 
-    /**
-     * Changes the key password for the default cert whose alias is s1as. The assumption here is that the keystore password
-     * is not the same as the key password. This is due to the fact that the keystore password should first be changed
-     * followed next by the key password. The end result is that the keystore and s1as key both have the same passwords.
-     * This function will tolerate deletion of the s1as alias, but it will not tolerate changing the s1as key from something
-     * other than the database password.
-     *
-     * @param config
-     * @param storePassword the keystore password
-     * @param oldKeyPassword the old password for the s1as alias
-     * @param newKeyPassword the new password for the s1as alias
-     * @throws DomainException
-     */
-    protected void changeKeyPasswords(RepositoryConfig config, String storePassword, String oldKeyPassword,
-        String newKeyPassword) throws DomainException {
-        if (storePassword.equals(oldKeyPassword) || oldKeyPassword.equals(newKeyPassword)) {
-            return;
-        }
-        final PEFileLayout layout = getFileLayout(config);
-        final File keystore = layout.getKeyStore();
-        try {
-            KeyStore keyStore = KeyStore.getInstance(keystore, storePassword.toCharArray());
-            List<String> aliases = Collections.list(keyStore.aliases());
-            List<String> keyAliases = new ArrayList<>();
-            for (String alias : aliases) {
-                Key key;
-                try {
-                    key = keyStore.getKey(alias, oldKeyPassword.toCharArray());
-                } catch (UnrecoverableKeyException e) {
-                    LOG.log(WARNING,
-                        "Key entry with alias {0} in a key store {1} could not be recovered with provided key password.",
-                        alias, keystore);
-                    continue;
-                }
-                if (key != null) {
-                    keyAliases.add(alias);
-                }
-            }
-            KeyTool keyTool = new KeyTool(keystore, storePassword.toCharArray());
-            for (String alias : keyAliases) {
-                keyTool.changeKeyPassword(alias, oldKeyPassword.toCharArray(), newKeyPassword.toCharArray());
-            }
-        } catch (Exception e) {
-            throw new DomainException(_strMgr.getString("s1asKeyPasswordNotChanged", keystore), e);
-        }
-    }
-
     /**
      * Changes the password of the keystore, truststore and the key password of the s1as alias. It is expected that the key
      * / 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,
     protected void changeSSLCertificateDatabasePassword(RepositoryConfig config, String oldPassword, String newPassword) throws DomainException {
         final PEFileLayout layout = getFileLayout(config);
         File keystore = layout.getKeyStore();
-        File truststore = layout.getTrustStore();
-
         if (keystore.exists()) {
-            // Change the password on the keystore
             changeKeystorePassword(oldPassword, newPassword, keystore);
-            changeKeyPasswords(config, newPassword, oldPassword, newPassword);
         }
-
+        File truststore = layout.getTrustStore();
         if (truststore.exists()) {
-            // Change the password on the truststore
             changeKeystorePassword(oldPassword, newPassword, truststore);
         }
     }

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

@@ -28,14 +28,18 @@
 import java.security.KeyStore;
 import java.security.KeyStoreException;
 import java.security.NoSuchAlgorithmException;
+import java.security.PrivateKey;
+import java.security.cert.Certificate;
 import java.security.cert.CertificateException;
+import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 
 import static java.lang.System.Logger.Level.DEBUG;
 import static java.lang.System.Logger.Level.ERROR;
 import static java.lang.System.Logger.Level.INFO;
+import static java.lang.System.Logger.Level.WARNING;
 
 /**
  * Java adapter to call the keytool command.
@@ -63,18 +67,34 @@ public class KeyTool {
     }
 
     private final File keyStore;
+    private final String keyStoreType;
     private char[] password;
 
     /**
      * Creates a new instance of KeyTool managing the keystore file.
      * The file may not exist yet.
+     * The type is detected automatically from the file extension.
      *
      * @param keyStore the file representing the keystore
      * @param password keystore and key password, must have at least 6 characters
      */
     public KeyTool(File keyStore, char[] password) {
+        this(keyStore, guessKeyStoreType(keyStore), password);
+    }
+
+
+    /**
+     * Creates a new instance of KeyTool managing the keystore file.
+     * The file may not exist yet.
+     *
+     * @param keyStore the file representing the keystore
+     * @param keyStoreType the type of the keystore, e.g. "PKCS12", "JKS"
+     * @param password keystore and key password, must have at least 6 characters
+     */
+    public KeyTool(File keyStore, String keyStoreType, char[] password) {
         this.keyStore = keyStore;
         this.password = password;
+        this.keyStoreType = keyStoreType;
     }
 
 
@@ -113,7 +133,7 @@ public void generateKeyPair(String alias, String dn, String keyAlgorithm, int ce
             "-keyalg", keyAlgorithm,
             "-validity", Integer.toString(certValidity),
             "-keystore", keyStore.getAbsolutePath(),
-            "-storetype", "JKS"
+            "-storetype", keyStoreType
         );
         if (keyStore.getParentFile().mkdirs()) {
             // The directory must exist, keytool will not create it
@@ -196,6 +216,7 @@ public void exportCertificate(String alias, final File outputFile) throws IOExce
 
     /**
      * Changes the key store password and remembers it.
+     * Changes also passwords of all keys in the key store which use the same password.
      *
      * @param newPassword the new key store password
      * @throws IOException
@@ -209,29 +230,61 @@ public void changeKeyStorePassword(char[] newPassword) throws IOException {
             "-keystore", this.keyStore.getAbsolutePath()
         );
         execute(command, password, newPassword, newPassword, newPassword);
+        final char[] oldPassword = password;
         this.password = newPassword;
+        if ("PKCS12".equals(this.keyStoreType)) {
+            // PKCS12 key store type changes passwords of keys together with the key store password
+            // JKS and JCEKS key store types require changing passwords of keys separately
+            return;
+        }
+        KeyStore ks = loadKeyStore();
+        final List<String> aliases;
+        try {
+            aliases = Collections.list(ks.aliases());
+        } catch (KeyStoreException e) {
+            throw new IOException("Could not list aliases in keystore: " + keyStore, e);
+        }
+        for (String alias : aliases) {
+            try {
+                if (ks.isKeyEntry(alias)) {
+                    changeKeyPassword(ks, alias, oldPassword, newPassword);
+                }
+            } catch (IOException | KeyStoreException e) {
+                LOG.log(WARNING, "Could not change key password for alias: {0}, it may use different password.", alias);
+            }
+        }
+        try (FileOutputStream output = new FileOutputStream(keyStore)) {
+            ks.store(output, password);
+        } catch (GeneralSecurityException e) {
+            throw new IOException(
+                "Keystore password successfuly changed, however failed changing key passwords: " + keyStore, e);
+        }
     }
 
 
     /**
      * Changes the key password
+     * <p>
+     * WARNING: This is not required for the PKCS12 key store type, as it changes passwords of keys
+     * together with the key store password.
      *
      * @param alias the alias of the key whose password should be changed
      * @param oldPassword the current key entry password
      * @param newPassword the new key entry password
      * @throws IOException
      */
     public void changeKeyPassword(String alias, char[] oldPassword, char[] newPassword) throws IOException {
-        List<String> command = List.of(
-            KEYTOOL,
-            "-J-Duser.language=en",
-            "-noprompt",
-            "-keypasswd",
-            "-alias", alias,
-            "-keystore", this.keyStore.getAbsolutePath()
-        );
-
-        execute(command, password, newPassword, newPassword);
+        try {
+            KeyStore sourceStore = loadKeyStore();
+            Certificate[] chain = sourceStore.getCertificateChain(alias);
+            PrivateKey key = (PrivateKey) sourceStore.getKey(alias, oldPassword);
+            sourceStore.setKeyEntry(alias, key, newPassword, chain);
+            try (FileOutputStream output = new FileOutputStream(keyStore)) {
+                sourceStore.store(output, password);
+            }
+        } catch (GeneralSecurityException e) {
+            throw new IOException("Could not change key password for alias: " + alias, e);
+        }
     }
 
 
@@ -240,6 +293,20 @@ private void execute(final List<String> command, char[]... stdinLines) throws IO
     }
 
 
+    /**
+     * Creates an empty key store file with the specified password.
+     * The type is detected from the file extension.
+     *
+     * @param file
+     * @param password
+     * @return KeyTool suitable to manage the newly created key store
+     * @throws IOException
+     */
+    public static KeyTool createEmptyKeyStore(File file, char[] password) throws IOException {
+        return createEmptyKeyStore(file, guessKeyStoreType(file), password);
+    }
+
+
     /**
      * Creates an empty key store file with the specified type and password.
      *
@@ -268,6 +335,41 @@ public static KeyTool createEmptyKeyStore(File file, String keyStoreType, char[]
     }
 
 
+    private static String guessKeyStoreType(File keyStore) {
+        String filename = keyStore.getName();
+        int lastDot = filename.lastIndexOf('.');
+        if (lastDot < 0) {
+            throw new IllegalArgumentException(
+                "Key store file name must have an extension to guess the key store type: " + keyStore);
+        }
+        String suffix = filename.substring(lastDot + 1).toUpperCase();
+        switch (suffix) {
+            case "JKS":
+                return "JKS";
+            case "P12":
+            case "PFX":
+                return "PKCS12";
+            case "JCEKS":
+                return "JCEKS";
+            default:
+                LOG.log(WARNING, "Unknown key store type for file {0}, using its suffix as a keystore type.", keyStore);
+                return suffix;
+        }
+    }
+
+
+    private static void changeKeyPassword(KeyStore keyStore, String alias, char[] oldPassword, char[] newPassword)
+        throws IOException {
+        try {
+            Certificate[] chain = keyStore.getCertificateChain(alias);
+            PrivateKey key = (PrivateKey) keyStore.getKey(alias, oldPassword);
+            keyStore.setKeyEntry(alias, key, newPassword, chain);
+        } catch (GeneralSecurityException e) {
+            throw new IOException("Could not change key password for alias: " + alias, e);
+        }
+    }
+
+
     private static void execute(final File keyStore, final List<String> command, final char[]... stdinLines) throws IOException {
         LOG.log(INFO, () -> "Executing command: " + command.stream().collect(Collectors.joining(" ")));
         final ProcessBuilder builder = new ProcessBuilder(command).directory(keyStore.getParentFile());