feat(mtls): Introduce DefaultMtlsProviderFactory and SecureConnectProvider #1730

andyrzhao · 2025-04-07T21:03:07Z

This PR allows the Java auth lib to provide either an X509 mTLS provider or SecureConnect mTLS provider based on availability and under a common "MtlsProvider" interface. The Java GAX lib will be refactored to use this factory and interface in a subsequent PR.

…vider

lqiu96 · 2025-04-09T17:33:58Z

oauth2_http/java/com/google/auth/mtls/DefaultMtlsProviderFactory.java

+        mtlsProvider = new SecureConnectProvider();
+        mtlsProvider.getKeyStore();
+        return mtlsProvider;


Can SecureConnectProvider be added in a future PR? Or is this required for this PR? From my last discussion with Alex, the requirements was the inclusion of X.509 and having the Auth library STS functionality use the X.509 MtlsProvider.

I'm a bit concerned that SecureConnectProvider will exist in both Gax and Auth at the same time. I know that in the future that will commit to this existing in this repo.

Thanks for taking a look Lawrence! So this PR is complementary to Alex's work to support X.509-based ADC auth. The goal of this PR is to bring Java SDK to feature parity with Golang SDK and Python SDK, both of which supports X.509-based ADC auth as well as X.509-backed mTLS transports (alongside SecureConnect backed mTLS transports). This is basically the final checkbox that allows the CBA team to declare client-side GA w.r.t X.509 work.

I think the fact that the SecureConnectProvider will exist in both Gax and Auth at the same time (in the near term) is a small inconvenience that we can solve with documentation updates. Btw, you will note that the SecureConnectProvider implementation in this CL does not have any of the "CBA environment variable look up logic" present in the gax version - this is intentional: I've already started a separate refactoring effort on the GAX side to create a new helper class "CertificateBasedAccess" that will handle all env var checks and endpoint calculations. This is necessary to ensure that both X.509 mTLS and SecureConnect mTLS are gated behind the same env var variables, regardless of which mTLS provider is used.

Ok, I think the SecureConnectProvider duplication is OK for now, given that the implementation isn't intended to be used by customers and we don't expose this on the public surface for customers to set anyways. There shouldn't be any confusion from their POV.

I'll need to double check this behavior in our downstream libraries, but i'm 90% confident that it can be fine for now.

Looks like although we do not expose the public surface for customers to set, downstream handwritten libraries have handles to set MtlsProvider from EndpointContext or InstantiatingGrpcChannelProvider? (I don't know if that's something touched in your related GAX changes?) It is unlikely a handwritten library used it, but can you double check this?

mTLSProvder references in EndpointContext and InstantiatingGrpcChannelProviders are internal logic that was authored around the same time the original mTLS support was added, and I'm 99% sure there are no one else is using the set MtlsProvider functionality (it's mainly used by unit tests from the same suite as far as I can tell.) We can check it again when working on the Gax refactoring. This PR alone should not introduce any breaking changes.

oauth2_http/java/com/google/auth/mtls/ContextAwareMetadataJson.java

oauth2_http/java/com/google/auth/mtls/DefaultMtlsProviderFactory.java

oauth2_http/java/com/google/auth/mtls/MtlsProvider.java

zhumin8

Taking a first round.

oauth2_http/java/com/google/auth/mtls/DefaultMtlsProviderFactory.java

zhumin8 · 2025-05-12T15:04:33Z

oauth2_http/java/com/google/auth/mtls/DefaultMtlsProviderFactory.java

