Add strict TLS mode

[EtiennePerot]

It appears that Syncplay's approach to TLS is to send an initial message over the TCP stream to upgrade the connection to use TLS:

{"TLS": {"startTLS": "send"}}

The server then acknowledges this TLS upgrade request with:

{"TLS": {"startTLS": "true"}}

And then the client sends the TLS ClientHello message, i.e. the TLS handshake begins in earnest, using the same TCP connection.

For this to work properly, the process handling the TCP connection has to speak Syncplay's protocol, or at least it has to understand this particular {"TLS": {"startTLS": "send"}} message. In practice, this means that the Syncplay server itself needs to have access to the certificate and private key.

As a general principle, each process that has such a key loaded in its working memory is a liability -- they may each have their own TLS implementations that may or may not be robust against the latest side-channel attacks or whatnot. In general, the fewer things handling a sensitive piece of data, the better. For this reason, it's useful for most HTTP-like application protocols (e.g. Matrix.org, CalDAV, Mozilla Sync, etc.) to support being run through a reverse proxy that does all the TLS encapsulation, and then passes on the decrypted traffic through a lo-only connection or through a socket file on the machine.

Since Syncplay's protocol isn't HTTP-like, it can't be run through a reverse HTTP proxy. As such, I was trying to get it to work through stunnel, a similar utility that handles the connection, handles all the TLS aspects of it, and forwards all the unencrypted traffic to a local Syncplay server that listens on lo only. However, because Syncplay's client first sends {"TLS": {"startTLS": "send"}} to initiate TLS, this confuses stunnel as it expects the client to directly start with the TLS ClientHello message.

Hence the following request: Would it be possible to add a TLS-only mode to both the Syncplay server and client, which directly talks TLS without this initial startTLS message? This would have the following benefits:

• No longer susceptible to Man-in-the-Middle attacks (Opportunistic approaches to TLS are inherently susceptible to such attacks)
• Ability to delegate TLS-handling to another process than Syncplay itself, such as stunnel.
• (Bonus) Ability to run Syncplay in an inetd-like fashion. As a personal Syncplay server is likely only seldomly used, the ability to have it automatically exit when no one is using it and re-started whenever someone connects would be a nice resource savings. stunnel provides just such a mode.

Somewhat-related issue: https://github.com/Syncplay/syncplay/issues/245

[Et0h]

I'm happy for @daniel-123, @albertosottile and others to weigh in on this because I don't really have the necessary understanding of TLS and Syncplay's implementation of it to assess the pros and cons (let alone to actually code the implementaiton).

Relevant principles from #315 that may come into play include:

• Simplicity. Unnecessary complexity can lead to unpredictable behaviour, features people never use, and a harder to manage (and therefore more likely to be buggy/unreliable) code base. We want to cover the main uses cases, not every possible circumstance (especially where there are reasonable workarounds).
• Functionality. The main reason we have many of these features and behaviours is to help improve the Syncplay experience and to help people focus on watching videos rather than managing Syncplay.
• Privacy. Try to avoid giving the user/client the ability to easily share their private information unintentionally, especially material from the user's clipboard.
• Compatibility. Forwards and backwards compatibility are both generally a good thing. As such, breaking compatibility should be the exception rather than the rule. However, in some cases it may be appropriate to drop support for older versions of media players, etc., in the interests of code simplicity and maintainability. There may also be security reasons to drop support for older media player builds or older operating systems.

[daniel-123]

My 2 cents in TL;DR: it's probably too complex to implement properly given our constraints for the amount of benefit it gives.

Most important point is handling compatibility - the initial decision to use opportunistic TLS was taken specifically to maintain backwards and forwards compatibility between clients and servers in first place. We would need a really long discussion what to do about it and probably plan to introduce it in steps over several versions.

Second point is that the benefits you listed, while definitely real, aren't going to used by vast majority of user base. And as small counterpoints to each:

• MitM attack would still result in lack of green padlock in Syncplay. Additionally Syncplay was never designed to be a secure means of communication - it's designed instead with assumption of just avoiding sending any sensitive information in first place (which is why there are options to not send any filename information and such).
• I guess main concern is the need for Syncplay process to access the certificate key. Though this is relatively easy to resolve by just getting another certificate for subdomain exclusively for Syncplay use.
• Syncplay server process uses very little resources, so there isn't that much of a benefit to having it run on-demand.

One idea I could see as much easier to implement would be an option for both server and client to just drop connections where opportunistic TLS didn't result in encrypted connection. Though it still pretty much has to default to "off" simply because we cannot just expect everybody who runs a sever to bother with certificates.

Lastly - we would need somebody to volunteer their time to code something like described in last paragraph. If you are interested @EtiennePerot, then we can discuss some details within this issue. If not I'll just close it in two weeks or so (feel free to reopen it if you need more time than that).

[EtiennePerot]

I actually ended up hacking up a solution for this without modifying the Syncplay source code by doing the following:

