From 6487d010fe43dacb846ebbc3f89e6eb06ee7e4c3 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Fri, 17 Jul 2020 15:35:36 -0700 Subject: [PATCH 01/16] L74: Java Channel Creds --- L74-java-channel-creds.md | 411 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 411 insertions(+) create mode 100644 L74-java-channel-creds.md diff --git a/L74-java-channel-creds.md b/L74-java-channel-creds.md new file mode 100644 index 000000000..a420e0de8 --- /dev/null +++ b/L74-java-channel-creds.md @@ -0,0 +1,411 @@ +Java: Channel and Server Credentials +---- +* Author(s): [Eric Anderson](https://github.com/ejona86) +* Approver: sanjaypujare +* Status: In Review {Draft, In Review, Ready for Implementation, Implemented} +* Implemented in: +* Last updated: 2020-06-17 +* Discussion at: (filled after thread exists) + +## Abstract + +Develop an API that separates transport credential construction from +channel/server construction. + +This is to allow: + +* implementing new transport credentials without creating a new + channel/server type, which allows users of the API to separate concerns +* providing stable APIs for advanced credential configuration like mutual + TLS, without polluting the general-purpose ManagedChannelBuilder API with + many overloads that don't apply to all transports +* composition of credentials, like necessary for xDS where a fallback + credential needs to be provided by the user +* aligning with C core and Go implementations + +_The design focuses on channel credentials, since they are the most complex and +most discussed._ Server credentials will mostly just mirror the channel +credentials; they suffer from most of the same problems but at a smaller +scale. + +## Background + +Prior to gRPC v0.12.0, C core had a "unified" `Credential` type that was +simultaneously a call and channel credential. This was the design proposed for +all languages. However, the Java implementation could not determine how such an +API could be implemented for Java with its multiple transports. The designer of +the API had too little bandwidth (it was a _very_ busy time), so Java (and Go) +developed its own API. Java used interceptors (which no other language had) and +credential-specific configuration on the ManagedChannelBuilder. + +In gRPC v0.12.0, C core swapped to a “split” CallCredential and +ChannelCredential. ChannelCredentials could contain CallCredentials. At this +point it was not a priority for Java to investigate the new API; Java was happy +they didn’t use the previous API, such that they didn’t have an API migration. + +gRPC Java 0.15.0 added CallCredentials to replace the interceptor-based call +credentials. At this point Java had looked into the full “split” credential +design, but decided it was too hard to implement at that moment. Java worked to +stabilize the `ManagedChannelBuilder.usePlaintext()` and +`useTransportSecurity()` APIs. + +Go has split call credentials (`credentials.PerRPCCredentials`) and channel +credentials (`credentials.TransportCredentials`), but with a separate type to +combine them (`credentials.Bundle`). The split credentials significantly +predate `Bundle`. + +### Related Proposals: + +N/A + +## Proposal + +`ChannelCredentials` will be introduced, which can be provided when +constructing a `ManagedChannelBuilder': + +```java +// instead of: +ManagedChannelBuilder.forTarget("example.com") + .build(); +ManagedChannelBuilder.forTarget("example.com").usePlaintext() + .build(); +// the user would now: +ManagedChannel.newBuilderForTarget("example.com", new TlsChannelCredentials()) + .build(); +ManagedChannel.newBuilderForTarget("example.com", new InsecureChannelCredentials()) + .build(); +``` + +`newBuilderForTarget()` will iterate through the `ManagedChannelProviders` +trying to create a builder for each in turn. The first provider that succeeds +will be used. If no provider is able to handle the credentials, then +`newBuilderForTarget()` will throw. + +The new ChannelCredentials API cannot be mixed with the pre-existing +`useTransportSecurity()` and `usePlaintext()` builder APIs. The old-style +methods will throw if called on a builder with a ChannelCredential provided. + +Note that this new API does not allow: 1) creating a channel without a +credential and supplying the credential later and 2) changing the credential +after creating the builder. This is not expected to be much of an issue as +users typically have the security information available before creating the +builder and reusing a builder is rare, especially to change the security. + +```java +package io.grpc; + +/** + * Represents a security configuration to be used for channels. There is no + * generic mechanism for processing arbitrary ChannelCredentials; the consumer + * of the credential (the channel) must support each implementation explicitly + * and separately. Consumers are not required to support all types or even all + * possible configurations for types that are partially supported, but they + * must at least fully support ChoiceChannelCredentials. + * + *

A Credential provides client identity and authenticates the server. This + * is different from CallCredentials, which only provides client identity. They + * can also influence types of encryption used and similar security + * configuration. + */ +public abstract class ChannelCredentials {} + +public final class TlsChannelCredentials extends ChannelCredentials { + /* Complicated. Described separately below. */ +} + +/** No client identity, authentication, or encryption is to be used. */ +public final class InsecureChannelCredentials extends ChannelCredentials {} + +/** + * Provides a list of ChoiceChannelCredentials, where any one may be used. The + * credentials are in preference order. + */ +public final class ChoiceChannelCredentials extends ChannelCredentials { + public ChoiceChannelCredentials(ChannelCredentials... creds) {...} + + public List getCredentialsList() { return creds; } +} +``` + +`ChoiceChannelCredentials` is intended to be used in cases like +`GoogleDefaultCredentials`, where TLS may be satisfactory, but there may also +be more optimal credentials if the transport is a certain type. + +To allow choosing `ChannelCredentials` and `CallCredentials` simultaneously, +there will be a composite credential. If `CallCredentials` are specified on the +Channel and per-RPC, then _both_ `CallCredentials` will be used. This matches +the behavior in other languages. + +```java +package io.grpc; + +public final class CompositeChannelCredentials extends ChannelCredentials { + public CompositeChannelCredentials( + ChannelCredentials chanCreds, CallCredentials callCreds) {...} + public ChannelCredentials getChannelCredentials() { return channelCredentials; } + public CallCredentials getCallCredentials() { return callCredentials; } +} +``` + +Unlike most of the credentials, TLS may have a considerable amount of +configuration. And as time goes on, more configuration options may be provided. +It could be a severe bug if certain TLS restrictions were ignored by a channel. +To detect cases where the channel does not understand enough of the TLS +configuration, there will be a method to check if there are required features +that are not understood. If some are not understood, enough information will be +returned to produce a useful error message. + +`TlsChannelCredentials` will support a no-arg constructor for the "all +defaults" configuration. For any more advanced configuration, a builder would +be used. + +```java +package io.grpc; + +public final class TlsChannelCredentials extends ChannelCredentials { + /** + * Returns an empty set if this credential can be adequately understood via + * the features listed, otherwise returns a hint of features that are lacking + * to understand the configuration to be used for manual debugging. + * + *

An "understood" feature does not imply the caller is able to fully + * handle the feature. It simply means the caller understands the feature + * enough to use the appropriate APIs to read the configuration. The caller + * may support just a subset of a feature, in which case the caller would + * need to look at the configuration to determine if only the supported + * subset is used. + * + *

This method may not be as simple as a set difference. There may be + * multiple features that can independently satisfy a piece of configuration. + * If the configuration is incomprehensible, all such features would be + * returned, even though only one may be necessary. + * + *

An empty set does not imply that the credentials are fully understood. + * There may be optional configuration that can be ignored if not understood. + */ + public Set incomprehensible(EnumSet understoodFeatures) {...} + + /* Other methods can be added to support various Features */ + + public enum Feature {} + + public final static class Builder { + public TlsChannelCredentials build() {...} + } +} +``` + +To support very advanced use cases, Netty will provide a credential that wraps +a `ProtocolNegotiator`. This allows implementations like ALTS and XDS to use +internal APIs without forcing their users to use experimental or internal APIs, +as their users would just interact with `ChannelCredentials`. + +```java +package io.grpc.netty; + +final class NettyChannelCredentials extends ChannelCredentials { + public NettyChannelCredentials(ProtocolNegotiator.Factory negotiator) {...} + public ProtocolNegotiator.Factory getNegotiator() { return negotiator; } +} + +@Internal +public final class InternalNettyChannelCredentials { + private InternalNettyChannelCredentials() {} + + public ChannelCredentials create(InternalProtocolNegotiator.Factory negotiator) + {...} + + /** + * Converts credentials to a negotiator, in similar fashion as for a new + * channel. + * + * @throws IllegalArgumentException if unable to convert + */ + public InternalProtocolNegotiator.Factory toNegotiator(ChannelCredentials creds) + {...} +} +``` + +### GoogleDefault ChannelCredentials Example + +The API provided here is not part of the design. It is meant as a +demonstration. + +```java +public final class GoogleDefaultChannelCredentials { + private GoogleDefaultChannelCredentials() {} + + public static ChannelCredentials create() { + return new ChoiceChannelCredentials( + InternalNettyChannelCredentials.create( + new AltsProtocolNegotiatorFactory()), + new CompositeChannelCredentials( + new TlsChannelCredentials(), + MoreCallCredentials.from(GoogleCredentials.getApplicationDefault()))); + } +} +``` + +### xDS ChannelCredentials Example + +The API provided here is not part of the design. It is meant as a +demonstration. + +```java +public final class XdsChannelCredentials { + private XdsChannelCredentials() {} + + /** + * Creates credentials to be configured by xDS, falling back to other + * credentials if no configuration is provided. + * + * @throws IllegalArgumentException if fallback is unable to be used + */ + public static ChannelCredentials createWithFallback( + ChannelCredentials fallback) { + return new InternalNettyChannelCredentials.create( + new XdsProtocolNegotiatorFactory( + InternalNettyChannelCredentials.toNegotiator(fallback))); + } +} +``` + +### Netty ChannelCredentials Processing Example + +The API provided here is not part of the design. It is meant as a +demonstration. + +```java +final class ProtocolNegotiators { + private static final EnumSet<> understoodTlsFeatures = + EnumSet.noneOf(TlsChannelCredentials.Feature.class); + + public static Result from(ChannelCredentials creds) { + if (creds instanceof TlsChannelCredentials) { + TlsChannelCredentials tlsCreds = (TlsChannelCredentials) creds; + Set incomprehensible = + tlsCreds.incomprehensible(understoodTlsFeatures); + if (!incomprehensible.isEmpty()) { + return Result.error("TLS features not understood: " + incomprehensible); + } + return Result.negotiator(tls()); + + } else if (creds instanceof InsecureChannelCredentials) { + return Result.negotiator(plaintext()); + + } else if (creds instanceof CompositeChannelCredentials) { + CompositeChannelCredentials compCreds = (CompositeChannelCredentials) creds; + return from(compCreds.getChannelCredentials()) + .withCallCredentials(compCreds.getCallCredentials()); + + } else if (creds instanceof NettyChannelCredentials) { + NettyChannelCredentials nettyCreds = (NettyChannelCredentials) creds; + return Result.negotiator(nettyCreds.getNegotiator()); + + } else if (creds instanceof ChoiceChannelCredentials) { + ChoiceChannelCredentials choiceCreds = (ChoiceChannelCredentials) creds; + StringBuilder error = new StringBuilder(); + for (ChannelCredentials innerCreds : choiceCreds.getCredentialsList()) { + Result result = from(innerCreds); + if (!result.isError()) { + return result; + } + error.append("; "); + error.append(result.getError()); + } + return Result.error(error.substring(2)); + + } else { +  return Result.error( + "Unsupported credential type: " + creds.getClass().getName()); + } + } + + public final class Result { + public static Result error(String error) {...} + public static Result negotiator(ProtocolNegotiator.Factory factory) {...} + public Result withCallCredentials(CallCredentials callCreds) {...} + } +} +``` + +### ServerCredentials + +Server-side will be a clone of client-side, but using the `Server` instead of +`Channel`. `ServerCredentials` cannot contain `CallCredentials`, so +`CompositeServerCredentials` will be omitted. There is also currently no known +use-case for `ChoiceServerCredentials`, so it will be omitted. + + +```java +package io.grpc; + +public abstract class ServerCredentials {} +``` + +## Rationale + +A channel credential API is substantially harder in Java than the other +languages as Java has multiple transports that behave radically differently. +OkHttp uses blocking I/O with a byte[]-backed Buffer whereas Netty uses +nonblocking I/O with a reference-counted, ByteBuffer-backed ByteBuf with an +event loop. Go and C use a code-based API where credentials implement an +interface and use a single I/O model. It is not practical to have one +code-based API to support both OkHttp and Netty, as it would have a +considerable amount of complexity in the implementations, be hard to optimize, +and have a wide public API surface. Instead, Java needs a data-based API, where +configuration values are communicated and each transport can have its own +implementation. A data-based API can also be used by InProcess and Binder +transports. + +There needs to be a `TlsChannelCredential` which is able to work with both +OkHttp and Netty transports. This credential needs to live in io.grpc and so +cannot depend on OkHttp nor Netty. Since it is not feasible for the credential +to provide the TLS implementation, it will provide configuration. Since new +configuration will be added over time, there needs to be a way for a transport +to know that it understands the important parts of the configuration. + +To support transport credentials like ALTS and TLS-offloading, there still +needs to be a way to provide implementations. However, these implementations +_can_ depend on the specific transport being used. Just the users of those +implementations should avoid depending _explicitly_ on a particular transport. + +The API surface should handle generic and transport-specific credentials in the +same manner, so the user’s code should not care about the implementation +details. + +It would be possible to use a versioning scheme for `TlsChannelCredentials`. +Version 1 could be all defaults. Version 2 could have file-based client +certificates and ca certificates for trust. Version 3 could include ciphers, +KeyManagers, TrustManagers, and session cache configuration. While versions +safely allow adding features in the future and is simpler than the Features +enum approach, it means a consumer that only supports a subset of features at a +version must check _all_ the features at a particular version to see if they +are set. Keeping track of which features were available at each version can be +error-prone. It's also hard to produce a user-meaningful error message, so they +are aware of which specific configuration is causing the incompatibility. + +## Implementation + +ejona86 will implement. TLS will initially be bare-bones, with just the +compatibility checker. Over the long-term TLS will be fleshed out with +additional configuration. ServerCredentials will be implemented shortly after +ServerCredentials. + +## Open issues (if applicable) + +Do we put the constructor in ManagedChannel instead of ManagedChannelBuilder? +On io.grpc.Grpc? The static methods on ManagedChannelBuilder have been a +problem, as they are exposed on all subclasses, but don’t operate within the +scope of the subclass, unless they do. + +It would be very convenient to be able to share the TlsChannelCredential API on +server-side, as it is quite a wide API and _almost_ the same. The consumer +would need to be slightly different, but much of the configuration can +potentially be shared. Should we make TlsChannelCredential and +TlsServerCredential just a holder for a TlsConfiguration class? For validation, +it would need to know whether it was for server-side or client-side. It would +also have the union of all client- and server-specific configuration. See +[Netty's SslContextBuilder]( +https://netty.io/4.1/api/io/netty/handler/ssl/SslContextBuilder.html) +for a view into how client and server differ. From a285fb99950691f2ecda865abde317954e0e77a7 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Fri, 17 Jul 2020 15:45:53 -0700 Subject: [PATCH 02/16] Add grpc-io discussion link --- L74-java-channel-creds.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/L74-java-channel-creds.md b/L74-java-channel-creds.md index a420e0de8..1129610d3 100644 --- a/L74-java-channel-creds.md +++ b/L74-java-channel-creds.md @@ -5,7 +5,7 @@ Java: Channel and Server Credentials * Status: In Review {Draft, In Review, Ready for Implementation, Implemented} * Implemented in: * Last updated: 2020-06-17 -* Discussion at: (filled after thread exists) +* Discussion at: https://groups.google.com/g/grpc-io/c/GNU9svnDsQs/m/kG6XsUy9BwAJ ## Abstract From be4a3be491828f831bfe87372f877723c986485b Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Fri, 24 Jul 2020 09:05:58 -0700 Subject: [PATCH 03/16] Swap to Grpc.newChannelBuilder --- L74-java-channel-creds.md | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/L74-java-channel-creds.md b/L74-java-channel-creds.md index 1129610d3..1cc76118b 100644 --- a/L74-java-channel-creds.md +++ b/L74-java-channel-creds.md @@ -65,21 +65,27 @@ constructing a `ManagedChannelBuilder': ```java // instead of: -ManagedChannelBuilder.forTarget("example.com") - .build(); -ManagedChannelBuilder.forTarget("example.com").usePlaintext() - .build(); +channel = ManagedChannelBuilder.forTarget("example.com") + .build(); +channel = ManagedChannelBuilder.forTarget("example.com").usePlaintext() + .build(); +channel = ManagedChannelBuilder.forAddress("example.com", 443) + .build(); // the user would now: -ManagedChannel.newBuilderForTarget("example.com", new TlsChannelCredentials()) - .build(); -ManagedChannel.newBuilderForTarget("example.com", new InsecureChannelCredentials()) - .build(); +channel = Grpc.newChannelBuilder("example.com", new TlsChannelCredentials()) + .build(); +channel = Grpc.newChannelBuilder("example.com", new InsecureChannelCredentials()) + .build(); +channel = Grpc.newChannelBuilderForAddress("example.com", 443, new TlsChannelCredentials()) + .build(); ``` -`newBuilderForTarget()` will iterate through the `ManagedChannelProviders` +`newChannelBuilder()` will iterate through the `ManagedChannelProviders` trying to create a builder for each in turn. The first provider that succeeds will be used. If no provider is able to handle the credentials, then -`newBuilderForTarget()` will throw. +`newChannelBuilder()` will throw. `newChannelBuilderForAddress()` will be a +convenience function that creates the target string and calls +`newChannelBuilder()`, mirroring its current behavior. The new ChannelCredentials API cannot be mixed with the pre-existing `useTransportSecurity()` and `usePlaintext()` builder APIs. The old-style @@ -394,11 +400,6 @@ ServerCredentials. ## Open issues (if applicable) -Do we put the constructor in ManagedChannel instead of ManagedChannelBuilder? -On io.grpc.Grpc? The static methods on ManagedChannelBuilder have been a -problem, as they are exposed on all subclasses, but don’t operate within the -scope of the subclass, unless they do. - It would be very convenient to be able to share the TlsChannelCredential API on server-side, as it is quite a wide API and _almost_ the same. The consumer would need to be slightly different, but much of the configuration can From b0511a38d6d18ff5239be18c9eeae5c07361c816 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Tue, 28 Jul 2020 10:43:49 -0700 Subject: [PATCH 04/16] Add more server-side details --- L74-java-channel-creds.md | 65 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/L74-java-channel-creds.md b/L74-java-channel-creds.md index 1cc76118b..9b4d8014d 100644 --- a/L74-java-channel-creds.md +++ b/L74-java-channel-creds.md @@ -342,6 +342,22 @@ Server-side will be a clone of client-side, but using the `Server` instead of `CompositeServerCredentials` will be omitted. There is also currently no known use-case for `ChoiceServerCredentials`, so it will be omitted. +```java +// instead of: +server = ServerBuilder.forPort(443) + .useTransportSecurity(new File("cert.pem"), new File("cert.key")) + .build().start(); +server = ServerBuilder.forPort(8080) + .build().start(); +// the user would now: +ChannelCredentials tlsCreds = new TlsServerCredentials( + new File("cert.pem"), new File("cert.key")); +server = Grpc.newServerBuilderForPort(443, tlsCreds) + .build().start(); +server = Grpc.newServerBuilderForPort(8080, new InsecureChannelCredentials()) + .build().start(); +``` + ```java package io.grpc; @@ -349,6 +365,55 @@ package io.grpc; public abstract class ServerCredentials {} ``` +Since servers using TLS require a certificate, the `TlsServerCredentials` +cannot have the simple no-arg configuration of `TlsChannelCredentials`. It will +provide convenience constructors for providing a certificate and unencrypted +key. If any other configuration is necessary, the `Builder` would be used. + +```java +package io.grpc; + +public final class TlsServerCredentials extends ServerCredentials { + /** + * Creates an instance using provided certificate chain and private key. + * Generally they should be PEM-encoded and the key is an unencrypted PKCS#8 + * key (file headers have "BEGIN CERTIFICATE" and "BEGIN PRIVATE KEY"). + */ + public TlsServerCredentials(File certChain, File privateKey) + throws IOException {...} + public TlsServerCredentials(InputStream certChain, InputStream privateKey) + throws IOException {...} + + /** Same as client-side */ + public Set incomprehensible(EnumSet understoodFeatures) {...} + + public byte[] getCertificateChain() { return Arrays.copyOf(...); } + public byte[] getPrivateKey() { return Arrays.copyOf(...); } + public String getPassword() {...} + + /* Other methods can be added to support various Features */ + + public enum Feature {} + + public static Builder newBuilder() { return new Builder(); } + + public final static class Builder { + public Builder keyManager(File certChain, File privateKey) + throws IOException {...} + public Builder keyManager(File certChain, File privateKey, String password) + throws IOException {...} + public Builder keyManager(InputStream certChain, InputStream privateKey) + throws IOException {...} + public Builder keyManager( + InputStream certChain, InputStream privateKey, String password) + throws IOException {...} + + // Throws if no certificate was provided + public TlsChannelCredentials build() {...} + } +} +``` + ## Rationale A channel credential API is substantially harder in Java than the other From d9f72a8162c2b53917a2848176ef6294c20925d9 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Tue, 28 Jul 2020 13:56:31 -0700 Subject: [PATCH 05/16] Commit missed comment --- L74-java-channel-creds.md | 1 + 1 file changed, 1 insertion(+) diff --git a/L74-java-channel-creds.md b/L74-java-channel-creds.md index 9b4d8014d..413693f45 100644 --- a/L74-java-channel-creds.md +++ b/L74-java-channel-creds.md @@ -381,6 +381,7 @@ public final class TlsServerCredentials extends ServerCredentials { */ public TlsServerCredentials(File certChain, File privateKey) throws IOException {...} + // Will not auto-close InputStream, unlike existing transportSecurity() public TlsServerCredentials(InputStream certChain, InputStream privateKey) throws IOException {...} From 387a0c3f721e3386c7aace83a6f4db3a84660d5b Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Thu, 30 Jul 2020 13:25:47 -0700 Subject: [PATCH 06/16] Add CLIENT_CERTIFICATE example Feature --- L74-java-channel-creds.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/L74-java-channel-creds.md b/L74-java-channel-creds.md index 413693f45..254217870 100644 --- a/L74-java-channel-creds.md +++ b/L74-java-channel-creds.md @@ -201,6 +201,13 @@ public final class TlsChannelCredentials extends ChannelCredentials { } ``` +An example `Feature` could be `CLIENT_CERTIFICATE`. When the `Feature` is +added, methods `getCertificateChain()`, `getPrivateKey()`, `getPassword()` can +be added. Observing the contents of those methods would be a requirement for +`CLIENT_CERTIFICATE`. An implementation understanding `CLIENT_CERTIFICATE` +might not support encrypted private keys and so could consider the credential +unsupported if `getPassword()` returned non-`null`. + To support very advanced use cases, Netty will provide a credential that wraps a `ProtocolNegotiator`. This allows implementations like ALTS and XDS to use internal APIs without forcing their users to use experimental or internal APIs, From b8e45d700015f39eb3ec66959305f025313ed9ad Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Thu, 6 Aug 2020 09:47:40 -0700 Subject: [PATCH 07/16] Simple fixes --- L74-java-channel-creds.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/L74-java-channel-creds.md b/L74-java-channel-creds.md index 254217870..237d7b5a0 100644 --- a/L74-java-channel-creds.md +++ b/L74-java-channel-creds.md @@ -108,9 +108,9 @@ package io.grpc; * possible configurations for types that are partially supported, but they * must at least fully support ChoiceChannelCredentials. * - *

A Credential provides client identity and authenticates the server. This - * is different from CallCredentials, which only provides client identity. They - * can also influence types of encryption used and similar security + *

A ChannelCredential provides client identity and authenticates the server. + * This is different from CallCredentials, which only provides client identity. + * They can also influence types of encryption used and similar security * configuration. */ public abstract class ChannelCredentials {} @@ -123,7 +123,7 @@ public final class TlsChannelCredentials extends ChannelCredentials { public final class InsecureChannelCredentials extends ChannelCredentials {} /** - * Provides a list of ChoiceChannelCredentials, where any one may be used. The + * Provides a list of ChannelCredentials, where any one may be used. The * credentials are in preference order. */ public final class ChoiceChannelCredentials extends ChannelCredentials { @@ -195,7 +195,7 @@ public final class TlsChannelCredentials extends ChannelCredentials { public enum Feature {} - public final static class Builder { + public static final class Builder { public TlsChannelCredentials build() {...} } } From 6e6829c935540a6c00a464be7641b8a1a014ac9c Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Thu, 6 Aug 2020 09:48:05 -0700 Subject: [PATCH 08/16] Avoid EnumSet in the API surface We don't actually gain anything by it, as EnumSet doesn't have any methods of its own. It doesn't really hurt anything if someone uses something other than EnumSet, and swapping to plain set resolves the minor oddity that the return value (Set) differed from the input (EnumSet). --- L74-java-channel-creds.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/L74-java-channel-creds.md b/L74-java-channel-creds.md index 237d7b5a0..eae9e4208 100644 --- a/L74-java-channel-creds.md +++ b/L74-java-channel-creds.md @@ -188,8 +188,12 @@ public final class TlsChannelCredentials extends ChannelCredentials { * *

An empty set does not imply that the credentials are fully understood. * There may be optional configuration that can be ignored if not understood. + * + *

Since {@code Feature} is an {@code enum}, {@code understoodFeatures} + * should generally be an {@link EnumSet}. {@code understoodFeatures} will not + * be modified. */ - public Set incomprehensible(EnumSet understoodFeatures) {...} + public Set incomprehensible(Set understoodFeatures) {...} /* Other methods can be added to support various Features */ @@ -393,7 +397,7 @@ public final class TlsServerCredentials extends ServerCredentials { throws IOException {...} /** Same as client-side */ - public Set incomprehensible(EnumSet understoodFeatures) {...} + public Set incomprehensible(Set understoodFeatures) {...} public byte[] getCertificateChain() { return Arrays.copyOf(...); } public byte[] getPrivateKey() { return Arrays.copyOf(...); } From 1dfdbc7d52852e138467d12d37131ba95c03fa2e Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Thu, 13 Aug 2020 12:51:49 -0700 Subject: [PATCH 09/16] Use create() methods everywhere, returning plain ChannelCredentials --- L74-java-channel-creds.md | 90 ++++++++++++++++++++++++++++++--------- 1 file changed, 69 insertions(+), 21 deletions(-) diff --git a/L74-java-channel-creds.md b/L74-java-channel-creds.md index eae9e4208..7096dd012 100644 --- a/L74-java-channel-creds.md +++ b/L74-java-channel-creds.md @@ -95,7 +95,7 @@ Note that this new API does not allow: 1) creating a channel without a credential and supplying the credential later and 2) changing the credential after creating the builder. This is not expected to be much of an issue as users typically have the security information available before creating the -builder and reusing a builder is rare, especially to change the security. +builder and reusing a builder is rare, especially to change the security. ```java package io.grpc; @@ -112,22 +112,42 @@ package io.grpc; * This is different from CallCredentials, which only provides client identity. * They can also influence types of encryption used and similar security * configuration. + * + * The concrete credential type should not be relevant to most users of the API + * and may be an implementation decision. Users should generally use the {@code + * ChannelCredentials} type for variables instead of the concrete type. + * Freshly-constructed credentials should be returned as {@code + * ChannelCredentials} instead of a concrete type to encourage this pattern. + * Concrete types would only be used after {@code instanceof} checks (which must + * consider {@code ChoiceChannelCredentials}!). */ public abstract class ChannelCredentials {} public final class TlsChannelCredentials extends ChannelCredentials { + public static ChannelCredentials create() {...} + /* Complicated. Described separately below. */ } /** No client identity, authentication, or encryption is to be used. */ -public final class InsecureChannelCredentials extends ChannelCredentials {} +public final class InsecureChannelCredentials extends ChannelCredentials { + private InsecureChannelCredentials() {} + + public ChannelCredentials create() { + return new InsecureChannelCredentials(); + } +} /** * Provides a list of ChannelCredentials, where any one may be used. The * credentials are in preference order. */ public final class ChoiceChannelCredentials extends ChannelCredentials { - public ChoiceChannelCredentials(ChannelCredentials... creds) {...} + private ChoiceChannelCredentials(ChannelCredentials... creds) {...} + + public static ChannelCredentials create(ChannelCredentials... creds) { + return new ChoiceChannelCredentials(creds); + } public List getCredentialsList() { return creds; } } @@ -146,8 +166,12 @@ the behavior in other languages. package io.grpc; public final class CompositeChannelCredentials extends ChannelCredentials { - public CompositeChannelCredentials( + private CompositeChannelCredentials( ChannelCredentials chanCreds, CallCredentials callCreds) {...} + public static ChannelCredentials create( + ChannelCredentials chanCreds, CallCredentials callCreds) { + return new CompositeChannelCredentials(chanCreds, callCreds); + } public ChannelCredentials getChannelCredentials() { return channelCredentials; } public CallCredentials getCallCredentials() { return callCredentials; } } @@ -169,6 +193,13 @@ be used. package io.grpc; public final class TlsChannelCredentials extends ChannelCredentials { + private TlsChannelCredentials(Builder builder) {...} + + /** Use TLS with its defaults. */ + public static ChannelCredentials create() { return newBuilder().build(); } + + public static Builder newBuilder() { return new Builder(); } + /** * Returns an empty set if this credential can be adequately understood via * the features listed, otherwise returns a hint of features that are lacking @@ -200,7 +231,7 @@ public final class TlsChannelCredentials extends ChannelCredentials { public enum Feature {} public static final class Builder { - public TlsChannelCredentials build() {...} + public ChannelCredentials build() {...} } } ``` @@ -221,16 +252,23 @@ as their users would just interact with `ChannelCredentials`. package io.grpc.netty; final class NettyChannelCredentials extends ChannelCredentials { - public NettyChannelCredentials(ProtocolNegotiator.Factory negotiator) {...} + private NettyChannelCredentials(ProtocolNegotiator.Factory negotiator) {...} + + public static ChannelCredentials create(ProtocolNegotiator.Factory negotiator) { + return new NettyChannelCredentials(negotiator); + } + public ProtocolNegotiator.Factory getNegotiator() { return negotiator; } } @Internal public final class InternalNettyChannelCredentials { - private InternalNettyChannelCredentials() {} + private InternalNettyChannelCredentials() {} // prevent instantiation - public ChannelCredentials create(InternalProtocolNegotiator.Factory negotiator) - {...} + public static ChannelCredentials create( + InternalProtocolNegotiator.Factory negotiator) { + return NettyChannelCredentials.create(negotiator); + } /** * Converts credentials to a negotiator, in similar fashion as for a new @@ -238,8 +276,8 @@ public final class InternalNettyChannelCredentials { * * @throws IllegalArgumentException if unable to convert */ - public InternalProtocolNegotiator.Factory toNegotiator(ChannelCredentials creds) - {...} + public static InternalProtocolNegotiator.Factory toNegotiator( + ChannelCredentials creds) {...} } ``` @@ -250,7 +288,7 @@ demonstration. ```java public final class GoogleDefaultChannelCredentials { - private GoogleDefaultChannelCredentials() {} + private GoogleDefaultChannelCredentials() {} // prevent instantiation public static ChannelCredentials create() { return new ChoiceChannelCredentials( @@ -270,7 +308,7 @@ demonstration. ```java public final class XdsChannelCredentials { - private XdsChannelCredentials() {} + private XdsChannelCredentials() {} // prevent instantiation /** * Creates credentials to be configured by xDS, falling back to other @@ -338,7 +376,7 @@ final class ProtocolNegotiators { } } - public final class Result { + public static final class Result { public static Result error(String error) {...} public static Result negotiator(ProtocolNegotiator.Factory factory) {...} public Result withCallCredentials(CallCredentials callCreds) {...} @@ -385,16 +423,24 @@ key. If any other configuration is necessary, the `Builder` would be used. package io.grpc; public final class TlsServerCredentials extends ServerCredentials { + private TlsServerCredentials(...) {} + /** * Creates an instance using provided certificate chain and private key. * Generally they should be PEM-encoded and the key is an unencrypted PKCS#8 * key (file headers have "BEGIN CERTIFICATE" and "BEGIN PRIVATE KEY"). */ - public TlsServerCredentials(File certChain, File privateKey) - throws IOException {...} + public static ServerCredentials create(File certChain, File privateKey) + throws IOException { + return newBuilder().keyManager(certChain, privateKey).build(); + } // Will not auto-close InputStream, unlike existing transportSecurity() - public TlsServerCredentials(InputStream certChain, InputStream privateKey) - throws IOException {...} + public static ServerCredentials create( + InputStream certChain, InputStream privateKey) throws IOException { + return newBuilder().keyManager(certChain, privateKey).build(); + } + + public static Builder newBuilder() { return new Builder(); } /** Same as client-side */ public Set incomprehensible(Set understoodFeatures) {...} @@ -407,8 +453,6 @@ public final class TlsServerCredentials extends ServerCredentials { public enum Feature {} - public static Builder newBuilder() { return new Builder(); } - public final static class Builder { public Builder keyManager(File certChain, File privateKey) throws IOException {...} @@ -421,7 +465,7 @@ public final class TlsServerCredentials extends ServerCredentials { throws IOException {...} // Throws if no certificate was provided - public TlsChannelCredentials build() {...} + public ChannelCredentials build() {...} } } ``` @@ -452,6 +496,10 @@ To support transport credentials like ALTS and TLS-offloading, there still needs to be a way to provide implementations. However, these implementations _can_ depend on the specific transport being used. Just the users of those implementations should avoid depending _explicitly_ on a particular transport. +Such "credentials" will not extend ChannelCredentials but instead create +pre-existing credentials types with specific configuration. To reduce the +strangeness involved, all ChannelCredentials construction returns the generic +ChannelCredentials type instead of the concrete type. The API surface should handle generic and transport-specific credentials in the same manner, so the user’s code should not care about the implementation From e7d0159694b418639b4cfe55721a7539cab700db Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Thu, 13 Aug 2020 12:57:27 -0700 Subject: [PATCH 10/16] Two TLS APIs is better, after further investigation --- L74-java-channel-creds.md | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/L74-java-channel-creds.md b/L74-java-channel-creds.md index 7096dd012..45cf1efe8 100644 --- a/L74-java-channel-creds.md +++ b/L74-java-channel-creds.md @@ -516,6 +516,15 @@ are set. Keeping track of which features were available at each version can be error-prone. It's also hard to produce a user-meaningful error message, so they are aware of which specific configuration is causing the incompatibility. +It would seems to be very convenient to be able to share the +TlsChannelCredential API on server-side, as it is quite a wide API and _almost_ +the same. However, on deeper inspection only the trust and key management is +likely to be the same. This is partly because not _all_ the TLS configuration is +expected to be exposed, and instead advanced use cases would use +transport-specific configuration or potentially "all encompassing" configuration +like providing a pre-configured SSLContext. Using separate APIs makes the APIs +more clear and would have limited cost. + ## Implementation ejona86 will implement. TLS will initially be bare-bones, with just the @@ -525,13 +534,4 @@ ServerCredentials. ## Open issues (if applicable) -It would be very convenient to be able to share the TlsChannelCredential API on -server-side, as it is quite a wide API and _almost_ the same. The consumer -would need to be slightly different, but much of the configuration can -potentially be shared. Should we make TlsChannelCredential and -TlsServerCredential just a holder for a TlsConfiguration class? For validation, -it would need to know whether it was for server-side or client-side. It would -also have the union of all client- and server-specific configuration. See -[Netty's SslContextBuilder]( -https://netty.io/4.1/api/io/netty/handler/ssl/SslContextBuilder.html) -for a view into how client and server differ. +N/A From 47d3f3bdd2fffbeb6513767ab4503d8838defca3 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Thu, 13 Aug 2020 13:01:39 -0700 Subject: [PATCH 11/16] Include ChoiceServerCredentials on server-side --- L74-java-channel-creds.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/L74-java-channel-creds.md b/L74-java-channel-creds.md index 45cf1efe8..5cf3f910b 100644 --- a/L74-java-channel-creds.md +++ b/L74-java-channel-creds.md @@ -153,7 +153,8 @@ public final class ChoiceChannelCredentials extends ChannelCredentials { } ``` -`ChoiceChannelCredentials` is intended to be used in cases like +`ChoiceChannelCredentials` allows code to use new credentials in the future without +risk of breaking the existing code flow. It can also be used in cases like `GoogleDefaultCredentials`, where TLS may be satisfactory, but there may also be more optimal credentials if the transport is a certain type. @@ -388,8 +389,7 @@ final class ProtocolNegotiators { Server-side will be a clone of client-side, but using the `Server` instead of `Channel`. `ServerCredentials` cannot contain `CallCredentials`, so -`CompositeServerCredentials` will be omitted. There is also currently no known -use-case for `ChoiceServerCredentials`, so it will be omitted. +`CompositeServerCredentials` will be omitted. ```java // instead of: From 296e473f8d5f8545f5d1c851b2872005af972355 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Thu, 13 Aug 2020 13:02:06 -0700 Subject: [PATCH 12/16] Fix typo s/ServerCredentials/ChannelCredentials/ --- L74-java-channel-creds.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/L74-java-channel-creds.md b/L74-java-channel-creds.md index 5cf3f910b..f982c5685 100644 --- a/L74-java-channel-creds.md +++ b/L74-java-channel-creds.md @@ -530,7 +530,7 @@ more clear and would have limited cost. ejona86 will implement. TLS will initially be bare-bones, with just the compatibility checker. Over the long-term TLS will be fleshed out with additional configuration. ServerCredentials will be implemented shortly after -ServerCredentials. +ChannelCredentials. ## Open issues (if applicable) From cb0c8b95928cd30baafa769790fe75949632c8a6 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Mon, 17 Aug 2020 11:49:03 -0700 Subject: [PATCH 13/16] Remove language about focusing on client-side Server side can just declare it mirrors client-side, and it is more fleshed out than it was earlier --- L74-java-channel-creds.md | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/L74-java-channel-creds.md b/L74-java-channel-creds.md index f982c5685..eca38dabc 100644 --- a/L74-java-channel-creds.md +++ b/L74-java-channel-creds.md @@ -23,11 +23,6 @@ This is to allow: credential needs to be provided by the user * aligning with C core and Go implementations -_The design focuses on channel credentials, since they are the most complex and -most discussed._ Server credentials will mostly just mirror the channel -credentials; they suffer from most of the same problems but at a smaller -scale. - ## Background Prior to gRPC v0.12.0, C core had a "unified" `Credential` type that was @@ -54,7 +49,7 @@ credentials (`credentials.TransportCredentials`), but with a separate type to combine them (`credentials.Bundle`). The split credentials significantly predate `Bundle`. -### Related Proposals: +### Related Proposals: N/A From 23d6df86a66bb2eb30d03a34edb5fb672f8c1032 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Mon, 17 Aug 2020 12:02:40 -0700 Subject: [PATCH 14/16] s/ChannelCredentials/ServerCredentials/ in one spot --- L74-java-channel-creds.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/L74-java-channel-creds.md b/L74-java-channel-creds.md index eca38dabc..2ba65917a 100644 --- a/L74-java-channel-creds.md +++ b/L74-java-channel-creds.md @@ -394,7 +394,7 @@ server = ServerBuilder.forPort(443) server = ServerBuilder.forPort(8080) .build().start(); // the user would now: -ChannelCredentials tlsCreds = new TlsServerCredentials( +ServerCredentials tlsCreds = new TlsServerCredentials( new File("cert.pem"), new File("cert.key")); server = Grpc.newServerBuilderForPort(443, tlsCreds) .build().start(); From ff3c104192a4fd1a40e1faa2dac08bc7edb29b94 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Mon, 17 Aug 2020 12:54:34 -0700 Subject: [PATCH 15/16] Add NettySslContextChannelCredentials, InsecureFromHttp1ChannelCredentials, SslSocketFactoryChannelCredentials These are necessary to allow migration from existing APIs. --- L74-java-channel-creds.md | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/L74-java-channel-creds.md b/L74-java-channel-creds.md index 2ba65917a..ac7ea7aa7 100644 --- a/L74-java-channel-creds.md +++ b/L74-java-channel-creds.md @@ -239,6 +239,45 @@ be added. Observing the contents of those methods would be a requirement for might not support encrypted private keys and so could consider the credential unsupported if `getPassword()` returned non-`null`. +To support migration from existing builder options, Netty and OkHttp will +provide transport-specific credentials. Note that neither transport will support +reading the configuration after creation, except by the transport. + +```java +package io.grpc.netty; + +/** A credential with full control over the security handshake. */ +public final class NettySslContextChannelCredentials { + private NettySslContextChannelCredentials() {} + + /** + * Create a credential using Netty's SslContext as configuration. It must have + * been configured with {@link GrpcSslContexts}, but options could have been + * overridden. + */ + public static ChannelCredentials create(SslContext sslContext) {...} +} + +/** An insecure credential that upgrades from HTTP/1 to HTTP/2. */ +public final class InsecureFromHttp1ChannelCredentials { + private InsecureFromHttp1ChannelCredentials() {} + + /** Creates an insecure credential that will upgrade from HTTP/1 to HTTP/2. */ + public static ChannelCredentials create() {...} +} +``` + +```java +package io.grpc.okhttp; + +/** A credential with full control over the SSLSocketFactory. */ +public final class SslSocketFactoryChannelCredentials { + private SslSocketFactoryChannelCredentials() {} + + public static ChannelCredentials create(SSLSocketFactory factory) {...} +} +``` + To support very advanced use cases, Netty will provide a credential that wraps a `ProtocolNegotiator`. This allows implementations like ALTS and XDS to use internal APIs without forcing their users to use experimental or internal APIs, From e7eaf60c5b6a3b4002376337b8aba98d22dd2325 Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Thu, 27 Aug 2020 17:57:14 -0700 Subject: [PATCH 16/16] Update status and last update date --- L74-java-channel-creds.md | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/L74-java-channel-creds.md b/L74-java-channel-creds.md index ac7ea7aa7..bedf6631e 100644 --- a/L74-java-channel-creds.md +++ b/L74-java-channel-creds.md @@ -2,9 +2,9 @@ Java: Channel and Server Credentials ---- * Author(s): [Eric Anderson](https://github.com/ejona86) * Approver: sanjaypujare -* Status: In Review {Draft, In Review, Ready for Implementation, Implemented} +* Status: Ready for Implementation {Draft, In Review, Ready for Implementation, Implemented} * Implemented in: -* Last updated: 2020-06-17 +* Last updated: 2020-08-27 * Discussion at: https://groups.google.com/g/grpc-io/c/GNU9svnDsQs/m/kG6XsUy9BwAJ ## Abstract @@ -561,11 +561,6 @@ more clear and would have limited cost. ## Implementation -ejona86 will implement. TLS will initially be bare-bones, with just the -compatibility checker. Over the long-term TLS will be fleshed out with -additional configuration. ServerCredentials will be implemented shortly after -ChannelCredentials. - -## Open issues (if applicable) - -N/A +ejona86 will implement. ServerCredentials will be implemented shortly after +ChannelCredentials. ChannelCredentials will be implemented in +https://github.com/grpc/grpc-java/pull/7294