Skip to content

Commit 9446dfb

Browse files
loosebazookaloosebazooka
committed
Rework keys to remove PemReader, user BC instead
Signed-off-by: Appu Goundan <[email protected]>
1 parent eaae62c commit 9446dfb

File tree

2 files changed

+45
-101
lines changed

2 files changed

+45
-101
lines changed

‎sigstore-java/src/main/java/dev/sigstore/encryption/Keys.java

Lines changed: 31 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -15,36 +15,39 @@
1515
*/
1616
package dev.sigstore.encryption;
1717

18-
import static org.bouncycastle.jce.ECPointUtil.*;
18+
import static org.bouncycastle.jce.ECPointUtil.decodePoint;
1919

20-
import com.google.common.annotations.VisibleForTesting;
2120
import java.io.ByteArrayInputStream;
2221
import java.io.IOException;
2322
import java.io.InputStreamReader;
2423
import java.nio.charset.StandardCharsets;
25-
import java.security.*;
26-
import java.security.spec.*;
27-
import java.util.Locale;
28-
import java.util.logging.Logger;
24+
import java.security.KeyFactory;
25+
import java.security.NoSuchAlgorithmException;
26+
import java.security.NoSuchProviderException;
27+
import java.security.PublicKey;
28+
import java.security.Security;
29+
import java.security.spec.ECPoint;
30+
import java.security.spec.ECPublicKeySpec;
31+
import java.security.spec.InvalidKeySpecException;
32+
import java.security.spec.RSAPublicKeySpec;
33+
import java.security.spec.X509EncodedKeySpec;
34+
import java.util.List;
2935
import org.bouncycastle.asn1.ASN1Integer;
3036
import org.bouncycastle.asn1.ASN1Sequence;
31-
import org.bouncycastle.crypto.params.AsymmetricKeyParameter;
32-
import org.bouncycastle.crypto.params.ECKeyParameters;
33-
import org.bouncycastle.crypto.params.Ed25519PublicKeyParameters;
34-
import org.bouncycastle.crypto.params.RSAKeyParameters;
35-
import org.bouncycastle.crypto.util.PublicKeyFactory;
37+
import org.bouncycastle.asn1.x509.SubjectPublicKeyInfo;
3638
import org.bouncycastle.jce.ECNamedCurveTable;
3739
import org.bouncycastle.jce.provider.BouncyCastleProvider;
3840
import org.bouncycastle.jce.spec.ECNamedCurveParameterSpec;
3941
import org.bouncycastle.jce.spec.ECNamedCurveSpec;
42+
import org.bouncycastle.openssl.PEMParser;
43+
import org.bouncycastle.openssl.jcajce.JcaPEMKeyConverter;
4044
import org.bouncycastle.util.encoders.DecoderException;
41-
import org.bouncycastle.util.io.pem.PemObject;
42-
import org.bouncycastle.util.io.pem.PemReader;
4345

4446
/** For internal use. Key related utility functions. */
4547
public class Keys {
4648

47-
private static final Logger log = Logger.getLogger(Keys.class.getName());
49+
private static final List<String> SUPPORTED_KEY_TYPES =
50+
List.of("ECDSA", "EC", "RSA", "Ed25519", "EdDSA");
4851

4952
static {
5053
Security.addProvider(new BouncyCastleProvider());
@@ -60,48 +63,26 @@ public class Keys {
6063
*/
6164
public static PublicKey parsePublicKey(byte[] keyBytes)
6265
throws InvalidKeySpecException, IOException, NoSuchAlgorithmException {
63-
PemReader pemReader =
64-
new PemReader(
65-
new InputStreamReader(new ByteArrayInputStream(keyBytes), StandardCharsets.UTF_8));
66-
PemObject section = null;
67-
try {
68-
section = pemReader.readPemObject();
69-
if (pemReader.readPemObject() != null) {
66+
try (PEMParser pemParser =
67+
new PEMParser(
68+
new InputStreamReader(new ByteArrayInputStream(keyBytes), StandardCharsets.UTF_8))) {
69+
var keyObj = pemParser.readObject(); // throws DecoderException
70+
if (keyObj == null) {
7071
throw new InvalidKeySpecException(
7172
"sigstore public keys must be only a single PEM encoded public key");
7273
}
74+
JcaPEMKeyConverter converter = new JcaPEMKeyConverter();
75+
if (keyObj instanceof SubjectPublicKeyInfo) {
76+
PublicKey pk = converter.getPublicKey((SubjectPublicKeyInfo) keyObj);
77+
if (!SUPPORTED_KEY_TYPES.contains(pk.getAlgorithm())) {
78+
throw new NoSuchAlgorithmException("Unsupported key type: " + pk.getAlgorithm());
79+
}
80+
return pk;
81+
}
82+
throw new InvalidKeySpecException("Could not parse PEM section into public key");
7383
} catch (DecoderException e) {
7484
throw new InvalidKeySpecException("Invalid key, could not parse PEM section");
7585
}
76-
// special handling for PKCS1 (rsa) public key
77-
// TODO: The length checking is not necessary after https://github.com/bcgit/bc-java/issues/1370
78-
// has been merged. Remove it when bc-java is updated with the merge.
79-
if ((section == null) || (section.getContent() == null) || (section.getContent().length == 0)) {
80-
throw new InvalidKeySpecException("Invalid key, empty PEM section");
81-
}
82-
if (section.getType().equals("RSA PUBLIC KEY")) {
83-
return parsePkcs1RsaPublicKey(section.getContent());
84-
}
85-
86-
// otherwise, we are dealing with PKIX X509 encoded keys
87-
byte[] content = section.getContent();
88-
EncodedKeySpec publicKeySpec = new X509EncodedKeySpec(content);
89-
AsymmetricKeyParameter keyParameters = null;
90-
91-
// Ensure PEM content can be parsed correctly
92-
try {
93-
keyParameters = PublicKeyFactory.createKey(content);
94-
} catch (IllegalStateException e) {
95-
throw new InvalidKeySpecException("Invalid key, could not parse PEM content");
96-
}
97-
if (keyParameters == null) {
98-
throw new InvalidKeySpecException("Invalid key, could not parse PEM content");
99-
}
100-
101-
// get algorithm inspecting the created class
102-
String keyAlgorithm = extractKeyAlgorithm(keyParameters);
103-
KeyFactory keyFactory = KeyFactory.getInstance(keyAlgorithm);
104-
return keyFactory.generatePublic(publicKeySpec);
10586
}
10687

10788
/**
@@ -184,34 +165,4 @@ public static PublicKey constructTufPublicKey(byte[] contents, String scheme)
184165
throw new RuntimeException(scheme + " not currently supported");
185166
}
186167
}
187-
188-
// https://stackoverflow.com/questions/42911637/get-publickey-from-key-bytes-not-knowing-the-key-algorithm
189-
private static String extractKeyAlgorithm(AsymmetricKeyParameter keyParameters)
190-
throws NoSuchAlgorithmException {
191-
if (keyParameters instanceof RSAKeyParameters) {
192-
return "RSA";
193-
} else if (keyParameters instanceof Ed25519PublicKeyParameters) {
194-
return "EdDSA";
195-
} else if (keyParameters instanceof ECKeyParameters) {
196-
return "EC";
197-
} else {
198-
String error =
199-
String.format(
200-
Locale.ROOT,
201-
"The key provided was of type: %s. We only support RSA, EdDSA, and EC ",
202-
keyParameters);
203-
log.warning(error);
204-
throw new NoSuchAlgorithmException(error);
205-
}
206-
}
207-
208-
@VisibleForTesting
209-
protected static int getJavaVersion() {
210-
return getJavaVersion(System.getProperty("java.version"));
211-
}
212-
213-
@VisibleForTesting
214-
protected static int getJavaVersion(String version) {
215-
return Integer.parseInt(version.substring(0, version.indexOf(".")));
216-
}
217168
}

‎sigstore-java/src/test/java/dev/sigstore/encryption/KeysTest.java

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,14 @@ class KeysTest {
4242
static final String ECDSA_SHA2_NISTP256 =
4343
"dev/sigstore/samples/keys/test-ecdsa-sha2-nistp256.pub";
4444

45+
@Test
46+
void parsePublicKey_invalid() {
47+
var key =
48+
"-----BEGIN Ã-----\nMGMGB1gFB00gFM0EEEEEEEzEEEEEEEEEEEEEEEEEEEEEEEEEEEEEFB00gFM0EEEEEEEzEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEFGB1g070v129B1700372=\n-----END ïI";
49+
Assertions.assertThrows(
50+
IOException.class, () -> Keys.parsePublicKey(key.getBytes(StandardCharsets.UTF_8)));
51+
}
52+
4553
@Test
4654
void parsePublicKey_rsa() throws IOException, InvalidKeySpecException, NoSuchAlgorithmException {
4755
PublicKey result =
@@ -61,7 +69,7 @@ void parsePublicKey_rsaPkcs1()
6169
void parsePublicKey_ec() throws IOException, InvalidKeySpecException, NoSuchAlgorithmException {
6270
PublicKey result =
6371
Keys.parsePublicKey(Resources.toByteArray(Resources.getResource(EC_PUB_PATH)));
64-
assertEquals("EC", result.getAlgorithm());
72+
assertEquals("ECDSA", result.getAlgorithm());
6573
}
6674

6775
@Test
@@ -70,7 +78,7 @@ void parsePublicKey_ed25519_withBouncyCastle()
7078
throws IOException, InvalidKeySpecException, NoSuchAlgorithmException {
7179
PublicKey result =
7280
Keys.parsePublicKey(Resources.toByteArray(Resources.getResource(ED25519_PUB_PATH)));
73-
// BouncyCastle names the algorithm differently than the JDK
81+
// BouncyCastle names the algorithm differently than the JDK (Ed25519 vs EdDSA)
7482
assertEquals("Ed25519", result.getAlgorithm());
7583
}
7684

@@ -95,7 +103,7 @@ void parseTufPublicKeyPemEncoded_sha2_nistp256()
95103
throws IOException, InvalidKeySpecException, NoSuchAlgorithmException {
96104
PublicKey result =
97105
Keys.parsePublicKey(Resources.toByteArray(Resources.getResource(ECDSA_SHA2_NISTP256)));
98-
assertEquals("EC", result.getAlgorithm());
106+
assertEquals("ECDSA", result.getAlgorithm());
99107
}
100108

101109
@Test
@@ -126,7 +134,7 @@ void parseTufPublicKey_ecdsaBad()
126134
@Test
127135
@EnabledForJreRange(min = JRE.JAVA_15)
128136
void parseTufPublicKey_ed25519_java15Plus()
129-
throws NoSuchAlgorithmException, InvalidKeySpecException, NoSuchProviderException {
137+
throws NoSuchAlgorithmException, InvalidKeySpecException {
130138
// {@code step crypto keypair ed25519.pub /dev/null --kty OKP --curve Ed25519}
131139
// copy just the key part out of ed25519.pub removing PEM header and footer
132140
// {@code echo $(copied content) | base64 -d | hexdump -v -e '/1 "%02x" '}
@@ -210,23 +218,8 @@ void parsePkixPublicKey_ecdsa() throws NoSuchAlgorithmException, InvalidKeySpecE
210218
}
211219

212220
@Test
213-
void parsePublicKey_failOnNullSection()
214-
throws IOException, NoSuchAlgorithmException, NoSuchProviderException {
215-
// This unit test is used to test the fix for a bug discovered by oss-fuzz
216-
// The bug happens when a malformed byte array is passed to the method
217-
// https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=57247
221+
void parsePublicKey_failOnBadPEM() throws Exception {
218222
byte[] byteArray = "-----BEGIN A-----\nBBBBB-----END A".getBytes(StandardCharsets.UTF_8);
219-
Assertions.assertThrows(InvalidKeySpecException.class, () -> Keys.parsePublicKey(byteArray));
220-
}
221-
222-
@Test
223-
void testGetJavaVersion() {
224-
assertEquals(1, Keys.getJavaVersion("1.6.0_23"));
225-
assertEquals(1, Keys.getJavaVersion("1.7.0"));
226-
assertEquals(1, Keys.getJavaVersion("1.6.0_23"));
227-
assertEquals(9, Keys.getJavaVersion("9.0.1"));
228-
assertEquals(11, Keys.getJavaVersion("11.0.4"));
229-
assertEquals(12, Keys.getJavaVersion("12.0.1"));
230-
assertEquals(15, Keys.getJavaVersion("15.0.1"));
223+
Assertions.assertThrows(IOException.class, () -> Keys.parsePublicKey(byteArray));
231224
}
232225
}

0 commit comments

Comments
 (0)