MySQL Bugs: #47320: OpenSSL client does not check YaSSL server certificate

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

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)

Verified against:

4.1: r2708
5.0: r2803
5.1: r3106

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

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();
}

we don't seem to need this callback at all, it's probably the best to drop it.

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.

FWIW, the verification callback is used to accept self-signed certificates (for OpenSSL linked builds).

Bug#37528 is somewhat related to this one.

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?

Right. This verification callback is a horrid hack.

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.

Pushed into 4.1.26.

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)

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)

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)

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)

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.

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!

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.

As for 4.1 change, I didn't review it, and it may be really the case that all verification is disabled. Bad, then.

I probably have written all that here as hidden comments, unfortunately I cannot see them myself anymore either :)

Domas, thank you for confirming!

Updated changelog description per Doumas, and added CVE number (CVE-2009-4028).

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)

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)

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)

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)