search

Martchus / syncthingtray

Tray application and Dolphin/Plasma integration for Syncthing
https://martchus.github.io/syncthingtray/
Other
1.66k stars 44 forks source link

tray can't connect to built-in syncthing via HTTPS unless cert field is configured manually #223

Closed martianrock closed 7 months ago

martianrock commented 9 months ago

Relevant components

Environment and versions

Bug description When switching syncthing to use TLS on web UI, syncthingtray fails to connect.

Steps to reproduce

  1. have syncthingtray work with built-in instance over http.
  2. Switch syncthing instance to use TLS for UI.
  3. Click "insert values from local syncthing configuration, url changes to https
  4. apply.
  5. get flooded with notifcations about "SSL handshake failed" and "root CA certificate not trusted for this purpose"

Expected behavior connection should succeed with TLS endpoint

Additional context The internal errors window shows this (Certificate and Expected Certificate are identical):

[2023-12-29T10:46:23] TLS error: The root CA certificate is not trusted for this purpose (17)
Certificate: 
-----BEGIN CERTIFICATE-----
MIICEDCCAZagAwIBAgIIcqyfYmTPsmUwCgYIKoZIzj0EAwIwRjESMBAGA1UEChMJ
...
TgIxAJNTA4r3oLHZzt2pflm72cJWLBKWp86HYCRKlyB/B5xvQSJlS9YtTgVYDT1T
vpvJYw==
-----END CERTIFICATE-----

Expected Certificate: 
-----BEGIN CERTIFICATE-----
MIICEDCCAZagAwIBAgIIcqyfYmTPsmUwCgYIKoZIzj0EAwIwRjESMBAGA1UEChMJ
...
TgIxAJNTA4r3oLHZzt2pflm72cJWLBKWp86HYCRKlyB/B5xvQSJlS9YtTgVYDT1T
vpvJYw==
-----END CERTIFICATE-----

Request URL: https://admin:redacted@127.0.0.1:8384/rest/system/status
[2023-12-29T10:46:23] TLS error: The root CA certificate is not trusted for this purpose (17)
Certificate: same as last
Request URL: https://admin:redacted@127.0.0.1:8384/rest/system/config
[2023-12-29T10:46:23] TLS error: The root CA certificate is not trusted for this purpose (17)
Certificate: same as last
Request URL: https://admin:redacted@127.0.0.1:8384/rest/system/status
[2023-12-29T10:46:23] TLS error: The root CA certificate is not trusted for this purpose (17)
Certificate: same as last
Request URL: https://admin:redacted@127.0.0.1:8384/rest/system/config
[2023-12-29T10:46:23] Unable to request Syncthing status: SSL handshake failed: The root CA certificate is not trusted for this purpose

the errors repeat indefinely until I switch back to plain http.

Martchus commented 9 months ago

Maybe the certificate cannot be located automatically. Does it work when you specify the certificate's location in the settings manually? In any case, it would be good to know where exactly the certificate is stored in your case.

It could be that Syncthing has changed something so the lookup that happens automatically needs to be adjusted.

Does this only happen for the built-in Syncthing? From my memory of the code I would say that's rather unlikely. I also cannot reproduce the issue right now under GNU/Linux with the system-provided Syncthing instance.

martianrock commented 9 months ago

It works correctly for non-built-in syncthing. (well, it works when I pick https-cert.pem file from syncthing's install directory. If I select wrong file (e.g. cert.pem) I get the same "not thrusted for this purpose" errors, not sure if this is relevant.)

I noticed that build-in instance is installed into the same directory as system-installed one (installing either via winget). The path to certs is the same - %LOCALAPPDATA%\Syncthing

For built-in instance I have to explicity select https-cert.pem file, then it works. If I don't select anything into cert field and rely on "insert values from local...." it can't connect. "insert values from local" does not populate anything into cert field, I am not sure how it should behave and if not populating that field is correct.

It looks like the root cause is somewhere in how "insert values from local" works. it is either not picking the cert at all or is picking the wrong one.

Martchus commented 9 months ago

"insert values from local" does not populate anything into cert field, I am not sure how it should behave and if not populating that field is correct.

The lookup is supposed to be done dynamically on the fly so that the insert button doesn't insert the certificate is normal. I'm not sure anymore why I made it this way.

For built-in instance I have to explicity select https-cert.pem file, then it works.

Good to know. That means it is just the dynamic lookup is broken under Windows and you can workaround the problem by specifying the certificate path manually for now.

martianrock commented 9 months ago

That means it is just the dynamic lookup is broken under Windows and you can workaround the problem by specifying the certificate path manually for now.

Agreed, not a blocker.

Martchus commented 9 months ago

Looks like I cannot reproduce this with a dynamically linked build done on Windows with MSYS2 packages. However, in the exact same environment with the same Syncthing config I can reproduce it with the cross-compiled build.

That makes it hard to debug right now. Judging by the code Syncthing Tray would actually return a specific error message when the certificate cannot be located. I suppose here it returns the lookup early thinking it actually doesn't have to locate a certificate.

Martchus commented 9 months ago

Ah, the difference between the builds is that the MSYS2 build uses the OpenSSL TLS backend apparently by default. When renaming its DLL to qopensslbackend.dll.bak it uses Schannel and the issue becomes reproducible.

Martchus commented 9 months ago

This should be fixed on master.

stale[bot] commented 7 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Martchus commented 7 months ago

I haven't gotten any feedback so I'm just closing the ticket. When I remember correctly this has worked in my tests.