Bug#20201864 : UPGRADE TO YASSL 2.3.7

Upgrading YaSSL from 2.3.5 to 2.3.7

Reviewed-by : Kristofer Pettersson <kristofer.pettersson@oracle.com>
Reviewed-by : Vamsikrishna Bhagi <vamsikrishna.bhagi@oracle.com>

extra/yassl/README

@@ -12,6 +12,16 @@ before calling SSL_new();
 
 *** end Note ***
 
+yaSSL Release notes, version 2.3.7 (12/10/2014)
+    This release of yaSSL fixes the potential to process duplicate handshake
+    messages by explicitly marking/checking received handshake messages.
+
+yaSSL Release notes, version 2.3.6 (11/25/2014)
+
+    This release of yaSSL fixes some valgrind warnings/errors including
+    uninitialized reads and off by one index errors induced from fuzzing
+    the handshake.  These were reported by Oracle. 
+
 yaSSL Release notes, version 2.3.5 (9/29/2014)
 
     This release of yaSSL fixes an RSA Padding check vulnerability reported by

extra/yassl/examples/client/client.cpp

@@ -18,6 +18,10 @@
 
 /* client.cpp  */
 
+// takes an optional command line argument of cipher list to make scripting
+// easier
+
+
 #include "../../testsuite/test.hpp"
 
 //#define TEST_RESUME
@@ -73,11 +77,16 @@ void client_test(void* args)
 #ifdef NON_BLOCKING
     tcp_set_nonblocking(sockfd);
 #endif
-
     SSL_METHOD* method = TLSv1_client_method();
     SSL_CTX*    ctx = SSL_CTX_new(method);
 
     set_certs(ctx);
+    if (argc >= 2) {
+        printf("setting cipher list to %s\n", argv[1]);
+        if (SSL_CTX_set_cipher_list(ctx, argv[1]) != SSL_SUCCESS) {
+            ClientError(ctx, NULL, sockfd, "set_cipher_list error\n");
+        }
+    }
     SSL* ssl = SSL_new(ctx);
 
     SSL_set_fd(ssl, sockfd);

extra/yassl/examples/server/server.cpp

@@ -18,6 +18,9 @@
 
 /* server.cpp */
 
+// takes 2 optional command line argument to make scripting
+// if the first  command line argument is 'n' client auth is disabled
+// if the second command line argument is 'd' DSA certs are used instead of RSA
 
 #include "../../testsuite/test.hpp"
 
@@ -69,6 +72,9 @@ THREAD_RETURN YASSL_API server_test(void* args)
     char**   argv     = 0;
 
     set_args(argc, argv, *static_cast<func_args*>(args));
+#ifdef SERVER_READY_FILE
+    set_file_ready("server_ready", *static_cast<func_args*>(args));
+#endif
     tcp_accept(sockfd, clientfd, *static_cast<func_args*>(args));
 
     tcp_close(sockfd);
@@ -77,8 +83,21 @@ THREAD_RETURN YASSL_API server_test(void* args)
     SSL_CTX*    ctx = SSL_CTX_new(method);
 
     //SSL_CTX_set_cipher_list(ctx, "RC4-SHA:RC4-MD5");
-    SSL_CTX_set_verify(ctx, SSL_VERIFY_PEER, 0);
+    
-    set_serverCerts(ctx);
+    // should we disable client auth
+    if (argc >= 2 && argv[1][0] == 'n')
+        printf("disabling client auth\n");
+    else
+        SSL_CTX_set_verify(ctx, SSL_VERIFY_PEER, 0);
+
+    // are we using DSA certs
+    if (argc >= 3 && argv[2][0] == 'd') {
+        printf("using DSA certs\n");
+        set_dsaServerCerts(ctx);
+    }
+    else {
+        set_serverCerts(ctx);
+    }
     DH* dh = set_tmpDH(ctx);
 
     SSL* ssl = SSL_new(ctx);

extra/yassl/include/openssl/ssl.h

@@ -35,7 +35,7 @@
 #include "rsa.h"
 
 
-#define YASSL_VERSION "2.3.5"
+#define YASSL_VERSION "2.3.7"
 
 
 #if defined(__cplusplus)

extra/yassl/include/yassl_int.hpp

@@ -107,6 +107,25 @@ enum AcceptState {
 };
 
 
+// track received messages to explicitly disallow duplicate messages
+struct RecvdMessages {
+    uint8 gotClientHello_;
+    uint8 gotServerHello_;
+    uint8 gotCert_;
+    uint8 gotServerKeyExchange_;
+    uint8 gotCertRequest_;
+    uint8 gotServerHelloDone_;
+    uint8 gotCertVerify_;
+    uint8 gotClientKeyExchange_;
+    uint8 gotFinished_;
+    RecvdMessages() : gotClientHello_(0), gotServerHello_(0), gotCert_(0),
+                      gotServerKeyExchange_(0), gotCertRequest_(0),
+                      gotServerHelloDone_(0), gotCertVerify_(0),
+                      gotClientKeyExchange_(0), gotFinished_(0)
+                    {} 
+};
+
+
 // combines all states
 class States {
     RecordLayerState recordLayer_;
@@ -115,6 +134,7 @@ class States {
     ServerState      serverState_;
     ConnectState     connectState_;
     AcceptState      acceptState_;
+    RecvdMessages    recvdMessages_;
     char             errorString_[MAX_ERROR_SZ];
     YasslError       what_;
 public:
@@ -137,6 +157,7 @@ public:
     AcceptState&      UseAccept();
     char*             useString();
     void              SetError(YasslError);
+    int               SetMessageRecvd(HandShakeType);
 private:
     States(const States&);              // hide copy
     States& operator=(const States&);   // and assign

extra/yassl/src/yassl_imp.cpp

@@ -242,6 +242,7 @@ void EncryptedPreMasterSecret::read(SSL& ssl, input_buffer& input)
     }
 
     opaque preMasterSecret[SECRET_LEN];
+    memset(preMasterSecret, 0, sizeof(preMasterSecret));
     rsa.decrypt(preMasterSecret, secret_, length_, 
                 ssl.getCrypto().get_random());
 
@@ -300,6 +301,11 @@ void ClientDiffieHellmanPublic::read(SSL& ssl, input_buffer& input)
     tmp[1] = input[AUTO];
     ato16(tmp, keyLength);
 
+    if (keyLength < dh.get_agreedKeyLength()/2) {
+        ssl.SetError(bad_input);
+        return;
+    }
+
     alloc(keyLength);
     input.read(Yc_, keyLength);
     if (input.get_error()) {
@@ -408,6 +414,10 @@ void DH_Server::read(SSL& ssl, input_buffer& input)
     tmp[1] = input[AUTO];
     ato16(tmp, length);
 
+    if (length == 0) {
+        ssl.SetError(bad_input);
+        return;
+    }
     signature_ = NEW_YS byte[length];
     input.read(signature_, length);
     if (input.get_error()) {
@@ -864,6 +874,12 @@ void ChangeCipherSpec::Process(input_buffer& input, SSL& ssl)
         return;
     }
 
+    // detect duplicate change_cipher
+    if (ssl.getSecurity().get_parms().pending_ == false) {
+        ssl.order_error();
+        return;
+    }
+
     ssl.useSecurity().use_parms().pending_ = false;
     if (ssl.getSecurity().get_resuming()) {
         if (ssl.getSecurity().get_parms().entity_ == client_end)
@@ -2047,12 +2063,8 @@ input_buffer& operator>>(input_buffer& input, CertificateRequest& request)
         tmp[0] = input[AUTO];
         tmp[1] = input[AUTO];
         ato16(tmp, dnSz);
-        
+       
-        DistinguishedName dn;
+        input.set_current(input.get_current() + dnSz);
-        request.certificate_authorities_.push_back(dn = NEW_YS 
-                                                  byte[REQUEST_HEADER + dnSz]);
-        memcpy(dn, tmp, REQUEST_HEADER);
-        input.read(&dn[REQUEST_HEADER], dnSz);
 
         sz -= dnSz + REQUEST_HEADER;
 
@@ -2191,6 +2203,11 @@ input_buffer& operator>>(input_buffer& input, CertificateVerify& request)
     ato16(tmp, sz);
     request.set_length(sz);
 
+    if (sz == 0) {
+        input.set_error();
+        return input;
+    }
+
     request.signature_ = NEW_YS byte[sz];
     input.read(request.signature_, sz);
 

extra/yassl/src/yassl_int.cpp

@@ -255,6 +255,77 @@ void States::SetError(YasslError ye)
 }
 
 
+// mark message recvd, check for duplicates, return 0 on success
+int States::SetMessageRecvd(HandShakeType hst)
+{
+    switch (hst) {
+        case hello_request:
+            break;  // could send more than one
+
+        case client_hello:
+            if (recvdMessages_.gotClientHello_)
+                return -1;
+            recvdMessages_.gotClientHello_ = 1;
+            break;
+
+        case server_hello:
+            if (recvdMessages_.gotServerHello_)
+                return -1;
+            recvdMessages_.gotServerHello_ = 1;
+            break;
+
+        case certificate:
+            if (recvdMessages_.gotCert_)
+                return -1;
+            recvdMessages_.gotCert_ = 1;
+            break;
+
+        case server_key_exchange:
+            if (recvdMessages_.gotServerKeyExchange_)
+                return -1;
+            recvdMessages_.gotServerKeyExchange_ = 1;
+            break;
+
+        case certificate_request:
+            if (recvdMessages_.gotCertRequest_)
+                return -1;
+            recvdMessages_.gotCertRequest_ = 1;
+            break;
+
+        case server_hello_done:
+            if (recvdMessages_.gotServerHelloDone_)
+                return -1;
+            recvdMessages_.gotServerHelloDone_ = 1;
+            break;
+
+        case certificate_verify:
+            if (recvdMessages_.gotCertVerify_)
+                return -1;
+            recvdMessages_.gotCertVerify_ = 1;
+            break;
+
+        case client_key_exchange:
+            if (recvdMessages_.gotClientKeyExchange_)
+                return -1;
+            recvdMessages_.gotClientKeyExchange_ = 1;
+            break;
+
+        case finished:
+            if (recvdMessages_.gotFinished_)
+                return -1;
+            recvdMessages_.gotFinished_ = 1;
+            break;
+
+
+        default:
+            return -1;
+
+    }
+
+    return 0;
+}
+
+
 sslFactory::sslFactory() :           
         messageFactory_(InitMessageFactory),
         handShakeFactory_(InitHandShakeFactory),
@@ -1199,6 +1270,11 @@ void SSL::verifyState(const HandShakeHeader& hsHeader)
         return;
     }
 
+    if (states_.SetMessageRecvd(hsHeader.get_handshakeType()) != 0) {
+        order_error();
+        return;
+    }
+
     if (secure_.get_parms().entity_ == client_end)
         verifyClientState(hsHeader.get_handshakeType());
     else

extra/yassl/taocrypt/src/asn.cpp

@@ -672,7 +672,7 @@ word32 CertDecoder::GetSignature()
     }
 
     sigLength_ = GetLength(source_);
-    if (sigLength_ == 0 || source_.IsLeft(sigLength_) == false) {
+    if (sigLength_ <= 1 || source_.IsLeft(sigLength_) == false) {
         source_.SetError(CONTENT_E);
         return 0;
     }
@@ -1001,11 +1001,17 @@ bool CertDecoder::ConfirmSignature(Source& pub)
         RSA_PublicKey pubKey(pub);
         RSAES_Encryptor enc(pubKey);
 
+        if (pubKey.FixedCiphertextLength() != sigLength_) {
+            source_.SetError(SIG_LEN_E);
+            return false;
+        }
+
         return enc.SSL_Verify(build.get_buffer(), build.size(), signature_);
     }
     else  { // DSA
         // extract r and s from sequence
         byte seqDecoded[DSA_SIG_SZ];
+        memset(seqDecoded, 0, sizeof(seqDecoded));
         DecodeDSA_Signature(seqDecoded, signature_, sigLength_);
 
         DSA_PublicKey pubKey(pub);

extra/yassl/taocrypt/src/integer.cpp

@@ -2605,18 +2605,20 @@ void Integer::Decode(Source& source)
 void Integer::Decode(const byte* input, unsigned int inputLen, Signedness s)
 {
     unsigned int idx(0);
-    byte b = input[idx++];
+    byte b = 0; 
+    if (inputLen>0)
+        b = input[idx];   // peek
     sign_  = ((s==SIGNED) && (b & 0x80)) ? NEGATIVE : POSITIVE;
 
     while (inputLen>0 && (sign_==POSITIVE ? b==0 : b==0xff))
     {
-        inputLen--;
+        idx++;   // skip
-        b = input[idx++];
+        if (--inputLen>0)
+            b = input[idx];  // peek
     }
 
     reg_.CleanNew(RoundupSize(BytesToWords(inputLen)));
 
-    --idx;
     for (unsigned int i=inputLen; i > 0; i--)
     {
         b = input[idx++];

extra/yassl/testsuite/cipher-test.sh

@@ -0,0 +1,130 @@
+#!/bin/bash
+
+# test all yassl cipher suties 
+# 
+
+
+server_pid=$no_pid
+
+
+do_cleanup() {
+    echo "in cleanup"
+
+    if [[ $server_pid != $no_pid ]]
+    then
+        echo "killing server"
+        kill -9 $server_pid
+    fi
+}
+
+do_trap() {
+    echo "got trap"
+    do_cleanup
+    exit -1
+}
+
+trap do_trap INT TERM
+
+
+# make sure example server and client are built
+if test ! -s ../examples/server/server; then
+    echo "Please build yaSSL first, example server missing"
+    exit -1
+fi
+
+if test ! -s ../examples/client/client; then
+    echo "Please build yaSSL first, example client missing"
+    exit -1
+fi
+
+
+# non DSA suites
+for suite in {"DHE-RSA-AES256-SHA","AES256-SHA","DHE-RSA-AES128-SHA","AES128-SHA","AES256-RMD","AES128-RMD","DES-CBC3-RMD","DHE-RSA-AES256-RMD","DHE-RSA-AES128-RMD","DHE-RSA-DES-CBC3-RMD","RC4-SHA","RC4-MD5","DES-CBC3-SHA","DES-CBC-SHA","EDH-RSA-DES-CBC3-SHA","EDH-RSA-DES-CBC-SHA"}
+do
+  for client_auth in {y,n}
+  do
+    echo "Trying $suite client auth = $client_auth ..."
+
+    if test -e server_ready; then
+        echo -e "removing exisitng server_ready file"
+        rm server_ready
+    fi
+    ../examples/server/server $client_auth &
+    server_pid=$!
+
+    while [ ! -s server_ready ]; do
+        echo -e "waiting for server_ready file..."
+        sleep 0.1
+    done
+
+    ../examples/client/client $suite
+    client_result=$?
+
+    wait $server_pid
+    server_result=$?
+
+    server_pid=$no_pid
+
+    if [[ $client_result != 0 ]]
+    then
+        echo "Client Error"
+        exit $client_result
+    fi
+
+    if [[ $server_result != 0 ]]
+    then
+        echo "Server Error"
+        exit $server_result
+    fi
+
+  done   # end client auth loop
+done  # end non dsa suite list
+echo -e "Non DSA Loop SUCCESS"
+
+
+
+# DSA suites
+for suite in {"DHE-DSS-AES256-SHA","DHE-DSS-AES128-SHA","DHE-DSS-AES256-RMD","DHE-DSS-AES128-RMD","DHE-DSS-DES-CBC3-RMD","EDH-DSS-DES-CBC3-SHA","EDH-DSS-DES-CBC-SHA"}
+do
+  for client_auth in {y,n}
+  do
+    echo "Trying $suite client auth = $client_auth ..."
+
+    if test -e server_ready; then
+        echo -e "removing exisitng server_ready file"
+        rm server_ready
+    fi
+    # d signifies DSA
+    ../examples/server/server $client_auth d &
+    server_pid=$!
+
+    while [ ! -s server_ready ]; do
+        echo -e "waiting for server_ready file..."
+        sleep 0.1
+    done
+
+    ../examples/client/client $suite
+    client_result=$?
+
+    wait $server_pid
+    server_result=$?
+
+    server_pid=$no_pid
+
+    if [[ $client_result != 0 ]]
+    then
+        echo "Client Error"
+        exit $client_result
+    fi
+
+    if [[ $server_result != 0 ]]
+    then
+        echo "Server Error"
+        exit $server_result
+    fi
+
+  done   # end client auth loop
+done  # end dsa suite list
+echo -e "DSA Loop SUCCESS"
+
+exit 0

extra/yassl/testsuite/test.hpp

@@ -131,9 +131,10 @@ struct func_args {
     int    argc;
     char** argv;
     int    return_code;
+    const char* file_ready;
     tcp_ready* signal_;
 
-    func_args(int c = 0, char** v = 0) : argc(c), argv(v) {}
+    func_args(int c = 0, char** v = 0) : argc(c), argv(v), file_ready(0) {}
 
     void SetSignal(tcp_ready* p) { signal_ = p; }
 };
@@ -146,6 +147,7 @@ void join_thread(THREAD_TYPE);
 // yaSSL
 const char* const    yasslIP      = "127.0.0.1";
 const unsigned short yasslPort    =  11111;
+const unsigned short proxyPort    =  12345;
 
 
 // client
@@ -172,13 +174,13 @@ const char* const svrKey3  = "../../../certs/server-key.pem";
 
 // server dsa
 const char* const dsaCert = "../certs/dsa-cert.pem";
-const char* const dsaKey  = "../certs/dsa512.der";
+const char* const dsaKey  = "../certs/dsa1024.der";
 
 const char* const dsaCert2 = "../../certs/dsa-cert.pem";
-const char* const dsaKey2  = "../../certs/dsa512.der";
+const char* const dsaKey2  = "../../certs/dsa1024.der";
 
 const char* const dsaCert3 = "../../../certs/dsa-cert.pem";
-const char* const dsaKey3  = "../../../certs/dsa512.der";
+const char* const dsaKey3  = "../../../certs/dsa1024.der";
 
 
 // CA 
@@ -222,6 +224,13 @@ inline void store_ca(SSL_CTX* ctx)
         if (SSL_CTX_load_verify_locations(ctx, certSuite, 0) != SSL_SUCCESS)
             if (SSL_CTX_load_verify_locations(ctx, certDebug,0) != SSL_SUCCESS)
                 err_sys("failed to use certificate: certs/client-cert.pem");
+
+    // DSA cert 
+    if (SSL_CTX_load_verify_locations(ctx, dsaCert, 0) != SSL_SUCCESS)
+        if (SSL_CTX_load_verify_locations(ctx, dsaCert2, 0) != SSL_SUCCESS)
+            if (SSL_CTX_load_verify_locations(ctx, dsaCert3, 0) != SSL_SUCCESS)
+                err_sys("failed to use certificate: certs/dsa-cert.pem");
+
 }
 
 
@@ -298,7 +307,7 @@ inline void set_dsaServerCerts(SSL_CTX* ctx)
             != SSL_SUCCESS) 
                 if (SSL_CTX_use_PrivateKey_file(ctx, dsaKey3,SSL_FILETYPE_ASN1)
                     != SSL_SUCCESS) 
-                    err_sys("failed to use key file: certs/dsa512.der");
+                    err_sys("failed to use key file: certs/dsa1024.der");
 }
 
 
