Bug #47320 OpenSSL client does not check YaSSL server certificate
Submitted: 15 Sep 2009 12:05 Modified: 18 Dec 2009 23:51
Reporter: Domas Mituzas Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: C API (client library) Severity:S1 (Critical)
Version:4.1, 5.0, 5.1 (bzr) OS:Any
Assigned to: Georgi Kodinov CPU Architecture:Any
Tags: Security
Triage: Triaged: D1 (Critical) / R2 (Low) / E1 (None/Negligible)

[15 Sep 2009 12:05] Domas Mituzas
Description:
MySQL clients linked against OpenSSL don't check server certificates presented by YaSSL-built server

* YaSSL clients do not have such issue
* OpenSSL clients don't have such issue when connecting to OpenSSL servers

The problem happens because there's really really bad shortcut in the code:

vio_verify_callback() at viosslfactories.c:

    /*
      Approve cert if depth is greater then "verify_depth", currently
      verify_depth is always 0 and there is no way to increase it.
     */
    if (verify_depth >= depth)
      ok= 1;

An attacker (or very valid YaSSL-based MySQL server) can specify 0-depth certificates, and client will not verify server certificate because of that. 

This essentially breaks all the SSL idea, and exposes SSL-based connections to MITM attacks. 

How to repeat:
use openssl client to connect to malicious or yassl based server

Suggested fix:
Nuke the condition, as we should never hit it in any real situation, or at least do this (though the code still means absolutely nothing). 

- if (verify_depth >= depth)
+ if (verify_depth > depth)
[15 Sep 2009 12:09] Domas Mituzas
Verified against:

4.1: r2708
5.0: r2803
5.1: r3106
[15 Sep 2009 12:17] Domas Mituzas
Rationale for triage:

D1 - Man-in-the middle session hijacking bug for security-conscious 
W2 - Rules out all OpenSSL-based builds, including distribution-specified ones
I3 - All SSL users using MySQL linked against OpenSSL
[15 Sep 2009 12:54] Domas Mituzas
The reason it doesn't affect YaSSL clients is because YaSSL entirely ignores verify callback:

void SSL_CTX_set_verify(SSL_CTX* ctx, int mode, VerifyCallback /*vc*/)
{
    if (mode & SSL_VERIFY_PEER)
        ctx->setVerifyPeer();

    if (mode == SSL_VERIFY_NONE)
        ctx->setVerifyNone();

    if (mode & SSL_VERIFY_FAIL_IF_NO_PEER_CERT)
        ctx->setFailNoCert();
}
[15 Sep 2009 13:07] Sergei Golubchik
we don't seem to need this callback at all, it's probably the best to drop it.
[15 Sep 2009 13:07] Domas Mituzas
Better patch would be ditching vio_verify_callback entirely - and using standard 
OpenSSL/YaSSL verification routines - current callback does absolutely nothing, 
except extended logging in debugging builds and backdoor functionality.
[23 Sep 2009 2:31] Davi Arnaut
FWIW, the verification callback is used to accept self-signed certificates (for OpenSSL linked builds).
[23 Sep 2009 2:33] Davi Arnaut
Bug#37528 is somewhat related to this one.
[23 Sep 2009 7:26] Domas Mituzas
Davi, correct way to accept self-signed certificates is using their public key as CA cert on client, then you don't need verification callback or backdoors in the code (or special knobs for not verifying certificates at all). 

Backdoors are bad, right?
[23 Sep 2009 11:20] Davi Arnaut
Right. This verification callback is a horrid hack.
[20 Oct 2009 10:09] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/87446

2709 Georgi Kodinov	2009-10-20
      Bug #47320: OpenSSL client does not check YaSSL server certificate
      
      Removed the verify callback, as it's not needed to verify even self
      signed certificates and is a security problem.
