Re: [openssl-dev] [openssl.org #4545] Crash in crypto/rand/md_rand.c

[Mick Saxton via RT]

I modified your patch to also catch the similar problem in ssleay_rand_bytes.
Results from the instrumented tests attached.

These tests were run on 64-bit Windows 7.
I have not specified a locking callback so will be using the default – could 
this be the problem?

Each thread has it’s own SSL_ctx and each connection is only ever serviced by 
the same thread.

It looks like state_index is going outside of the expected range.

This is possible if one or more threads do
   state_index += num_ceil;

and then another thread reads it before
   if ( state_index > state_num )
  state_index %= st_num.;

Thanks for your help


From: Matt Caswell via RT [mailto:r...@openssl.org]
Sent: 18 June 2016 00:08
To: Mick Saxton
Cc: openssl-dev@openssl.org
Subject: Re: [openssl-dev] [openssl.org #4545] Crash in crypto/rand/md_rand.c



On 17/06/16 20:56, Matt Caswell via RT wrote:
>
>
> On 17/06/16 19:43, Mick Saxton via RT wrote:
>> Perhaps we should consider if there are any negative consequences to my 
>> solution?
>> It does work.
>>
>> I am trying really hard to get contention but I am only seeing this problem 
>> in about 1 out of 100,000 successful TLSv1.2 connections
>> On a heavily congested network.
>> I require three machines to just to run the test that causes the failure.
>>
>> All we are trying to do is get a random number – surely getting a slightly 
>> less random number is better than crashing?
>> It could be that the problematic instances were going to disconnect anyway 
>> due to TCP/IP problems.
>>
>
> I think we need to try instrumenting the code to see if we can get some
> more information out. I will try and pull something together - but it
> might be Monday before I get the opportunity.

I got to it quicker than I thought. Please see attached patch. Can you
apply it to the latest git 1.0.2 version and re-run your test (capture
stderr output). I'd like to see what we get.

Also is this 32-bit or 64-bit Windows? Are you able to share your
locking callback implementation?

Thanks

Matt


--
Ticket here: 
http://rt.openssl.org/Ticket/Display.html?id=4545<http://rt.openssl.org/Ticket/Display.html?id=4545>
Please log in as guest with password guest if prompted




Legal Notice: This email is intended only for the person(s) to whom it is 
addressed. If you are not an intended recipient and have received this message 
in error, please notify the sender immediately by replying to this email or 
calling +44(0) 2083269015 (UK) or +1 866 592 4214 (USA). This email and any 
attachments may be privileged and/or confidential. The unauthorized use, 
disclosure, copying or printing of any information it contains is strictly 
prohibited. The opinions expressed in this email are those of the author and do 
not necessarily represent the views of 1E Ltd. Nothing in this email will 
operate to bind 1E to any order or other contract.

-- 
Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4545
Please log in as guest with password guest if prompted

MD_RAND ERROR: ssleay_rand_add: k == 35, j == 4, st_idx == 1054, state_num == 
1023, do_not_lock 0, crypto_lock_rand == 0, locking_threadid == 3204, 
thisthreadid == 2756
 MD_RAND ERROR: ssleay_rand_add: k == 35, j == 4, st_idx == 1054, state_num == 
1023, do_not_lock 0, crypto_lock_rand == 0, locking_threadid == 3204, 
thisthreadid == 2756
MD_RAND ERROR: ssleay_rand_bytes: k == 23, j == 10, st_idx == 1036, state_num 
== 1023, do_not_lock N/A, crypto_lock_rand == 0, locking_threadid == 8176, 
thisthreadid == 8176-- 
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev

Re: [openssl-dev] [openssl.org #4545] Crash in crypto/rand/md_rand.c

[Mick Saxton via RT]

Perhaps we should consider if there are any negative consequences to my 
solution?
It does work.

I am trying really hard to get contention but I am only seeing this problem in 
about 1 out of 100,000 successful TLSv1.2 connections
On a heavily congested network.
I require three machines to just to run the test that causes the failure.

All we are trying to do is get a random number – surely getting a slightly less 
random number is better than crashing?
It could be that the problematic instances were going to disconnect anyway due 
to TCP/IP problems.



Rather than my previous suggestion – I am now suggesting:-

So in ssleay_rand_add

If ( j-k>0 ) MD_Update(, &(state[st_idx]), j – k);

And a similar fix in ssleay_rand_bytes


This also avoids adding zero bytes to the hash – which it does quite often.




From: Salz, Rich via RT [mailto:r...@openssl.org]
Sent: 17 June 2016 18:39
To: Mick Saxton
Cc: openssl-dev@openssl.org
Subject: RE: [openssl-dev] [openssl.org #4545] Crash in crypto/rand/md_rand.c

Sending mail re-opens the ticket.

Rats, wish it was fixed. Going to need something to more easily reproduce it, I 
guess.

--
Ticket here: 
http://rt.openssl.org/Ticket/Display.html?id=4545
Please log in as guest with password guest if prompted




Legal Notice: This email is intended only for the person(s) to whom it is 
addressed. If you are not an intended recipient and have received this message 
in error, please notify the sender immediately by replying to this email or 
calling +44(0) 2083269015 (UK) or +1 866 592 4214 (USA). This email and any 
attachments may be privileged and/or confidential. The unauthorized use, 
disclosure, copying or printing of any information it contains is strictly 
prohibited. The opinions expressed in this email are those of the author and do 
not necessarily represent the views of 1E Ltd. Nothing in this email will 
operate to bind 1E to any order or other contract.

-- 
Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4545
Please log in as guest with password guest if prompted

-- 
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev

Re: [openssl-dev] [openssl.org #4545] Crash in crypto/rand/md_rand.c

[Mick Saxton via RT]

Hi Rich

Many thanks for doing that – unfortunately it is not the whole fix.
I checked out the latest v1.0.2i-dev and built that but I still get the crash.
I know the ticket is now “closed” – do you need me to reopen it?

I am still convinced that I don’t get it with the “master” build – but that is 
v1.1 and even the build process is significantly different on Windows.
One thing that I did notice is that CPU load seems lower on the v1.1 build 
which would be really great/
.. but it may be making this problem less obvious (frequent).

Thanks Mick

From: Rich Salz via RT [mailto:r...@openssl.org]
Sent: 16 June 2016 19:41
To: Mick Saxton
Cc: openssl-dev@openssl.org
Subject: [openssl.org #4545] Crash in crypto/rand/md_rand.c

commit d0a2bb1f94e26c2e7b44676e9b739c23ad763a79 just pushed to 1.0.2
closing ticket.

--
Ticket here: 
http://rt.openssl.org/Ticket/Display.html?id=4545
Please log in as guest with password guest if prompted




Legal Notice: This email is intended only for the person(s) to whom it is 
addressed. If you are not an intended recipient and have received this message 
in error, please notify the sender immediately by replying to this email or 
calling +44(0) 2083269015 (UK) or +1 866 592 4214 (USA). This email and any 
attachments may be privileged and/or confidential. The unauthorized use, 
disclosure, copying or printing of any information it contains is strictly 
prohibited. The opinions expressed in this email are those of the author and do 
not necessarily represent the views of 1E Ltd. Nothing in this email will 
operate to bind 1E to any order or other contract.

-- 
Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4545
Please log in as guest with password guest if prompted

-- 
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev

Re: [openssl-dev] [openssl.org #4545] Resolved: Crash in crypto/rand/md_rand.c

[Mick Saxton via RT]

Hi Rich

I may be confused but as I see it.
it is not really resolved in v1.0.2 branch? (or will it be in the next v1.0.2 
release?)
– it just will be when v1.1 becomes available.
But I don’t know when that will be.

Mick


From: Rich Salz via RT [mailto:r...@openssl.org]
Sent: 16 June 2016 12:31
To: Mick Saxton 
Subject: [openssl.org #4545] Resolved: Crash in crypto/rand/md_rand.c

According to our records, your request has been resolved. If you have any
further questions or concerns, please respond to this message.

--
Ticket here: 
http://rt.openssl.org/Ticket/Display.html?id=4545
Please log in as guest with password guest if prompted




Legal Notice: This email is intended only for the person(s) to whom it is 
addressed. If you are not an intended recipient and have received this message 
in error, please notify the sender immediately by replying to this email or 
calling +44(0) 2083269015 (UK) or +1 866 592 4214 (USA). This email and any 
attachments may be privileged and/or confidential. The unauthorized use, 
disclosure, copying or printing of any information it contains is strictly 
prohibited. The opinions expressed in this email are those of the author and do 
not necessarily represent the views of 1E Ltd. Nothing in this email will 
operate to bind 1E to any order or other contract.

-- 
Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4545
Please log in as guest with password guest if prompted

-- 
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev

Re: [openssl-dev] [openssl.org #4545] Crash in crypto/rand/md_rand.c

[Mick Saxton via RT]

Hi Rich

Sorry for delay in responding – it takes longer to say that an intermittent 
problem doesn’t exist than to see it happen once.

I have tested against latest master (15 June) and the v1.1-pre5 – so far both 
are looking good.

As a control, I went back and retested with v1.0.2h on the same hardware and it 
exhibited the problem within 3-5 passes of my tests.
(each pass is approx. 110K connections ).

The later software has not exhibited this fault after 50 passes.
Hope this helps.

Thanks Mick


From: Rich Salz via RT [mailto:r...@openssl.org]
Sent: 10 June 2016 20:44
To: Mick Saxton
Cc: openssl-dev@openssl.org
Subject: [openssl.org #4545] Crash in crypto/rand/md_rand.c

Can you test against a recent master, it has some rand bugfixes that might
address this.

--
Ticket here: 
http://rt.openssl.org/Ticket/Display.html?id=4545
Please log in as guest with password guest if prompted




Legal Notice: This email is intended only for the person(s) to whom it is 
addressed. If you are not an intended recipient and have received this message 
in error, please notify the sender immediately by replying to this email or 
calling +44(0) 2083269015 (UK) or +1 866 592 4214 (USA). This email and any 
attachments may be privileged and/or confidential. The unauthorized use, 
disclosure, copying or printing of any information it contains is strictly 
prohibited. The opinions expressed in this email are those of the author and do 
not necessarily represent the views of 1E Ltd. Nothing in this email will 
operate to bind 1E to any order or other contract.

-- 
Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4545
Please log in as guest with password guest if prompted

-- 
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev

Re: [openssl-dev] [openssl.org #4545] Crash in crypto/rand/md_rand.c

[Mick Saxton via RT]

Hi Matt

The test program runs against our major new development so I cannot share it as 
is.

I will try to produce a skeleton version which I could let you have.

-  But that will be end if next week as I am away for a few days

-  That is providing that exhibits the bug.

Mick


From: Matt Caswell via RT [mailto:r...@openssl.org]
Sent: 27 May 2016 10:46
To: Mick Saxton
Cc: openssl-dev@openssl.org
Subject: [openssl.org #4545] Crash in crypto/rand/md_rand.c

On Fri May 20 15:49:49 2016, mi...@1e.com wrote:
> Hi
>
> Before going any further I would like to state that I have only seen
> this problem when we have 1 or more concurrent connections.
>
> Mostly we notice it on Windows but I have seen it on linux (Ubuntu).
>
> I first noticed it when using v1.0.2d but have seen it again since
> upgrading to v1.0.2h.
>
> It can happen in one of two places and results in a call to MD_Update
> with a negative value.
>
> I have come up with a temporary fix which avoids the possibility of
> crashing at the expense of some randomness.
> The system is very highly stressed at this point so debugging further
> is difficult.
>
> The fix I am using is probably not what you eventually will want to
> implement but it does improve stability.
>
> 273: MD_Update(, &(state[st_idx]), (j - k) );
> Change to
> 273: MD_Update(, &(state[st_idx]), (j - k) > 0 ? j - k :
> 1); // mi...@1e.com (j -k) must not be negative
>
> And
>
> 495: MD_Update(, &(state[st_idx]), MD_DIGEST_LENGTH / 2 -
> k )
> Change to
> 495: MD_Update(, &(state[st_idx]), MD_DIGEST_LENGTH / 2 -
> k > 0 ? MD_DIGEST_LENGTH / 2 - k : 1); // mi...@1e.com 
> (j -k) must
> not be negative
>
>
> I do have a test program which can reproduce this behaviour.

Hi Mick

Are you able to share your test program?

Thanks

Matt

--
Ticket here: 
http://rt.openssl.org/Ticket/Display.html?id=4545
Please log in as guest with password guest if prompted




Legal Notice: This email is intended only for the person(s) to whom it is 
addressed. If you are not an intended recipient and have received this message 
in error, please notify the sender immediately by replying to this email or 
calling +44(0) 2083269015 (UK) or +1 866 592 4214 (USA). This email and any 
attachments may be privileged and/or confidential. The unauthorized use, 
disclosure, copying or printing of any information it contains is strictly 
prohibited. The opinions expressed in this email are those of the author and do 
not necessarily represent the views of 1E Ltd. Nothing in this email will 
operate to bind 1E to any order or other contract.

-- 
Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4545
Please log in as guest with password guest if prompted

-- 
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev

[openssl-dev] [openssl.org #4545] Crash in crypto/rand/md_rand.c

[Mick Saxton via RT]

Hi

Before going any further I would like to state that I have only seen this 
problem when we have 1 or more concurrent connections.

Mostly we notice it on Windows but I have seen it on linux (Ubuntu).

I first noticed it when using v1.0.2d but have seen it again since upgrading to 
v1.0.2h.

It can happen in one of two places and results in a call to MD_Update with a 
negative value.

I have come up with a temporary fix which avoids the possibility of crashing at 
the expense of some randomness.
The system is very highly stressed at this point so debugging further is 
difficult.

The fix I am using is probably not what you eventually will want to implement 
but it does improve stability.

273:MD_Update(, &(state[st_idx]), (j - k) );
Change to
273:MD_Update(, &(state[st_idx]), (j - k) > 0 ? j - k : 1);  // 
mi...@1e.com (j -k) must not be negative

And

495:MD_Update(, &(state[st_idx]), MD_DIGEST_LENGTH / 2 - k )
Change to
495:MD_Update(, &(state[st_idx]), MD_DIGEST_LENGTH / 2 - k > 0 ? 
MD_DIGEST_LENGTH / 2 - k : 1);  // mi...@1e.com (j -k) must not be negative


I do have a test program which can reproduce this behaviour.

If I can be of further help - please contact me by email.

Regards
Mick Saxton




Legal Notice: This email is intended only for the person(s) to whom it is 
addressed. If you are not an intended recipient and have received this message 
in error, please notify the sender immediately by replying to this email or 
calling +44(0) 2083269015 (UK) or +1 866 592 4214 (USA). This email and any 
attachments may be privileged and/or confidential. The unauthorized use, 
disclosure, copying or printing of any information it contains is strictly 
prohibited. The opinions expressed in this email are those of the author and do 
not necessarily represent the views of 1E Ltd. Nothing in this email will 
operate to bind 1E to any order or other contract.

-- 
Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4545
Please log in as guest with password guest if prompted



crash.log
Description: Binary data
-- 
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev