Skip to content

[Backport] Fix parsing of PBES2 encrypted PKCS#8 keys #79352

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Oct 25, 2021
Merged

[Backport] Fix parsing of PBES2 encrypted PKCS#8 keys #79352

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3f94a96
Fix parsing of PBES2 encrypted PKCS#8 keys
tvernum Oct 18, 2021
d13dea3
Fix broken merge
tvernum Oct 19, 2021
cee4d34
Merge branch '7.x' into backport/7.x/78904-Fix-parsing-of-PBES2
tvernum Oct 19, 2021
ee6c3d3
Handle JDK8
tvernum Oct 19, 2021
f930709
Oracle JDK8 does support PBES2
tvernum Oct 19, 2021
8e5034b
Imports
tvernum Oct 19, 2021
ea67f9b
Assert / Assume, whatever
tvernum Oct 19, 2021
ebf8175
Merge branch '7.x' into backport/7.x/78904-Fix-parsing-of-PBES2
elasticmachine Oct 20, 2021
6ba28dc
Improve PBES2+JDK8 error message
tvernum Oct 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 45 additions & 28 deletions libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/DerParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,21 +36,22 @@ 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;
Expand All @@ -60,6 +61,22 @@ final class DerParser {
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
*/
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;
}

Asn1Object readAsn1Object() throws IOException {
int tag = derInputStream.read();
if (tag == -1) {
Expand Down Expand Up @@ -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);
Expand All @@ -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:
Expand All @@ -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);
Expand Down
124 changes: 120 additions & 4 deletions libs/ssl-config/src/main/java/org/elasticsearch/common/ssl/PemUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
package org.elasticsearch.common.ssl;

import org.elasticsearch.core.CharArrays;
import org.elasticsearch.jdk.JavaVersion;

import javax.crypto.Cipher;
import javax.crypto.EncryptedPrivateKeyInfo;
Expand All @@ -26,6 +27,7 @@
import java.nio.file.Files;
import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.security.AlgorithmParameters;
import java.security.GeneralSecurityException;
import java.security.KeyFactory;
import java.security.KeyPairGenerator;
Expand Down Expand Up @@ -69,6 +71,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");
}
Expand Down Expand Up @@ -336,17 +341,78 @@ 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
);
}
if (JavaVersion.current().compareTo(JavaVersion.parse("11.0.0")) < 0) {
// PBES2 appears to be supported on Oracle 8, but not OpenJDK8
// We don't both clarifying that here because it is supported on the bundled JDK, and that's what people should use
Copy link
Contributor

@jkakavas jkakavas Oct 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

both -> bother ( in case you need to make another commit for another reason)

throw new GeneralSecurityException(
"PKCS#8 Private Key is encrypted with PBES2 which is not supported on this JDK [" + JavaVersion.current() + "]",
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
*
Expand Down Expand Up @@ -575,7 +641,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 {
Expand All @@ -593,6 +659,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
Expand Down
Loading