SignalK / signalk-server

An implementation of a Signal K central server for boats.
http://signalk.org
Apache License 2.0
306 stars 152 forks source link

Add TLS Value to mDNS TXT Record #80

Open timmathews opened 8 years ago

timmathews commented 8 years ago

The mDNS responses should include a TLS key/value pair if the connection supports it. This is less necessary for HTTP/WS service records as the server should perform a 301 redirect if TLS is configured. However, for future lower-level transports (e.g. directly over UDP/TCP or a PUB/SUB protocol like NATS or MQTT), it will be useful for the client to know that it should initiate the connection with TLS (or upgrade via a mechanism like STARTLS).

tkurki commented 8 years ago

We discussed in Slack adding the version of the TLS supported. That may turn out to be not much value added, unless there is a standard way to represent the current & future versions. Node tlsSocket will report something like version: 'TLSv1/SSLv3'.

However do we really need this? In the APIs and conventions doc we specify that

A Signal K server should advertise its services over mDNS/Bonjour. The server must use the service types - _signalk-http._tcp for http API - _signalk-ws._tcp for WebSocket - _signalk-https._tcp for HTTPS API - _signalk-wss._tcp for secure WebSocket

I think that should be enough, client code needs to have logic to handle the different service types.

If a server has available and advertises standard and secure protocols there should be a way for a client to figure that they are from the same server identity and it should choose which it prefers. For this purpose at least node server includes self identity in the TXT record.

nohal commented 8 years ago

Well, then we just need to fix the bug in signalk-server-node not behaving according to the specification - it always reports only _signalk-ws._tcp / _signalk-http._tcp, regardless of ssl setting....

tkurki commented 8 years ago

Definitely!

nohal commented 8 years ago

I have completely missed that part of the specification, BTW.

nohal commented 8 years ago

It will also mean that the client, at least the implementation I'm using in OpenCPN will have to query the DNS twice, but it is certainly doable.

tkurki commented 8 years ago

FYI node binding to Bonjour/Avahi implementation is event based: you start to listen to a service type and you receive events as advertisements appear/repeat.

nohal commented 8 years ago

So the solution that is acceptable and correct for you is

diff --git a/lib/interfaces/rest.js b/lib/interfaces/rest.js
index bf02504..d016006 100644
--- a/lib/interfaces/rest.js
+++ b/lib/interfaces/rest.js
@@ -75,7 +75,7 @@ module.exports = function(app) {
     },

     mdns: {
-      name: "_signalk-http",
+      name: app.config.ssl ? "_signalk-http" : "_signalk-https",
       type: "tcp",
       port: app.config.port
     }
diff --git a/lib/interfaces/ws.js b/lib/interfaces/ws.js
index 6ef7f35..5e287b1 100644
--- a/lib/interfaces/ws.js
+++ b/lib/interfaces/ws.js
@@ -26,7 +26,7 @@ module.exports = function(app) {
     primus;

   api.mdns = {
-    name: "_signalk-ws",
+    name: app.config.ssl ? "_signalk-ws" : "_signalk-wss",
     type: "tcp",
     port: app.config.port
   };
diff --git a/lib/mdns.js b/lib/mdns.js
index a0e7c67..a3293ca 100644
--- a/lib/mdns.js
+++ b/lib/mdns.js
@@ -54,7 +54,7 @@ module.exports = function mdns(app) {
   }

   types.push({
-    type: mdns['tcp']('http'),
+    type: app.config.ssl ? mdns['tcp']('http') : mdns['tcp']('https'),
     port: app.config.port
   });

?

tkurki commented 8 years ago

Yes.

nohal commented 8 years ago

OK, will update the PR. BTW, _https.tcp seems not to be recognized by Bonjour Browser, while _http.tcp is. I don't think it matters much, just an observation

tkurki commented 8 years ago

Yeah, I think I've seen comments about that. I think zero conf discovery could be a lot more useful in IoT, seems like a solution finding a problem, but too bad there is no generic browser support.

timmathews commented 8 years ago

I believe that's because HTTPS isn't actually a different protocol. It's just HTTP wrapped in TLS. If http were being developed today, IANA and the IETF would insist that HTTPS version use the same port as HTTP and use something like STARTTLS to negotiate encryption.

So, mDNS treats them as the same thing and expects implementors (us) to do what we're doing with including a flag indicating TLS support.

-Tim

On Apr 20, 2016, at 12:25 PM, Pavel Kalian notifications@github.com wrote:

OK, will update the PR. BTW, _https.tcp seems not to be recognized by Bonjour Browser, while _http.tcp is. I don't think it matters much, just an observation

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub

nohal commented 8 years ago

Well, this solution was just refused :) Anyway, I have redone the PR to follow the specification.

nohal commented 8 years ago

I finally found something relevant in the HTTP section of http://dns-sd.org/ServiceTypes.html It does not look like people take the advice too seriously though- for example Konqueror seems to support _https.tcp according to https://web.archive.org/web/20150922075901/http://www.avahi.org/wiki/AdministrativeAvahiApplication (avahi.org site currently defunct)

tkurki commented 8 years ago

Signalk-js-client discovery does not support tls services with the double lookup method yet as nohal pointed out in https://github.com/SignalK/signalk-js-client/commit/907dd20247171d633994136a84d8cd931de0fa43