I'm not sure that the issue is the lock on the "state" variable because
this is a char array with a fixed size. No dynamic allocations and free.
However, the crash is on this part of code:
MD_Update(&m, &(state[0]), k);
On 14 January 2016 at 14:37, The default queue via RT <r...@openssl.org>
wrote:
>
> Greetings,
>
> This message has been automatically generated in response to the
> creation of a trouble ticket regarding:
> "Crash on ssleay_rand_bytes - global variable is not protected",
> a summary of which appears below.
>
> There is no need to reply to this message right now. Your ticket has been
> assigned an ID of [openssl.org #4235].
>
> Please include the string:
>
> [openssl.org #4235]
>
> in the subject line of all future correspondence about this issue. To do
> so,
> you may reply to this message.
>
> Thank you,
> r...@openssl.org
>
> -------------------------------------------------------------------------
> *Context:*
>
> openssl 1.0.2e
> Windows 2012
> gsoap 2.8.7 - WSSE
> Application multithread
>
>
> *crash dump: *
>
> d:\opensource\openssl-1.0.2e\crypto\sha\sha_locl.h (320):
> sha1_block_data_order
> d:\opensource\openssl-1.0.2e\tmp32\md32_common.h (343): SHA1_Update
> d:\opensource\openssl-1.0.2e\crypto\evp\m_sha1.c (78): update
> d:\opensource\openssl-1.0.2e\crypto\rand\md_rand.c (496): ssleay_rand_bytes
> d:\opensource\openssl-1.0.2e\crypto\rand\md_rand.c (544):
> ssleay_rand_pseudo_bytes
> d:\dev\3rdparty\gsoap-2.8\gsoap\plugin\wsseapi.cpp (3545): calc_nonce
> d:\dev\3rdparty\gsoap-2.8\gsoap\plugin\wsseapi.cpp (1718):
> soap_wsse_add_UsernameTokenDigest
>
> *Analyze:*
>
> I think this issue occurs because of the global static variable "state" is
> not protected into the functions (md_rand.c).
>
> sample:
>
> int ssleay_rand_bytes(unsigned char *buf, int num, int pseudo, int lock)
> {
> ...
>
> *if (lock)*
> * CRYPTO_w_lock(CRYPTO_LOCK_RAND);*
>
> * /* prevent ssleay_rand_bytes() from trying to obtain the lock again
> */*
> * CRYPTO_w_lock(CRYPTO_LOCK_RAND2);*
> * CRYPTO_THREADID_current(&locking_threadid);*
> * CRYPTO_w_unlock(CRYPTO_LOCK_RAND2);*
> * crypto_lock_rand = 1;*
>
> if (!initialized) {
> RAND_poll();
> initialized = 1;
> }
>
> if (!stirred_pool)
> do_stir_pool = 1;
>
> ok = (entropy >= ENTROPY_NEEDED);
> if (!ok) {
> /*
> * If the PRNG state is not yet unpredictable, then seeing the PRNG
> * output may help attackers to determine the new state; thus we
> have
> * to decrease the entropy estimate. Once we've had enough initial
> * seeding we don't bother to adjust the entropy count, though,
> * because we're not ambitious to provide *information-theoretic*
> * randomness. NOTE: This approach fails if the program forks
> before
> * we have enough entropy. Entropy should be collected in a
> separate
> * input pool and be transferred to the output pool only when the
> * entropy limit has been reached.
> */
> entropy -= num;
> if (entropy < 0)
> entropy = 0;
> }
>
> if (do_stir_pool) {
> /*
> * In the output function only half of 'md' remains secret, so we
> * better make sure that the required entropy gets 'evenly
> * distributed' through 'state', our randomness pool. The input
> * function (ssleay_rand_add) chains all of 'md', which makes it
> more
> * suitable for this purpose.
> */
>
> int n = STATE_SIZE; /* so that the complete pool gets accessed
> */
> while (n > 0) {
> #if MD_DIGEST_LENGTH > 20
> # error "Please adjust DUMMY_SEED."
> #endif
> #define DUMMY_SEED "...................." /* at least MD_DIGEST_LENGTH */
> /*
> * Note that the seed does not matter, it's just that
> * ssleay_rand_add expects to have something to hash.
> */
> ssleay_rand_add(DUMMY_SEED, MD_DIGEST_LENGTH, 0.0);
> n -= MD_DIGEST_LENGTH;
> }
> if (ok)
> stirred_pool = 1;
> }
>
> st_idx = state_index;
> st_num = state_num;
> md_c[0] = md_count[0];
> md_c[1] = md_count[1];
> memcpy(local_md, md, sizeof md);
>
> state_index += num_ceil;
> if (state_index > state_num)
> state_index %= state_num;
>
> /*
> * state[st_idx], ..., state[(st_idx + num_ceil - 1) % st_num] are now
> * ours (but other threads may use them too)
> */
>
> md_count[0] += 1;
>
> /* before unlocking, we must clear 'crypto_lock_rand' */
> crypto_lock_rand = 0;
> * if (lock)*
> * CRYPTO_w_unlock(CRYPTO_LOCK_RAND);*
>
> while (num > 0) {
> /* num_ceil -= MD_DIGEST_LENGTH/2 */
> j = (num >= MD_DIGEST_LENGTH / 2) ? MD_DIGEST_LENGTH / 2 : num;
> num -= j;
> MD_Init(&m);
> #ifndef GETPID_IS_MEANINGLESS
> if (curr_pid) { /* just in the first iteration to save time
> */
> MD_Update(&m, (unsigned char *)&curr_pid, sizeof curr_pid);
> curr_pid = 0;
> }
> #endif
> MD_Update(&m, local_md, MD_DIGEST_LENGTH);
> MD_Update(&m, (unsigned char *)&(md_c[0]), sizeof(md_c));
>
> #ifndef PURIFY /* purify complains */
> /*
> * The following line uses the supplied buffer as a small source of
> * entropy: since this buffer is often uninitialised it may cause
> * programs such as purify or valgrind to complain. So for those
> * builds it is not used: the removal of such a small source of
> * entropy has negligible impact on security.
> */
> MD_Update(&m, buf, j);
> #endif
>
> // no lock to protect "state" global variable
>
> k = (st_idx + MD_DIGEST_LENGTH / 2) - st_num;
> if (k > 0) {
> MD_Update(&m, &(state[st_idx]), MD_DIGEST_LENGTH / 2 - k);
> *
>
> MD_Update(&m, &(state[0]), k); // CRASH*
> } else
> MD_Update(&m, &(state[st_idx]), MD_DIGEST_LENGTH / 2);
> MD_Final(&m, local_md);
>
> for (i = 0; i < MD_DIGEST_LENGTH / 2; i++) {
> /* may compete with other threads */
> state[st_idx++] ^= local_md[i];
> if (st_idx >= st_num)
> st_idx = 0;
> if (i < j)
> *(buf++) = local_md[i + MD_DIGEST_LENGTH / 2];
> }
> }
>
> MD_Init(&m);
> MD_Update(&m, (unsigned char *)&(md_c[0]), sizeof(md_c));
> MD_Update(&m, local_md, MD_DIGEST_LENGTH);
> * if (lock)*
> * CRYPTO_w_lock(CRYPTO_LOCK_RAND);*
> MD_Update(&m, md, MD_DIGEST_LENGTH);
> MD_Final(&m, md);
> * if (lock)*
> * CRYPTO_w_unlock(CRYPTO_LOCK_RAND);*
>
> EVP_MD_CTX_cleanup(&m);
>
>
>
> Best Regards,
> Alex
>
>
_______________________________________________
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
