Skip to content

Commit 664646a

Browse files
adutraadutra
authored
fix(crypto): Fix BC and JCA private key reading (#229)
This change fixes 2 issues: - The JCA PEM reader was incorrectly hard-coding the algorithm to RSA, thus making it impossible to read an ECDSA file in PKCS#8 format. - The BC PEM reader was not setting the key ID to "EC" when reading PKCS#8 format, causing Nimbus JOSE JWT to fail for any ECDSA key in PKCS#8 format.
1 parent baaea6f commit 664646a

9 files changed

Lines changed: 107 additions & 37 deletions

File tree

‎docs/client-authentication.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,12 +112,12 @@ in the file is used.
112112

113113
The following format is always supported:
114114

115-
1. RSA keys in PKCS#8 format (`BEGIN PRIVATE KEY`)
115+
1. RSA or EC (Elliptic Curve) keys in PKCS#8 format (`BEGIN PRIVATE KEY`)
116116

117117
If the BouncyCastle library is available at runtime, the following formats are also supported:
118118

119119
2. RSA keys in PKCS#1 format (`BEGIN RSA PRIVATE KEY`)
120-
3. EC (Elliptic Curve) keys (`BEGIN EC PRIVATE KEY`)
120+
3. EC (Elliptic Curve) keys in EC SEC 1 format (`BEGIN EC PRIVATE KEY`)
121121

122122
Only unencrypted private keys are supported.
123123

‎docs/configuration.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -458,9 +458,9 @@ The file must be in PEM format; it may contain a private key, or a private key a
458458

459459
Supported key formats are:
460460

461-
- RSA PKCS#8 (`BEGIN PRIVATE KEY`): always supported
462-
- RSA PKCS#1 (`BEGIN RSA PRIVATE KEY`): requires the BouncyCastle library
463-
- ECDSA (`BEGIN EC PRIVATE KEY`): requires the BouncyCastle library
461+
- RSA & ECDSA in PKCS#8 format (`BEGIN PRIVATE KEY`): always supported
462+
- RSA in PKCS#1 format (`BEGIN RSA PRIVATE KEY`): requires the BouncyCastle library
463+
- ECDSA in EC SEC 1 format (`BEGIN EC PRIVATE KEY`): requires the BouncyCastle library
464464

465465
Only unencrypted keys are supported currently.
466466

‎oauth2/core/src/intTest/java/com/dremio/iceberg/authmgr/oauth2/agent/OAuth2AgentKeycloakIT.java‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,11 +170,14 @@ void clientSecretJwt(
170170
void privateKeyJwt(
171171
@EnumLike(excludes = "urn:ietf:params:oauth:grant-type:token-exchange")
172172
GrantType initialGrantType,
173-
@Values(strings = {"rsa_pkcs8", "rsa_pkcs1", "ecdsa_sec1"}) String keyType,
173+
@Values(strings = {"rsa_pkcs8", "rsa_pkcs1", "ecdsa_pkcs8", "ecdsa_sec1"}) String keyType,
174174
Builder envBuilder)
175175
throws Exception {
176176
TestCertificates certs = TestCertificates.instance();
177-
assumeThat(certs.isBouncyCastleAvailable() || keyType.equals("rsa_pkcs8"))
177+
assumeThat(
178+
certs.isBouncyCastleAvailable()
179+
|| keyType.equals("rsa_pkcs8")
180+
|| keyType.equals("ecdsa_pkcs8"))
178181
.as("BouncyCastle is required for RSA PKCS#1 and ECDSA SEC 1 keys")
179182
.isTrue();
180183
Path privateKeyPath;
@@ -191,6 +194,11 @@ void privateKeyJwt(
191194
algorithm = JWSAlgorithm.RS256;
192195
clientId = CLIENT_ID4;
193196
break;
197+
case "ecdsa_pkcs8":
198+
privateKeyPath = certs.getEcdsaPrivateKeyPkcs8Pem();
199+
algorithm = JWSAlgorithm.ES256;
200+
clientId = CLIENT_ID5;
201+
break;
194202
case "ecdsa_sec1":
195203
privateKeyPath = certs.getEcdsaPrivateKeySec1Pem();
196204
algorithm = JWSAlgorithm.ES256;

‎oauth2/core/src/main/java/com/dremio/iceberg/authmgr/oauth2/config/JwtClientAuthConfig.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,9 +116,10 @@ public interface JwtClientAuthConfig {
116116
* <p>Supported key formats are:
117117
*
118118
* <ul>
119-
* <li>RSA PKCS#8 ({@code BEGIN PRIVATE KEY}): always supported
120-
* <li>RSA PKCS#1 ({@code BEGIN RSA PRIVATE KEY}): requires the BouncyCastle library
121-
* <li>ECDSA ({@code BEGIN EC PRIVATE KEY}): requires the BouncyCastle library
119+
* <li>RSA & ECDSA in PKCS#8 format ({@code BEGIN PRIVATE KEY}): always supported
120+
* <li>RSA in PKCS#1 format ({@code BEGIN RSA PRIVATE KEY}): requires the BouncyCastle library
121+
* <li>ECDSA in EC SEC 1 format ({@code BEGIN EC PRIVATE KEY}): requires the BouncyCastle
122+
* library
122123
* </ul>
123124
*
124125
* Only unencrypted keys are supported currently.

‎oauth2/core/src/main/java/com/dremio/iceberg/authmgr/oauth2/crypto/BouncyCastlePemReader.java‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,9 @@ final class BouncyCastlePemReader implements PemReader {
4242
* Reads a private key from a PEM file. Supported key formats:
4343
*
4444
* <ul>
45+
* <li>RSA or EC PKCS#8 (BEGIN PRIVATE KEY)
4546
* <li>RSA PKCS#1 (BEGIN RSA PRIVATE KEY)
46-
* <li>RSA PKCS#8 (BEGIN PRIVATE KEY)
47-
* <li>EC (BEGIN EC PRIVATE KEY)
47+
* <li>EC SEC 1 (BEGIN EC PRIVATE KEY)
4848
* </ul>
4949
*
5050
* <p>Only unencrypted keys are supported.
@@ -72,16 +72,16 @@ public PrivateKey readPrivateKey(Path file) {
7272
}
7373

7474
private static PrivateKey extractPrivateKey(Object pemObject) throws PEMException {
75+
// Nimbus JOSE JWT uses "EC" as the algorithm name for EC keys,
76+
// but BouncyCastle uses "ECDSA"; normalize to "EC" for compatibility.
77+
JcaPEMKeyConverter converter =
78+
new JcaPEMKeyConverter().setAlgorithmMapping(X9ObjectIdentifiers.id_ecPublicKey, "EC");
7579
if (pemObject instanceof PEMKeyPair) {
76-
// Handle PKCS#1 format (BEGIN RSA PRIVATE KEY) or EC (BEGIN EC PRIVATE KEY)
77-
return new JcaPEMKeyConverter()
78-
// Nimbus JOSE JWT uses "EC" as the algorithm name for EC keys,
79-
// but BouncyCastle uses "ECDSA"
80-
.setAlgorithmMapping(X9ObjectIdentifiers.id_ecPublicKey, "EC")
81-
.getPrivateKey(((PEMKeyPair) pemObject).getPrivateKeyInfo());
80+
// Handle PKCS#1 format (BEGIN RSA PRIVATE KEY) or SEC 1 (BEGIN EC PRIVATE KEY)
81+
return converter.getPrivateKey(((PEMKeyPair) pemObject).getPrivateKeyInfo());
8282
} else if (pemObject instanceof PrivateKeyInfo) {
8383
// Handle PKCS#8 format (BEGIN PRIVATE KEY)
84-
return new JcaPEMKeyConverter().getPrivateKey((PrivateKeyInfo) pemObject);
84+
return converter.getPrivateKey((PrivateKeyInfo) pemObject);
8585
}
8686
return null;
8787
}

‎oauth2/core/src/main/java/com/dremio/iceberg/authmgr/oauth2/crypto/JcaPemReader.java‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,32 @@
2121
import java.nio.file.Path;
2222
import java.security.KeyFactory;
2323
import java.security.PrivateKey;
24+
import java.security.spec.InvalidKeySpecException;
2425
import java.security.spec.PKCS8EncodedKeySpec;
2526
import org.apache.commons.codec.binary.Base64;
2627

2728
final class JcaPemReader implements PemReader {
2829

30+
private static final String[] ALGORITHMS = {"RSA", "EC"};
31+
2932
@Override
3033
public PrivateKey readPrivateKey(Path file) {
3134
try {
3235
byte[] encoded = Base64.decodeBase64(readPemEncodedPrivateKey(file));
33-
KeyFactory keyFactory = KeyFactory.getInstance("RSA");
3436
PKCS8EncodedKeySpec keySpec = new PKCS8EncodedKeySpec(encoded);
35-
return keyFactory.generatePrivate(keySpec);
37+
InvalidKeySpecException toThrow = null;
38+
for (String algorithm : ALGORITHMS) {
39+
try {
40+
return KeyFactory.getInstance(algorithm).generatePrivate(keySpec);
41+
} catch (InvalidKeySpecException e) {
42+
if (toThrow == null) {
43+
toThrow = e;
44+
} else {
45+
toThrow.addSuppressed(e);
46+
}
47+
}
48+
}
49+
throw toThrow;
3650
} catch (Exception e) {
3751
throw new IllegalArgumentException("Failed to read PEM file: " + file, e);
3852
}

‎oauth2/core/src/main/java/com/dremio/iceberg/authmgr/oauth2/crypto/PemReader.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,10 @@ static PemReader getInstance() {
2828
/**
2929
* Reads a private key from a PEM file.
3030
*
31-
* <p>RSA keys in the PKCS#8 format are always supported.
31+
* <p>RSA and ECDSA keys in the PKCS#8 format are always supported.
3232
*
33-
* <p>Support for additional key formats, such as RSA in the PKCS#1 format or ECDSA keys, is
34-
* supported if the BouncyCastle library is available.
33+
* <p>Support for additional key formats, such as RSA in the PKCS#1 format or ECDSA keys in SEC 1
34+
* format, is supported if the BouncyCastle library is available.
3535
*
3636
* <p>Only unencrypted keys are supported.
3737
*

‎oauth2/core/src/test/java/com/dremio/iceberg/authmgr/oauth2/crypto/BouncyCastlePemReaderTest.java‎

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,11 @@
2626
import java.security.PrivateKey;
2727
import java.security.interfaces.ECPrivateKey;
2828
import java.security.interfaces.RSAPrivateKey;
29+
import java.util.stream.Stream;
2930
import org.junit.jupiter.api.Test;
3031
import org.junit.jupiter.api.io.TempDir;
32+
import org.junit.jupiter.params.ParameterizedTest;
33+
import org.junit.jupiter.params.provider.MethodSource;
3134

3235
class BouncyCastlePemReaderTest {
3336

@@ -64,7 +67,21 @@ void testReadPkcs1RsaPrivateKey() {
6467
}
6568

6669
@Test
67-
void testReadEcPrivateKey() {
70+
void testReadEcPkcs8PrivateKey() {
71+
// Given
72+
Path privateKeyFile = TestCertificates.instance().getEcdsaPrivateKeyPkcs8Pem();
73+
74+
// When
75+
PrivateKey privateKey = new BouncyCastlePemReader().readPrivateKey(privateKeyFile);
76+
77+
// Then
78+
assertThat(privateKey).isNotNull();
79+
assertThat(privateKey.getAlgorithm()).isEqualTo("EC");
80+
assertThat(privateKey).isInstanceOf(ECPrivateKey.class);
81+
}
82+
83+
@Test
84+
void testReadEcSec1PrivateKey() {
6885
// Given
6986
Path privateKeyFile = TestCertificates.instance().getEcdsaPrivateKeySec1Pem();
7087

@@ -73,7 +90,7 @@ void testReadEcPrivateKey() {
7390

7491
// Then
7592
assertThat(privateKey).isNotNull();
76-
assertThat(privateKey.getAlgorithm()).isIn("EC", "ECDSA"); // BouncyCastle may return "ECDSA"
93+
assertThat(privateKey.getAlgorithm()).isEqualTo("EC");
7794
assertThat(privateKey).isInstanceOf(ECPrivateKey.class);
7895
}
7996

@@ -104,19 +121,25 @@ void testReadEmptyFile() throws IOException {
104121
.hasMessageContaining("No private key found in file");
105122
}
106123

107-
@Test
108-
void testReadFileWithoutPrivateKey() {
109-
// Given
110-
Path privateKeyFile = TestCertificates.instance().getRsaCertificatePem();
111-
124+
@ParameterizedTest
125+
@MethodSource("invalidPemFiles")
126+
void testReadFileWithoutPrivateKey(Path certificateFile) {
112127
// When - Then
113-
assertThatThrownBy(() -> new BouncyCastlePemReader().readPrivateKey(privateKeyFile))
128+
assertThatThrownBy(() -> new BouncyCastlePemReader().readPrivateKey(certificateFile))
114129
.isInstanceOf(IllegalArgumentException.class)
115130
.hasMessageContaining("Failed to read PEM file")
116131
.rootCause()
117132
.hasMessageContaining("No private key found in file");
118133
}
119134

135+
static Stream<Path> invalidPemFiles() {
136+
return Stream.of(
137+
TestCertificates.instance().getRsaPublicKeyPem(),
138+
TestCertificates.instance().getRsaCertificatePem(),
139+
TestCertificates.instance().getEcdsaPublicKeyPem(),
140+
TestCertificates.instance().getEcdsaCertificatePem());
141+
}
142+
120143
@Test
121144
void testReadInvalidPemContent() throws IOException {
122145
// Given

‎oauth2/core/src/test/java/com/dremio/iceberg/authmgr/oauth2/crypto/JcaPemReaderTest.java‎

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,13 @@
2424
import java.nio.file.NoSuchFileException;
2525
import java.nio.file.Path;
2626
import java.security.PrivateKey;
27+
import java.security.interfaces.ECPrivateKey;
2728
import java.security.interfaces.RSAPrivateKey;
29+
import java.util.stream.Stream;
2830
import org.junit.jupiter.api.Test;
2931
import org.junit.jupiter.api.io.TempDir;
32+
import org.junit.jupiter.params.ParameterizedTest;
33+
import org.junit.jupiter.params.provider.MethodSource;
3034

3135
class JcaPemReaderTest {
3236

@@ -47,6 +51,20 @@ void testReadPkcs8RsaPrivateKey() {
4751
assertThat(((RSAPrivateKey) privateKey).getModulus().bitLength()).isEqualTo(2048);
4852
}
4953

54+
@Test
55+
void testReadEcPkcs8PrivateKey() {
56+
// Given
57+
Path privateKeyFile = TestCertificates.instance().getEcdsaPrivateKeyPkcs8Pem();
58+
59+
// When
60+
PrivateKey privateKey = new JcaPemReader().readPrivateKey(privateKeyFile);
61+
62+
// Then
63+
assertThat(privateKey).isNotNull();
64+
assertThat(privateKey.getAlgorithm()).isEqualTo("EC");
65+
assertThat(privateKey).isInstanceOf(ECPrivateKey.class);
66+
}
67+
5068
@Test
5169
void testReadNonExistentFile() {
5270
// Given
@@ -74,11 +92,9 @@ void testReadEmptyFile() throws IOException {
7492
.hasMessageContaining("No private key found in file");
7593
}
7694

77-
@Test
78-
void testReadFileWithoutPrivateKey() {
79-
// Given
80-
Path certificateFile = TestCertificates.instance().getRsaCertificatePem();
81-
95+
@ParameterizedTest
96+
@MethodSource("invalidPemFiles")
97+
void testReadFileWithoutPrivateKey(Path certificateFile) {
8298
// When - Then
8399
assertThatThrownBy(() -> new JcaPemReader().readPrivateKey(certificateFile))
84100
.isInstanceOf(IllegalArgumentException.class)
@@ -87,6 +103,14 @@ void testReadFileWithoutPrivateKey() {
87103
.hasMessageContaining("No private key found in file");
88104
}
89105

106+
static Stream<Path> invalidPemFiles() {
107+
return Stream.of(
108+
TestCertificates.instance().getRsaPublicKeyPem(),
109+
TestCertificates.instance().getRsaCertificatePem(),
110+
TestCertificates.instance().getEcdsaPublicKeyPem(),
111+
TestCertificates.instance().getEcdsaCertificatePem());
112+
}
113+
90114
@Test
91115
void testReadInvalidPemContent() throws IOException {
92116
// Given

0 commit comments

Comments
 (0)