I was going through the crypto part and I thought about a possible attack:
A malicious client could gather certificates of other peers by examining the certificates offered after the TLS handshake, since InsecureSkipVerify is set to true, the certificates are not verified as far as I know.
Then he could impersonate other device ids, given that he has already gathered certificates from other peers.
Through the clusterConfig message he can gather the list of shared folders and other devices so this attack can spread further
With knowledge of other peers client certificate, and thus device id and shared folders, he could sync folders which he doesn’t have access to.
Is this attack possible? I didn’t verify, just thought of the possibility.
At the point you perform the TLS handshake, the server presents it’s public key, the client presents some random nounce encrypted with servers public key, and then the server presents the same nounce+1 (or something like that) to verify that the server has the private key.
InsecureSkipVerify only disables CA signature checking for the given certificate chain.
This mostly used mostly by HTTP, because when you connect to somedomain.com, you don’t know what is the valid public key for a given domain. What your browser does, is has a list of certification authorities that it trusts (CA’s) and then verifies that the public key presented to somedomain.com is signed by some CA this way establishing the trust.
In syncthing there is no CA, and we know whether or not the public key is valid for a given connection just by hashing it and verifying that it matches the expected device ID, and the part where the server proves that it has the private key is verified by TLS before the connection is even available to syncthing to verify if the device ID matches.
TL;DR It’s not enough to have just the public key to impersonate someone, and the private key is never given to anyone else.