search

flackr / circ

An IRC packaged chrome app
BSD 3-Clause "New" or "Revised" License
388 stars 79 forks source link

For SSL connections, server name and other options are not verified? #429

Open DrJosh9000 opened 5 years ago

DrJosh9000 commented 5 years ago

I found that I was easily able to do IRC over TLS over an SSH port-forward, but I expected it to fail because localhost != the name of the host in the server certificate.

https://github.com/flackr/circ/blob/master/package/bin/net/ssl_socket.js#L84 (and options always being {}) seems to show that the verify function will always return true. Docs for forge indicate this is where server name validation should happen (https://github.com/digitalbazaar/forge#options).

I haven't spent very long looking into this, I may be wrong. But it was surprising.

alexgartrell commented 4 years ago

I want to +1 this. I installed circ today and played with different certificates. First, circ doesn't do any name validation at all, so if I generate a cert with the wrong name, it still works. Second, in the event that I have a self-signed cert with the right name, it also works. So man-in-the-middling secure CIRC connections is trivial.

alexgartrell commented 4 years ago

See Pull Request #433

flackr commented 4 years ago

I tried using chrome.socket as well but found that it fails on many popular ssl irc servers with net::ERR_SSL_CLIENT_AUTH_CERT_NEEDED (error 110). A more compatible change would be to use forge's verification, and ideally trusting a self signed certificates if the user chooses to run a bouncer or similar.