Dialer: add optional method NetDialTLSContext#746
Merged
1 commit merged intogorilla:masterfrom Jan 4, 2022
Merged
Conversation
lluiscampos
force-pushed
the
issue-745-net-dial-tls
branch
from
f50b198 to
e87c21b
Compare
Contributor
Author
|
The first commit is unrelated to the issue at hand, but otherwise the CI was failing on |
lluiscampos
added a commit
to lluiscampos/mender
that referenced
this pull request
Dec 10, 2021
By switching to the "enhanced" API for websocket.Dialer from mendersoftware's fork. There is a limitation in current gorilla/websocket.Dialer API in that the user cannot specify a dial method for TLS/TCP connections. The TLS handshake is always done by the library based on user's TLSClientConfig, but that is not enough for Mender as we need it to be done via OpenSSL (aka our dial wrapper for TLS) so that advance auth features like getting the keys from HSM. This commit switches to mendersoftware's fork and modifies the code accordingly (one line change!). The patch has been submitted upstream. See: * gorilla/websocket#745 * gorilla/websocket#746 Changelog: None No changelog, commit 84204a3 claims to support websockets, this commit just fixes a bug there which has not been released. Signed-off-by: Lluis Campos <[email protected]>
Fixes issue: gorilla#745 With the previous interface, NetDial and NetDialContext were used for both TLS and non-TLS TCP connections, and afterwards TLSClientConfig was used to do the TLS handshake. While this API works for most cases, it prevents from using more advance authentication methods during the TLS handshake, as this is out of the control of the user. This commits introduces another a new dial method, NetDialTLSContext, which is used when dialing for TLS/TCP. The code then assumes that the handshake is done there and TLSClientConfig is not used. This API change is fully backwards compatible and it better aligns with net/http.Transport API, which has these two dial flavors. See: https://pkg.go.dev/net/http#Transport Signed-off-by: Lluis Campos <[email protected]>
lluiscampos
force-pushed
the
issue-745-net-dial-tls
branch
from
e87c21b to
18650c5
Compare
Contributor
Author
|
Thanks @garyburd for your feedback. I updated now the pull request following your suggestion. I updated the commit message accordingly too.
As
Roger that 👍 |
lluiscampos
changed the title
ghost
merged commit
lluiscampos
added a commit
to lluiscampos/mender
that referenced
this pull request
Jan 10, 2022
Now that upstream have merged our PR. Slightly modify our code accordingly. See: * gorilla/websocket#745 * gorilla/websocket#746 Changelog: None Signed-off-by: Lluis Campos <[email protected]>
lluiscampos
added a commit
to lluiscampos/mender
that referenced
this pull request
Jan 10, 2022
Now that upstream have merged our PR. Slightly modify our code accordingly. See: * gorilla/websocket#745 * gorilla/websocket#746 Changelog: None Signed-off-by: Lluis Campos <[email protected]>
This pull request was closed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes issue: #745
With the previous interface, NetDial and NetDialContext were used for
both TLS and non-TLS TCP connections, and afterwards TLSClientConfig was
used to do the TLS handshake.
While this API works for most cases, it prevents from using more advance
authentication methods during the TLS handshake, as this is out of the
control of the user.
This commits introduces another a new dial method, NetDialTLSContext,
which is used when dialing for TLS/TCP. The code then assumes that the
handshake is done there and TLSClientConfig is not used.
This API change is fully backwards compatible and it better aligns with
net/http.Transport API, which has these two dial flavors. See:
https://pkg.go.dev/net/http#Transport