• First, running a Syncplay server (optionally: on localhost only; there's no flag in syncplay-server for this, but can be done by running it in a Docker container).
• Second, running stunnel on a localhost port as well, standing in front of syncplay-server, in order to handle TLS decapsulation. The idea is to have stunnel receive a plain strictly-TLS connection, and forward the plaintext of that to `syncplay-server.
• Third, running a Python script on a public port that trims off the initial {"TLS": {"startTLS": "send"}} message, replies blindly with {"TLS": {"startTLS": "true"}}, and from that point on acts as a transparent TCP proxy to stunnel which handles the TLS handshakes that come next.

Here's the stunnel configuration that works for me:

foreground = yes
options = NO_SSLv2
options = NO_SSLv3
options = SINGLE_ECDH_USE
options = SINGLE_DH_USE
[syncplay]
accept  = 9998 # Port that stunnel will listen on
connect = 9999 # Port that syncplay-server is listening on
cert = <path to a file that has both certificate AND private key data in it>

And here's the Python script sitting in front of that:

#!/usr/bin/python3

# See https://github.com/Syncplay/syncplay/issues/346 for this script's raison-d'etre.

import select
import socket
import socketserver
import sys

listen_port = int(sys.argv[1])
forward_port = int(sys.argv[2])

class SyncplayRequestHandler(socketserver.BaseRequestHandler):
    def handle(self):
        print('Handling connection from:', self.client_address)
        data = self.request.recv(1024)
        if data.strip() != b'{"TLS": {"startTLS": "send"}}':
            print('Bad connection header from:', self.client_address)
            try:
                self.request.close()
            except:
                pass
            return
        print('Opening forwarding connection on behalf of:', self.client_address)
        forwarded_conn = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
        forwarded_conn.connect(('localhost', forward_port))
        # Acknowledge to the client that we are ready.
        self.request.sendall(b'{"TLS": {"startTLS": "true"}}\r\n')
        # Start proxying.
        print('Proxying started for:', self.client_address)
        sockets = (self.request, forwarded_conn)
        while True:
            rlist, _, xlist = select.select(sockets, (), sockets)
            if len(xlist):
                break
            for s in rlist:
                if s is self.request:
                    forwarded_conn.sendall(self.request.recv(1024))
                elif s is forwarded_conn:
                    self.request.sendall(forwarded_conn.recv(1024))
        print('Proxying stopped for:', self.client_address)
        try:
            forwarded_conn.close()
        except:
            pass
        try:
            self.request.close()
        except:
            pass

class SyncplayServer(socketserver.ThreadingTCPServer):
    def server_bind(self):
        self.socket.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
        socketserver.ThreadingTCPServer.server_bind(self)

with SyncplayServer(('', listen_port), SyncplayRequestHandler) as server:
    server.serve_forever()

This is launched with arguments 9997 9998 (first is the port for the script to listen on, second is the port stunnel is listening on).

Then Syncplay clients can be pointed at servername:9997 and will connect to the Python script, which will strip off the startTLS header, and from then the TLS stream goes to stunnel. The Syncplay client negotiates TLS with stunnel, and once done stunnel forwards plaintext syncplay traffic to syncplay-server, none the wiser than TLS is happening.

This is "strict" TLS in the sense that no unencrypted traffic (beyond the initial startTLS message) is present on the wire, and that if the client were to try to talk plaintext to it, neither the Python script nor stunnel would be OK with that, and so the connection would never make it to syncplay-server.

I understand this isn't exactly code that is appropriate to check in as some official solution, just thought I'd share in case someone stumbles on this issue. I agree that your proposed solution would be superior and I encourage anyone who wants to implement it to do so rather than using the hack above.

[tacerus]

Thank you very much for your detailed comment including the proxy script, @EtiennePerot. Whilst setting up secure services on the internet with TLS is part of my daily job, I eventually gave up trying to make SyncPlay speak TLS - the built-in STARTTLS would not work (https://github.com/Syncplay/syncplay/issues/250#issuecomment-1200415061), and external TLS proxies like stunnel would choke on the required response.

With your workaround, secure, implicit, TLS, worked on the first try.

Small improvement: I made the proxy listen on dual stack IPv4 and IPv6 and talk to the backend using IPv6 as well:

$ diff -u syncplay-proxy.orig /usr/local/bin/syncplay-proxy 
--- syncplay-proxy.orig 2022-07-31 18:50:07.067941580 +0200
+++ /usr/local/bin/syncplay-proxy       2022-07-31 18:50:28.748540145 +0200
@@ -22,7 +22,7 @@
                 pass
             return
         print('Opening forwarding connection on behalf of:', self.client_address)
-        forwarded_conn = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+        forwarded_conn = socket.socket(socket.AF_INET6, socket.SOCK_STREAM)
         forwarded_conn.connect(('localhost', forward_port))
         # Acknowledge to the client that we are ready.
         self.request.sendall(b'{"TLS": {"startTLS": "true"}}\r\n')
@@ -49,6 +49,7 @@
             pass

 class SyncplayServer(socketserver.ThreadingTCPServer):
+    address_family = socket.AF_INET6
     def server_bind(self):
         self.socket.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
         socketserver.ThreadingTCPServer.server_bind(self)

The client reports:

[18:55:07] Secure connection established (TLSv1.3)
[18:55:07] Successfully connected to server

I additionally wrote a systemd service for this, if anyone is interested: https://git.casa/syncplay.git/tree/syncplay-proxy