+        mtlsProvider.getKeyStore();
+        return mtlsProvider;
+      } catch (CertificateSourceUnavailableException ex) {
+        throw new CertificateSourceUnavailableException(


Is it useful here to wrap the the original exception? It's usually good practice for debugging to wrap the original exception (the one from X509Provider or SecureConnectProvider) as the cause.

So with the update to use "isCertificateSourceAvailable()", we no longer have the raw exceptions here. I don't have a strong preference here either-way - this is a non-blocking error message for the general use-case.

oauth2_http/java/com/google/auth/mtls/X509Provider.java

zhumin8 · 2025-05-12T15:27:22Z

oauth2_http/java/com/google/auth/mtls/SecureConnectProvider.java

+    while (remainTime > 0) {
+      Thread.sleep(Math.min(remainTime + 1, 100));
+      remainTime -= System.currentTimeMillis() - startTime;


Instead of manual timeout logic implementation, for Java 8+, Process.waitFor(long timeout, TimeUnit unit) is the standard and much cleaner way to wait for a process with a timeout.
If check "every 100 milliseconds" itself is not a requirement, this would simplify logic here, to something along the lines of

static int runCertificateProviderCommand(Process commandProcess, long timeoutMilliseconds) throws IOException, InterruptedException { boolean terminated = commandProcess.waitFor(timeoutMilliseconds, TimeUnit.MILLISECONDS); if (!terminated) { commandProcess.destroy(); throw new IOException("Cert provider command timed out"); } return commandProcess.exitValue(); }

oauth2_http/java/com/google/auth/mtls/SecureConnectProvider.java

zhumin8 · 2025-05-12T15:40:55Z

oauth2_http/java/com/google/auth/mtls/SecureConnectProvider.java

+      if (metadata == null) {
+        return null;
+      }


if metadata == null a valid scenario? If so, when later calling methods on process can lead to NullPointerException?

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

Lines 114 to 123 in 7a2b2ce

Process process = processProvider.createProcess(metadata);

// Run the command and timeout after 1000 milliseconds.

int exitCode = runCertificateProviderCommand(process, 1000);

if (exitCode != 0) {

throw new IOException("Cert provider command failed with exit code: " + exitCode);

}

// Create mTLS key store with the input certificates from shell command.

return SecurityUtils.createMtlsKeyStore(process.getInputStream());

Good point! "metadata == null" in createProcess is not a case we expect to reach within the current code logic, since "new FileInputStream" would result in "FileNotFoundException" first. To safeguard against null pointer possibility, I updated "return null" to "throw new IOException("Error creating Process: metadata is null");"

zhumin8 · 2025-05-12T15:51:28Z

oauth2_http/java/com/google/auth/mtls/DefaultMtlsProviderFactory.java

+        mtlsProvider = new SecureConnectProvider();
+        mtlsProvider.getKeyStore();
+        return mtlsProvider;


Looks like although we do not expose the public surface for customers to set, downstream handwritten libraries have handles to set MtlsProvider from EndpointContext or InstantiatingGrpcChannelProvider? (I don't know if that's something touched in your related GAX changes?) It is unlikely a handwritten library used it, but can you double check this?

oauth2_http/java/com/google/auth/mtls/SecureConnectProvider.java

zhumin8 · 2025-05-16T19:12:49Z

oauth2_http/javatests/com/google/auth/mtls/SecureConnectProviderTest.java

+import java.util.List;
+import org.junit.Test;
+
+public class SecureConnectProviderTest {


Does it makes sense to create similar tests for the new X509Provider?

So X509ProviderTest already exists (and was written by AION team), which should suffice for now?

If possible, can we also have tests for DefaultMtlsProviderFactory?

I think some small tests to cover the fallback logic and a test to cover if both options fail? Hopefully it's not too much work to add these cases.

zhumin8

LGTM

oauth2_http/java/com/google/auth/mtls/DefaultMtlsProviderFactory.java

oauth2_http/java/com/google/auth/mtls/ContextAwareMetadataJson.java

oauth2_http/java/com/google/auth/mtls/X509Provider.java

oauth2_http/java/com/google/auth/mtls/ContextAwareMetadataJson.java

oauth2_http/java/com/google/auth/mtls/MtlsProvider.java

lqiu96

Changes generally LGTM. Added a few small comments if you could take a look at.

andyrzhao · 2025-05-30T18:15:29Z

Changes generally LGTM. Added a few small comments if you could take a look at.

Thanks for the thorough review Lawrence and Min!

| Package | Type | Package file | Manager | Update | Change | |---|---|---|---|---|---| | [io.netty:netty-handler](https://netty.io/) ([source](https://github.com/netty/netty)) | dependencies | misk/gradle/libs.versions.toml | gradle | patch | `4.2.1.Final` -> `4.2.2.Final` | | [io.netty:netty-bom](https://netty.io/) ([source](https://github.com/netty/netty)) | dependencies | misk/gradle/libs.versions.toml | gradle | patch | `4.2.1.Final` -> `4.2.2.Final` | | [com.google.auth:google-auth-library-oauth2-http](https://github.com/googleapis/google-auth-library-java) | dependencies | misk/gradle/libs.versions.toml | gradle | minor | `1.36.0` -> `1.37.0` | | [com.google.auth:google-auth-library-credentials](https://github.com/googleapis/google-auth-library-java) | dependencies | misk/gradle/libs.versions.toml | gradle | minor | `1.36.0` -> `1.37.0` | | [software.amazon.awssdk:sdk-core](https://aws.amazon.com/sdkforjava) | dependencies | misk/gradle/libs.versions.toml | gradle | patch | `2.31.56` -> `2.31.57` | | [software.amazon.awssdk:sqs](https://aws.amazon.com/sdkforjava) | dependencies | misk/gradle/libs.versions.toml | gradle | patch | `2.31.56` -> `2.31.57` | | [software.amazon.awssdk:dynamodb-enhanced](https://aws.amazon.com/sdkforjava) | dependencies | misk/gradle/libs.versions.toml | gradle | patch | `2.31.56` -> `2.31.57` | | [software.amazon.awssdk:dynamodb](https://aws.amazon.com/sdkforjava) | dependencies | misk/gradle/libs.versions.toml | gradle | patch | `2.31.56` -> `2.31.57` | | [software.amazon.awssdk:aws-core](https://aws.amazon.com/sdkforjava) | dependencies | misk/gradle/libs.versions.toml | gradle | patch | `2.31.56` -> `2.31.57` | | [software.amazon.awssdk:bom](https://aws.amazon.com/sdkforjava) | dependencies | misk/gradle/libs.versions.toml | gradle | patch | `2.31.56` -> `2.31.57` | | [software.amazon.awssdk:auth](https://aws.amazon.com/sdkforjava) | dependencies | misk/gradle/libs.versions.toml | gradle | patch | `2.31.56` -> `2.31.57` | --- ### Release Notes <details> <summary>googleapis/google-auth-library-java (com.google.auth:google-auth-library-oauth2-http)</summary> ### [`v1.37.0`](https://github.com/googleapis/google-auth-library-java/blob/HEAD/CHANGELOG.md#1370-2025-06-04) ##### Features - **mtls:** Introduce DefaultMtlsProviderFactory and SecureConnectProvider ([#1730](googleapis/google-auth-library-java#1730)) ([c9fd1b1](googleapis/google-auth-library-java@c9fd1b1)) ##### Bug Fixes - Correct capitalization of GitHub ([#1761](googleapis/google-auth-library-java#1761)) ([f79a2e4](googleapis/google-auth-library-java@f79a2e4)) - Correct extra spaces in README heading ([#1760](googleapis/google-auth-library-java#1760)) ([8d26666](googleapis/google-auth-library-java@8d26666)) - Correct misspelling of OAuth in comments ([#1762](googleapis/google-auth-library-java#1762)) ([42b9602](googleapis/google-auth-library-java@42b9602)) - Correct typo in ServiceAccountJwtAccessCredentials.java comment ([#1765](googleapis/google-auth-library-java#1765)) ([3058b06](googleapis/google-auth-library-java@3058b06)) - Update Javadoc reference in ExternalAccountCredentials ([#1763](googleapis/google-auth-library-java#1763)) ([5eb3659](googleapis/google-auth-library-java@5eb3659)) ##### Documentation - Duplicate "the" in Javadoc comments ([#1764](googleapis/google-auth-library-java#1764)) ([5f7a084](googleapis/google-auth-library-java@5f7a084)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - "after 6pm every weekday,before 2am every weekday" in timezone Australia/Melbourne, Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Never, or you tick the rebase/retry checkbox. 👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://github.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). GitOrigin-RevId: 57d30e592601468e7e82f86d4dcf27523cdba475

feat(mtls): Introduce DefaultMtlsProviderFactory and SecureConnectPro…

3a4bae6

…vider

andyrzhao requested review from a team as code owners April 7, 2025 21:03

product-auto-label bot added the size: l Pull request size is large. label Apr 7, 2025

feat(mtls): fix lint

67a93b3

lqiu96 reviewed Apr 9, 2025

View reviewed changes

oauth2_http/java/com/google/auth/mtls/ContextAwareMetadataJson.java Show resolved Hide resolved

lqiu96 requested a review from zhumin8 April 9, 2025 18:42

zhumin8 reviewed Apr 9, 2025

View reviewed changes

oauth2_http/java/com/google/auth/mtls/DefaultMtlsProviderFactory.java Outdated Show resolved Hide resolved

oauth2_http/java/com/google/auth/mtls/MtlsProvider.java Show resolved Hide resolved

feat(mtls): Update code comments

7a2b2ce

andyrzhao requested review from lqiu96 and zhumin8 April 9, 2025 22:44

zhumin8 reviewed May 12, 2025

View reviewed changes

andyrzhao added 2 commits May 13, 2025 15:54

Address PR comments

d09ec5a

Merge branch 'main' into main

047f391

andyrzhao requested a review from zhumin8 May 13, 2025 23:01

zhumin8 reviewed May 16, 2025

View reviewed changes

oauth2_http/java/com/google/auth/mtls/SecureConnectProvider.java Show resolved Hide resolved

zhumin8 reviewed May 16, 2025

View reviewed changes

andyrzhao added 3 commits May 19, 2025 14:07

feat(mtls): Fix import and test

f499899

Merge branch 'main' into main

47c4978

feat(mtls): Fix lint.

8671818

andyrzhao requested a review from zhumin8 May 20, 2025 04:07

zhumin8 previously approved these changes May 27, 2025

View reviewed changes

lqiu96 reviewed May 27, 2025

View reviewed changes

oauth2_http/java/com/google/auth/mtls/DefaultMtlsProviderFactory.java Show resolved Hide resolved

feat(mtls): Add link to github issue on MtlsProvider interface.

47a9719

andyrzhao dismissed zhumin8’s stale review via 47a9719 May 27, 2025 23:31

feat(mtls): Fix lint.

54108ff

andyrzhao requested review from lqiu96 and zhumin8 May 28, 2025 17:13

zhumin8 reviewed May 28, 2025

View reviewed changes

oauth2_http/java/com/google/auth/mtls/DefaultMtlsProviderFactory.java Show resolved Hide resolved

zhumin8 reviewed May 28, 2025

View reviewed changes

oauth2_http/java/com/google/auth/mtls/ContextAwareMetadataJson.java Outdated Show resolved Hide resolved

lqiu96 reviewed May 28, 2025

View reviewed changes

oauth2_http/java/com/google/auth/mtls/X509Provider.java Show resolved Hide resolved

lqiu96 reviewed May 28, 2025

View reviewed changes

oauth2_http/java/com/google/auth/mtls/X509Provider.java Outdated Show resolved Hide resolved

lqiu96 reviewed May 28, 2025

View reviewed changes

oauth2_http/java/com/google/auth/mtls/ContextAwareMetadataJson.java Show resolved Hide resolved

lqiu96 reviewed May 28, 2025

View reviewed changes

oauth2_http/java/com/google/auth/mtls/MtlsProvider.java Outdated Show resolved Hide resolved

feat(mtls): Update Java docs.

dcab27f

zhumin8 previously approved these changes May 29, 2025

View reviewed changes

lqiu96 previously approved these changes May 29, 2025

View reviewed changes

feat(mtls): Rename to isAvailable() and minor comment tweaks.

c895cb9

andyrzhao dismissed stale reviews from lqiu96 and zhumin8 via c895cb9 May 30, 2025 18:13

Merge branch 'main' into main

da7cb32

andyrzhao requested a review from lqiu96 May 30, 2025 18:44

lqiu96 approved these changes May 30, 2025

View reviewed changes

andyrzhao requested a review from zhumin8 May 30, 2025 21:13

zhumin8 approved these changes Jun 3, 2025

View reviewed changes

andyrzhao merged commit c9fd1b1 into googleapis:main Jun 3, 2025
23 of 24 checks passed

release-please bot mentioned this pull request Jun 4, 2025

chore(main): release 1.37.0 #1766

Merged

release-please bot mentioned this pull request Dec 11, 2025

chore(protobuf-4.x-rc): release 1.42.0-rc1 #1861

Merged

	Process process = processProvider.createProcess(metadata);

	// Run the command and timeout after 1000 milliseconds.
	int exitCode = runCertificateProviderCommand(process, 1000);
	if (exitCode != 0) {
	throw new IOException("Cert provider command failed with exit code: " + exitCode);
	}

	// Create mTLS key store with the input certificates from shell command.
	return SecurityUtils.createMtlsKeyStore(process.getInputStream());

feat(mtls): Introduce DefaultMtlsProviderFactory and SecureConnectProvider #1730

feat(mtls): Introduce DefaultMtlsProviderFactory and SecureConnectProvider #1730

Uh oh!

Conversation

andyrzhao commented Apr 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zhumin8 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhumin8 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lqiu96 left a comment

Choose a reason for hiding this comment

Uh oh!

andyrzhao commented May 30, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants