Skip to content

Commit 215fabb

Browse files
authored
fix(auth): present the full certificate chain for mTLS (#14621)
## Why `X509Provider` and `SecureConnectProvider` build their mTLS key store with google-http-client's `SecurityUtils.createMtlsKeyStore`. That method keeps only the **first** `CERTIFICATE` in the PEM input, so Java presents only the leaf certificate in the TLS handshake. This is unchanged in the latest google-http-client release (2.2.0). - **Google APIs over `*.mtls.googleapis.com` are unaffected**, because Google's front ends already know the intermediates. - **Peers that trust only a root CA can't verify the leaf without the intermediate**, so they reject the handshake. - A common case is agent-to-agent mTLS, where the receiving agent trusts the SPIFFE trust bundle. - Agent identity certificates on GKE (and Cloud Run) are issued by an intermediate CA, so a Java agent that reuses the SDK's mTLS key store can't call such a peer. - Python sends the whole chain (`load_cert_chain`). This bug predates the agent identity work; the agent identity testing caught it. ## What changes **New package-private `MtlsKeyStoreUtils.createMtlsKeyStore(InputStream)`.** It replaces the upstream method. - **The change:** the key entry's chain holds **every distinct certificate**, in input order. The first certificate is still the leaf. - **Same as upstream:** key store type (JKS), alias, empty password, accepted key format (PKCS#8 `PRIVATE KEY`), which private key is chosen when there are several, and error messages. - **Also different from upstream:** the input is decoded as UTF-8 instead of the platform default charset, and it's read to the end. Upstream stopped once it had a certificate and a key. **Used by:** - `X509Provider.getKeyStore()`, on both the certificate-configuration path (Cloud Run, WIF X.509) and the GKE credential-bundle path; - `SecureConnectProvider.getKeyStore()` (ECP), which drains the bytes already buffered on the helper's stdout after the process exits so a background child process holding stdout open cannot block waiting for EOF. **Inputs that worked before still work.** Because more of the input is now parsed, once a certificate and a private key have been found (the point where upstream stopped reading): - a malformed later PEM section ends the input instead of failing; - an extra certificate that can't be parsed is left out of the chain (and logged at `FINE`). **Unchanged:** - exception wrapping in the providers; - the token side: the `cnf` binding is still the leaf thumbprint, read from the certificate file; - the public API (no clirr impact). **Known limitations (unchanged from before):** - GAX's deprecated `com.google.api.gax.rpc.mtls.MtlsProvider` (ECP-only) still uses the upstream method. GAX's transports don't use it; they use `com.google.auth.mtls.MtlsProvider` through `DefaultMtlsProviderFactory`. - There's no check that the private key matches the first certificate (same as upstream). ## Tests ### Unit tests - `MtlsKeyStoreUtilsTest` (new), using a generated test chain (root → intermediate → leaf, EC P-256, long validity) in `testresources/mtls/test_chain_{cert,key}.pem`: - upstream `SecurityUtils.createMtlsKeyStore` keeps only the leaf for the same input. This pins down the bug, and the test fails if upstream ever fixes it; - leaf + intermediate (and 3-certificate chains) in order, with the certificates first or the key first, and duplicate certificates deduplicated; - the private key matches the leaf (sign/verify); - a single certificate gives the same alias, type, certificate and key as `SecurityUtils.createMtlsKeyStore`; - non-certificate sections are ignored; - with several private keys, the same key is chosen as upstream: the last one before the first certificate, or the first one after it. Each of these tests runs upstream on the same input and compares the encoded key bytes; - a malformed trailing section after a certificate and key is ignored, while a malformed section after only a certificate or only a key throws `IllegalArgumentException`, and an `IOException` mid-stream propagates; - an unparsable or empty extra certificate is left out; - an unparsable leaf, a missing certificate, or a missing key throws, with the same messages as before. - `X509ProviderTest`: a GKE bundle with an intermediate (certificates first, and key first as GKE writes it), and a certificate config whose `cert_path` holds a chain, all give leaf + intermediate. - `SecureConnectProviderTest`: helper output with a chain gives leaf + intermediate, and only buffered stdout bytes are read after the process exits. The provider tests fail without the fix (`expected: <2> but was: <1>`). Full `oauth2_http` suite passes. `fmt` is clean. ### Live tests | Environment | Check | Before | After | |---|---|---|---| | GKE agent identity | Agent-to-agent mTLS using the SDK's `DefaultMtlsProviderFactory` key store, against a receiver that trusts only the SPIFFE root | `Broken pipe` (handshake rejected) | **200** | | GKE agent identity | GCS over `storage.mtls.googleapis.com`, GAX clients over gRPC and HTTP/JSON, bound tokens | pass | pass (unchanged) | | Workload identity federation X.509 (STS mTLS) | Token exchange with a leaf-only `cert_path` | success, leaf sent | success, leaf sent | | Workload identity federation X.509 (STS mTLS) | Token exchange with a leaf + intermediate `cert_path` | success, **leaf only** sent | success, **leaf + intermediate** sent | The certificates sent were confirmed from the TLS handshake debug log. The live tests ran on the first commit (`b2876a4d3a9`). Subsequent commits only refine how unusual or malformed inputs are handled, plus tests, so well-formed GKE and WIF files behave the same. **Internal links (Googlers only):** [live test results](https://paste.googleplex.com/5884065727774720) · GKE [setup](https://paste.googleplex.com/5635555912712192), [manifests](https://paste.googleplex.com/6258309762514944) and [harness](https://paste.googleplex.com/4882197073494016) (shared with #14595)
1 parent 4aad503 commit 215fabb

8 files changed

Lines changed: 706 additions & 11 deletions

File tree

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
/*
2+
* Copyright 2026 Google LLC
3+
*
4+
* Redistribution and use in source and binary forms, with or without
5+
* modification, are permitted provided that the following conditions are
6+
* met:
7+
*
8+
* * Redistributions of source code must retain the above copyright
9+
* notice, this list of conditions and the following disclaimer.
10+
* * Redistributions in binary form must reproduce the above
11+
* copyright notice, this list of conditions and the following disclaimer
12+
* in the documentation and/or other materials provided with the
13+
* distribution.
14+
* * Neither the name of Google LLC nor the names of its
15+
* contributors may be used to endorse or promote products derived from
16+
* this software without specific prior written permission.
17+
*
18+
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
19+
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
20+
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
21+
* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
22+
* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
23+
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
24+
* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
25+
* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
26+
* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
27+
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
28+
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
29+
*/
30+
31+
package com.google.auth.mtls;
32+
33+
import com.google.api.client.util.PemReader;
34+
import java.io.ByteArrayInputStream;
35+
import java.io.IOException;
36+
import java.io.InputStream;
37+
import java.io.InputStreamReader;
38+
import java.nio.charset.StandardCharsets;
39+
import java.security.GeneralSecurityException;
40+
import java.security.KeyFactory;
41+
import java.security.KeyStore;
42+
import java.security.PrivateKey;
43+
import java.security.cert.Certificate;
44+
import java.security.cert.CertificateException;
45+
import java.security.cert.CertificateFactory;
46+
import java.security.cert.X509Certificate;
47+
import java.security.spec.PKCS8EncodedKeySpec;
48+
import java.util.ArrayList;
49+
import java.util.Arrays;
50+
import java.util.List;
51+
import java.util.logging.Level;
52+
import java.util.logging.Logger;
53+
import org.jspecify.annotations.NullMarked;
54+
55+
/**
56+
* Builds the mTLS {@link KeyStore} from PEM-encoded certificates and a private key.
57+
*
58+
* <p>This replaces {@code com.google.api.client.util.SecurityUtils#createMtlsKeyStore}, which keeps
59+
* only the first certificate. Here the key entry holds every distinct certificate in the input, so
60+
* the intermediate certificates are presented too. Some peers (for example, another workload that
61+
* trusts only a root CA) need them to verify the client certificate. Because every certificate in
62+
* the input is sent in the TLS handshake, the input should hold only the leaf certificate and its
63+
* intermediates.
64+
*
65+
* <p>Everything else matches the upstream method: key store type (JKS), alias, empty password,
66+
* accepted key format (PKCS#8 {@code PRIVATE KEY}), which private key is chosen, and error
67+
* messages. Any input that the upstream method accepted is still accepted, even though more of it
68+
* is now used: once a certificate and a private key have been found (the point where the upstream
69+
* method stopped reading), a malformed later PEM section ends the input instead of failing, and an
70+
* extra certificate that cannot be parsed is left out of the chain. Unlike the upstream method, the
71+
* input is decoded as UTF-8 rather than the platform default charset, and it is read to the end.
72+
*/
73+
@NullMarked
74+
final class MtlsKeyStoreUtils {
75+
76+
private static final Logger LOGGER = Logger.getLogger(MtlsKeyStoreUtils.class.getName());
77+
78+
private MtlsKeyStoreUtils() {}
79+
80+
/**
81+
* Creates a key store holding one private key entry.
82+
*
83+
* @param certAndKey PEM input containing one or more {@code CERTIFICATE} sections and a PKCS#8
84+
* {@code PRIVATE KEY} section, in any order. The first certificate must be the one that
85+
* matches the private key (the leaf); the remaining distinct certificates are kept in input
86+
* order as the rest of the chain. If there are several private keys, the one used is the same
87+
* as upstream: the last one before the first certificate, or else the first one after it. The
88+
* stream is read to the end and is not closed.
89+
* @throws IllegalArgumentException if the input has no certificate or no private key, or if a PEM
90+
* section before both a certificate and a private key have been read is malformed
91+
* @throws GeneralSecurityException if the first certificate or the private key cannot be parsed,
92+
* or the key store cannot be created
93+
* @throws IOException if the input cannot be read
94+
*/
95+
static KeyStore createMtlsKeyStore(InputStream certAndKey)
96+
throws GeneralSecurityException, IOException {
97+
List<byte[]> certificates = new ArrayList<>();
98+
byte[] privateKey = null;
99+
PemReader reader = new PemReader(new InputStreamReader(certAndKey, StandardCharsets.UTF_8));
100+
while (true) {
101+
PemReader.Section section;
102+
try {
103+
section = reader.readNextSection();
104+
} catch (IllegalArgumentException e) {
105+
if (!certificates.isEmpty() && privateKey != null) {
106+
// Malformed trailing content after a usable certificate and key: stop here, as the
107+
// upstream method (which stopped reading at this point) would have.
108+
break;
109+
}
110+
throw e;
111+
}
112+
if (section == null) {
113+
break;
114+
}
115+
if ("CERTIFICATE".equals(section.getTitle())) {
116+
byte[] certBytes = section.getBase64DecodedBytes();
117+
if (!containsCertificate(certificates, certBytes)) {
118+
certificates.add(certBytes);
119+
}
120+
} else if ("PRIVATE KEY".equals(section.getTitle())
121+
&& (privateKey == null || certificates.isEmpty())) {
122+
// Same choice as upstream: a later key replaces an earlier one until the first certificate
123+
// has been seen.
124+
privateKey = section.getBase64DecodedBytes();
125+
}
126+
}
127+
if (certificates.isEmpty()) {
128+
throw new IllegalArgumentException("certificate is missing from certAndKey string");
129+
}
130+
if (privateKey == null) {
131+
throw new IllegalArgumentException("private key is missing from certAndKey string");
132+
}
133+
134+
CertificateFactory certFactory = CertificateFactory.getInstance("X.509");
135+
List<X509Certificate> chain = new ArrayList<>(certificates.size());
136+
chain.add(parseCertificate(certFactory, certificates.get(0)));
137+
for (int i = 1; i < certificates.size(); i++) {
138+
try {
139+
chain.add(parseCertificate(certFactory, certificates.get(i)));
140+
} catch (CertificateException | RuntimeException e) {
141+
// The upstream method never parsed extra certificates; leave this one out rather than
142+
// fail.
143+
LOGGER.log(Level.FINE, "Skipping unparsable extra certificate in mTLS chain", e);
144+
}
145+
}
146+
PrivateKey key =
147+
KeyFactory.getInstance(chain.get(0).getPublicKey().getAlgorithm())
148+
.generatePrivate(new PKCS8EncodedKeySpec(privateKey));
149+
150+
KeyStore keyStore = KeyStore.getInstance("JKS");
151+
keyStore.load(null);
152+
keyStore.setKeyEntry("alias", key, new char[] {}, chain.toArray(new X509Certificate[0]));
153+
return keyStore;
154+
}
155+
156+
private static boolean containsCertificate(List<byte[]> certificates, byte[] candidate) {
157+
for (byte[] existing : certificates) {
158+
if (Arrays.equals(existing, candidate)) {
159+
return true;
160+
}
161+
}
162+
return false;
163+
}
164+
165+
private static X509Certificate parseCertificate(CertificateFactory certFactory, byte[] der)
166+
throws CertificateException {
167+
Certificate cert = certFactory.generateCertificate(new ByteArrayInputStream(der));
168+
if (cert == null) {
169+
throw new CertificateException("Empty certificate");
170+
}
171+
return (X509Certificate) cert;
172+
}
173+
}

‎google-auth-library-java/oauth2_http/java/com/google/auth/mtls/SecureConnectProvider.java‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,11 @@
3232

3333
import com.google.api.client.json.JsonParser;
3434
import com.google.api.client.json.gson.GsonFactory;
35-
import com.google.api.client.util.SecurityUtils;
3635
import com.google.api.core.InternalApi;
3736
import com.google.common.annotations.VisibleForTesting;
3837
import com.google.common.collect.ImmutableList;
38+
import java.io.ByteArrayInputStream;
39+
import java.io.ByteArrayOutputStream;
3940
import java.io.FileInputStream;
4041
import java.io.FileNotFoundException;
4142
import java.io.IOException;
@@ -147,8 +148,25 @@ static KeyStore getKeyStore(InputStream metadata, ProcessProvider processProvide
147148
"SecureConnect: Cert provider command failed with exit code: " + exitCode);
148149
}
149150

150-
// Create mTLS key store with the input certificates from shell command.
151-
return SecurityUtils.createMtlsKeyStore(process.getInputStream());
151+
// Create mTLS key store with the input certificates from shell command. Read only the bytes
152+
// already buffered now that the process has exited, so a background child process holding
153+
// stdout open cannot block waiting for EOF.
154+
ByteArrayOutputStream stdout = new ByteArrayOutputStream();
155+
try (InputStream processOut = process.getInputStream()) {
156+
if (processOut != null) {
157+
for (int available = processOut.available();
158+
available > 0;
159+
available = processOut.available()) {
160+
byte[] buffer = new byte[available];
161+
int read = processOut.read(buffer);
162+
if (read <= 0) {
163+
break;
164+
}
165+
stdout.write(buffer, 0, read);
166+
}
167+
}
168+
}
169+
return MtlsKeyStoreUtils.createMtlsKeyStore(new ByteArrayInputStream(stdout.toByteArray()));
152170
}
153171

154172
@VisibleForTesting

‎google-auth-library-java/oauth2_http/java/com/google/auth/mtls/X509Provider.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030

3131
package com.google.auth.mtls;
3232

33-
import com.google.api.client.util.SecurityUtils;
3433
import com.google.api.core.InternalApi;
3534
import com.google.auth.oauth2.EnvironmentProvider;
3635
import com.google.auth.oauth2.PropertyProvider;
@@ -143,7 +142,8 @@ public X509Provider() {
143142
* which contains both the certificate chain and the private key.
144143
* </ul>
145144
*
146-
* @return a KeyStore containing the X.509 certificate specified by the certificate configuration.
145+
* @return a KeyStore containing the X.509 certificate chain and private key from the certificate
146+
* source above.
147147
* @throws CertificateSourceUnavailableException if the certificate source is unavailable (ex.
148148
* missing configuration file)
149149
* @throws IOException if a general I/O error occurs while creating the KeyStore
@@ -169,7 +169,7 @@ public KeyStore getKeyStore() throws CertificateSourceUnavailableException, IOEx
169169
MtlsUtils.getGkeCredentialBundlePath(envProvider, propProvider);
170170
if (gkeCredentialBundlePath != null) {
171171
try (InputStream bundleStream = new FileInputStream(new File(gkeCredentialBundlePath))) {
172-
return SecurityUtils.createMtlsKeyStore(bundleStream);
172+
return MtlsKeyStoreUtils.createMtlsKeyStore(bundleStream);
173173
} catch (FileNotFoundException e) {
174174
// The bundle was removed (e.g. mid-rotation) after the availability check.
175175
throw new CertificateSourceUnavailableException(
@@ -201,7 +201,7 @@ public KeyStore getKeyStore() throws CertificateSourceUnavailableException, IOEx
201201
new SequenceInputStream(certStream, privateKeyStream)) {
202202

203203
// Build a key store using the combined stream.
204-
return SecurityUtils.createMtlsKeyStore(certAndPrivateKeyStream);
204+
return MtlsKeyStoreUtils.createMtlsKeyStore(certAndPrivateKeyStream);
205205
} catch (CertificateSourceUnavailableException e) {
206206
// Throw the CertificateSourceUnavailableException without wrapping.
207207
throw e;

0 commit comments

Comments
 (0)