[27 Oct 2009 13:36] Georgi Kodinov
Pushed into 4.1.26.
[4 Nov 2009 9:16] Bugs System
Pushed into 5.0.88 (revid:joro@sun.com-20091104091355-hpz6dwgkrfmokj3k) (version source revid:joro@sun.com-20091027131106-1w5i5wrb27oqewk2) (merge vers: 5.0.88) (pib:13)
[4 Nov 2009 9:24] Bugs System
Pushed into 5.1.41 (revid:joro@sun.com-20091104092152-qz96bzlf2o1japwc) (version source revid:kristofer.pettersson@sun.com-20091103162305-08l4gkeuif2ozsoj) (merge vers: 5.1.41) (pib:13)
[11 Nov 2009 6:49] Bugs System
Pushed into 6.0.14-alpha (revid:alik@sun.com-20091110093407-rw5g8dys2baqkt67) (version source revid:alik@sun.com-20091109080109-7dxapd5y5pxlu08w) (merge vers: 6.0.14-alpha) (pib:13)
[11 Nov 2009 6:57] Bugs System
Pushed into 5.5.0-beta (revid:alik@sun.com-20091109115615-nuohp02h8mdrz8m2) (version source revid:alik@sun.com-20091105092041-sp6eyod7sdlfuj3b) (merge vers: 5.5.0-beta) (pib:13)
[11 Nov 2009 16:44] Paul Dubois
Noted in 4.1.26, 5.0.88, 5.1.41, 5.5.0, 6.0.14 changelogs.

MySQL clients linked against OpenSSL did not check server
certificates presented by a server linked against yaSSL.
[27 Nov 2009 15:40] Tomas Hoger
Domas, can you clarify why you believe this does not affect OpenSSL clients talking to OpenSSL servers?  I can reproduce with OpenSSL client/server (tested with 5.0.77), where server has no ssl-ca set, only ssl-key and ssl-cert, and client is run with --ssl --ssl-ca /dev/null connects successfully.  It does not happen when ssl-ca is configured for mysqld (depth 1 verification error occurs).

I also see the fix pushed to 4.1-branch.  I believe all SSL verification is disabled there as of following commit:
http://bazaar.launchpad.net/~mysql/mysql-server/mysql-4.1/revision/1346.251.1

Is there some other change I may be missing?  Thank you!
[17 Dec 2009 11:24] Domas Mituzas
I believe this affects all openssl clients talking to any servers. 
Synopsis is too narrow, and should probably be changed into "MySQL clients with OpenSSL can be tricked not to check server certificates". 

As I'm not working anymore for Sun/MySQL, I'll leave it for them to change anything if needed.
[17 Dec 2009 11:26] Domas Mituzas
As for 4.1 change, I didn't review it, and it may be really the case that all verification is disabled. Bad, then.
[17 Dec 2009 11:29] Domas Mituzas
I probably have written all that here as hidden comments, unfortunately I cannot see them myself anymore either :)
[17 Dec 2009 14:23] Tomas Hoger
Domas, thank you for confirming!
[17 Dec 2009 17:29] Paul Dubois
Updated changelog description per Doumas, and added CVE number (CVE-2009-4028).
[18 Dec 2009 10:30] Bugs System
Pushed into 5.1.41-ndb-7.1.0 (revid:jonas@mysql.com-20091218102229-64tk47xonu3dv6r6) (version source revid:jonas@mysql.com-20091218095730-26gwjidfsdw45dto) (merge vers: 5.1.41-ndb-7.1.0) (pib:15)
[18 Dec 2009 10:45] Bugs System
Pushed into 5.1.41-ndb-6.2.19 (revid:jonas@mysql.com-20091218100224-vtzr0fahhsuhjsmt) (version source revid:jonas@mysql.com-20091217101452-qwzyaig50w74xmye) (merge vers: 5.1.41-ndb-6.2.19) (pib:15)
[18 Dec 2009 11:01] Bugs System
Pushed into 5.1.41-ndb-6.3.31 (revid:jonas@mysql.com-20091218100616-75d9tek96o6ob6k0) (version source revid:jonas@mysql.com-20091217154335-290no45qdins5bwo) (merge vers: 5.1.41-ndb-6.3.31) (pib:15)
[18 Dec 2009 11:15] Bugs System
Pushed into 5.1.41-ndb-7.0.11 (revid:jonas@mysql.com-20091218101303-ga32mrnr15jsa606) (version source revid:jonas@mysql.com-20091218064304-ezreonykd9f4kelk) (merge vers: 5.1.41-ndb-7.0.11) (pib:15)