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

This adds an option to skip the TLS connection wrapper #1146

Closed
wants to merge 1 commit into from

Conversation

oderwat
Copy link
Contributor

@oderwat oderwat commented Nov 29, 2022

This option to skip the TLS wrapper is meant to be used if a custom dialer already handled the TLS handshake. I discussed my use case, which is using NATS from a Wasm module running in the browser in Slack with @derekcollison. There may be other use cases when the custom dialer is using some kind of tunneling, for example.

This option to skip the TLS wrapper is meant to be used if a custom
dialer already handled the TLS handshake. I discussed my use case,
which is using NATS from a Wasm module running in the browser in
Slack with @derekcollison. There may be other use cases when the
custom dialer is using some kind of tunneling, for example.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.09%) to 85.763% when pulling c9b2fd8 on metatexx:MTX-SkipTLSWrapper into 398a1ec on nats-io:main.

if nc.Opts.SkipTLSWrapper {
// we do nothing when asked to skip the TLS wrapper
return nil
}
Copy link
Member

Choose a reason for hiding this comment

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

I think we can achieve some of this with adding extra behavior to the implementation of the CustomDialer interface, to avoid having another option that is coupled to using a CustomDialer. We can have an internal interface as follows in the client, which the CustomDialer could implement:

type skipTLSDialer interface {
	SkipTLS() bool
}

// makeTLSConn will wrap an existing Conn using TLS.
func (nc *Conn) makeTLSConn() error {
	if nc.Opts.CustomDialer != nil {
		// we do nothing when asked to skip the TLS wrapper.
		sd, ok := nc.Opts.CustomDialer.(skipTLSDialer)
		if ok && sd.SkipTLS() {
			return nil
		}
	}

Then from the app perspective you would implement Dial and toggle the behavior by implementing SkipTLS like this:

type testSkipTLSDialer struct {
	dialer *net.Dialer
}

func (sd *testSkipTLSDialer) Dial(network, address string) (net.Conn, error) {
	return sd.dialer.Dial(network, address)
}

func (sd *testSkipTLSDialer) SkipTLS() bool {
	return true
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had the same though about relating that functionality to the custom dialer, but I failed to come up with a practical solution. That said, I am happy with any way to have the skip functionality merged because I do not want to deal with a fork. This way seems very unobtrusive.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Who can / will decide about the way of the implementation? What can I do?

Copy link
Member

Choose a reason for hiding this comment

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

I have branch with the approach above on top of your commit, will send PR shortly for review

Copy link
Member

Choose a reason for hiding this comment

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

opened PR with this change: #1147 /cc @piotrpio

@oderwat
Copy link
Contributor Author

oderwat commented Nov 29, 2022

I want to dump my code to get working NATS from WASM through web socket working here:

//go:build wasm
// This expects to be in a WASM build and uses:
import "nhooyr.io/websocket"

func (cw *ConnectionWrapper) Dial(network, address string) (net.Conn, error) {
	//dbg.Logf("Got Request for Network: %q / Address: %q", network, address)
	ctx, cancel := context.WithTimeout(context.Background(), time.Second)
	defer cancel()

	// The address we get has to be a string without a protocol. We will
	// always connect throught wss though!
	//dbg.Logf("Dialing Address: wss://%s", address)
	c, _, err := websocket.Dial(ctx, "wss://"+address, nil)
	if err != nil {
		return nil, fmt.Errorf("websocket.Dial failed %w", err)
	}
	netconn := websocket.NetConn(context.Background(), c, websocket.MessageBinary)
	return netconn, nil
}

func Connect() (*nats.Conn, error) {
   var err error
   // Here we do not specify wss because if we do specify one with "wss://" it
   // wants to handle the TLS by itself, but our CustomDialer does that already.
   // That way it seems to work flawlessly.
   nc, err = nats.Connect("tls.domain.io:5222,tls.domain.io:5223,tls.domain.io:5224",
      UserCredentialsFromContent(CW.CredsFileContent),
      nats.SetCustomDialer(&CW),
      nats.SetSkipTLSWrapper(true) /* may change with implementing this PR */ )
   if err != nil {
      return nil, fmt.Errorf("Native Go Connect did fail: %w", err)
   } else {
      return nc, nil
   }
}

This is the "magic" code in my Wasm frontend. The CustomDialer and the Connect(). This is from proprietary code. But I guess I could make an example for a simplified Go-App client that uses NATS. People always wonder how to connect to the backend and I think NATS would be a wonderful solution because a go-app "app" is always a backend and the frontend in Go and embedding the server in the backend code is trivial

@oderwat
Copy link
Contributor Author

oderwat commented Nov 29, 2022

I close this because of #1147 will solve it. Thank you for the work on it @wallyqs !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants