Description: yaSSL was only handling the cases of zero or

one leading zeros for the key agreement instead of
potentially any number.
There is about 1 in 50,000 connections to fail
when using DHE cipher suites.  The second problem was the
case where a server would send a public value shorter than
the prime value, causing about 1 in 128 client connections
to fail, and also caused the yaSSL client to read off the
end of memory.
All client side DHE cipher suite users should update.
Note: The patch is received from YaSSL people

6 files changed, 40 insertions, 15 deletions

diff --git a/extra/yassl/README b/extra/yassl/README
index bf0e1c9f40f..81d573d0b20 100644
--- a/extra/yassl/README
+++ b/extra/yassl/README
@@ -12,6 +12,17 @@ before calling SSL_new();
 
 *** end Note ***
 
+yaSSL Release notes, version 2.3.9 (12/01/2015)
+    This release of yaSSL fixes two client side Diffie-Hellman problems.
+    yaSSL was only handling the cases of zero or one leading zeros for the key
+    agreement instead of potentially any number.  This caused about 1 in 50,000
+    connections to fail when using DHE cipher suites.  The second problem was
+    the case where a server would send a public value shorter than the prime
+    value, causing about 1 in 128 client connections to fail, and also
+    caused the yaSSL client to read off the end of memory.  All client side
+    DHE cipher suite users should update.
+    Thanks to Adam Langely (agl@imperialviolet.org) for the detailed report!
+
 yaSSL Release notes, version 2.3.8 (9/17/2015)
     This release of yaSSL fixes a high security vulnerability.  All users
     SHOULD update.  If using yaSSL for TLS on the server side with private
diff --git a/extra/yassl/include/crypto_wrapper.hpp b/extra/yassl/include/crypto_wrapper.hpp
index b09b662c88c..0472b304679 100644
--- a/extra/yassl/include/crypto_wrapper.hpp
+++ b/extra/yassl/include/crypto_wrapper.hpp
@@ -378,6 +378,7 @@ public:
 
     uint        get_agreedKeyLength() const;
     const byte* get_agreedKey()       const;
+    uint        get_publicKeyLength() const;
     const byte* get_publicKey()       const;
     void        makeAgreement(const byte*, unsigned int);
 
diff --git a/extra/yassl/include/openssl/ssl.h b/extra/yassl/include/openssl/ssl.h
index b0a7592f870..095b3c6aa80 100644
--- a/extra/yassl/include/openssl/ssl.h
+++ b/extra/yassl/include/openssl/ssl.h
@@ -35,7 +35,7 @@
 #include "rsa.h"
 
 
-#define YASSL_VERSION "2.3.8"
+#define YASSL_VERSION "2.3.9"
 
 
 #if defined(__cplusplus)
