From a323b74db5bb2d369457ca644afa7c08a6f45883 Mon Sep 17 00:00:00 2001 From: Matt Castelaz Date: Thu, 29 Oct 2026 02:22:03 +0000 Subject: [PATCH 1/3] fix(auth): verify public/private key match with retries in mTLS keystore loading Verify that the parsed private key matches the leaf certificate's public key in MtlsKeyStoreUtils.createMtlsKeyStore() via a test signature and verification, and retry up to 3 times (with a 100ms interval) in X509Provider.getKeyStore() when loading separate cert_path and key_path files from disk to handle transient mid-rotation mismatches. --- .../google/auth/mtls/MtlsKeyStoreUtils.java | 59 ++++++- .../com/google/auth/mtls/X509Provider.java | 73 ++++++-- .../auth/mtls/MtlsKeyStoreUtilsTest.java | 41 +++++ .../google/auth/mtls/X509ProviderTest.java | 167 ++++++++++++++++++ 4 files changed, 324 insertions(+), 16 deletions(-) diff --git a/google-auth-library-java/oauth2_http/java/com/google/auth/mtls/MtlsKeyStoreUtils.java b/google-auth-library-java/oauth2_http/java/com/google/auth/mtls/MtlsKeyStoreUtils.java index d8f51c61935f..dabccfdb2548 100644 --- a/google-auth-library-java/oauth2_http/java/com/google/auth/mtls/MtlsKeyStoreUtils.java +++ b/google-auth-library-java/oauth2_http/java/com/google/auth/mtls/MtlsKeyStoreUtils.java @@ -40,11 +40,14 @@ import java.security.KeyFactory; import java.security.KeyStore; import java.security.PrivateKey; +import java.security.PublicKey; +import java.security.Signature; import java.security.cert.Certificate; import java.security.cert.CertificateException; import java.security.cert.CertificateFactory; import java.security.cert.X509Certificate; import java.security.spec.PKCS8EncodedKeySpec; +import java.security.spec.X509EncodedKeySpec; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -75,6 +78,9 @@ final class MtlsKeyStoreUtils { private static final Logger LOGGER = Logger.getLogger(MtlsKeyStoreUtils.class.getName()); + private static final byte[] VERIFICATION_DATA = + "verification-data".getBytes(StandardCharsets.UTF_8); + private MtlsKeyStoreUtils() {} /** @@ -89,7 +95,8 @@ private MtlsKeyStoreUtils() {} * @throws IllegalArgumentException if the input has no certificate or no private key, or if a PEM * section before both a certificate and a private key have been read is malformed * @throws GeneralSecurityException if the first certificate or the private key cannot be parsed, - * or the key store cannot be created + * if the private key does not match the leaf certificate's public key, or if the key store + * cannot be created * @throws IOException if the input cannot be read */ static KeyStore createMtlsKeyStore(InputStream certAndKey) @@ -143,9 +150,11 @@ static KeyStore createMtlsKeyStore(InputStream certAndKey) LOGGER.log(Level.FINE, "Skipping unparsable extra certificate in mTLS chain", e); } } + X509Certificate leaf = chain.get(0); PrivateKey key = - KeyFactory.getInstance(chain.get(0).getPublicKey().getAlgorithm()) + KeyFactory.getInstance(leaf.getPublicKey().getAlgorithm()) .generatePrivate(new PKCS8EncodedKeySpec(privateKey)); + verifyKeyPair(leaf, key); KeyStore keyStore = KeyStore.getInstance("JKS"); keyStore.load(null); @@ -153,6 +162,52 @@ static KeyStore createMtlsKeyStore(InputStream certAndKey) return keyStore; } + /** + * Verifies that {@code privateKey} corresponds to the public key in {@code cert} by performing a + * test signature and verification. + */ + private static void verifyKeyPair(X509Certificate cert, PrivateKey privateKey) + throws GeneralSecurityException { + PublicKey publicKey = cert.getPublicKey(); + String keyAlgorithm = publicKey.getAlgorithm(); + String sigAlg; + if ("RSA".equals(keyAlgorithm)) { + sigAlg = "SHA256withRSA"; + } else if ("EC".equals(keyAlgorithm) || "ECDSA".equals(keyAlgorithm)) { + sigAlg = "SHA256withECDSA"; + } else { + return; + } + + boolean matches = false; + GeneralSecurityException failureCause = null; + try { + if ("ECDSA".equals(keyAlgorithm) && publicKey.getEncoded() != null) { + // SunEC rejects keys whose algorithm is "ECDSA" rather than "EC"; normalize the key. + publicKey = + KeyFactory.getInstance("EC") + .generatePublic(new X509EncodedKeySpec(publicKey.getEncoded())); + } + + Signature signer = Signature.getInstance(sigAlg); + signer.initSign(privateKey); + signer.update(VERIFICATION_DATA); + byte[] signature = signer.sign(); + + Signature verifier = Signature.getInstance(sigAlg); + verifier.initVerify(publicKey); + verifier.update(VERIFICATION_DATA); + + matches = verifier.verify(signature); + } catch (GeneralSecurityException e) { + failureCause = e; + } + + if (!matches) { + throw new GeneralSecurityException("Certificate and private key do not match", failureCause); + } + } + private static boolean containsCertificate(List certificates, byte[] candidate) { for (byte[] existing : certificates) { if (Arrays.equals(existing, candidate)) { diff --git a/google-auth-library-java/oauth2_http/java/com/google/auth/mtls/X509Provider.java b/google-auth-library-java/oauth2_http/java/com/google/auth/mtls/X509Provider.java index fa7deeda21ca..3d107a0cde5a 100644 --- a/google-auth-library-java/oauth2_http/java/com/google/auth/mtls/X509Provider.java +++ b/google-auth-library-java/oauth2_http/java/com/google/auth/mtls/X509Provider.java @@ -35,6 +35,7 @@ import com.google.auth.oauth2.PropertyProvider; import com.google.auth.oauth2.SystemEnvironmentProvider; import com.google.auth.oauth2.SystemPropertyProvider; +import com.google.common.annotations.VisibleForTesting; import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; @@ -42,6 +43,8 @@ import java.io.InputStream; import java.io.SequenceInputStream; import java.security.KeyStore; +import java.util.logging.Level; +import java.util.logging.Logger; import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.Nullable; @@ -52,6 +55,25 @@ @NullMarked @InternalApi public class X509Provider implements MtlsProvider { + private static final Logger LOGGER = Logger.getLogger(X509Provider.class.getName()); + + // Retries for verifying certificate and private key matching during atomic key rotation. + static final int CERT_KEY_MATCH_RETRIES = 3; + static final long CERT_KEY_MATCH_RETRY_INTERVAL_MS = 100; + + @VisibleForTesting + @FunctionalInterface + interface TimeService { + void sleep(long millis) throws InterruptedException; + } + + private static volatile TimeService timeService = Thread::sleep; + + @VisibleForTesting + static void setTimeServiceForTesting(@Nullable TimeService service) { + timeService = service != null ? service : Thread::sleep; + } + private final EnvironmentProvider envProvider; private final PropertyProvider propProvider; private final @Nullable String certConfigPathOverride; @@ -193,22 +215,45 @@ public KeyStore getKeyStore() throws CertificateSourceUnavailableException, IOEx throw configUnavailable; } - // Read the certificate and private key file paths into streams. - try (InputStream certStream = new FileInputStream(new File(workloadCertConfig.getCertPath())); - InputStream privateKeyStream = - new FileInputStream(new File(workloadCertConfig.getPrivateKeyPath())); - SequenceInputStream certAndPrivateKeyStream = - new SequenceInputStream(certStream, privateKeyStream)) { + // Read the certificate and private key file paths into streams, retrying to handle transient + // mid-rotation mismatches or file replacements when separate cert_path and key_path files are + // updated non-atomically relative to each other. + Exception lastException = null; + for (int attempt = 0; attempt < CERT_KEY_MATCH_RETRIES; attempt++) { + try (InputStream certStream = + new FileInputStream(new File(workloadCertConfig.getCertPath())); + InputStream privateKeyStream = + new FileInputStream(new File(workloadCertConfig.getPrivateKeyPath())); + SequenceInputStream certAndPrivateKeyStream = + new SequenceInputStream(certStream, privateKeyStream)) { - // Build a key store using the combined stream. - return MtlsKeyStoreUtils.createMtlsKeyStore(certAndPrivateKeyStream); - } catch (CertificateSourceUnavailableException e) { - // Throw the CertificateSourceUnavailableException without wrapping. - throw e; - } catch (Exception e) { - // Wrap all other exception types to an IOException. - throw new IOException("X509Provider: Unexpected IOException:", e); + // Build a key store using the combined stream. + return MtlsKeyStoreUtils.createMtlsKeyStore(certAndPrivateKeyStream); + } catch (CertificateSourceUnavailableException e) { + // Throw the CertificateSourceUnavailableException without wrapping. + throw e; + } catch (Exception e) { + lastException = e; + if (attempt + 1 < CERT_KEY_MATCH_RETRIES) { + LOGGER.log( + Level.FINE, + "X509Provider: Failed to load or verify certificate and private key on attempt " + + (attempt + 1) + + ", retrying", + e); + try { + timeService.sleep(CERT_KEY_MATCH_RETRY_INTERVAL_MS); + } catch (InterruptedException interrupted) { + Thread.currentThread().interrupt(); + throw new IOException( + "X509Provider: Interrupted while waiting for certificate and private key match.", + interrupted); + } + } + } } + // Wrap all other exception types to an IOException. + throw new IOException("X509Provider: Unexpected IOException:", lastException); } /** diff --git a/google-auth-library-java/oauth2_http/javatests/com/google/auth/mtls/MtlsKeyStoreUtilsTest.java b/google-auth-library-java/oauth2_http/javatests/com/google/auth/mtls/MtlsKeyStoreUtilsTest.java index 5f1e8fca0545..87308b416700 100644 --- a/google-auth-library-java/oauth2_http/javatests/com/google/auth/mtls/MtlsKeyStoreUtilsTest.java +++ b/google-auth-library-java/oauth2_http/javatests/com/google/auth/mtls/MtlsKeyStoreUtilsTest.java @@ -44,6 +44,9 @@ import java.io.IOException; import java.io.InputStream; import java.nio.file.Files; +import java.security.GeneralSecurityException; +import java.security.KeyPair; +import java.security.KeyPairGenerator; import java.security.KeyStore; import java.security.PrivateKey; import java.security.Signature; @@ -51,6 +54,7 @@ import java.security.cert.CertificateException; import java.security.cert.X509Certificate; import java.security.spec.InvalidKeySpecException; +import java.util.Base64; import java.util.Collections; import org.junit.jupiter.api.Test; @@ -310,6 +314,43 @@ void createMtlsKeyStore_missingPrivateKey_throws() throws Exception { assertEquals("private key is missing from certAndKey string", e.getMessage()); } + @Test + void createMtlsKeyStore_mismatchedRsaKeyPair_throws() throws Exception { + KeyPairGenerator kpg = KeyPairGenerator.getInstance("RSA"); + kpg.initialize(1024); + KeyPair otherRsaKeyPair = kpg.generateKeyPair(); + String otherRsaKeyPem = toPkcs8Pem(otherRsaKeyPair.getPrivate()); + + GeneralSecurityException e = + assertThrows( + GeneralSecurityException.class, + () -> + MtlsKeyStoreUtils.createMtlsKeyStore( + stream(read(SINGLE_CERT_PATH), otherRsaKeyPem))); + assertEquals("Certificate and private key do not match", e.getMessage()); + } + + @Test + void createMtlsKeyStore_mismatchedEcKeyPair_throws() throws Exception { + KeyPairGenerator kpg = KeyPairGenerator.getInstance("EC"); + kpg.initialize(256); + KeyPair otherEcKeyPair = kpg.generateKeyPair(); + String otherEcKeyPem = toPkcs8Pem(otherEcKeyPair.getPrivate()); + + GeneralSecurityException e = + assertThrows( + GeneralSecurityException.class, + () -> + MtlsKeyStoreUtils.createMtlsKeyStore(stream(read(CHAIN_CERT_PATH), otherEcKeyPem))); + assertEquals("Certificate and private key do not match", e.getMessage()); + } + + static String toPkcs8Pem(PrivateKey privateKey) { + return "-----BEGIN PRIVATE KEY-----\n" + + Base64.getMimeEncoder(64, "\n".getBytes(UTF_8)).encodeToString(privateKey.getEncoded()) + + "\n-----END PRIVATE KEY-----\n"; + } + static String read(String path) throws IOException { return new String(Files.readAllBytes(new File(path).toPath()), UTF_8); } diff --git a/google-auth-library-java/oauth2_http/javatests/com/google/auth/mtls/X509ProviderTest.java b/google-auth-library-java/oauth2_http/javatests/com/google/auth/mtls/X509ProviderTest.java index e9d7182cf310..c4f6650a30da 100644 --- a/google-auth-library-java/oauth2_http/javatests/com/google/auth/mtls/X509ProviderTest.java +++ b/google-auth-library-java/oauth2_http/javatests/com/google/auth/mtls/X509ProviderTest.java @@ -46,11 +46,16 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.security.GeneralSecurityException; +import java.security.KeyPair; +import java.security.KeyPairGenerator; import java.security.KeyStore; import java.security.KeyStoreException; import java.security.cert.Certificate; import java.security.cert.CertificateException; import java.security.cert.CertificateFactory; +import java.util.ArrayList; +import java.util.List; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -403,6 +408,168 @@ void x509Provider_publicConstructor_ignoresGkeCredentialBundle(@TempDir Path tem } } + @Test + void x509Provider_mismatchedKeyPair_retriesAndSucceedsWhenKeyRotates(@TempDir Path tempDir) + throws Exception { + Path certFile = tempDir.resolve("cert.pem"); + Path keyFile = tempDir.resolve("key.pem"); + Files.copy(new File(TEST_CERT_PATH).toPath(), certFile); + + KeyPairGenerator kpg = KeyPairGenerator.getInstance("RSA"); + kpg.initialize(1024); + KeyPair oldRsaKeyPair = kpg.generateKeyPair(); + Files.write( + keyFile, MtlsKeyStoreUtilsTest.toPkcs8Pem(oldRsaKeyPair.getPrivate()).getBytes(UTF_8)); + + Path config = writeWorkloadConfig(tempDir, certFile, keyFile); + List sleeps = new ArrayList<>(); + X509Provider.setTimeServiceForTesting( + millis -> { + sleeps.add(millis); + if (sleeps.size() == 2) { + // Simulate external rotator finishing key_path update before the 3rd attempt. + try { + Files.copy( + new File(TEST_KEY_PATH).toPath(), + keyFile, + java.nio.file.StandardCopyOption.REPLACE_EXISTING); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + }); + try { + X509Provider testProvider = new X509Provider(config.toString()); + + KeyStore store = testProvider.getKeyStore(); + + assertEquals(1, store.size()); + assertNotNull(store.getCertificateAlias(loadTestCert())); + assertEquals(2, sleeps.size()); + assertEquals(X509Provider.CERT_KEY_MATCH_RETRY_INTERVAL_MS, sleeps.get(0)); + assertEquals(X509Provider.CERT_KEY_MATCH_RETRY_INTERVAL_MS, sleeps.get(1)); + } finally { + X509Provider.setTimeServiceForTesting(null); + } + } + + @Test + void x509Provider_mismatchedKeyPair_throwsAfterExhaustingRetries(@TempDir Path tempDir) + throws Exception { + Path certFile = tempDir.resolve("cert.pem"); + Path keyFile = tempDir.resolve("key.pem"); + Files.copy(new File(TEST_CERT_PATH).toPath(), certFile); + + KeyPairGenerator kpg = KeyPairGenerator.getInstance("RSA"); + kpg.initialize(1024); + KeyPair mismatchedRsaKeyPair = kpg.generateKeyPair(); + Files.write( + keyFile, + MtlsKeyStoreUtilsTest.toPkcs8Pem(mismatchedRsaKeyPair.getPrivate()).getBytes(UTF_8)); + + Path config = writeWorkloadConfig(tempDir, certFile, keyFile); + List sleeps = new ArrayList<>(); + X509Provider.setTimeServiceForTesting(sleeps::add); + try { + X509Provider testProvider = new X509Provider(config.toString()); + + IOException exception = assertThrows(IOException.class, testProvider::getKeyStore); + + assertEquals("X509Provider: Unexpected IOException:", exception.getMessage()); + assertNotNull(exception.getCause()); + assertTrue(exception.getCause() instanceof GeneralSecurityException); + assertEquals("Certificate and private key do not match", exception.getCause().getMessage()); + assertEquals(X509Provider.CERT_KEY_MATCH_RETRIES - 1, sleeps.size()); + } finally { + X509Provider.setTimeServiceForTesting(null); + } + } + + @Test + void x509Provider_transientlyEmptyCertFile_retriesAndSucceeds(@TempDir Path tempDir) + throws Exception { + Path certFile = tempDir.resolve("cert.pem"); + Path keyFile = tempDir.resolve("key.pem"); + // Simulate cert_path being truncated mid-write on the first attempt. + Files.write(certFile, new byte[0]); + Files.copy(new File(TEST_KEY_PATH).toPath(), keyFile); + + Path config = writeWorkloadConfig(tempDir, certFile, keyFile); + List sleeps = new ArrayList<>(); + X509Provider.setTimeServiceForTesting( + millis -> { + sleeps.add(millis); + try { + Files.copy( + new File(TEST_CERT_PATH).toPath(), + certFile, + java.nio.file.StandardCopyOption.REPLACE_EXISTING); + } catch (IOException e) { + throw new RuntimeException(e); + } + }); + try { + X509Provider testProvider = new X509Provider(config.toString()); + + KeyStore store = testProvider.getKeyStore(); + + assertEquals(1, store.size()); + assertNotNull(store.getCertificateAlias(loadTestCert())); + assertEquals(1, sleeps.size()); + } finally { + X509Provider.setTimeServiceForTesting(null); + } + } + + @Test + void x509Provider_interruptedDuringRetrySleep_restoresInterruptAndThrows(@TempDir Path tempDir) + throws Exception { + Path certFile = tempDir.resolve("cert.pem"); + Path keyFile = tempDir.resolve("key.pem"); + Files.copy(new File(TEST_CERT_PATH).toPath(), certFile); + + KeyPairGenerator kpg = KeyPairGenerator.getInstance("RSA"); + kpg.initialize(1024); + KeyPair mismatchedRsaKeyPair = kpg.generateKeyPair(); + Files.write( + keyFile, + MtlsKeyStoreUtilsTest.toPkcs8Pem(mismatchedRsaKeyPair.getPrivate()).getBytes(UTF_8)); + + Path config = writeWorkloadConfig(tempDir, certFile, keyFile); + X509Provider.setTimeServiceForTesting( + millis -> { + throw new InterruptedException("simulated interrupt"); + }); + try { + X509Provider testProvider = new X509Provider(config.toString()); + + IOException exception = assertThrows(IOException.class, testProvider::getKeyStore); + + assertTrue(Thread.currentThread().isInterrupted()); + assertEquals( + "X509Provider: Interrupted while waiting for certificate and private key match.", + exception.getMessage()); + assertTrue(exception.getCause() instanceof InterruptedException); + } finally { + Thread.interrupted(); // Clear interrupt flag + X509Provider.setTimeServiceForTesting(null); + } + } + + private static Path writeWorkloadConfig(Path tempDir, Path certFile, Path keyFile) + throws IOException { + Path config = tempDir.resolve("certificate_config.json"); + Files.write( + config, + ("{\"cert_configs\":{\"workload\":{\"cert_path\":\"" + + certFile.toString() + + "\",\"key_path\":\"" + + keyFile.toString() + + "\"}}}") + .getBytes(UTF_8)); + return config; + } + /** Property provider whose user.home has no gcloud certificate_config.json. */ private static TestPropertyProvider noGcloudConfig(Path tempDir) { TestPropertyProvider propProvider = new TestPropertyProvider(); From 357797945e9d265e1b58ef37ef7fce0870b18694 Mon Sep 17 00:00:00 2001 From: Matt Castelaz Date: Fri, 30 Oct 2026 02:54:13 +0000 Subject: [PATCH 2/3] test(auth): escape Windows backslashes in writeWorkloadConfig and update MtlsKeyStoreUtils javadoc --- .../java/com/google/auth/mtls/MtlsKeyStoreUtils.java | 12 +++++++----- .../com/google/auth/mtls/X509ProviderTest.java | 4 ++-- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/google-auth-library-java/oauth2_http/java/com/google/auth/mtls/MtlsKeyStoreUtils.java b/google-auth-library-java/oauth2_http/java/com/google/auth/mtls/MtlsKeyStoreUtils.java index dabccfdb2548..25ef0d0eb5d7 100644 --- a/google-auth-library-java/oauth2_http/java/com/google/auth/mtls/MtlsKeyStoreUtils.java +++ b/google-auth-library-java/oauth2_http/java/com/google/auth/mtls/MtlsKeyStoreUtils.java @@ -67,11 +67,13 @@ * *

Everything else matches the upstream method: key store type (JKS), alias, empty password, * accepted key format (PKCS#8 {@code PRIVATE KEY}), which private key is chosen, and error - * messages. Any input that the upstream method accepted is still accepted, even though more of it - * is now used: once a certificate and a private key have been found (the point where the upstream - * method stopped reading), a malformed later PEM section ends the input instead of failing, and an - * extra certificate that cannot be parsed is left out of the chain. Unlike the upstream method, the - * input is decoded as UTF-8 rather than the platform default charset, and it is read to the end. + * messages. Any valid certificate-and-key pair that the upstream method accepted is still accepted, + * even though more of the input is now used: once a certificate and a private key have been found + * (the point where the upstream method stopped reading), a malformed later PEM section ends the + * input instead of failing, and an extra certificate that cannot be parsed is left out of the + * chain. Unlike the upstream method, the input is decoded as UTF-8 rather than the platform default + * charset, it is read to the end, and the private key is verified against the leaf certificate's + * public key. */ @NullMarked final class MtlsKeyStoreUtils { diff --git a/google-auth-library-java/oauth2_http/javatests/com/google/auth/mtls/X509ProviderTest.java b/google-auth-library-java/oauth2_http/javatests/com/google/auth/mtls/X509ProviderTest.java index c4f6650a30da..b50f4c82507c 100644 --- a/google-auth-library-java/oauth2_http/javatests/com/google/auth/mtls/X509ProviderTest.java +++ b/google-auth-library-java/oauth2_http/javatests/com/google/auth/mtls/X509ProviderTest.java @@ -562,9 +562,9 @@ private static Path writeWorkloadConfig(Path tempDir, Path certFile, Path keyFil Files.write( config, ("{\"cert_configs\":{\"workload\":{\"cert_path\":\"" - + certFile.toString() + + certFile.toString().replace("\\", "\\\\") + "\",\"key_path\":\"" - + keyFile.toString() + + keyFile.toString().replace("\\", "\\\\") + "\"}}}") .getBytes(UTF_8)); return config; From 3da50e73ce865d31601aaba7e8a814e0eaee7cdf Mon Sep 17 00:00:00 2001 From: Matt Castelaz Date: Fri, 30 Oct 2026 03:02:39 +0000 Subject: [PATCH 3/3] fix(auth): scope verifyKeyPair catch to SignatureException and add deterministic tests --- .../google/auth/mtls/MtlsKeyStoreUtils.java | 36 +++++------ .../auth/mtls/MtlsKeyStoreUtilsTest.java | 60 ++++++++++++++++++- 2 files changed, 73 insertions(+), 23 deletions(-) diff --git a/google-auth-library-java/oauth2_http/java/com/google/auth/mtls/MtlsKeyStoreUtils.java b/google-auth-library-java/oauth2_http/java/com/google/auth/mtls/MtlsKeyStoreUtils.java index 25ef0d0eb5d7..78775f086bc9 100644 --- a/google-auth-library-java/oauth2_http/java/com/google/auth/mtls/MtlsKeyStoreUtils.java +++ b/google-auth-library-java/oauth2_http/java/com/google/auth/mtls/MtlsKeyStoreUtils.java @@ -42,12 +42,12 @@ import java.security.PrivateKey; import java.security.PublicKey; import java.security.Signature; +import java.security.SignatureException; import java.security.cert.Certificate; import java.security.cert.CertificateException; import java.security.cert.CertificateFactory; import java.security.cert.X509Certificate; import java.security.spec.PKCS8EncodedKeySpec; -import java.security.spec.X509EncodedKeySpec; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -175,38 +175,30 @@ private static void verifyKeyPair(X509Certificate cert, PrivateKey privateKey) String sigAlg; if ("RSA".equals(keyAlgorithm)) { sigAlg = "SHA256withRSA"; - } else if ("EC".equals(keyAlgorithm) || "ECDSA".equals(keyAlgorithm)) { + } else if ("EC".equals(keyAlgorithm)) { sigAlg = "SHA256withECDSA"; } else { return; } - boolean matches = false; - GeneralSecurityException failureCause = null; - try { - if ("ECDSA".equals(keyAlgorithm) && publicKey.getEncoded() != null) { - // SunEC rejects keys whose algorithm is "ECDSA" rather than "EC"; normalize the key. - publicKey = - KeyFactory.getInstance("EC") - .generatePublic(new X509EncodedKeySpec(publicKey.getEncoded())); - } + Signature signer = Signature.getInstance(sigAlg); + signer.initSign(privateKey); + signer.update(VERIFICATION_DATA); + byte[] signature = signer.sign(); - Signature signer = Signature.getInstance(sigAlg); - signer.initSign(privateKey); - signer.update(VERIFICATION_DATA); - byte[] signature = signer.sign(); - - Signature verifier = Signature.getInstance(sigAlg); - verifier.initVerify(publicKey); - verifier.update(VERIFICATION_DATA); + Signature verifier = Signature.getInstance(sigAlg); + verifier.initVerify(publicKey); + verifier.update(VERIFICATION_DATA); + boolean matches; + try { matches = verifier.verify(signature); - } catch (GeneralSecurityException e) { - failureCause = e; + } catch (SignatureException e) { + throw new GeneralSecurityException("Certificate and private key do not match", e); } if (!matches) { - throw new GeneralSecurityException("Certificate and private key do not match", failureCause); + throw new GeneralSecurityException("Certificate and private key do not match"); } } diff --git a/google-auth-library-java/oauth2_http/javatests/com/google/auth/mtls/MtlsKeyStoreUtilsTest.java b/google-auth-library-java/oauth2_http/javatests/com/google/auth/mtls/MtlsKeyStoreUtilsTest.java index 87308b416700..4df67b0c373f 100644 --- a/google-auth-library-java/oauth2_http/javatests/com/google/auth/mtls/MtlsKeyStoreUtilsTest.java +++ b/google-auth-library-java/oauth2_http/javatests/com/google/auth/mtls/MtlsKeyStoreUtilsTest.java @@ -34,6 +34,7 @@ import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -50,9 +51,12 @@ import java.security.KeyStore; import java.security.PrivateKey; import java.security.Signature; +import java.security.SignatureException; import java.security.cert.Certificate; import java.security.cert.CertificateException; +import java.security.cert.CertificateFactory; import java.security.cert.X509Certificate; +import java.security.interfaces.RSAPublicKey; import java.security.spec.InvalidKeySpecException; import java.util.Base64; import java.util.Collections; @@ -315,9 +319,45 @@ void createMtlsKeyStore_missingPrivateKey_throws() throws Exception { } @Test - void createMtlsKeyStore_mismatchedRsaKeyPair_throws() throws Exception { + void createMtlsKeyStore_mismatchedRsaKeyPairSameKeySize_throws() throws Exception { + // Generate a 1024-bit RSA key whose modulus is strictly smaller than SINGLE_CERT_PATH's + // 1024-bit RSA modulus, guaranteeing that Signature.verify() returns false (rather than + // throwing SignatureException on s >= n). + X509Certificate singleCert = + (X509Certificate) + CertificateFactory.getInstance("X.509") + .generateCertificate( + new ByteArrayInputStream(read(SINGLE_CERT_PATH).getBytes(UTF_8))); + RSAPublicKey certPublicKey = (RSAPublicKey) singleCert.getPublicKey(); KeyPairGenerator kpg = KeyPairGenerator.getInstance("RSA"); kpg.initialize(1024); + KeyPair otherRsaKeyPair; + do { + otherRsaKeyPair = kpg.generateKeyPair(); + } while (((RSAPublicKey) otherRsaKeyPair.getPublic()) + .getModulus() + .compareTo(certPublicKey.getModulus()) + >= 0); + String otherRsaKeyPem = toPkcs8Pem(otherRsaKeyPair.getPrivate()); + + GeneralSecurityException e = + assertThrows( + GeneralSecurityException.class, + () -> + MtlsKeyStoreUtils.createMtlsKeyStore( + stream(read(SINGLE_CERT_PATH), otherRsaKeyPem))); + assertEquals("Certificate and private key do not match", e.getMessage()); + assertNull(e.getCause()); + } + + @Test + void createMtlsKeyStore_mismatchedRsaKeyPairDifferentKeySize_throws() throws Exception { + // Pairing a 2048-bit RSA private key with the 1024-bit RSA SINGLE_CERT_PATH deterministically + // causes Signature.verify() to throw SignatureException ("Bad signature length: got 256 but was + // expecting 128"), which must be translated into GeneralSecurityException("Certificate and + // private key do not match"). + KeyPairGenerator kpg = KeyPairGenerator.getInstance("RSA"); + kpg.initialize(2048); KeyPair otherRsaKeyPair = kpg.generateKeyPair(); String otherRsaKeyPem = toPkcs8Pem(otherRsaKeyPair.getPrivate()); @@ -328,6 +368,7 @@ void createMtlsKeyStore_mismatchedRsaKeyPair_throws() throws Exception { MtlsKeyStoreUtils.createMtlsKeyStore( stream(read(SINGLE_CERT_PATH), otherRsaKeyPem))); assertEquals("Certificate and private key do not match", e.getMessage()); + assertTrue(e.getCause() instanceof SignatureException); } @Test @@ -337,6 +378,23 @@ void createMtlsKeyStore_mismatchedEcKeyPair_throws() throws Exception { KeyPair otherEcKeyPair = kpg.generateKeyPair(); String otherEcKeyPem = toPkcs8Pem(otherEcKeyPair.getPrivate()); + GeneralSecurityException e = + assertThrows( + GeneralSecurityException.class, + () -> + MtlsKeyStoreUtils.createMtlsKeyStore(stream(read(CHAIN_CERT_PATH), otherEcKeyPem))); + assertEquals("Certificate and private key do not match", e.getMessage()); + assertNull(e.getCause()); + } + + @Test + void createMtlsKeyStore_mismatchedEcKeyPairDifferentCurve_throws() throws Exception { + // Pairing a 384-bit (secp384r1) EC private key with the 256-bit (secp256r1) EC CHAIN_CERT_PATH. + KeyPairGenerator kpg = KeyPairGenerator.getInstance("EC"); + kpg.initialize(384); + KeyPair otherEcKeyPair = kpg.generateKeyPair(); + String otherEcKeyPem = toPkcs8Pem(otherEcKeyPair.getPrivate()); + GeneralSecurityException e = assertThrows( GeneralSecurityException.class,