[macastelaz]

Summary

Verifies that the parsed PrivateKey cryptographically matches the leaf X509Certificate's PublicKey in MtlsKeyStoreUtils.createMtlsKeyStore(), and adds retry handling (3 attempts with a 100ms backoff, matching AgentIdentityUtils.loadAndVerifyCredentials()) in X509Provider.getKeyStore() when loading separate cert_path and key_path files from disk.

Problem

1. Unverified KeyStore entries: KeyStore.getInstance("JKS").setKeyEntry("alias", key, new char[] {}, chain) does not verify that key corresponds to chain[0].getPublicKey() as long as the key algorithm matches (e.g., both are RSA or both are EC).
2. Non-atomic rotation across separate cert_path and key_path files: When an external certificate rotator updates cert_path (cert_A -> cert_B) slightly before key_path (key_A -> key_B), transport channel refresh (CertificateRotationTracker, which tracks cert_path's SHA-256 fingerprint) can trigger X509Provider.getKeyStore() during the intermediate (cert_B, key_A) window. Without key-pair verification and retries, getKeyStore() succeeds with a mismatched KeyStore and the channel marks FP(cert_B) as refreshed, leaving the mTLS channel with a mismatched certificate and private key until the next rotation.

Solution

• MtlsKeyStoreUtils.createMtlsKeyStore(): Perform a test signature and verification (SHA256withRSA for RSA keys, SHA256withECDSA for EC keys) to ensure the parsed private key matches the leaf certificate's public key, throwing GeneralSecurityException("Certificate and private key do not match") on mismatch.
• X509Provider.getKeyStore(): Retry up to 3 times (100ms interval, with injectable TimeService for deterministic testing) when reading and verifying separate cert_path and key_path files from WorkloadCertificateConfiguration, handling transient mid-rotation key mismatches or partial file writes while preserving interrupt restoration (InterruptedIOException) on InterruptedException.

Testing

• Unit Tests:
  • Added unit tests in MtlsKeyStoreUtilsTest covering mismatched RSA (same and different key sizes) and EC key pairs, as well as pass-through for non-RSA/EC algorithms (e.g. DSA).
  • Added deterministic unit tests in X509ProviderTest covering:
    • Retrying and succeeding when key_path finishes rotating to match cert_path on a subsequent attempt (1 sleep of 100ms).
    • Failing with IOException caused by GeneralSecurityException("Certificate and private key do not match") after exhausting all 3 attempts (2 sleeps of 100ms).
    • Retrying and succeeding when cert_path is transiently empty/truncated on the first attempt.
    • Restoring the thread interrupt flag and failing fast with InterruptedIOException (0 subsequent attempts) if interrupted during retry backoff.
    • Single-attempt execution (0 sleeps) for the GKE combined credential bundle (x509.credential-bundle.private-key.pem).
• Manual & Live Cloud Run Verification:
  • Full test setup, 5-scenario local end-to-end simulation output, and live Cloud Run Agent Identity (--functional-type=agent --identity-type=agent-identity) execution logs (agent-bound-token-java-test-28m2v): https://paste.googleplex.com/6755399436524638

[gemini-code-assist]

Code Review

This pull request introduces key pair verification when creating an mTLS KeyStore to ensure the private key matches the leaf certificate's public key. It also adds retry logic in X509Provider to handle transient mismatches during atomic key rotations. The review feedback recommends separating cryptographic setup/execution failures from actual key verification mismatches to prevent misleading error messages, and advises against adding out-of-scope algorithm normalization (such as converting 'ECDSA' to 'EC') to maintain behavioral alignment.

[nbayati]

Left a comment on possibly reconciling the cert-key matching logic between this flow and the agentic path that already has it implemented.

Also the PR description mentions a 0-sleep GKE bundle test in X509ProviderTest and a DSA pass-through test in MtlsKeyStoreUtilsTest that don't seem to be in the diff (the GKE bundle one seems worth adding).

[nbayati]

This cert-key check already existed in the Agent Identity flow, and this PR duplicates some of that logic. The two copies have already diverged (for example on "ECDSA" handling and how errors are treated; not sure if that was intentional).

Can we share the core check in a single internal helper (e.g. in MtlsUtils)?

[macastelaz]

Good call — extracted the shared check into @internalapi MtlsUtils.verifyKeyPair(X509Certificate, PrivateKey) in 17102f6 (including the "ECDSA" → "EC" KeyFactory normalization for SunEC). Both MtlsKeyStoreUtils.createMtlsKeyStore and AgentIdentityCertificateValidationUtils.verifyKeyPair now delegate to MtlsUtils.verifyKeyPair, while AgentIdentityCertificateValidationUtils retains its strict RSA/EC/ECDSA algorithm allowlist (UnsupportedKeyAlgorithmException) and boolean/logging contract.

[nbayati]

nit: The description says an interrupt surfaces as InterruptedIOException, which is what MtlsUtils throws, but this throws a plain IOException. Callers can't tell an interrupt apart from a read failure without digging into the cause.

[macastelaz]

Fixed! X509Provider.getKeyStore() now throws InterruptedIOException (with initCause(interrupted)) to match MtlsUtils and the PR description, and X509ProviderTest now asserts InterruptedIOException.class.

[macastelaz]

Thanks for the review @nbayati!

1. Consolidated the certificate/private-key matching logic into MtlsUtils.verifyKeyPair(X509Certificate, PrivateKey) and updated both MtlsKeyStoreUtils and AgentIdentityCertificateValidationUtils to delegate to it.

2. Updated X509Provider.getKeyStore() to throw InterruptedIOException (with initCause(interrupted)) when interrupted during retry sleep.

3. Added both tests from the PR description: x509Provider_gkeCredentialBundleWithMismatchedKeyPair_throwsWithoutRetrying in X509ProviderTest (verifying 0 sleeps on a mismatched GKE combined bundle) and createMtlsKeyStore_nonRsaOrEcKeyPair_passesThroughLikeUpstream in MtlsKeyStoreUtilsTest (verifying DSA pass-through).