Support custom KeyManager with TlsChannelCredentials

[arithmetic1728]

I am working on adding mtls feature to Google gax-java library PR. We need to add ssl context to ManagedChannelBuilder. The code in my PR looks like this:

import io.grpc.netty.shaded.io.grpc.netty.GrpcSslContexts;
import io.grpc.netty.shaded.io.grpc.netty.NettyChannelBuilder;
import io.grpc.netty.shaded.io.netty.handler.ssl.SslContext;
import io.grpc.netty.shaded.io.netty.handler.ssl.SslContextBuilder;

private ManagedChannel createSingleChannel() throws IOException {
    ...
    ManagedChannelBuilder builder;
    SslContext sslContext = createSslContext();
    if (sslContext != null) {
      builder = ((NettyChannelBuilder) builder).sslContext(sslContext);
    }
    ...
    ManagedChannel managedChannel = builder.build();
    return managedChannel;
}

In the code I cast the builder type from ManagedChannelBuilder to NettyChannelBuilder, and add the ssl context. This code is for grpc so the cast works.

Our concern is that NettyChannelBuilder is from io.grpc.netty.shaded.io.grpc.netty. It is supposed to be private to grpc-java I guess.

My question is: is this OK? If not, how can I pass the ssl context to the ManagedChannelBuilder. We must use ManagedChannelBuilder to build the channel (because we provide the user a callback to configure ManagedChannelBuilder), otherwise this will be a major breaking change for all googleapis client libraries.

[elharo]

Can you eliminate the cast by adding the relevant method to ManagedChannelBuilder in this repo and delegating? Accessing the shaded class is asking for trouble. It makes me very uncomfortable.

[arithmetic1728]

@elharo I did some research. It seems several people asked for this before, but adding ssl context to ManagedChannelBuilder was opposed as stated here and here.

The posts suggest using the public version of NettyChannelBuilder, but it is not a option for us since it is not compatible with ManagedChannelBuilder. I guess one solution might be some helper methods to convert between ManagedChannelBuilder and NettyChannelBuilder? @ejona86

[ejona86]

Accessing the shaded class is bad, but even if the code was using the non-shaded NettyChannelBuilder (grpc-netty vs grpc-netty-shaded) that would still be inappropriate for gax-java as NettyChannelBuilder is unstable API.

it is not a option for us since it is not compatible with ManagedChannelBuilder

It is actually compatible. Maybe you mean something non-obvious by "compatible."

I guess one solution might be some helper methods to convert between ManagedChannelBuilder and NettyChannelBuilder

Nope. NettyChannelBuilder is a dead end for you, as it is unstable API and has no plans to become a stable API. Is gax-java not using the experimental API detector, because I'd have hoped that sort of stuff would fail the build.

What you need is currently not available. If you could provide more background that would be helpful.

ChannelCredentials is being introduced in the current impending release. I expect that will be the way mTLS will be stabilized. But no work has started to add mTLS to TlsChannelCredentials, so it would need to be designed and implemented, and then stabilized.

[elharo]

context here: go/java-gapic-client-mtls

@ejona86 your feedback on this doc would be very much appreciated

[ejona86]

Oh, I missed the link to googleapis/gax-java#1249 in the initial comment.

This is related to https://google.aip.dev/auth/4114 . That mentions "Application Default Credentials," but com.google.auth.Credentials (as returned by GoogleCredentials.getApplicationDefault()) only supports bearer tokens. I believe that is true mostly cross-language, so that is mostly from the perspective of gcloud CLI. But this does present terminology confusions.

I see that the current PoC code uses a KeyManager. I was originally expecting a first-cut of mTLS to use file/inputstream as input, like is present in TlsServerCredentials. KeyManager can be supported too, it's just that I expect it will rarely be used. Creating a KeyManager is a PITA for users (especially the key file), but I see that is implemented by google-http-java-client.

[arithmetic1728]

Oh, I missed the link to googleapis/gax-java#1249 in the initial comment.

This is related to https://google.aip.dev/auth/4114 . That mentions "Application Default Credentials," but com.google.auth.Credentials (as returned by GoogleCredentials.getApplicationDefault()) only supports bearer tokens. I believe that is true mostly cross-language, so that is mostly from the perspective of gcloud CLI. But this does present terminology confusions.

Application default credentials becomes a broad concept as we move to ADC v2.0 go/adc-v2, so for mTLS, it is not returned from GoogleCredentials.getApplicationDefault(), it just means the client certificate and private key is the default one installed on the device. The default client cert and private key are obtained by running some command on the device, and then parse the stdout.

I see that the current PoC code uses a KeyManager. I was originally expecting a first-cut of mTLS to use file/inputstream as input, like is present in TlsServerCredentials. KeyManager can be supported too, it's just that I expect it will rarely be used. Creating a KeyManager is a PITA for users (especially the key file), but I see that is implemented by google-http-java-client.

