[openssl-dev] [openssl.org #4119] DTLS resets handshake hash too frequently for ClientHello

[David Benjamin via RT]

On Wed, Nov 4, 2015 at 7:04 AM Matt Caswell via RT <rt at openssl.org> wrote:

>
>
> On 03/11/15 17:43, David Benjamin via RT wrote:
>
> > I'm not sure that fix quite works though. If BIO_flush completes
> > asynchronously
>
> Ahhh, yes good point. Updated patch attached.
>
> > (hrm, it's missing an rwstate update),
>
> Yes, just discovered that myself and then came back and reread your
> email to find out you already pointed it out! Also addressed in updated
> patch.
>

The new patch seems to almost work. I merged it into our codebase since we
hadn't diverged too much on that function yet and ran it against our tests
(fixed to actually stress low MTUs). The s->init_off <=
DTLS1_HM_HEADER_LENGTH assertion is only true in the frag_off > 0 case.
After moving it there, everything passes.

For reference, here's the merged version:
https://boringssl-review.googlesource.com/#/c/6424/

David

PS: By the way, you might want to consider doing something similar to test
all the resume points, since they're apparently pretty subtle and hard to
get right sometimes. :-)

I added an async mode to our tests which installs a filter BIO that only
lets one byte/datagram through at a time.
https://boringssl.googlesource.com/boringssl/+/a97b737fb09509dcedf0d15b51b60d2349821605/ssl/test/async_bio.cc
It also makes every callback take two steps where possible.
https://boringssl.googlesource.com/boringssl/+/a97b737fb09509dcedf0d15b51b60d2349821605/ssl/test/bssl_shim.cc#510

That's driven like this:
https://boringssl.googlesource.com/boringssl/+/a97b737fb09509dcedf0d15b51b60d2349821605/ssl/test/bssl_shim.cc#802
https://boringssl.googlesource.com/boringssl/+/a97b737fb09509dcedf0d15b51b60d2349821605/ssl/test/bssl_shim.cc#873

Then that gets run through a series of tests intended to cover all the
various possible handshake shapes.
https://boringssl.googlesource.com/boringssl/+/a97b737fb09509dcedf0d15b51b60d2349821605/ssl/test/runner/runner.go#2539

It's worked pretty well, assuming I remember to cover all the interesting
cases. (Apparently I missed one this time around, so it's not perfect. :-)
Still, it's caught a lot of mistakes early.)