-
Notifications
You must be signed in to change notification settings - Fork 252
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
Add TCP transport #490
base: master
Are you sure you want to change the base?
Add TCP transport #490
Conversation
6490781
to
250a844
Compare
250a844
to
70a49b2
Compare
Tests for |
@legastero |
First off, you've done a good job getting TCP and TLS connections to work here. Merging and supporting this as a feature in stanza, though, is going to take some more work to prepare for it. I do recognize that you need this for your Bonfire client, so as you noticed I've started updating the transport code to make it at least possible to load your TCP transport here in your app so you don't have to maintain as many fork changes until stanza gains support for TCP/TLS connections. Some variation of: const client = createClient({
transports: { tcp: true },
transportPreferenceOrder: ['tcp']
});
client.transports.tcp = TCP; As for what it will take to properly integrate this, here's the shortlist of things:
Basically, arbitrary TLS connections require a lot to ensure they are handled securely in a library. Using just HTTPS/WSS has done a lot of heavy lifting and side stepping in that regard. |
That makes sense. For the time being that snippet seems like a clean way to do transports, although using TCP/TLS in its current state would surely not be as secure as it deserves to be for the reasons you've mentioned. Some thoughts:
This would only be doable on Node of course, which is already handled now that
This could just be a matter of connecting within the preference loop and awaiting transport connection success/failure. Success would return and failure would fall through until the
Generalizing "transport candidates" as transports plus endpoint information could be a clean solution to this combined with the priority-order falling through described above. This would take one pass to generate the candidates and another to attempt/pick one. It would be interesting and very likely possible to do this in parallel and use This would also be very useful for cases where transport encryption is not required (although this should probably be default on). For instance
This is very important and technically applies to websocket/BOSH as well. Instead of only using secure websocket/BOSH protocol schemes, you could also use raw HTTP or websocket connections unless a
Combining SASL with the transports would be interesting. Salted hash password challenges would obviously be heavily preferable if |
Additionally, a A major architectural change to all transports is the new promise-based structure of Tests exist for as much of this as possible, especially for security critical features. |
Adds a transport called
tcp
. Connections to port 5223 are implicitly direct TLS.Handles unencrypted, STARTTLS, and direct TLS connections. TLS renegotiation is not currently handled.
Additional transport options:
directTLS
: Explicitly enables direct TLS connections.port
: Sets the TCP port to connect to. This overrides the port given inurl
.Packages
net
andtls
shim the respective node modules to allow building for web. This change also checks if a transport exists before attempting to create it, so while TCP is the preferred transport on node, it will not be attempted on web.