Skip to content
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

Add support for Network.framework TLS options #1221

Merged
merged 4 commits into from
Jul 13, 2021
Merged

Conversation

glbrntt
Copy link
Collaborator

@glbrntt glbrntt commented Jul 12, 2021

Motivation:

gRPC Swift currently half supports Network.framework via
NIOTransportServices, that is, we make use of the network stack where
it's available but we always use NIOSSL for TLS. We should offer users a
way to use Network.framework's TLS stack.

Modifications:

  • Add 'GRPCTLSConfiguration' which is a wrapper around configuration
    structures for both NIOSSL and Network.framework. This is for both
    client and server configuration.
  • Add static factory methods making it easier for callers to construct
    the right configuration with the right defauls.
  • Add static factory methods allowing the caller to construct a
    GRPCTLSConfiguration from the 'base' configuration (i.e. a
    NIOSSL.TLSConfiguration or NWProtocolTLS.Options)
  • Add new methods to the client and server builders to make a builder
    backed by a given TLS backend. In addition the client has a helper for
    automatically choosing the right backend from the type of the provided
    event loop group.
  • Add backend specific options to each builder.
  • Deprecate the generic 'secure' builders for both the client and the
    server.

Result:

Users can configure their client/server to use the TLS implementation
provided by Network.framework where appropriate.

@glbrntt glbrntt added the 🆕 semver/minor Adds new public API. label Jul 12, 2021
@glbrntt glbrntt requested a review from Lukasa July 12, 2021 13:39
Motivation:

gRPC Swift currently half supports Network.framework via
NIOTransportServices, that is, we make use of the network stack where
it's available but we always use NIOSSL for TLS. We should offer users a
way to use Network.framework's TLS stack.

Modifications:

- Add 'GRPCTLSConfiguration' which is a wrapper around configuration
  structures for both NIOSSL and Network.framework. This is for both
  client and server configuration.
- Add static factory methods making it easier for callers to construct
  the right configuration with the right defauls.
- Add static factory methods allowing the caller to construct a
  `GRPCTLSConfiguration` from the 'base' configuration (i.e. a
  `NIOSSL.TLSConfiguration` or `NWProtocolTLS.Options`)
- Add new methods to the client and server builders to make a builder
  backed by a given TLS backend. In addition the client has a helper for
  automatically choosing the right backend from the type of the provided
  event loop group.
- Add backend specific options to each builder.
- Deprecate the generic 'secure' builders for both the client and the
  server.

Result:

Users can configure their client/server to use the TLS implementation
provided by Network.framework where appropriate.
Sources/GRPC/ClientConnection.swift Show resolved Hide resolved
//
// 'nil' means we're not using TLS, or we're using the Network.framework TLS backend. We'll
// check and apply the Network.framework TLS options when we create a bootstrap.
let sslContext: Result<NIOSSLContext, Error>?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, Optional<Result> is sufficiently opaque that I'm never going to be quite sure what each thing means. I wonder if this needs a temporary type.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it is a little unclear. I extracted this into a TLSMode.

Sources/GRPC/ConnectionManagerChannelProvider.swift Outdated Show resolved Hide resolved
Sources/GRPC/GRPCTLSConfiguration.swift Show resolved Hide resolved
Sources/GRPC/GRPCTLSConfiguration.swift Outdated Show resolved Hide resolved
Sources/GRPC/GRPCTLSConfiguration.swift Show resolved Hide resolved
Sources/GRPC/GRPCTLSConfiguration.swift Outdated Show resolved Hide resolved
@glbrntt glbrntt requested a review from Lukasa July 13, 2021 10:25
@glbrntt glbrntt merged commit 725154a into grpc:main Jul 13, 2021
@glbrntt glbrntt deleted the gb-nwfw branch July 13, 2021 13:37
glbrntt added a commit to glbrntt/grpc-swift that referenced this pull request Aug 3, 2021
Motivation:

In grpc#1221 we added Network.framework as a provider for TLS. However in a
few places we incorrectly set the availability for watchOS as 5.0. It
should be 6.0 (as required by NIOTS).

If the watchOS deployment target was set to 5.0 then the build would
result in a number of errors (as the availability requirements for using
various NIOTS types would not be sufficient).

Modifications:

- Raise the availability version for various APIs which use Network.framework

Result:

No build errors for types which aren't availabile on watchOS 5.0.
glbrntt added a commit that referenced this pull request Aug 6, 2021
Motivation:

In #1221 we added Network.framework as a provider for TLS. However in a
few places we incorrectly set the availability for watchOS as 5.0. It
should be 6.0 (as required by NIOTS).

If the watchOS deployment target was set to 5.0 then the build would
result in a number of errors (as the availability requirements for using
various NIOTS types would not be sufficient).

Modifications:

- Raise the availability version for various APIs which use Network.framework

Result:

No build errors for types which aren't availabile on watchOS 5.0.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🆕 semver/minor Adds new public API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants