- Notifications
You must be signed in to change notification settings - Fork 68
fix(mtls): Fix EndpointContext's determineEndpoint logic to respect env var #3912
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| } catch (IOException e) { | ||
| LOG.log( | ||
| Level.WARNING, | ||
| "DefaultMtlsProviderFactory encountered unexpected IOException: " + e.getMessage()); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There may be a reason for this and I've since lost the context for this. Do you remember why this is logged and not thrown back to the user?
This IOException occurs when trying to create the MtlsProvider when a user enables Mtls (via Env Var).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this warning is logged when the SDK detects the necessary mTLS config on the device, but the mTLS helper fails unexpectedly (ex. WARNING: DefaultMtlsProviderFactory encountered unexpected IOException: SecureConnect: Cert provider command failed with exit code: 139). In this case, mTLS is still disabled, the app doesn't crash, but a warning is logged because the helper command should not be failing - this reveals an underlying bug with the helper that should be fixed by the owning team. In other words, WARN if the user intends to use mTLS and it fails, and NO WARN if user never intended to use mTLS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, in that case can the logs add some like mTLS is disabled and falling back to TLS?
Would there be any possible security implications where we fall back to TLS and not use mTLS without erroring out? This may be already covered/ approved, but I'm wondering if the user doesn't see this warning and assumes that they're running on mTLS when it turns out it isn't.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question - what happens is that if the user (or the workload) is under CAA-policy enforcement ("require mTLS"), they will receive an "Access Denied" error when trying to access GCP resources. I will include the message you suggested to help with debugging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One follow up question for this. What about if the user/ workload is not under CAA-policy enforcement? Or is that going to be enabled for everyone in the future?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the user/workload is not under CAA-policy enforcement, then the requests will simply succeed even when hitting the non-mTLS endpoints.
| /gcbrun |
| /gcbrun |
| there looks to be some flaky tests in the CI. Re-running the jobs. |
| /gcbrun |
🤖 I have created a release *beep* *boop* --- <details><summary>2.62.3</summary> ## [2.62.3](v2.62.2...v2.62.3) (2025-10-02) ### Bug Fixes * **mtls:** Fix EndpointContext's determineEndpoint logic to respect env var ([#3912](#3912)) ([e5948d0](e5948d0)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Mridula <66699525+mpeddada1@users.noreply.github.com>
Only attempt to create a default MtlsProvider in "determineEndpoint" if client certificate usage is enabled by the env var GOOGLE_API_USE_CLIENT_CERTIFICATE.
Advisory Note: GOOGLE_API_USE_CLIENT_CERTIFICATE will default to true (if not set) in a future release, so if you do not wish to enable mTLS (such as for testing environments) please explicitly set GOOGLE_API_USE_CLIENT_CERTIFICATE to false before executing your test suite.
Fixes #3911 ☕️