luvit / lit

Toolkit for developing, sharing, and running luvit/lua programs and libraries.
http://lit.luvit.io/
Apache License 2.0
249 stars 58 forks source link

fix: secure-socket does not check for certificate domain #326

Closed Bilal2453 closed 4 months ago

Bilal2453 commented 4 months ago

After secure-socket verifies a certificates and deems it to be valid, it does not check whether this certificate is actually provided by the same server that claims to be the owner of the certificate. This results in a possible attack vector where an attacker that is intercepting a connection (through a MITM attack) to provide any valid SSL certificate (for example, one they made themselves for a domain they own) and secure-socket (and by extension, coro-net/coro-http etc) would accept that certificate.

  1. A connection is intercepted by an attacker between google.com and a user-agent using secure-socket.
  2. The attacker provides their valid certificate for the domain they own doodle.com.
  3. secure-socket will check for the validity of the certificate (which it is valid), and start encrypting the packets following that.
  4. The attacker can now decrypt all packets and read what is going on with the connection, potentially stealing the user-agent session tokens, passwords, etc.

A Common Weakness Enumeration for this would be 297:

cert = SSL_get_peer_certificate(ssl);
if (cert && (SSL_get_verify_result(ssl)==X509_V_OK)) {

// do secret things
}

Even though the "verify" step returns X509_V_OK, this step does not include checking the Common Name against the name of the host. That is, there is no guarantee that the certificate is for the desired host. The SSL connection could have been established with a malicious host that provided a valid certificate.

This has been mitigated in this PR by applying an identity check using x509:check_host and error if the domain of the certificate does not match the server name.

This was caught when I was (out of curiosity) testing out https://badssl.com/dashboard/ with coro-http. As such I also added tests for this using coro-http.

P.S: We also had a bug in how we handled coroutine resumption, where in the tests coro-net was constantly trying to resume a coroutine after it was already resumed by the openssl certificate error. That is now fixed by checking the thread status, and only attempt resuming it if it was suspended.