rootkiwi / an2linuxserver

Sync Android notifications encrypted to a Linux desktop
Other
402 stars 43 forks source link

Client Certificate Cryptography Issues #1

Closed Javantea closed 7 years ago

Javantea commented 7 years ago

Instead of using TLS client certificates, the implementation uses the sha1 hash of a password called client_cert.

sha1 = hashlib.sha1(client_cert + SERVER_CERT_DER).hexdigest().upper()

That is not how client certificates work, You don't just have to prove that you have the public key of a certificate, you have to prove that you have the private key of the certificate. This is done through RSA, not SHA1. This authentication mechanism appears to be vulnerable to man-in-the-middle attacks. This is why cryptographers will tell you not to roll your own crypto.

rootkiwi commented 7 years ago

No that sha1 hash is only used for pairing. That hash is displayed on the phone and on the server during pairing to prevent a man-in-the-middle attack. It's is kinda like during bluetooth pairing, to confirm that the numbers match.

Because if the hashes match it means that the client and the server have the same certificates, which the user can confirm before saving.

And what do you mean by client_cert being a password?

client_cert is the certificate that the client (android) send during pairing.

Then after pairing is completed the certificate is saved in authorized_certs file.

Then during a notification connection, after a device already have done the pairing process, the certificates is validated during the TLS handshake. The client trust the one from the server and the server trust the one from the client, which was exchanged during pairing.

If you look closer in the handle_notification_connection() instead of handle_pair_request() you can see this:

notif_tls_ctx.load_verify_locations(cadata=parse_authorized_certs()) and you can also see this in the program: notif_tls_ctx.verify_mode = ssl.CERT_REQUIRED

So, the TLS handshake takes care of the authentication, I just have to provide a list of trusted certificates: cadata=parse_authorized_certs() and if the client does not provide a certificate and also prove ownership, the handshake will fail.

ssl.CERT_REQUIRED means that the server will send a CertificateRequest during the TLS handshake which means that the client will have to send theirs certificate as well.

Javantea commented 7 years ago

Thank you for the clarification. I missed the part of the code where it handles client certs and sets CERT_REQUIRED. At this point, the only issue I can see in this code is that SHA1 has been proven to collide and that the attacks against SHA1 will continue to improve. Both of the values being hashed are available to an attacker. Since the TLS prevents the attacker from modifying the cert, this should be fine.