Skip to content

Commit 9be88f6

Browse files
committed
fix(auth,gax): align mTLS certificate discovery and error handling with go/sdk-mtls-by-default-cert-discovery
Address PR 13995 review feedback from @nbayati: - Align discovery and error behavior with go/sdk-mtls-by-default-cert-discovery: - Fail closed (IllegalStateException) when GOOGLE_API_CERTIFICATE_CONFIG points to a missing, unreadable, malformed, or missing cert/key configuration. - Safe fallback (return null) when implicit default gcloud config is missing or is an ECP-only configuration without a workload block. - Fail closed with clear source identification if default gcloud config is unreadable, malformed, or points to missing cert/key files. - Replace .exists() with .isFile() && .canRead() checks across config, certificate, and key paths. - Make getGkeWorkloadCertPath and getGceWorkloadCertPath package-private stubs returning null with explanatory comments for phased rollout. - Explicitly identify the resolution source (GOOGLE_API_CERTIFICATE_CONFIG vs default gcloud location) in all error messages. - Update getCertificatePath exception message to reference 'cert_configs.workload.cert_path' rather than legacy 'certificate_file'. - Add comprehensive test coverage in MtlsUtilsTest and CertificateBasedAccessTest.
1 parent be0a495 commit 9be88f6

3 files changed

Lines changed: 435 additions & 56 deletions

File tree

google-auth-library-java/oauth2_http/java/com/google/auth/mtls/MtlsUtils.java

Lines changed: 89 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -79,47 +79,78 @@ public static boolean useMtlsClientCertificate(
7979
return null;
8080
}
8181

