Comment 2 for bug 925657

> I just worked on refactoring, simplifying and cleaning up the
> certificate validation code. It's now much easier to read but it's not
> fixed with regards to this bug report.

Hmmm, hopefully this doesn't void me review....

> tls_verify_certificate() first uses x509_verify_certificate() to attempt
> to validate the certificate using OpenSSL and ~/.freerdp/certs as a
> lookup path. My understanding is that one can add trusted CAs in
> ~/.freerdp/certs and have that be used with this first technique. Would
> that be what the first issue was about?

No. The issue is that the server's hostname is not validated against the Subject Alternative Name or the Common Name field of the X509 certificate that the server presents to freerdp. This is important for thwarting man in the middle attacks (part of the purpose of having certificate verification in the first place). Consider this: with no mitm attacker, suppose freerdp connects to the rdp.foo.com server, gets the rdp.foo.com certificate and then properly verifies it via libfreerdp-core/crypto.c's x509_verify_cert() by setting everything up right and calling openssl's X509_verify_cert(). X509_verify_cert() makes sure the trust chain validates all the way back to the root CA and if there is no error, we are sure that this certificate is ok. Now consider a mitm attacker obtains a properly signed certificate for mitm.attacker.org (which is easy in today's world) and our user is at a coffee shop and is mitm'd. The user tries to connect to rdp.foo.com but instead is redirected to the attacker's machine. The attacker presents the mitm.attacker.org certificate, which is verifiable via X509_verify_cert(), so freerdp continues without error and the user's communications are viewable by the attacker. To stop this, freerdp should verify that the server's hostname matches the Common Name or Subject Alternative Name that is in the X509 certificate. In this case, freerdp should check that rdp.foo.com (what it is trying to connect to) matches the certificate that mitm.attacker.org provides. Because rdp.foo.com won't match the mitm.attacker.org Common Name or Subject Alternative Name, verification fails.

Since libfreerdp-core/crypto.c's x509_verify_cert() failed we then hit the fallback code in tls_verify_certificate() in libfreerdp-core/tls.c, which also doesn't verify the server's hostname against the Common Name or Subject Alternative Name that is in the X509 certificate that is in the user's ~/.freerdp/cacert/ certificate store.

In one instance, this still sort of works: if rdp.foo.com uses a properly signed certificate then if this code is hit, freerdp will presumably prompt the user to accept the certificate for mitm.attacker.org. This dialog should show all the information that would indicate that there is a problem (ie, it is obvious the certificate is not for rdp.foo.com but instead for mitm.attacker.org). In this way there is hope that the user would not accept the certificate. But you can do better than that by doing the server hostname check you and denying the connection right there and not prompt the user at all.

The other instance is when rdp.foo.com uses a self-signed certificate that is in the ~/.freerdp/cacert/. Normally (ie, without a mitm attacker) x509_verify_cert() would fail and we would hit the fallback code in tls.c and check that the cert matches what we have in our certificate store in ~/.freerdp/cacert/. However, with the lack of server hostname checking when the mitm.attacker.org certificate is presented, which then verfies in x509_verify_cert() and the fallback code is not executed and the self-signed certificate in ~/.freerdp/cacert/ is not consulted.