@@ -310,6 +319,12 @@ inline void set_args(int& argc, char**& argv, func_args& args)
 }
 
 
+inline void set_file_ready(const char* name, func_args& args)
+{
+    args.file_ready = name;
+}
+
+
 inline void tcp_set_nonblocking(SOCKET_T& sockfd)
 {
 #ifdef NON_BLOCKING
@@ -349,7 +364,11 @@ inline void tcp_socket(SOCKET_T& sockfd, SOCKADDR_IN_T& addr)
         */   // end external testing later
 #else
     addr.sin_family = AF_INET_V;
+#ifdef YASSL_PROXY_PORT
+    addr.sin_port = htons(proxyPort);
+#else
     addr.sin_port = htons(yasslPort);
+#endif
     addr.sin_addr.s_addr = inet_addr(yasslIP);
 #endif
 
@@ -401,6 +420,16 @@ inline void tcp_listen(SOCKET_T& sockfd)
 }
 
 
+inline void create_ready_file(func_args& args)
+{
+    FILE* f = fopen(args.file_ready, "w+");
+
+    if (f) {
+        fputs("ready", f);
+        fclose(f);
+    }
+}
+
 
 inline void tcp_accept(SOCKET_T& sockfd, SOCKET_T& clientfd, func_args& args)
 {
@@ -418,6 +447,9 @@ inline void tcp_accept(SOCKET_T& sockfd, SOCKET_T& clientfd, func_args& args)
     pthread_mutex_unlock(&ready.mutex_);
 #endif
 
+    if (args.file_ready)
+        create_ready_file(args);
+
     clientfd = accept(sockfd, (sockaddr*)&client, (ACCEPT_THIRD_T)&client_len);
 
     if (clientfd == (SOCKET_T) -1) {