Bug #39178 Server crash in YaSSL with non-RSA-requesting client if server uses RSA key
Submitted: 2 Sep 2008 9:30 Modified: 10 Dec 2008 20:24
Reporter: Tonci Grgin Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: General Severity:S1 (Critical)
Version:5.0/5.1/6.0 OS:Any (XP Pro SP2)
Assigned to: Chad MILLER CPU Architecture:Any
Tags: SSL, yassl
Triage: Triaged: D1 (Critical) / R2 (Low) / E3 (Medium)

[2 Sep 2008 9:30] Tonci Grgin
Description:
Using attached test case *from* Vistax64 client, MySQL server on WinXP 32bit crashes.
However, the other way around (client on WinXP, MySQL server on Vistax64) no crash happens.

MySQL server used on Vistax64 is standard community version 5.0.67 as opposed to MySQL server used on WinXP (5.0.68PB10!) which crashes.

This happened during testing of Bug#35895. Mysql cl client works as expected. I tested MySQL server 5.1 (both latest community version and my 5.1.28PB12) and they appear not to crash but I can't be sure as there is no 100% repeatable test case.

How to repeat:
o) On WinXP box start mysql cl client and add new user:
  CREATE USER 'ssl2'@'%';
  GRANT ALL PRIVILEGES ON *.* TO 'ssl2'@'%' REQUIRE SSL;
o) On Vista box, start attached test case from VS2005. Change addresses as necessary.

MySQL server error log:
Version: '5.0.68-pb10-log'  socket: ''  port: 5068  MySQL Pushbuild Edition, build 10
080902  8:10:15 - mysqld got exception 0xc0000005 ;
--<cut>--
key_buffer_size=402653184
read_buffer_size=2097152
max_used_connections=3
max_connections=100
threads_connected=3
It is possible that mysqld could use up to 
key_buffer_size + (read_buffer_size + sort_buffer_size)*max_connections = 802816 Kbytes of memory
--<cut>--

thd=03514620
Attempting backtrace. You can use the following information to find out
where mysqld died. If you see no messages after this, something went
terribly wrong...
7C911E58    ntdll.dll!RtlInitializeCriticalSection()
7C910D5C    ntdll.dll!wcsncpy()
0074ABF1    mysqld.exe!???
0074E9CC    mysqld.exe!???
Trying to get some variables.
Some pointers may be invalid and cause the dump to abort...
thd->query at 00000000=(null)
thd->thread_id=8

