| 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: | |
| 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 | ||
[2 Sep 2008 9:30]
Tonci Grgin
[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)
