-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Dialer: add optional method NetDialTLSContext #746
Conversation
f50b198
to
e87c21b
Compare
The first commit is unrelated to the issue at hand, but otherwise the CI was failing on |
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]>
e87c21b
to
18650c5
Compare
Thanks @garyburd for your feedback. I updated now the pull request following your suggestion. I updated the commit message accordingly too.
As
Roger that 👍 |
Thank you for your contribution. 👍 for the included test. |
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]>
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]>
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