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

Verify that certificate is valid for server hostname #107

Merged
merged 1 commit into from
Apr 23, 2017

Conversation

blunden
Copy link
Contributor

@blunden blunden commented Apr 22, 2017

Without this change, the WebSocket library will accept
a trusted certificate issued for domain A when connecting
to domain B. This could be exploited for Man-in-the-middle
attacks.

The underlying issue is that Java considers hostname
verification to be a part of HTTPS and as such, will
not perform it by default.

This change adds the default HostnameVerifier used
by Android, which in recent versions is derived from
OkHttp. Minor changes were made to make it build for
Java 1.6.

Tested with and without a proxy configured.

Without this change, the WebSocket library will accept
a trusted certificate issued for domain A when connecting
to domain B. This could be exploited for Man-in-the-middle
attacks.

The underlying issue is that Java considers hostname
verification to be a part of HTTPS and as such, will
not perform it by default.

This change adds the default HostnameVerifier used
by Android, which in recent versions is derived from
OkHttp. Minor changes were made to make it build for
Java 1.6.

Tested with and without a proxy configured.
@blunden
Copy link
Contributor Author

blunden commented Apr 22, 2017

@TakahikoKawasaki A public test server that can be used to reliably reproduce this issue is available to you upon request.

Status with this change:

  • No proxy: Working
  • Standard HTTP proxy: Working (verifies that hostname of the wss:// request matches certificate of the endpoint server)
  • HTTPS proxy: Untested (likely not working but I was unable to find a suitable proxy to test this with)

@TakahikoKawasaki
Copy link
Owner

@blunden Thank you very much. Apparently, you have much more knowledge about this field than I.

@TakahikoKawasaki TakahikoKawasaki merged commit fb23d30 into TakahikoKawasaki:master Apr 23, 2017
TakahikoKawasaki added a commit that referenced this pull request Apr 23, 2017
instead of SSLPeerUnverifiedException when the certificate of
the peer does not match the expected hostname.

This change is related to the pull request #107 (Verify that
certificate is valid for server hostname).
@TakahikoKawasaki
Copy link
Owner

@blunden Released nv-websocket-client version 2.2. WebSocket.connect() method throws HostnameUnverifiedException (a subclass of WebSocketException) when the certificate of the peer does not match the expected hostname. Thank you for your contribution.

@blunden
Copy link
Contributor Author

blunden commented Apr 23, 2017

@TakahikoKawasaki Thanks for merging it.

You will likely need to add hostname verification of the proxy's certificate if it's an HTTPS proxy. Like I said above, I don't have any of those to test with so currently it won't verify the hostname of the proxy, which it really should. It would probably be a simple fix but the ProxyHandshaker or Address would have to be able to return whether the user specified an http or https proxy address. You could then just add an if-statement based on that and a similar hostname verification of mAddress.getHostname() (which I think will be the proxy hostname) inside it. It did not do it myself because I couldn't test it and didn't want to risk breaking it.

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