diff --git a/extra/yassl/src/crypto_wrapper.cpp b/extra/yassl/src/crypto_wrapper.cpp
index 3f819ee2349..95065932c5f 100644
--- a/extra/yassl/src/crypto_wrapper.cpp
+++ b/extra/yassl/src/crypto_wrapper.cpp
@@ -751,9 +751,10 @@ struct DiffieHellman::DHImpl {
     byte* publicKey_;
     byte* privateKey_;
     byte* agreedKey_;
+    uint  pubKeyLength_;
 
     DHImpl(TaoCrypt::RandomNumberGenerator& r) : ranPool_(r), publicKey_(0),
-                                               privateKey_(0), agreedKey_(0) {}
+                              privateKey_(0), agreedKey_(0), pubKeyLength_(0) {}
     ~DHImpl() 
     {   
         ysArrayDelete(agreedKey_); 
@@ -762,7 +763,7 @@ struct DiffieHellman::DHImpl {
     }
 
     DHImpl(const DHImpl& that) : dh_(that.dh_), ranPool_(that.ranPool_),
-                                 publicKey_(0), privateKey_(0), agreedKey_(0)
+                  publicKey_(0), privateKey_(0), agreedKey_(0), pubKeyLength_(0)
     {
         uint length = dh_.GetByteLength();
         AllocKeys(length, length, length);
@@ -810,7 +811,7 @@ DiffieHellman::DiffieHellman(const byte* p, unsigned int pSz, const byte* g,
     using TaoCrypt::Integer;
 
     pimpl_->dh_.Initialize(Integer(p, pSz).Ref(), Integer(g, gSz).Ref());
-    pimpl_->publicKey_ = NEW_YS opaque[pubSz];
+    pimpl_->publicKey_ = NEW_YS opaque[pimpl_->pubKeyLength_ = pubSz];
     memcpy(pimpl_->publicKey_, pub, pubSz);
 }
 
@@ -869,6 +870,10 @@ const byte* DiffieHellman::get_agreedKey() const
     return pimpl_->agreedKey_;
 }
 
+uint DiffieHellman::get_publicKeyLength() const
+{
+    return pimpl_->pubKeyLength_;
+}
 
 const byte* DiffieHellman::get_publicKey() const
 {
diff --git a/extra/yassl/src/yassl_imp.cpp b/extra/yassl/src/yassl_imp.cpp
index 1baa5adedf8..f190761604d 100644
--- a/extra/yassl/src/yassl_imp.cpp
+++ b/extra/yassl/src/yassl_imp.cpp
@@ -109,15 +109,12 @@ void ClientDiffieHellmanPublic::build(SSL& ssl)
     uint keyLength = dhClient.get_agreedKeyLength(); // pub and agree same
 
     alloc(keyLength, true);
-    dhClient.makeAgreement(dhServer.get_publicKey(), keyLength);
+    dhClient.makeAgreement(dhServer.get_publicKey(),
+                           dhServer.get_publicKeyLength());
     c16toa(keyLength, Yc_);
     memcpy(Yc_ + KEY_OFFSET, dhClient.get_publicKey(), keyLength);
 
-    // because of encoding first byte might be zero, don't use it for preMaster
-    if (*dhClient.get_agreedKey() == 0) 
-        ssl.set_preMaster(dhClient.get_agreedKey() + 1, keyLength - 1);
-    else
-        ssl.set_preMaster(dhClient.get_agreedKey(), keyLength);
+    ssl.set_preMaster(dhClient.get_agreedKey(), keyLength);
 }
 
 
@@ -321,11 +318,7 @@ void ClientDiffieHellmanPublic::read(SSL& ssl, input_buffer& input)
     }
     dh.makeAgreement(Yc_, keyLength); 
 
-    // because of encoding, first byte might be 0, don't use for preMaster 
-    if (*dh.get_agreedKey() == 0) 
-        ssl.set_preMaster(dh.get_agreedKey() + 1, dh.get_agreedKeyLength() - 1);
-    else
-        ssl.set_preMaster(dh.get_agreedKey(), dh.get_agreedKeyLength());
+    ssl.set_preMaster(dh.get_agreedKey(), dh.get_agreedKeyLength());
     ssl.makeMasterSecret();
 }
 
diff --git a/extra/yassl/src/yassl_int.cpp b/extra/yassl/src/yassl_int.cpp
index c04c5a53352..19ba6386efd 100644
--- a/extra/yassl/src/yassl_int.cpp
+++ b/extra/yassl/src/yassl_int.cpp
@@ -807,6 +807,19 @@ void SSL::set_random(const opaque* random, ConnectionEnd sender)
 // store client pre master secret
 void SSL::set_preMaster(const opaque* pre, uint sz)
 {
+    uint i(0);  // trim leading zeros
+    uint fullSz(sz);
+
+    while (i++ < fullSz && *pre == 0) {
+        sz--;
+        pre++;
+    }
+
+    if (sz == 0) {
+        SetError(bad_input);
+        return;
+    }
+
     secure_.use_connection().AllocPreSecret(sz);
     memcpy(secure_.use_connection().pre_master_secret_, pre, sz);
 }
@@ -924,6 +937,8 @@ void SSL::order_error()
 // Create and store the master secret see page 32, 6.1
 void SSL::makeMasterSecret()
 {
+    if (GetError()) return;
+
     if (isTLS())
         makeTLSMasterSecret();
     else {