eclipse-hono / hono

Eclipse Hono™ Project
https://eclipse.dev/hono
Eclipse Public License 2.0
452 stars 137 forks source link

MQTT adapter : clients connection stealing #275

Open ppatierno opened 7 years ago

ppatierno commented 7 years ago

As described in the MQTT 3.1.1 spec :

If the ClientId represents a Client already connected to the Server then the Server MUSTdisconnect the existing Client.

we should have to implement such feature in the MQTT adapter.

sophokles73 commented 7 years ago

IMHO this depends on whether we want the clientId to be the Hono device ID or a surrogate identifier. Currently we are matching the client ID against the device ID specified in the topic when sending a message, i.e. we assume that the client ID is the device ID. If we want to enforce the behavior defined in the MQTT spec you are referring to then this would mean that no two devices from different tenants can have the same device ID. Alternatively, we need to either make sure that the client ID contains both the tenant and the device ID, e.g. myTenant/myDeviceId, or that the device ID is a surrogate identifier. I would actually opt for the former ...

sysexcontrol commented 7 years ago

The myTenant/myDeviceId solution seems to be a step into the right direction for me, but if I see things correctly, there still is the need to prevent devices that are connecting that they do not send a wrong tenantId in their clientId (and as side effect close the connection for devices belonging to other tenants).

To be exact, I would think it needs the following sequence:

WDYT?

pellmann commented 7 years ago

If think this part of the spec is especially to be sure, that a message can reach the correct device over one defined channel. So this will be more of a problem with command and control. And who can decide if a device is already connected in a clustered environment?

ppatierno commented 7 years ago

The big problem is related even on scaling. When we scale out the MQTT adapter having more instances on Kubernetes/OpenShift for example, each connection from a remote MQTT client is delivered to different MQTT adapter instances so we should know if a client with a client-id is already connected to another MQTT adapter instance. In EnMasse the MQTT gateway works to have one AMQP connection for every MQTT connection (so not how the MQTT adapter in Hono works). Because all the MQTT gateway instances are connected to the same router network, detecting client-id duplication (and connection stealing) should be done at router level. There is this JIRA (https://issues.apache.org/jira/browse/DISPATCH-630) waiting for development in order to do that. In the Hono case we have only one AMQP connection from adapter to router network and checking client-id duplication on more adapter instances could be more complicated. In any case another problem will be asking to the other MQTT adapter to shut down the connected client because a "connection stealing" is in progress on another instance.

sophokles73 commented 7 years ago

As long as we are only talking about uploading telemetry data, I do not think that we have a problem here, i.e. it doesn't really matter if a device has one or more connections open to one or more adapter instances. As long as the connection is properly authenticated and complies with the rules, we can safely forward it downstream.

Once we start implementing Command & Control, we will need to determine, which adapter instance a device is connected to anyway, in order to be able to route the outbound commands to the adapter instance the device is connected to. For that purpose we will probably need to have the adapters report to some central service (Device Registry?) when a device connects successfully. This information could then later be used to determine, if a device is already connected to another adapter instance ...

ppatierno commented 7 years ago

Could we have an hacked and cloned device sending data with same username/password and client-id as the original one ? In this case it could be useful to close the second connection and not the first ... so the opposite of the MQTT spec. My only concern is that even if for telemetry it could not be a problem we won't be fully MQTT 3.1.1 compliant.

If we need this for Command & Control, we don't design it now and using for telemetry as well ? It should be a service with information related to the device lifecycle, is the device registry thought for that ?

sophokles73 commented 7 years ago

How would the adapter know which one is the hacked & cloned one, if they both connect using the same client-id and credentials?

ppatierno commented 7 years ago

I was just semplifying to the second one which opens the connection.

sophokles73 commented 7 years ago

Hmm, FMPOV this does not sound like a robust mechanism. What if the real device needs to continuously connect, send data and then disconnect again, e.g. due to power constraints? I think that many devices will work like this, basically sleeping most of the time. In this case, I do not think that considering the second connection to be the hacked device is very helpful ...

Apart from that, I wouldn't actually be too concerned with full MQTT 3.1.1 compliance in each and every detail. FMPOV it is most important that devices can exchange Telemetry and Event data in the way we have specified it using MQTT as the underlying transport protocol. We should not make any claims regarding implementing the full MQTT 3.1.1 spec with our protocol adapter.