Commit 8c65e082 authored by V S Murthy Sidagam's avatar V S Murthy Sidagam

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
parent cb15cce7
...@@ -12,6 +12,17 @@ before calling SSL_new(); ...@@ -12,6 +12,17 @@ before calling SSL_new();
*** end Note *** *** 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) yaSSL Release notes, version 2.3.8 (9/17/2015)
This release of yaSSL fixes a high security vulnerability. All users This release of yaSSL fixes a high security vulnerability. All users
SHOULD update. If using yaSSL for TLS on the server side with private SHOULD update. If using yaSSL for TLS on the server side with private
......
...@@ -378,6 +378,7 @@ public: ...@@ -378,6 +378,7 @@ public:
uint get_agreedKeyLength() const; uint get_agreedKeyLength() const;
const byte* get_agreedKey() const; const byte* get_agreedKey() const;
uint get_publicKeyLength() const;
const byte* get_publicKey() const; const byte* get_publicKey() const;
void makeAgreement(const byte*, unsigned int); void makeAgreement(const byte*, unsigned int);
......
...@@ -35,7 +35,7 @@ ...@@ -35,7 +35,7 @@
#include "rsa.h" #include "rsa.h"
#define YASSL_VERSION "2.3.8" #define YASSL_VERSION "2.3.9"
#if defined(__cplusplus) #if defined(__cplusplus)
......
...@@ -751,9 +751,10 @@ struct DiffieHellman::DHImpl { ...@@ -751,9 +751,10 @@ struct DiffieHellman::DHImpl {
byte* publicKey_; byte* publicKey_;
byte* privateKey_; byte* privateKey_;
byte* agreedKey_; byte* agreedKey_;
uint pubKeyLength_;
DHImpl(TaoCrypt::RandomNumberGenerator& r) : ranPool_(r), publicKey_(0), DHImpl(TaoCrypt::RandomNumberGenerator& r) : ranPool_(r), publicKey_(0),
privateKey_(0), agreedKey_(0) {} privateKey_(0), agreedKey_(0), pubKeyLength_(0) {}
~DHImpl() ~DHImpl()
{ {
ysArrayDelete(agreedKey_); ysArrayDelete(agreedKey_);
...@@ -762,7 +763,7 @@ struct DiffieHellman::DHImpl { ...@@ -762,7 +763,7 @@ struct DiffieHellman::DHImpl {
} }
DHImpl(const DHImpl& that) : dh_(that.dh_), ranPool_(that.ranPool_), 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(); uint length = dh_.GetByteLength();
AllocKeys(length, length, length); AllocKeys(length, length, length);
...@@ -810,7 +811,7 @@ DiffieHellman::DiffieHellman(const byte* p, unsigned int pSz, const byte* g, ...@@ -810,7 +811,7 @@ DiffieHellman::DiffieHellman(const byte* p, unsigned int pSz, const byte* g,
using TaoCrypt::Integer; using TaoCrypt::Integer;
pimpl_->dh_.Initialize(Integer(p, pSz).Ref(), Integer(g, gSz).Ref()); 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); memcpy(pimpl_->publicKey_, pub, pubSz);
} }
...@@ -869,6 +870,10 @@ const byte* DiffieHellman::get_agreedKey() const ...@@ -869,6 +870,10 @@ const byte* DiffieHellman::get_agreedKey() const
return pimpl_->agreedKey_; return pimpl_->agreedKey_;
} }
uint DiffieHellman::get_publicKeyLength() const
{
return pimpl_->pubKeyLength_;
}
const byte* DiffieHellman::get_publicKey() const const byte* DiffieHellman::get_publicKey() const
{ {
......
...@@ -109,15 +109,12 @@ void ClientDiffieHellmanPublic::build(SSL& ssl) ...@@ -109,15 +109,12 @@ void ClientDiffieHellmanPublic::build(SSL& ssl)
uint keyLength = dhClient.get_agreedKeyLength(); // pub and agree same uint keyLength = dhClient.get_agreedKeyLength(); // pub and agree same
alloc(keyLength, true); alloc(keyLength, true);
dhClient.makeAgreement(dhServer.get_publicKey(), keyLength); dhClient.makeAgreement(dhServer.get_publicKey(),
dhServer.get_publicKeyLength());
c16toa(keyLength, Yc_); c16toa(keyLength, Yc_);
memcpy(Yc_ + KEY_OFFSET, dhClient.get_publicKey(), keyLength); memcpy(Yc_ + KEY_OFFSET, dhClient.get_publicKey(), keyLength);
// because of encoding first byte might be zero, don't use it for preMaster ssl.set_preMaster(dhClient.get_agreedKey(), keyLength);
if (*dhClient.get_agreedKey() == 0)
ssl.set_preMaster(dhClient.get_agreedKey() + 1, keyLength - 1);
else
ssl.set_preMaster(dhClient.get_agreedKey(), keyLength);
} }
...@@ -321,11 +318,7 @@ void ClientDiffieHellmanPublic::read(SSL& ssl, input_buffer& input) ...@@ -321,11 +318,7 @@ void ClientDiffieHellmanPublic::read(SSL& ssl, input_buffer& input)
} }
dh.makeAgreement(Yc_, keyLength); dh.makeAgreement(Yc_, keyLength);
// because of encoding, first byte might be 0, don't use for preMaster ssl.set_preMaster(dh.get_agreedKey(), dh.get_agreedKeyLength());
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.makeMasterSecret(); ssl.makeMasterSecret();
} }
......
...@@ -807,6 +807,19 @@ void SSL::set_random(const opaque* random, ConnectionEnd sender) ...@@ -807,6 +807,19 @@ void SSL::set_random(const opaque* random, ConnectionEnd sender)
// store client pre master secret // store client pre master secret
void SSL::set_preMaster(const opaque* pre, uint sz) 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); secure_.use_connection().AllocPreSecret(sz);
memcpy(secure_.use_connection().pre_master_secret_, pre, sz); memcpy(secure_.use_connection().pre_master_secret_, pre, sz);
} }
...@@ -924,6 +937,8 @@ void SSL::order_error() ...@@ -924,6 +937,8 @@ void SSL::order_error()
// Create and store the master secret see page 32, 6.1 // Create and store the master secret see page 32, 6.1
void SSL::makeMasterSecret() void SSL::makeMasterSecret()
{ {
if (GetError()) return;
if (isTLS()) if (isTLS())
makeTLSMasterSecret(); makeTLSMasterSecret();
else { else {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment