Here's a tentative patch that causes the test cases to pass with OpenSSL
1.1.1e+ for me on Debian Sid.
James, can you please give it a go and confirm that it works for you?
Before I commit, we'd also need to gate this based on the new OpenSSL
constant. We could gate based upon the OpenSSL version number, but I think
if we can confine to this one area, gating based on ifdef is likely prudent.
I'd also like to get the scons patch you sent earlier into a new release.
I'm hopeful that I can cut a new 1.3 release sometime this week and have
others vote on it - that should make your downstream efforts a bit easier.
As I mentioned earlier, I may also try to add a new test case that confirms
that the client can handle a premature EOF from server-side without
erroring out as that is the more interesting case from Serf's
perspective...though, I think I may try to just get a new 1.3 out first.
If it does fail in that case (and I can get around to mocking it up), we
might have to do a fast-follow with another release if it doesn't alter the
API in any meaningful manner.
Cheers. -- justin
--- ../../serf-1.3.9/test/server/test_sslserver.c 2016-06-30
11:45:07.000000000 -0400
+++ test/server/test_sslserver.c 2020-03-30 14:41:20.773365217 -0400
@@ -424,6 +424,11 @@
*len = 0;
return APR_EAGAIN;
case SSL_ERROR_SSL:
+ if (ERR_GET_REASON(ERR_peek_error()) ==
SSL_R_UNEXPECTED_EOF_WHILE_READING) {
+ *len = 0;
+ return APR_EOF;
+ }
+ /* Fallthrough */
default:
*len = 0;
serf__log(TEST_VERBOSE, __FILE__,
On Sat, Mar 28, 2020 at 9:49 AM Justin Erenkrantz <jus...@erenkrantz.com>
wrote:
> Ok, I was finally able to reproduce this with Debian sid's serf 1.3 source
> packages.
>
> In short, I went down a wild goose chase - as it's actually the test
> harness (our mock server) that is returning this SSL error - not serf
> itself.
>
> The test suite isn't handling the new SSL_ERROR_SSL return code when the
> *client* (eg serf) disconnects as the test case only wants to send one
> request and then closes the connection. So, we likely just need to ignore
> the returned SSL_ERROR_SSL in this case - I'm still trying to figure out
> how we can distinguish that EOF case from a general SSL (probably
> SSL_R_UNEXPECTED_EOF_WHILE_READING); but, I'm timing out on my cycles right
> now.
>
> I'd actually like to see if I can create a test case where the *server*
> disconnects abnormally over SSL and ensure that Serf handles it
> appropriately. (I haven't yet checked to confirm that we have this case
> already; but, it'd likely be relatively straightforward to do this.)
>
> Cheers. -- justin
>
> P.S. Note to future self and others:
> https://wiki.debian.org/HowToGetABacktrace and
> https://wiki.debian.org/BuildingTutorial#Get_the_source_package were
> useful. I ultimately needed the libssl1.1-dbgsym package; then I was able
> to break on the ssl3_read_n call and see the callstack.
>
> On Fri, Mar 27, 2020 at 5:56 PM Lucas Nussbaum <lu...@debian.org> wrote:
>
>> Hi Justin,
>>
>> Most likely, this is due to the new openssl version in unstable.
>>
>> Lucas
>>
>>
>> On 27/03/20 at 17:15 -0400, Justin Erenkrantz wrote:
>> > James,
>> >
>> > I finally got a Debian sid environment up. However, I'm seeing a
>> different
>> > sets of test failures right now against vanilla serf 1.4.x and trunk
>> (which
>> > works with the scons/python3 in sid without a patch AFAICT) - this is
>> with
>> > 1.4.x branch:
>> >
>> > ---
>> >
>> > == Running the unit tests ==
>> >
>> >
>> ...................................................................F......................F..FFF.F.............................
>> >
>> > There were 6 failures:
>> >
>> > 1) test_ssl_handshake_nosslv2: test/test_ssl.c:589: Serf does not
>> disable
>> > SSLv2, but it should!
>> >
>> > 2) test_ssl_missing_client_certificate: test/test_ssl.c:1925: expected
>> > <120172> but was <120171>
>> >
>> > 3) test_ssl_server_cert_with_cn_nul_byte: test/test_util.c:551: expected
>> > <0> but was <120199>
>> >
>> > 4) test_ssl_server_cert_with_san_nul_byte: test/test_util.c:551:
>> expected
>> > <0> but was <120199>
>> >
>> > 5) test_ssl_server_cert_with_cnsan_nul_byte: test/test_util.c:551:
>> expected
>> > <0> but was <120199>
>> >
>> > 6) test_ssl_renegotiate: test/test_ssl.c:1881: expected <0> but was
>> <120199>
>> >
>> >
>> > !!!FAILURES!!!
>> >
>> > Runs: 127 Passes: 121 Fails: 6
>> > ---
>> >
>> > I'll try to dig into this more over the weekend, but I wonder if I'm
>> seeing
>> > something different than you (or the builder) are...I'll also try to
>> pull
>> > in your patch set against vanilla 1.3.x to see if I can match the
>> reported
>> > error.
>> >
>> > Cheers. -- justin
>> >
>> > On Wed, Mar 25, 2020 at 8:17 PM James McCoy <james...@debian.org>
>> wrote:
>> >
>> > > On Wed, Mar 25, 2020 at 08:57:14AM -0400, Justin Erenkrantz wrote:
>> > > > James,
>> > > >
>> > > > Thanks for the bug report. For reference, the upstream OpenSSL
>> commit
>> > > looks to
>> > > > be:
>> > > >
>> > > > https://github.com/openssl/openssl/commit/
>> > > > d924dbf4ae127c68463bcbece04b6e06abc58928
>> > > >
>> > > > I strongly suspect that the patch on our side (against 1.3.x) is
>> > > something akin
>> > > > to below. I'm having trouble getting a test environment up with the
>> > > latest
>> > > > OpenSSL to reproduce, so if anyone can test in the interim, that'd
>> be
>> > > > appreciated.
>> > >
>> > > Latest Debian sid has everything ready to test, although you'll need
>> the
>> > > patch I'm carrying to build since SCons is using Python 3. That
>> reminds
>> > > me, I was supposed to send that to the list awhile back.
>> > >
>> > > > If not, I'll try again as time (and kiddo) permit.
>> > >
>> > > Unfortunately, that didn't have any effect.
>> > >
>> > > Cheers,
>> > > --
>> > > James
>> > > GPG Key: 4096R/91BF BF4D 6956 BD5D F7B7 2D23 DFE6 91AE 331B A3DB
>> > >
>>
>
