Can't use TLS in client on Fedora 31

[Phidica]

Fedora 31, using current Git master of Syncplay (23ca271).

Despite having all of the required dependencies for TLS in the client, I was still seeing "This client does not support TLS" at startup. Running in debug mode, I saw the following:

$ ./syncplayClient.py --debug
UnicodeEncodeError: 'ascii' codec can't encode character '\u0151' in position 126065: ordinal not in range(128)
[...]

After digging through the source code and testing commands related to TLS, I found that the error arises from the following block in syncplay/client.py

try:
    caCertFP = open(os.environ['SSL_CERT_FILE'])
    caCertTwisted = Certificate.loadPEM(caCertFP.read())
    caCertFP.close()
    self.protocolFactory.options = optionsForClientTLS(hostname=host)
    self._clientSupportsTLS = True
except Exception as e:
    self.ui.showDebugMessage(str(e))

What's happening is that my system CA bundle (ca-certificates-2020.2.40-1.1.fc31.noarch) contains Unicode in one of the comments between certs. Specifically on the line

# NetLock Arany (Class Gold) Főtanúsítvány

Then Certificate.loadPEM(caCertFP.read()) raises an exception with the following trace (reproduced in an interactive Python session, running the commands myself):

Traceback (most recent call last):
  File "/usr/lib64/python3.7/site-packages/twisted/internet/_sslverify.py", line 500, in loadPEM
    return Class.load(data, crypto.FILETYPE_PEM)
  File "/usr/lib64/python3.7/site-packages/twisted/internet/_sslverify.py", line 477, in load
    return Class(crypto.load_certificate(format, requestData), *args)
  File "/usr/lib/python3.7/site-packages/OpenSSL/crypto.py", line 1824, in load_certificate
    buffer = buffer.encode("ascii")
UnicodeEncodeError: 'ascii' codec can't encode character '\u0151' in position 126065: ordinal not in range(128)

Here's the thing though: caCertTwisted is never even used for anything. It looks like we attempt to load the system CA bundle just as a test that it...exists, or something? But if I remove the line

    caCertTwisted = Certificate.loadPEM(caCertFP.read())

and then launch the client, TLS works! So, the UnicodeEncodeError raised by PyOpenSSL is actually irrelevant when it comes to actually starting the client with TLS, and I am able to successfully connect to the public server over TLSv1.3 after applying this change.

So, I think that this test done to determine whether the client can launch with TLS needs to be reviewed.

(Alternatively, this could be considered a bug upstream in Twisted or PyOpenSSL, however it kind of looks like these methods are only intended to be loading one certificate, not a bundle containing comments between certificates... So I'm not sure they would accept it as a bug?)

[Et0h]

I don't use Fedora so am not in a great position to review the test, but if you can figure out a simple tweak to Syncplay which would fix it then please make a PR.

[Phidica]

Are you able to explain what exactly is the need to attempt loading the system CA bundle? Is it enough just to test that the file exists instead?

[Et0h]

@Phidica I can't be of much use as I wasn't particularly involved in the development of that feature, but it is possible that @albertosottile or @daniel-123 would be able to comment.

[albertosottile]

So... I remember writing that code. In the beginning of the TLS client implementation I tried (quite desperately) several approaches to make Twisted load the certifi CA file but, I failed miserably. Then I found out this environmental variable approach which, despite being ugly, has worked perfectly up to now. So, I am quite convinced that the line that is causing your error is a remnant of my experimentations and its not fundamentally needed anymore. Nevertheless, I would still assert if the file exists In the same try block, just to be extra safe.

[Et0h]

@Phidica If there is a suitable pull request soon in line with @albertosottile's comments then this can hopefully be included in the release candidate of Syncplay v1.6.5.

[Phidica]

Thanks @daniel-123!