WinDBG stack trace:
WARNING: Stack unwind information not available. Following frames may be wrong.
ntdll!RtlInitializeCriticalSection+0x32b
ntdll!wcsncpy+0x2cd
mysqld!free+0x6b
mysqld!_aligned_free+0x13
mysqld!TaoCrypt::AlignedAllocator<unsigned int>::deallocate+0x1d
mysqld!TaoCrypt::DSA_PrivateKey::~DSA_PrivateKey+0x33
mysqld!yaSSL::ysDelete<yaSSL::DSS::DSSImpl>+0x11
mysqld!yaSSL::DSS::`scalar deleting destructor'+0x12
mysqld!yaSSL::DH_Server::build+0x3ee
mysqld!yaSSL::ServerKeyExchange::build+0x10
mysqld!yaSSL::sendServerKeyExchange+0x35
mysqld!yaSSL_accept+0xaa
mysqld!ssl_do+0x73
mysqld!sslaccept+0x19
mysqld!check_connection+0x3a4
mysqld!handle_one_connection+0xe7
mysqld!pthread_start+0x3b
mysqld!_threadstart+0x6c
kernel32!GetModuleFileNameA+0x1b4

Suggested fix:
Exact error is: ERROR 2013 (HY000): Lost connection to MySQL server at 'reading initial communication packet', system error: 0

Similar situation was with 5.0.58 (don't quite remember the version).
I also wonder why free is calling wcsncpy...
[2 Sep 2008 9:32] Tonci Grgin
VS2005 test case

Attachment: Issue23213-netssl.zip (application/x-zip-compressed, text), 315.23 KiB.

[23 Oct 2008 21:06] Chad MILLER
If there is no user role set to accept connections from the packet source, then there is no crash.  The auth system discards the connection before the SSL system crashes.

GDB:

Program received signal SIGABRT, Aborted.
[Switching to Thread 0xb36e3b90 (LWP 20379)]
0xb7f6d410 in __kernel_vsyscall ()
(gdb) bt
#0  0xb7f6d410 in __kernel_vsyscall ()
#1  0xb7d8c085 in raise () from /lib/tls/i686/cmov/libc.so.6
#2  0xb7d8da01 in abort () from /lib/tls/i686/cmov/libc.so.6
#3  0xb7dc4b7c in __libc_message () from /lib/tls/i686/cmov/libc.so.6
#4  0xb7dd061b in free () from /lib/tls/i686/cmov/libc.so.6
#5  0x0876572b in operator delete (ptr=0x8fa1740) at yassl_int.cpp:52
#6  0x08765757 in operator delete[] (ptr=0x8fa1740, nt={<No data fields>})
    at yassl_int.cpp:64
#7  0x08756fe9 in yaSSL::ysArrayDelete<unsigned char> (ptr=0x8fa1740 "¡")
    at ./../include/yassl_types.hpp:71
#8  0x08768764 in ~output_buffer (this=0xb36e2d58) at buffer.cpp:214
#9  0x0875f003 in yaSSL::DH_Server::build (this=0x8fa0d38, ssl=@0x8f73548)
    at yassl_imp.cpp:207
#10 0x08758a35 in yaSSL::ServerKeyExchange::build (this=0xb36e2e58, 
    ssl=@0x8f73548) at yassl_imp.cpp:1603
#11 0x08773c87 in yaSSL::sendServerKeyExchange (ssl=@0x8f73548, 
    buffer=yaSSL::buffered) at handshake.cpp:856
#12 0x08750624 in yaSSL_accept (ssl=0x8f73548) at ssl.cpp:346
#13 0x086ee5c2 in ssl_do (ptr=0x8a37e68, vio=0x8f409e0, timeout=60, 
    connect_accept_func=0x875050a <yaSSL_accept>) at viossl.c:202
#14 0x086ee957 in sslaccept (ptr=0x8a37e68, vio=0x8f409e0, timeout=60)
    at viossl.c:256
#15 0x082c799f in check_connection (thd=0x8f70160) at sql_connect.cc:805
---Type <return> to continue, or q <return> to quit---
#16 0x082c7fed in login_connection (thd=0x8f70160) at sql_connect.cc:955
#17 0x082c81a8 in handle_one_connection (arg=0x8f70160) at sql_connect.cc:1107
#18 0xb7f3e4fb in start_thread () from /lib/tls/i686/cmov/libpthread.so.0
#19 0xb7e37e5e in clone () from /lib/tls/i686/cmov/libc.so.6
(gdb) f 5 
#5  0x0876572b in operator delete (ptr=0x8fa1740) at yassl_int.cpp:52
warning: Source file is more recent than executable.
52          if (ptr) free(ptr);
(gdb) print ptr
$1 = (void *) 0x8fa1740
(gdb) up
#6  0x08765757 in operator delete[] (ptr=0x8fa1740, nt={<No data fields>})
    at yassl_int.cpp:64
64          ::operator delete(ptr, nt);
(gdb) up
#7  0x08756fe9 in yaSSL::ysArrayDelete<unsigned char> (ptr=0x8fa1740 "¡")
    at ./../include/yassl_types.hpp:71
71          ::operator delete[](ptr, yaSSL::ys);
(gdb) up
#8  0x08768764 in ~output_buffer (this=0xb36e2d58) at buffer.cpp:214
warning: Source file is more recent than executable.
214         ysArrayDelete(buffer_); 
(gdb) l
209     }
210
211
212     output_buffer::~output_buffer() 
213     { 
214         ysArrayDelete(buffer_); 
215     }
216
217
218     uint output_buffer::get_size() const 
(gdb) up
#9  0x0875f003 in yaSSL::DH_Server::build (this=0x8fa0d38, ssl=@0x8f73548)
    at yassl_imp.cpp:207
207         memcpy(keyMessage_, tmp.get_buffer(), tmp.get_size());
(gdb) print tmp.buffer_
$2 = (unsigned char *) 0x8fa1740 "¡"
[23 Oct 2008 21:08] Chad MILLER
master.err containing valgrind errors

Attachment: master.err (application/octet-stream, text), 24.37 KiB.

[24 Oct 2008 0:04] Chad MILLER
The problem is in here or in something referenced from here.  'sigSz' is too large by 1.

extras/yassl/src/yassl_imp.cpp +135

    mySTL::auto_ptr<Auth> auth;
    const CertManager& cert = ssl.getCrypto().get_certManager();

    if (ssl.getSecurity().get_parms().sig_algo_ == rsa_sa_algo)
        auth.reset(NEW_YS RSA(cert.get_privateKey(),
                   cert.get_privateKeyLength(), false));
    else {
        auth.reset(NEW_YS DSS(cert.get_privateKey(),
                   cert.get_privateKeyLength(), false));
        sigSz += DSS_ENCODED_EXTRA;
    }

    sigSz += auth->get_signatureLength();

...and this should be added.

=== modified file 'extra/yassl/src/yassl_imp.cpp'
--- extra/yassl/src/yassl_imp.cpp	2008-02-22 15:14:27 +0000
+++ extra/yassl/src/yassl_imp.cpp	2008-10-24 00:02:35 +0000
@@ -195,6 +195,7 @@
                    ssl.getCrypto().get_random());
         byte encoded[DSS_SIG_SZ + DSS_ENCODED_EXTRA];
         TaoCrypt::EncodeDSA_Signature(signature_, encoded);
+        assert(sizeof(encoded) <= sigSz);
         memcpy(signature_, encoded, sizeof(encoded));
     }
[29 Oct 2008 20:10] Chad MILLER
Okay, the problem may appear by using 32-bit Vista also.  Confirmed by iggy.

The choice of crypto params is what matters.  c/net uses MSFT crypto libraries, which didn't have crypto params TLS_DHE_DSS_WITH_AES_256_CBC_SHA until Vista.

So, I think only the bug is on the server end, in use of Diffie-Hellman key agreement and non-RSA cipher.
[31 Oct 2008 22:18] Chad MILLER
Confirmed, and I do not need Windows (of any word size) to reproduce it now.

All Diffie-Hellman-key-exchange PLUS non-RSA-ciphers in YaSSL are affected.  YaSSL supposedly implements these ciphers:

RC4-MD5
RC4-SHA
DES-CBC-SHA
DES-CBC3-SHA
EDH-DSS-DES-CBC-SHA
EDH-DSS-DES-CBC3-SHA
EDH-RSA-DES-CBC-SHA
EDH-RSA-DES-CBC3-SHA
AES128-SHA
DHE-DSS-AES128-SHA
DHE-RSA-AES128-SHA
AES256-SHA
DHE-DSS-AES256-SHA
DHE-RSA-AES256-SHA
DHE-DSS-DES-CBC3-RMD
DHE-DSS-AES128-RMD
DHE-DSS-AES256-RMD
DHE-RSA-DES-CBC3-RMD
DHE-RSA-AES128-RMD
DHE-RSA-AES256-RMD
DES-CBC3-RMD
AES128-RMD
AES256-RMD

and of them, all that have "DHE" in the name but not "RSA" are buggy and cause crashes in the server.

This affected Windows Vista first because it tries to use 
"TLS_DHE_DSS_WITH_AES_CBC_SHA", which we (in yassl) call "DHE-DSS-AES256-SHA".
[3 Nov 2008 19:19] Magnus Blåudd
Patch looks fine, run test also with OpenSSL.

Please remember to notify Todd about the bug you filed at SourceForge.
[17 Nov 2008 16:52] 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/58972

2679 Chad MILLER	2008-11-17
      Bug#39178: non-RSA keys in connection to a RSA-keyed yaSSL-using server \
      	using crashes server
      
      When the server is configured to use a RSA key, and when the client sends
      a cipher-suite list that contains a non-RSA key as acceptable, the server 
      would try to process that key even though it was impossible.
      
      Now, yaSSL sets its own acceptable-cipher list according to what kind of
      key the server is started with, and will never explore and try to pair 
      impossible combinations.
      
      This involves a partial import of the current YaSSL tree, not the whole
      thing, so as to try to avoid introducing new bugs.
[18 Nov 2008 15:11] 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/59082

2680 Chad MILLER	2008-11-18
      Additional patch for Bug#39178.  Fix interface header and update link-
      collision-avoidance CPP-rewrite rules.
[18 Nov 2008 15:16] Magnus Blåudd
Reviewed the two new patches. Ok
[18 Nov 2008 16:35] 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/59095

2679 Chad MILLER	2008-11-18
      Bug#39178: non-RSA keys in connection to a RSA-keyed yaSSL-using server \
      		using crashes server
      
      When the server is configured to use a RSA key, and when the client sends
      a cipher-suite list that contains a non-RSA key as acceptable, the server 
      would try to process that key even though it was impossible.
      
      Now, yaSSL sets its own acceptable-cipher list according to what kind of
      key the server is started with, and will never explore and try to pair 
      impossible combinations.
      
      This involves a partial import of the current YaSSL tree, not the whole
      thing, so as to try to avoid introducing new bugs.
      
      (Updated to avoid many whitespace changes and make diff smaller.)
[18 Nov 2008 16:46] 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/59098

2679 Chad MILLER	2008-11-18
      Bug#39178: non-RSA keys in connection to a RSA-keyed yaSSL-using server \
      		using crashes server
      
      When the server is configured to use a RSA key, and when the client sends
      a cipher-suite list that contains a non-RSA key as acceptable, the server 
      would try to process that key even though it was impossible.
      
      Now, yaSSL sets its own acceptable-cipher list according to what kind of
      key the server is started with, and will never explore and try to pair 
      impossible combinations.
      
      This involves a partial import of the current YaSSL tree, not the whole
      thing, so as to try to avoid introducing new bugs.
      
      (Updated to avoid many whitespace changes and make diff smaller.)
[18 Nov 2008 22:07] Chad MILLER
Pushed into 5.1-bugteam and 6.0-bugteam.
[2 Dec 2008 11:16] 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/60364

2725 Georgi Kodinov	2008-12-02
      Addendum to the fix for bug #39178: Server crash in YaSSL 
      with non-RSA-requesting client if server uses RSA key
      
      matchSuite() may not find a match. 
      It will return error in this case.
      Added a error checking code that will prevent using uninitialized 
      memory in the code based on the assumption 
      that matchSuite() has found a match.
[8 Dec 2008 10:22] Bugs System
Pushed into 5.1.31  (revid:kgeorge@mysql.com-20081202111528-gdxu5521wgqrrxl8) (version source revid:kgeorge@mysql.com-20081202111528-gdxu5521wgqrrxl8) (pib:5)
[8 Dec 2008 11:33] Bugs System
Pushed into 6.0.9-alpha  (revid:kgeorge@mysql.com-20081202111528-gdxu5521wgqrrxl8) (version source revid:kgeorge@mysql.com-20081202112543-novjesppm4r05vkj) (pib:5)
[10 Dec 2008 20:24] Paul Dubois
Noted in 5.1.31, 6.0.9 changelogs.

A server built using yaSSL for SSL support would crash if configured
to use an RSA key and a client sent a cipher list containing a
non-RSA key as acceptable.
[19 Jan 2009 11:23] Bugs System
Pushed into 5.1.31-ndb-6.2.17 (revid:tomas.ulin@sun.com-20090119095303-uwwvxiibtr38djii) (version source revid:tomas.ulin@sun.com-20090108105244-8opp3i85jw0uj5ib) (merge vers: 5.1.31-ndb-6.2.17) (pib:6)
[19 Jan 2009 13:01] Bugs System
Pushed into 5.1.31-ndb-6.3.21 (revid:tomas.ulin@sun.com-20090119104956-guxz190n2kh31fxl) (version source revid:tomas.ulin@sun.com-20090119104956-guxz190n2kh31fxl) (merge vers: 5.1.31-ndb-6.3.21) (pib:6)
[19 Jan 2009 16:07] Bugs System
Pushed into 5.1.31-ndb-6.4.1 (revid:tomas.ulin@sun.com-20090119144033-4aylstx5czzz88i5) (version source revid:tomas.ulin@sun.com-20090119144033-4aylstx5czzz88i5) (merge vers: 5.1.31-ndb-6.4.1) (pib:6)