Add TLS to the NIO implementation #406
Conversation
glbrntt
commented
Mar 19, 2019
glbrntt
commented
- Tests to follow in a separate PR
- Certificate and key from Echo have been updated since the certificate had expired
- EchoNIO uses the same certificate and key
- Also adds diagram to explain the server pipeline
| /// Wrapper object to manage the lifecycle of a gRPC server. | ||
| /// | ||
| /// The pipeline is configured in three stages detailed below. Note: handlers marked with | ||
| /// a '*' are responsible for handling errors. |
|
I also submitted a PR against master with the updated Echo certificate and key (#407) |
MrMage
left a comment
MrMage
left a comment
There was a problem hiding this comment.
Great work, especially the flow chart! I do wonder, though, if we should provide a more convenient way of setting up "vanilla" SSL, especially on the client (see the gRPC-Core implementation, where you'd essentially just need to add , secure: true). Happy to have that in a later PR, though.
When you add tests in a follow-up PR, consider doing it similar to the gRPC-Core implementation, where we essentially run all tests a second time with SSL enabled.
Sources/Examples/EchoNIO/main.swift
Outdated
|
|
||
| func makeClientTLSConfiguration() throws -> TLSConfiguration { | ||
| let certificate = try NIOSSLCertificate(file: "ssl.crt", format: .pem) | ||
| // The certificate common name is "example.com", so skip verification. |
There was a problem hiding this comment.
nit: mind inserting "hostname" after "skip"?
That's a good point. It's worth adding now since it should be fairly straightforward.
Exactly what I had in mind ;) |
MrMage
left a comment
MrMage
left a comment
There was a problem hiding this comment.
LGTM; I'd appreciate if @rebello95 could also take a look before merging.
|
@MrMage I trust your review - I'm not very familiar with the NIO implementation 😄 |
4 participants