Yes we need an api for keystore and keystorepassword. For the end user, even though we give the user the option to provide their own keystore and keystorepassword, we don't expect them to do so (they should use the default client cert/key as stated in the AIP, and let the library do everything, user only needs to use an environment variable to control whether to use this mTLS feature or not). This api will be very helpful for us, even though it might be rarely used for the end user.
@ejona86

[arithmetic1728]

@ejona86 To make the feature request clear, we would like a few lines of code to achieve either of the following:

(1)
Input: keystore and keystorePassword
Output: a new ManagedChannelBuilder instance

(2)
Input: keystore, keystorePassword, ManagedChannelBuilder
Output: the same or a new ManagedChannelBuilder

We need ManagedChannelBuilder because we provided the users a ApiFunction<ManagedChannelBuilder, ManagedChannelBuilder> channelConfigurator to configure ManagedChannelBuilder. We call user provided channelConfigurator to configure ManagedChannelBuilder before we call ManagedChannelBuilder.build(), so ManagedChannelBuilder is the only thing we can use to create a ManagedChannel.

[elharo]

That raises a possible red flag. Are you passing around plain text passwords in method invocations? Might be OK, I'm not sure; but it would be worth a security review.

[ejona86]

The keystorePassword protects the keystore and, if set, is commonly something useless like "changeit" or "password" or <emptystring>. Keystore passwords serve little purpose because they have to be plaintext or equivalent for the software, but some keystore formats require them. It doesn't look like ADC uses a keystore password. Passing keystore file and password together is very common in the Java APIs that interact with these things.

@arithmetic1728, the types of arguments are not clear. Is keystore a KeyStore or a File? If it is a KeyStore, then keystorePassword is unnecessary.

(1) is what the ChannelCredentials provide. You would construct TlsChannelCredentials with the keystore information, and then pass it when the builder is instantiated. You can see the approach in the gRFC. Note that those credentials should generally also include any per-RPC credentials via CompositeChannelCredentials, if they are necessary.

We have no API plans for (2).

[arithmetic1728]

The keystorePassword protects the keystore and, if set, is commonly something useless like "changeit" or "password" or <emptystring>. Keystore passwords serve little purpose because they have to be plaintext or equivalent for the software, but some keystore formats require them. It doesn't look like ADC uses a keystore password. Passing keystore file and password together is very common in the Java APIs that interact with these things.

@arithmetic1728, the types of arguments are not clear. Is keystore a KeyStore or a File? If it is a KeyStore, then keystorePassword is unnecessary.

It is a java.security.KeyStore object. I think the password is used to recover the private key in the keystore, for instance it is used in KeyManagerFactory.init. Not sure if they are the same password.

For the default KeyStore (created with the default device cert and private key), the key is unencrypted so there is no password. It is designed this way because the key is generated in memory by running some command and KeyStore only exists in memory. We need encrypted key only if we have to save the file on disk (this only happens for some Python transport library which reads key only from file, Java has no such issue).

I don't see any problem passing the password as it only exists in memory. Java api like KeyManagerFactory.init needs the password so we have to pass it anyway.

(1) is what the ChannelCredentials provide. You would construct TlsChannelCredentials with the keystore information, and then pass it when the builder is instantiated. You can see the approach in the gRFC. Note that those credentials should generally also include any per-RPC credentials via CompositeChannelCredentials, if they are necessary.

We have no API plans for (2).

Awesome, (1) will be enough. It would be greatly appreciated if you can provide a code snippet showing how to do the following for Netty transport:
Input: keystore, keystorePassword (if needed), credentials (such as Google Application Default Credentials)
Output: a new ManagedChannelBuilder instance

[elharo]

1. There seems to be some concern about whether users need this functionality or not. If you're unsure, it's better to leave it out now and add it when a need is proven.

2. I'm not sure about the specifics of Java keystore files. In general though even when passing keys around in memory in Java it is better to:

A) use arrays of chars or bytes rather than strings
B) Zero the array after use to wipe the key from memory

This avoids the key from living in memory for longer than needed and potentially being swapped out to disk. Again, it's probably good to have someone who's a real security expert review this.

[ejona86]

Support for this was added in #7885 . gax-java would provide a KeyManager in the API, not a KeyStore, and so there isn't any password for this flow.

The API is not yet stable and so can't quite be used by a library. That is being tracked by #7479 . We'd still want someone to play with the API to verify it meets your needs before we stabilize.

[arithmetic1728]

Support for this was added in #7885 . gax-java would provide a KeyManager in the API, not a KeyStore, and so there isn't any password for this flow.

The API is not yet stable and so can't quite be used by a library. That is being tracked by #7479 . We'd still want someone to play with the API to verify it meets your needs before we stabilize.

I just did end to end test with pubsub client library, the API worked perfectly. The test environment is specially designed for testing mtls feature (project has VPC policy enforcement) - test only passes if mtls is configured correctly, otherwise request is rejected with 403 error. The API looks pretty good to us.