From 9446dfb035b88ad5d962270550b59c2c72aa694f Mon Sep 17 00:00:00 2001 From: Appu Goundan Date: Wed, 1 Nov 2023 14:40:45 -0400 Subject: [PATCH] Rework keys to remove PemReader, user BC instead Signed-off-by: Appu Goundan --- .../java/dev/sigstore/encryption/Keys.java | 111 +++++------------- .../dev/sigstore/encryption/KeysTest.java | 35 +++--- 2 files changed, 45 insertions(+), 101 deletions(-) diff --git a/sigstore-java/src/main/java/dev/sigstore/encryption/Keys.java b/sigstore-java/src/main/java/dev/sigstore/encryption/Keys.java index 265674ad..1a5d90e1 100644 --- a/sigstore-java/src/main/java/dev/sigstore/encryption/Keys.java +++ b/sigstore-java/src/main/java/dev/sigstore/encryption/Keys.java @@ -15,36 +15,39 @@ */ package dev.sigstore.encryption; -import static org.bouncycastle.jce.ECPointUtil.*; +import static org.bouncycastle.jce.ECPointUtil.decodePoint; -import com.google.common.annotations.VisibleForTesting; import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStreamReader; import java.nio.charset.StandardCharsets; -import java.security.*; -import java.security.spec.*; -import java.util.Locale; -import java.util.logging.Logger; +import java.security.KeyFactory; +import java.security.NoSuchAlgorithmException; +import java.security.NoSuchProviderException; +import java.security.PublicKey; +import java.security.Security; +import java.security.spec.ECPoint; +import java.security.spec.ECPublicKeySpec; +import java.security.spec.InvalidKeySpecException; +import java.security.spec.RSAPublicKeySpec; +import java.security.spec.X509EncodedKeySpec; +import java.util.List; import org.bouncycastle.asn1.ASN1Integer; import org.bouncycastle.asn1.ASN1Sequence; -import org.bouncycastle.crypto.params.AsymmetricKeyParameter; -import org.bouncycastle.crypto.params.ECKeyParameters; -import org.bouncycastle.crypto.params.Ed25519PublicKeyParameters; -import org.bouncycastle.crypto.params.RSAKeyParameters; -import org.bouncycastle.crypto.util.PublicKeyFactory; +import org.bouncycastle.asn1.x509.SubjectPublicKeyInfo; import org.bouncycastle.jce.ECNamedCurveTable; import org.bouncycastle.jce.provider.BouncyCastleProvider; import org.bouncycastle.jce.spec.ECNamedCurveParameterSpec; import org.bouncycastle.jce.spec.ECNamedCurveSpec; +import org.bouncycastle.openssl.PEMParser; +import org.bouncycastle.openssl.jcajce.JcaPEMKeyConverter; import org.bouncycastle.util.encoders.DecoderException; -import org.bouncycastle.util.io.pem.PemObject; -import org.bouncycastle.util.io.pem.PemReader; /** For internal use. Key related utility functions. */ public class Keys { - private static final Logger log = Logger.getLogger(Keys.class.getName()); + private static final List SUPPORTED_KEY_TYPES = + List.of("ECDSA", "EC", "RSA", "Ed25519", "EdDSA"); static { Security.addProvider(new BouncyCastleProvider()); @@ -60,48 +63,26 @@ public class Keys { */ public static PublicKey parsePublicKey(byte[] keyBytes) throws InvalidKeySpecException, IOException, NoSuchAlgorithmException { - PemReader pemReader = - new PemReader( - new InputStreamReader(new ByteArrayInputStream(keyBytes), StandardCharsets.UTF_8)); - PemObject section = null; - try { - section = pemReader.readPemObject(); - if (pemReader.readPemObject() != null) { + try (PEMParser pemParser = + new PEMParser( + new InputStreamReader(new ByteArrayInputStream(keyBytes), StandardCharsets.UTF_8))) { + var keyObj = pemParser.readObject(); // throws DecoderException + if (keyObj == null) { throw new InvalidKeySpecException( "sigstore public keys must be only a single PEM encoded public key"); } + JcaPEMKeyConverter converter = new JcaPEMKeyConverter(); + if (keyObj instanceof SubjectPublicKeyInfo) { + PublicKey pk = converter.getPublicKey((SubjectPublicKeyInfo) keyObj); + if (!SUPPORTED_KEY_TYPES.contains(pk.getAlgorithm())) { + throw new NoSuchAlgorithmException("Unsupported key type: " + pk.getAlgorithm()); + } + return pk; + } + throw new InvalidKeySpecException("Could not parse PEM section into public key"); } catch (DecoderException e) { throw new InvalidKeySpecException("Invalid key, could not parse PEM section"); } - // special handling for PKCS1 (rsa) public key - // TODO: The length checking is not necessary after https://github.com/bcgit/bc-java/issues/1370 - // has been merged. Remove it when bc-java is updated with the merge. - if ((section == null) || (section.getContent() == null) || (section.getContent().length == 0)) { - throw new InvalidKeySpecException("Invalid key, empty PEM section"); - } - if (section.getType().equals("RSA PUBLIC KEY")) { - return parsePkcs1RsaPublicKey(section.getContent()); - } - - // otherwise, we are dealing with PKIX X509 encoded keys - byte[] content = section.getContent(); - EncodedKeySpec publicKeySpec = new X509EncodedKeySpec(content); - AsymmetricKeyParameter keyParameters = null; - - // Ensure PEM content can be parsed correctly - try { - keyParameters = PublicKeyFactory.createKey(content); - } catch (IllegalStateException e) { - throw new InvalidKeySpecException("Invalid key, could not parse PEM content"); - } - if (keyParameters == null) { - throw new InvalidKeySpecException("Invalid key, could not parse PEM content"); - } - - // get algorithm inspecting the created class - String keyAlgorithm = extractKeyAlgorithm(keyParameters); - KeyFactory keyFactory = KeyFactory.getInstance(keyAlgorithm); - return keyFactory.generatePublic(publicKeySpec); } /** @@ -184,34 +165,4 @@ public static PublicKey constructTufPublicKey(byte[] contents, String scheme) throw new RuntimeException(scheme + " not currently supported"); } } - - // https://stackoverflow.com/questions/42911637/get-publickey-from-key-bytes-not-knowing-the-key-algorithm - private static String extractKeyAlgorithm(AsymmetricKeyParameter keyParameters) - throws NoSuchAlgorithmException { - if (keyParameters instanceof RSAKeyParameters) { - return "RSA"; - } else if (keyParameters instanceof Ed25519PublicKeyParameters) { - return "EdDSA"; - } else if (keyParameters instanceof ECKeyParameters) { - return "EC"; - } else { - String error = - String.format( - Locale.ROOT, - "The key provided was of type: %s. We only support RSA, EdDSA, and EC ", - keyParameters); - log.warning(error); - throw new NoSuchAlgorithmException(error); - } - } - - @VisibleForTesting - protected static int getJavaVersion() { - return getJavaVersion(System.getProperty("java.version")); - } - - @VisibleForTesting - protected static int getJavaVersion(String version) { - return Integer.parseInt(version.substring(0, version.indexOf("."))); - } } diff --git a/sigstore-java/src/test/java/dev/sigstore/encryption/KeysTest.java b/sigstore-java/src/test/java/dev/sigstore/encryption/KeysTest.java index d7ef316d..fed8c3c8 100644 --- a/sigstore-java/src/test/java/dev/sigstore/encryption/KeysTest.java +++ b/sigstore-java/src/test/java/dev/sigstore/encryption/KeysTest.java @@ -42,6 +42,14 @@ class KeysTest { static final String ECDSA_SHA2_NISTP256 = "dev/sigstore/samples/keys/test-ecdsa-sha2-nistp256.pub"; + @Test + void parsePublicKey_invalid() { + var key = + "-----BEGIN Ã-----\nMGMGB1gFB00gFM0EEEEEEEzEEEEEEEEEEEEEEEEEEEEEEEEEEEEEFB00gFM0EEEEEEEzEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEFGB1g070v129B1700372=\n-----END ïI"; + Assertions.assertThrows( + IOException.class, () -> Keys.parsePublicKey(key.getBytes(StandardCharsets.UTF_8))); + } + @Test void parsePublicKey_rsa() throws IOException, InvalidKeySpecException, NoSuchAlgorithmException { PublicKey result = @@ -61,7 +69,7 @@ void parsePublicKey_rsaPkcs1() void parsePublicKey_ec() throws IOException, InvalidKeySpecException, NoSuchAlgorithmException { PublicKey result = Keys.parsePublicKey(Resources.toByteArray(Resources.getResource(EC_PUB_PATH))); - assertEquals("EC", result.getAlgorithm()); + assertEquals("ECDSA", result.getAlgorithm()); } @Test @@ -70,7 +78,7 @@ void parsePublicKey_ed25519_withBouncyCastle() throws IOException, InvalidKeySpecException, NoSuchAlgorithmException { PublicKey result = Keys.parsePublicKey(Resources.toByteArray(Resources.getResource(ED25519_PUB_PATH))); - // BouncyCastle names the algorithm differently than the JDK + // BouncyCastle names the algorithm differently than the JDK (Ed25519 vs EdDSA) assertEquals("Ed25519", result.getAlgorithm()); } @@ -95,7 +103,7 @@ void parseTufPublicKeyPemEncoded_sha2_nistp256() throws IOException, InvalidKeySpecException, NoSuchAlgorithmException { PublicKey result = Keys.parsePublicKey(Resources.toByteArray(Resources.getResource(ECDSA_SHA2_NISTP256))); - assertEquals("EC", result.getAlgorithm()); + assertEquals("ECDSA", result.getAlgorithm()); } @Test @@ -126,7 +134,7 @@ void parseTufPublicKey_ecdsaBad() @Test @EnabledForJreRange(min = JRE.JAVA_15) void parseTufPublicKey_ed25519_java15Plus() - throws NoSuchAlgorithmException, InvalidKeySpecException, NoSuchProviderException { + throws NoSuchAlgorithmException, InvalidKeySpecException { // {@code step crypto keypair ed25519.pub /dev/null --kty OKP --curve Ed25519} // copy just the key part out of ed25519.pub removing PEM header and footer // {@code echo $(copied content) | base64 -d | hexdump -v -e '/1 "%02x" '} @@ -210,23 +218,8 @@ void parsePkixPublicKey_ecdsa() throws NoSuchAlgorithmException, InvalidKeySpecE } @Test - void parsePublicKey_failOnNullSection() - throws IOException, NoSuchAlgorithmException, NoSuchProviderException { - // This unit test is used to test the fix for a bug discovered by oss-fuzz - // The bug happens when a malformed byte array is passed to the method - // https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=57247 + void parsePublicKey_failOnBadPEM() throws Exception { byte[] byteArray = "-----BEGIN A-----\nBBBBB-----END A".getBytes(StandardCharsets.UTF_8); - Assertions.assertThrows(InvalidKeySpecException.class, () -> Keys.parsePublicKey(byteArray)); - } - - @Test - void testGetJavaVersion() { - assertEquals(1, Keys.getJavaVersion("1.6.0_23")); - assertEquals(1, Keys.getJavaVersion("1.7.0")); - assertEquals(1, Keys.getJavaVersion("1.6.0_23")); - assertEquals(9, Keys.getJavaVersion("9.0.1")); - assertEquals(11, Keys.getJavaVersion("11.0.4")); - assertEquals(12, Keys.getJavaVersion("12.0.1")); - assertEquals(15, Keys.getJavaVersion("15.0.1")); + Assertions.assertThrows(IOException.class, () -> Keys.parsePublicKey(byteArray)); } }