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

Add secure method to tcp socket for firefox #12

Merged
merged 3 commits into from
Aug 12, 2014
Merged

Conversation

dborkan
Copy link
Contributor

@dborkan dborkan commented Aug 11, 2014

Adding a secure method to tcp socket for firefox.

Firefox sockets can be upgraded to TLS using by calling nsISocketTransportService.createTransport with type 'starttls' and the same hostname and port as an existing insecure tcp socket. Documentation is here: https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsISocketTransportService#createTransport()

Tested by connecting to chat.facebook.com on port 5222, then upgrading the socket to TLS, and verifying we get prompted with a challenge from facebook in response.

@dborkan
Copy link
Contributor Author

dborkan commented Aug 11, 2014

FYI, this pull request is also related to freedomjs/freedom#92 (freedom interface) and freedomjs/freedom-typescript-api#19 (typescript API update)

}
this.clientSocket = new ClientSocket();
this.clientSocket.setOnDataListener(this._onData.bind(this));
this.clientSocket.connect(this.hostname, this.port, true);
Copy link
Member

Choose a reason for hiding this comment

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

Does this continue the existing TCP session, or start a new one? It looks like it makes a new connection to the host/port.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It continues the existing TCP session. The firefox API is weird and unlike in Chrome you don't get a unique socket identifier which you can upgrade to TLS, nor is there a socket object with any upgradeToTls method. Instead it appears to keep a mapping from hostname/port to a socket which can be upgraded to tls by creating a new nsISocketTransport object (not necessarily the same as creating a new connection) with type 'starttls'.

We will need to test this in the future and make sure it doesn't result in weird race conditions if we have 2 pieces of code both trying to connect to the same hostname/port and do a starttls flow (e.g. if there are 2 instances of a GTalk social provider that are both trying to connect to GTalk at the same time with different logins)

Copy link
Member

Choose a reason for hiding this comment

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

seems worth a comment.

On Tue, Aug 12, 2014 at 8:51 AM, dborkan notifications@github.com wrote:

In providers/tcp_socket.js:

  • this.port = port;
  • continuation();
    +};

+// TODO: handle failures.
+Socket_firefox.prototype.secure = function(continuation) {

  • if (!this.hostname || !this.port || !this.clientSocket) {
  • continuation(undefined, {
  •  "errcode": "SOCKET_NOT_CONNECTED",
    
  •  "message": "Cannot Secure Not Connected Socket"
    
  • });
  • return;
  • }
  • this.clientSocket = new ClientSocket();
  • this.clientSocket.setOnDataListener(this._onData.bind(this));
  • this.clientSocket.connect(this.hostname, this.port, true);

It continues the existing TCP session. The firefox API is weird and unlike
in Chrome you don't get a unique socket identifier which you can upgrade to
TLS, nor is there a socket object with any upgradeToTls method. Instead it
appears to keep a mapping from hostname/port to a socket which can be
upgraded to tls by creating a new nsISocketTransport object (not
necessarily the same as creating a new connection) with type 'starttls'.

We will need to test this in the future and make sure it doesn't result in
weird race conditions if we have 2 pieces of code both trying to connect to
the same hostname/port and do a starttls flow (e.g. if there are 2
instances of a GTalk social provider that are both trying to connect to
GTalk at the same time with different logins)


Reply to this email directly or view it on GitHub
https://github.com/freedomjs/freedom-for-firefox/pull/12/files#r16121843
.

@dborkan
Copy link
Contributor Author

dborkan commented Aug 12, 2014

Just added a test for starttls, PTAL

@willscott
Copy link
Member

👍

dborkan added a commit that referenced this pull request Aug 12, 2014
Add secure method to tcp socket for firefox
@dborkan dborkan merged commit 543f814 into master Aug 12, 2014
@ryscheng ryscheng deleted the dborkan-secure branch December 12, 2014 02:15
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

2 participants