82-
String certConfigPath = envProvider.getEnv(CERTIFICATE_CONFIGURATION_ENV_VARIABLE);
83-
if (!Strings.isNullOrEmpty(certConfigPath)) {
82+
String explicitConfigPath = envProvider.getEnv(CERTIFICATE_CONFIGURATION_ENV_VARIABLE);
83+
84+
// 1. Explicit Configuration Path (Fail Closed)
85+
if (!Strings.isNullOrEmpty(explicitConfigPath)) {
86+
File configFile = new File(explicitConfigPath);
87+
if (!configFile.exists()) {
88+
throw new IllegalStateException(
89+
"Certificate configuration file specified via GOOGLE_API_CERTIFICATE_CONFIG at '"
90+
+ explicitConfigPath
91+
+ "' does not exist.");
92+
}
93+
if (!configFile.isFile() || !configFile.canRead()) {
94+
throw new IllegalStateException(
95+
"Failed to read certificate configuration file specified via"
96+
+ " GOOGLE_API_CERTIFICATE_CONFIG at '"
97+
+ explicitConfigPath
98+
+ "'.");
99+
}
84100
try {
85101
WorkloadCertificateConfiguration config =
86-
getWorkloadCertificateConfiguration(envProvider, propProvider, certConfigPath);
87-
88-
File certFile = new File(config.getCertPath());
89-
File keyFile = new File(config.getPrivateKeyPath());
90-
if (!certFile.exists() || !keyFile.exists()) {
91-
throw new IllegalStateException(
92-
"Certificate config points to certificate/key files that do not exist on disk: "
93-
+ "cert_path="
94-
+ config.getCertPath()
95-
+ ", key_path="
96-
+ config.getPrivateKeyPath());
97-
}
102+
getWorkloadCertificateConfiguration(envProvider, propProvider, explicitConfigPath);
103+
validateCertAndKeyFiles(config, explicitConfigPath, false);
98104
return config.getCertPath();
99105
} catch (CertificateSourceUnavailableException e) {
100-
// Certificate config file does not exist on disk -> safe fallback
106+
// ECP / PKCS11 configuration without workload section; safe fallback
107+
return null;
101108
} catch (IllegalStateException e) {
102109
throw e;
103110
} catch (Exception e) {
104-
throw new IllegalStateException("Failed to parse certificate config: " + certConfigPath, e);
111+
throw new IllegalStateException(
112+
"Certificate configuration file specified via GOOGLE_API_CERTIFICATE_CONFIG at '"
113+
+ explicitConfigPath
114+
+ "' is malformed: "
115+
+ e.getMessage(),
116+
e);
117+
}
118+
}
119+
120+
// 2. Implicit / Default gcloud Configuration Path
121+
File defaultConfigFile = null;
122+
try {
123+
defaultConfigFile = getWellKnownCertificateConfigFile(envProvider, propProvider);
124+
} catch (IOException e) {
125+
// APPDATA missing on Windows, etc. Safe fallback.
126+
}
127+
if (defaultConfigFile != null && defaultConfigFile.exists()) {
128+
if (!defaultConfigFile.isFile() || !defaultConfigFile.canRead()) {
129+
throw new IllegalStateException(
130+
"Default certificate configuration file at '"
131+
+ defaultConfigFile.getAbsolutePath()
132+
+ "' exists but could not be read.");
105133
}
106-
} else {
107134
try {
108135
WorkloadCertificateConfiguration config =
109136
getWorkloadCertificateConfiguration(envProvider, propProvider, null);
110-
File certFile = new File(config.getCertPath());
111-
File keyFile = new File(config.getPrivateKeyPath());
112-
if (certFile.exists() && keyFile.exists()) {
113-
return config.getCertPath();
114-
}
137+
validateCertAndKeyFiles(config, defaultConfigFile.getAbsolutePath(), true);
138+
return config.getCertPath();
115139
} catch (CertificateSourceUnavailableException e) {
116-
// Well-known gcloud certificate_config.json does not exist. Safe fallback to
117-
// SPIFFE/well-known paths.
140+
// ECP-only configuration without workload section; safe fallback
141+
} catch (IllegalStateException e) {
142+
throw e;
118143
} catch (Exception e) {
119-
// Ignore parsing errors for well-known config fallback
144+
throw new IllegalStateException(
145+
"Default certificate configuration file at '"
146+
+ defaultConfigFile.getAbsolutePath()
147+
+ "' is malformed: "
148+
+ e.getMessage(),
149+
e);
120150
}
121151
}
122152

153+
// 3. Platform SPIFFE Fallbacks (Stubs)
123154
String gkeCertPath = getGkeWorkloadCertPath();
124155
if (gkeCertPath != null) {
125156
return gkeCertPath;
@@ -133,24 +164,40 @@ public static boolean useMtlsClientCertificate(
133164
return null;
134165
}
135166

136-
/** Dedicated GKE Fallback Resolution Path */
137-
public static @Nullable String getGkeWorkloadCertPath() {
138-
String gkePath = "/var/run/secrets/workload-spiffe-credentials";
139-
File bundleFile = new File(gkePath, "credentialbundle.pem");
140-
if (bundleFile.exists()) {
141-
return bundleFile.getAbsolutePath();
167+
private static void validateCertAndKeyFiles(
168+
WorkloadCertificateConfiguration config, String configPath, boolean isDefaultConfig) {
169+
File certFile = new File(config.getCertPath());
170+
File keyFile = new File(config.getPrivateKeyPath());
171+
if (!certFile.isFile() || !certFile.canRead() || !keyFile.isFile() || !keyFile.canRead()) {
172+
String sourcePrefix =
173+
isDefaultConfig
174+
? "referenced by default configuration '"
175+
: "referenced by configuration '";
176+
throw new IllegalStateException(
177+
"Failed to read certificate/key file at '"
178+
+ config.getCertPath()
179+
+ "' or '"
180+
+ config.getPrivateKeyPath()
181+
+ "' "
182+
+ sourcePrefix
183+
+ configPath
184+
+ "'.");
142185
}
186+
}
187+
188+
/** Dedicated GKE Fallback Resolution Path */
189+
static @Nullable String getGkeWorkloadCertPath() {
190+
// GKE workload certificate resolution is temporarily disabled (returns null)
191+
// pending Phase 1 rollout of bound token support on GKE
192+
// (go/agentic-bound-token-sdk-rollout-plan).
143193
return null;
144194
}
145195

146196
/** Dedicated GCE Fallback Resolution Path */
147-
public static @Nullable String getGceWorkloadCertPath() {
148-
String gcePath = "/var/run/secrets/workload-spiffe-credentials";
149-
File certFile = new File(gcePath, "certificates.pem");
150-
File keyFile = new File(gcePath, "private_key.pem");
151-
if (certFile.exists() && keyFile.exists()) {
152-
return certFile.getAbsolutePath();
153-
}
197+
static @Nullable String getGceWorkloadCertPath() {
198+
// GCE workload certificate resolution is temporarily disabled (returns null)
199+
// pending Phase 2 rollout of bound token support on GCE
200+
// (go/agentic-bound-token-sdk-rollout-plan).
154201
return null;
155202
}
156203

@@ -160,7 +207,7 @@ public static boolean useMtlsClientCertificate(
160207
return null;
161208
}
162209
File file = new File(certPath);
163-
if (!file.exists()) {
210+
if (!file.isFile() || !file.canRead()) {
164211
return null;
165212
}
166213
try {
@@ -199,8 +246,8 @@ public static String getCertificatePath(
199246
.getCertPath();
200247
if (Strings.isNullOrEmpty(certPath)) {
201248
throw new CertificateSourceUnavailableException(
202-
"Certificate configuration loaded successfully, but does not contain a 'certificate_file'"
203-
+ " path.");
249+
"Certificate configuration loaded successfully, but does not contain a"
250+
+ " 'cert_configs.workload.cert_path' path.");
204251
}
205252
return certPath;
206253
}
@@ -236,7 +283,7 @@ static WorkloadCertificateConfiguration getWorkloadCertificateConfiguration(
236283
}
237284
}
238285

239-
if (!certConfig.isFile()) {
286+
if (!certConfig.isFile() || !certConfig.canRead()) {
240287
throw new CertificateSourceUnavailableException(
241288
"Certificate configuration file does not exist or is not a file: "
242289
+ certConfig.getAbsolutePath());

0 commit comments

Comments
 (0)