feat(tls): Support async TlsHandshake in protocol.Conn#83
Conversation
louisruch
changed the title
irenarindos
left a comment
irenarindos
left a comment
There was a problem hiding this comment.
Just two small things
louisruch
force-pushed
the
louis-conn
branch
3 times, most recently
from
ea52521 to
c25aa37
Compare
ddebko
left a comment
ddebko
left a comment
There was a problem hiding this comment.
I left a few non-blocking suggestions, mostly defensive checks. I also had one question about a possible cleanup issue in case I’m overlooking something.
louisruch
dismissed stale reviews from ddebko, RyanDerr, and irenarindos
via
83f004a
| } | ||
|
|
||
| _ = c.waitForHandshake() | ||
|
|
There was a problem hiding this comment.
Meta comment about this overall approach. Custom reads and writes I think can break the ability for some kernel speedups in conns. We can't really use that anyways but in downstream code we may want to pull out the *tls.Conn when possible to be closer to the bare connection. At least in current Boundary it shouldn't matter, but if we use a different proxying method it might be important.
There was a problem hiding this comment.
thanks for this note, I am going to merge this PR as is and then add some additional comments in future PR to allow time for Boundary team to test these changes before getting it to the customer
jefferai
left a comment
jefferai
left a comment
There was a problem hiding this comment.
I think it looks good! Pending testing in Boundary to make sure everything still works properly.
5 participants
Description
Runs TlsHandshake in goroutine and adds timeout (defaults 1 minute) to the TlsHandshake
PCI review checklist
I have documented a clear reason for, and description of, the change I am making.
If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.
If applicable, I've documented the impact of any changes to security controls.
Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.