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

The Paho Java client does not perform peer verification on the connected socket #38

Closed
jpwsutton opened this issue Feb 4, 2016 · 0 comments

Comments

@jpwsutton
Copy link
Member

migrated from Bugzilla #425195
status REOPENED severity enhancement in component MQTT-Java for 1.2
Reported in version 1.1 on platform PC
Assigned to: Ian Craggs

On 2014-01-09 09:26:26 -0500, Ian Craggs wrote:

Reported by: alexrhelder@outlook.com

The Paho Java client does not perform peer verification on the connected socket. This allows peer spoofing / MITM attacks.

Proposed Solution # 1

Like HttpsURLConnection, the IMQTTClient interface could get something like the following:

void setHostnameVerifier(HostnameVerifier hv);

where Java5 built-in HostNameVerifier interface is either reused as-is or inspires a Paho equivalent.

http://docs.oracle.com/javase/1.5.0/docs/api/javax/net/ssl/HostnameVerifier.html

Proposed Solution # 2
Instead of SSLNetworkModule / TCPNetworkModule creating a disconnected socket via

SocketFactory.createSocket(), use SocketFactory.createSocket(String hostname, int port)

A custom SSLFactory implementation could look like:

class MySSLSocketFactory {

SSLSocketFactory delegate;

SSLSocket createSocket(String hostname, int port) throws IOException {
    SSLSocket s = delegate.createSocket(hostname, port);
    s.startHandshake();
    verifyHostName(s, host);
}

void verifyHostName(Socket s, String host) {
    // Throw exception if fail verification
}

}

In any case, I think the Paho client should not create a disconnected socket; this allows the SSLSocketFactory to apply alternative settings and policies on the created socket.

Note: Java 7 has X509ExtendedTrustManager which is a connection-sensitive trust manager. This may also be leveraged in the future, but is relatively new.

On 2014-01-09 09:36:26 -0500, Ian Craggs wrote:

Al disagrees on the seriousness of this bug report, don't you Al? (And whether it is truly a vulnerability)

On 2014-01-09 09:49:18 -0500, Al Stockdill-Mander wrote:

I do :) In that it depends on your use case, If you're deploying an app that will talk to known servers over SSL you will preseed your trust store with only the certificates relevant to the systems you wish to connect to and the lack of hostname verification doesn't seem to be a problem.

That's not to say I think we shouldn't make this an option anyway.

On 2014-04-22 10:50:27 -0400, Ian Craggs wrote:

So, this is a worthwhile enhancement, but not a vulnerability.

On 2014-05-01 10:03:19 -0400, Al Stockdill-Mander wrote:

what's the vulnerability? You have to steal the private key first to do anything with it.
once you've done that you can already decrypt the communications
i need to review it again. but if a mitm is possible (and i'm not hallucinating) it seems very serious to me. this in conjunction with the likely scenario that mqtt will often be used in IoT scenario, where updating clients may be difficult because they are in remote locations or the sheer number of them
you can't mitm just because we don't do cn checking
you'd have to redirect the client and have stolen a private key that the client would verify
these devices are not going to be webservers/clients. I would very much not recommend putting the whole swathe of root CA certs on them
13:15 < alsm> these devices are not going to be webservers/clients. I would very much not recommend putting the whole swathe of root CA certs on them
This is a point worth repeating.
OK, sounds good, i'm getting rusty at this topic already, its so tricky
if its been reviewed, thats good
But it will be added to the client

Once the J2ME codebase is split off into its own repository we can utilise language features > 1.4.2 and implement the HostNameVerifier interface.

On 2014-11-10 07:14:52 -0500, Davy De Waele wrote:

I noticed this is marked for v0.5 but according to https://projects.eclipse.org/projects/technology.paho/releases/1.1.0/bugs the next release will be v1.1.0

Will this be fixed or will there be a workaround so we are able to do hostname verification ?

On 2015-02-02 11:52:01 -0500, Ian Craggs wrote:

Reassigning to Bin.

On 2015-07-09 07:29:17 -0400, Ian Craggs wrote:

Reassiging back to me.

@jpwsutton jpwsutton added the bug label Feb 12, 2016
@jpwsutton jpwsutton modified the milestone: Backlog Jun 1, 2016
w7849516230 added a commit to w7849516230/paho.mqtt.java that referenced this issue Nov 25, 2016
Bug:eclipse#38

Signed-off-by: shawn <w7849516230@gmail.com>
w7849516230 added a commit to w7849516230/paho.mqtt.java that referenced this issue Nov 25, 2016
Bug:eclipse#38

Signed-off-by: shawn <w7849516230@gmail.com>
w7849516230 added a commit to w7849516230/paho.mqtt.java that referenced this issue Nov 25, 2016
Bug:eclipse#38

Signed-off-by: shawn <w7849516230@gmail.com>
@w7849516230 w7849516230 mentioned this issue Nov 25, 2016
4 tasks
jpwsutton pushed a commit that referenced this issue Mar 27, 2017
Bug:#38

Signed-off-by: shawn <w7849516230@gmail.com>
@jpwsutton jpwsutton modified the milestones: 1.3.0, Backlog Mar 27, 2017
lucesape pushed a commit to repairnator/repairnator-experiments that referenced this issue Apr 11, 2017
lucesape pushed a commit to repairnator/repairnator-experiments that referenced this issue Apr 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant