Fix parsing of PBES2 encrypted PKCS#8 keys (#78904)

This commit adds support for decrypting PKCS#8 encoded private keys
that have been encrypted using a PBES2 based scheme (AES only).

Unfortunately `java.crypto.EncryptedPrivateKeyInfo` doesn't make this
easy as the underlying encryption algorithm is hidden within the
`AlgorithmParameters`, and can only be extracted by calling
`toString()` on the parameters object.

See: https://datatracker.ietf.org/doc/html/rfc8018#appendix-A.4
See: AlgorithmParameters#toString()
See: com.sun.crypto.provider.PBES2Parameters#toString()

Resolves: #78901, #32021

Author: tvernum

libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/DerParser.java

@@ -36,21 +36,22 @@ public final class DerParser {
     private static final int CONSTRUCTED = 0x20;
 
     // Tag and data types
-    private static final int INTEGER = 0x02;
-    private static final int OCTET_STRING = 0x04;
-    private static final int OBJECT_OID = 0x06;
-    private static final int NUMERIC_STRING = 0x12;
-    private static final int PRINTABLE_STRING = 0x13;
-    private static final int VIDEOTEX_STRING = 0x15;
-    private static final int IA5_STRING = 0x16;
-    private static final int GRAPHIC_STRING = 0x19;
-    private static final int ISO646_STRING = 0x1A;
-    private static final int GENERAL_STRING = 0x1B;
-
-    private static final int UTF8_STRING = 0x0C;
-    private static final int UNIVERSAL_STRING = 0x1C;
-    private static final int BMP_STRING = 0x1E;
-
+    static final class Type {
+        static final int INTEGER = 0x02;
+        static final int OCTET_STRING = 0x04;
+        static final int OBJECT_OID = 0x06;
+        static final int SEQUENCE = 0x10;
+        static final int NUMERIC_STRING = 0x12;
+        static final int PRINTABLE_STRING = 0x13;
+        static final int VIDEOTEX_STRING = 0x15;
+        static final int IA5_STRING = 0x16;
+        static final int GRAPHIC_STRING = 0x19;
+        static final int ISO646_STRING = 0x1A;
+        static final int GENERAL_STRING = 0x1B;
+        static final int UTF8_STRING = 0x0C;
+        static final int UNIVERSAL_STRING = 0x1C;
+        static final int BMP_STRING = 0x1E;
+    }
 
     private InputStream derInputStream;
     private int maxAsnObjectLength;
@@ -60,6 +61,22 @@ public DerParser(byte[] bytes) {
         this.maxAsnObjectLength = bytes.length;
     }
 
+    /**
+     * Read an object and verify its type
+     * @param requiredType The expected type code
+     * @throws IOException if data can not be parsed
+     * @throws IllegalStateException if the parsed object is of the wrong type
+     */
+    public Asn1Object readAsn1Object(int requiredType) throws IOException {
+        final Asn1Object obj = readAsn1Object();
+        if (obj.type != requiredType) {
+            throw new IllegalStateException(
+                "Expected ASN.1 object of type 0x" + Integer.toHexString(requiredType) + " but was 0x" + Integer.toHexString(obj.type)
+            );
+        }
+        return obj;
+    }
+
     public Asn1Object readAsn1Object() throws IOException {
         int tag = derInputStream.read();
         if (tag == -1) {
@@ -207,7 +224,7 @@ public DerParser getParser() throws IOException {
          * @return BigInteger
          */
         public BigInteger getInteger() throws IOException {
-            if (type != DerParser.INTEGER)
+            if (type != Type.INTEGER)
                 throw new IOException("Invalid DER: object is not integer"); //$NON-NLS-1$
 
             return new BigInteger(value);
@@ -218,28 +235,28 @@ public String getString() throws IOException {
             String encoding;
 
             switch (type) {
-                case DerParser.OCTET_STRING:
+                case Type.OCTET_STRING:
                     // octet string is basically a byte array
                     return toHexString(value);
-                case DerParser.NUMERIC_STRING:
-                case DerParser.PRINTABLE_STRING:
-                case DerParser.VIDEOTEX_STRING:
-                case DerParser.IA5_STRING:
-                case DerParser.GRAPHIC_STRING:
-                case DerParser.ISO646_STRING:
-                case DerParser.GENERAL_STRING:
+                case Type.NUMERIC_STRING:
+                case Type.PRINTABLE_STRING:
+                case Type.VIDEOTEX_STRING:
+                case Type.IA5_STRING:
+                case Type.GRAPHIC_STRING:
+                case Type.ISO646_STRING:
+                case Type.GENERAL_STRING:
                     encoding = "ISO-8859-1"; //$NON-NLS-1$
                     break;
 
-                case DerParser.BMP_STRING:
+                case Type.BMP_STRING:
                     encoding = "UTF-16BE"; //$NON-NLS-1$
                     break;
 
-                case DerParser.UTF8_STRING:
+                case Type.UTF8_STRING:
                     encoding = "UTF-8"; //$NON-NLS-1$
                     break;
 
-                case DerParser.UNIVERSAL_STRING:
+                case Type.UNIVERSAL_STRING:
                     throw new IOException("Invalid DER: can't handle UCS-4 string"); //$NON-NLS-1$
 
                 default:
@@ -251,7 +268,7 @@ public String getString() throws IOException {
 
         public String getOid() throws IOException {
 
-            if (type != DerParser.OBJECT_OID) {
+            if (type != Type.OBJECT_OID) {
                 throw new IOException("Ivalid DER: object is not object OID");
             }
             StringBuilder sb = new StringBuilder(64);

libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/PemUtils.java

@@ -25,6 +25,7 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.security.AccessControlException;
+import java.security.AlgorithmParameters;
 import java.security.GeneralSecurityException;
 import java.security.KeyFactory;
 import java.security.KeyPairGenerator;
@@ -68,6 +69,9 @@ public final class PemUtils {
     private static final String OPENSSL_EC_PARAMS_FOOTER = "-----END EC PARAMETERS-----";
     private static final String HEADER = "-----BEGIN";
 
+    private static final String PBES2_OID = "1.2.840.113549.1.5.13";
+    private static final String AES_OID = "2.16.840.1.101.3.4.1";
+
     private PemUtils() {
         throw new IllegalStateException("Utility class should not be instantiated");
     }
@@ -365,17 +369,70 @@ private static PrivateKey parsePKCS8Encrypted(BufferedReader bReader, char[] key
         }
         byte[] keyBytes = Base64.getDecoder().decode(sb.toString());
 
-        EncryptedPrivateKeyInfo encryptedPrivateKeyInfo = new EncryptedPrivateKeyInfo(keyBytes);
-        SecretKeyFactory secretKeyFactory = SecretKeyFactory.getInstance(encryptedPrivateKeyInfo.getAlgName());
+        final EncryptedPrivateKeyInfo encryptedPrivateKeyInfo = getEncryptedPrivateKeyInfo(keyBytes);
+        String algorithm = encryptedPrivateKeyInfo.getAlgName();
+        if (algorithm.equals("PBES2") || algorithm.equals("1.2.840.113549.1.5.13")) {
+            algorithm = getPBES2Algorithm(encryptedPrivateKeyInfo);
+        }
+        SecretKeyFactory secretKeyFactory = SecretKeyFactory.getInstance(algorithm);
         SecretKey secretKey = secretKeyFactory.generateSecret(new PBEKeySpec(keyPassword));
-        Cipher cipher = Cipher.getInstance(encryptedPrivateKeyInfo.getAlgName());
+        Cipher cipher = Cipher.getInstance(algorithm);
         cipher.init(Cipher.DECRYPT_MODE, secretKey, encryptedPrivateKeyInfo.getAlgParameters());
         PKCS8EncodedKeySpec keySpec = encryptedPrivateKeyInfo.getKeySpec(cipher);
         String keyAlgo = getKeyAlgorithmIdentifier(keySpec.getEncoded());
         KeyFactory keyFactory = KeyFactory.getInstance(keyAlgo);
         return keyFactory.generatePrivate(keySpec);
     }
 
+    private static EncryptedPrivateKeyInfo getEncryptedPrivateKeyInfo(byte[] keyBytes) throws IOException, GeneralSecurityException {
+        try {
+            return new EncryptedPrivateKeyInfo(keyBytes);
+        } catch (IOException e) {
+            // The Sun JCE provider can't handle non-AES PBES2 data (but it can handle PBES1 DES data - go figure)
+            // It's not worth our effort to try and decrypt it ourselves, but we can detect it and give a good error message
+            DerParser parser = new DerParser(keyBytes);
+            final DerParser.Asn1Object rootSeq = parser.readAsn1Object(DerParser.Type.SEQUENCE);
+            parser = rootSeq.getParser();
+            final DerParser.Asn1Object algSeq = parser.readAsn1Object(DerParser.Type.SEQUENCE);
+            parser = algSeq.getParser();
+            final String algId = parser.readAsn1Object(DerParser.Type.OBJECT_OID).getOid();
+            if (PBES2_OID.equals(algId)) {
+                final DerParser.Asn1Object algData = parser.readAsn1Object(DerParser.Type.SEQUENCE);
+                parser = algData.getParser();
+                final DerParser.Asn1Object ignoreKdf = parser.readAsn1Object(DerParser.Type.SEQUENCE);
+                final DerParser.Asn1Object cryptSeq = parser.readAsn1Object(DerParser.Type.SEQUENCE);
+                parser = cryptSeq.getParser();
+                final String encryptionId = parser.readAsn1Object(DerParser.Type.OBJECT_OID).getOid();
+                if (encryptionId.startsWith(AES_OID) == false) {
+                    final String name = getAlgorithmNameFromOid(encryptionId);
+                    throw new GeneralSecurityException(
+                        "PKCS#8 Private Key is encrypted with unsupported PBES2 algorithm ["
+                            + encryptionId
+                            + "]"
+                            + (name == null ? "" : " (" + name + ")"),
+                        e
+                    );
+                }
+            }
+            throw e;
+        }
+    }
+
+    /**
+     * This is horrible, but it's the only option other than to parse the encoded ASN.1 value ourselves
+     * @see AlgorithmParameters#toString() and com.sun.crypto.provider.PBES2Parameters#toString()
+     */
+    private static String getPBES2Algorithm(EncryptedPrivateKeyInfo encryptedPrivateKeyInfo) {
+        final AlgorithmParameters algParameters = encryptedPrivateKeyInfo.getAlgParameters();
+        if (algParameters != null) {
+            return algParameters.toString();
+        } else {
+            // AlgorithmParameters can be null when running on BCFIPS.
+            // However, since BCFIPS doesn't support any PBE specs, nothing we do here would work, so we just do enough to avoid an NPE
+            return encryptedPrivateKeyInfo.getAlgName();
+        }
+    }
+
     /**
      * Decrypts the password protected contents using the algorithm and IV that is specified in the PEM Headers of the file
      *
@@ -604,7 +661,7 @@ private static String getKeyAlgorithmIdentifier(byte[] keyBytes) throws IOExcept
                 return "EC";
         }
         throw new GeneralSecurityException("Error parsing key algorithm identifier. Algorithm with OID [" + oidString +
-            "] is not żsupported");
+            "] is not supported");
     }
 
     public static List<Certificate> readCertificates(Collection<Path> certPaths) throws CertificateException, IOException {
@@ -622,6 +679,56 @@ public static List<Certificate> readCertificates(Collection<Path> certPaths) thr
         return certificates;
     }
 
+    private static String getAlgorithmNameFromOid(String oidString) throws GeneralSecurityException {
+        switch (oidString) {
+            case "1.2.840.10040.4.1":
+                return "DSA";
+            case "1.2.840.113549.1.1.1":
+                return "RSA";
+            case "1.2.840.10045.2.1":
+                return "EC";
+            case "1.3.14.3.2.7":
+                return "DES-CBC";
+            case "2.16.840.1.101.3.4.1.1":
+                return "AES-128_ECB";
+            case "2.16.840.1.101.3.4.1.2":
+                return "AES-128_CBC";
+            case "2.16.840.1.101.3.4.1.3":
+                return "AES-128_OFB";
+            case "2.16.840.1.101.3.4.1.4":
+                return "AES-128_CFB";
+            case "2.16.840.1.101.3.4.1.6":
+                return "AES-128_GCM";
+            case "2.16.840.1.101.3.4.1.21":
+                return "AES-192_ECB";
+            case "2.16.840.1.101.3.4.1.22":
+                return "AES-192_CBC";
+            case "2.16.840.1.101.3.4.1.23":
+                return "AES-192_OFB";
+            case "2.16.840.1.101.3.4.1.24":
+                return "AES-192_CFB";
+            case "2.16.840.1.101.3.4.1.26":
+                return "AES-192_GCM";
+            case "2.16.840.1.101.3.4.1.41":
+                return "AES-256_ECB";
+            case "2.16.840.1.101.3.4.1.42":
+                return "AES-256_CBC";
+            case "2.16.840.1.101.3.4.1.43":
+                return "AES-256_OFB";
+            case "2.16.840.1.101.3.4.1.44":
+                return "AES-256_CFB";
+            case "2.16.840.1.101.3.4.1.46":
+                return "AES-256_GCM";
+            case "2.16.840.1.101.3.4.1.5":
+                return "AESWrap-128";
+            case "2.16.840.1.101.3.4.1.25":
+                return "AESWrap-192";
+            case "2.16.840.1.101.3.4.1.45":
+                return "AESWrap-256";
+        }
+        return null;
+    }
+
     private static String getEcCurveNameFromOid(String oidString) throws GeneralSecurityException {
         switch (oidString) {
             // see https://tools.ietf.org/html/rfc5480#section-2.1.1.1

libs/ssl-config/src/test/java/org/elasticsearch/common/ssl/PemUtilsTests.java

@@ -15,6 +15,7 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.security.AlgorithmParameters;
+import java.security.GeneralSecurityException;
 import java.security.Key;
 import java.security.KeyStore;
 import java.security.PrivateKey;
@@ -26,6 +27,7 @@
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.notNullValue;
+import static org.hamcrest.Matchers.startsWith;
 import static org.hamcrest.core.StringContains.containsString;
 
 public class PemUtilsTests extends ESTestCase {
@@ -79,17 +81,49 @@ public void testReadPKCS8EcKey() throws Exception {
         assertThat(privateKey, equalTo(key));
     }
 
-    public void testReadEncryptedPKCS8Key() throws Exception {
+    public void testReadEncryptedPKCS8PBES1Key() throws Exception {
         assumeFalse("Can't run in a FIPS JVM, PBE KeySpec is not available", inFipsJvm());
         Key key = getKeyFromKeystore("RSA");
         assertThat(key, notNullValue());
         assertThat(key, instanceOf(PrivateKey.class));
-        PrivateKey privateKey = PemUtils.parsePrivateKey(getDataPath
-            ("/certs/pem-utils/key_pkcs8_encrypted.pem"), TESTNODE_PASSWORD);
+        PrivateKey privateKey = PemUtils.parsePrivateKey(
+            getDataPath("/certs/pem-utils/key_pkcs8_encrypted_pbes1_des.pem"),
+            TESTNODE_PASSWORD
+        );
         assertThat(privateKey, notNullValue());
         assertThat(privateKey, equalTo(key));
     }
 
+    public void testReadEncryptedPKCS8PBES2AESKey() throws Exception {
+        assumeFalse("Can't run in a FIPS JVM, PBE KeySpec is not available", inFipsJvm());
+        Key key = getKeyFromKeystore("RSA");
+        assertThat(key, notNullValue());
+        assertThat(key, instanceOf(PrivateKey.class));
+        PrivateKey privateKey = PemUtils.parsePrivateKey(
+            getDataPath("/certs/pem-utils/key_pkcs8_encrypted_pbes2_aes.pem"),
+            TESTNODE_PASSWORD
+        );
+        assertThat(privateKey, notNullValue());
+        assertThat(privateKey, equalTo(key));
+    }
+
+    public void testReadEncryptedPKCS8PBES2DESKey() throws Exception {
+        assumeFalse("Can't run in a FIPS JVM, PBE KeySpec is not available", inFipsJvm());
+
+        // Sun JSE cannot read keys encrypted with PBES2 DES (but does support AES with PBES2 and DES with PBES1)
+        // Rather than add our own support for this we just detect that our error message is clear and meaningful
+        final GeneralSecurityException exception = expectThrows(
+            GeneralSecurityException.class,
+            () -> PemUtils.parsePrivateKey(getDataPath("/certs/pem-utils/key_pkcs8_encrypted_pbes2_des.pem"), TESTNODE_PASSWORD)
+        );
+        assertThat(
+            exception.getMessage(),
+            equalTo("PKCS#8 Private Key is encrypted with unsupported PBES2 algorithm [1.3.14.3.2.7] (DES-CBC)")
+        );
+        assertThat(exception.getCause(), instanceOf(IOException.class));
+        assertThat(exception.getCause().getMessage(), startsWith("PBE parameter parsing error"));
+    }
+
     public void testReadDESEncryptedPKCS1Key() throws Exception {
         Key key = getKeyFromKeystore("RSA");
         assertThat(key, notNullValue());
@@ -134,8 +168,10 @@ public void testReadOpenSslDsaKeyWithParams() throws Exception {
         Key key = getKeyFromKeystore("DSA");
         assertThat(key, notNullValue());
         assertThat(key, instanceOf(PrivateKey.class));
-        PrivateKey privateKey = PemUtils.parsePrivateKey(getDataPath("/certs/pem-utils/dsa_key_openssl_plain_with_params.pem"),
-            EMPTY_PASSWORD);
+        PrivateKey privateKey = PemUtils.parsePrivateKey(
+            getDataPath("/certs/pem-utils/dsa_key_openssl_plain_with_params.pem"),
+            EMPTY_PASSWORD
+        );
 
         assertThat(privateKey, notNullValue());
         assertThat(privateKey, equalTo(key));
@@ -165,8 +201,10 @@ public void testReadOpenSslEcKeyWithParams() throws Exception {
         Key key = getKeyFromKeystore("EC");
         assertThat(key, notNullValue());
         assertThat(key, instanceOf(PrivateKey.class));
-        PrivateKey privateKey = PemUtils.parsePrivateKey(getDataPath("/certs/pem-utils/ec_key_openssl_plain_with_params.pem"),
-            EMPTY_PASSWORD);
+        PrivateKey privateKey = PemUtils.parsePrivateKey(
+            getDataPath("/certs/pem-utils/ec_key_openssl_plain_with_params.pem"),
+            EMPTY_PASSWORD
+        );
 
         assertThat(privateKey, notNullValue());
         assertThat(privateKey, equalTo(key));