Bug#978353: serf: FTBFS: test_ssl_handshake fails with OpenSSL 1.1.1i

2021-01-18 Thread Justin Erenkrantz
On Fri, Jan 15, 2021 at 9:13 PM James McCoy  wrote:

> On Fri, Jan 15, 2021 at 08:36:22AM -0500, Justin Erenkrantz wrote:
> > Sadly, my Debian sid box ran into other issues and is currently
> inaccessible.
> >
> > I *think* that this would address the 1.3.x test issues, but 1.3.x
> doesn't
> > build on Mac OS for me for other reasons...so, let me know how it goes?
> =)  --
>
> Success!
>

Great! I'd recommend just picking that up as a local patch for now and
we'll work towards releasing a 1.4.0 with this and a bunch of other
goodness in the coming weeks/months.

>From a quick look at debian-devel-announce, it looks like the Bullseye
freeze has just started ; so, probably by the time the unstable window
opens again post-Bullseye, we'll be able to land a proper 1.4.0.

Cheers.  -- justin


Bug#978353: serf: FTBFS: test_ssl_handshake fails with OpenSSL 1.1.1i

2021-01-15 Thread Justin Erenkrantz
Sadly, my Debian sid box ran into other issues and is currently
inaccessible.

I *think* that this would address the 1.3.x test issues, but 1.3.x doesn't
build on Mac OS for me for other reasons...so, let me know how it goes?
=)  -- justin

Index: test/test_context.c
===
--- test/test_context.c (revision 1885525)
+++ test/test_context.c (working copy)
@@ -1138,7 +1138,7 @@

 /* We expect an error from the certificate validation function. */
 if (failures & expected_failures)
-return APR_SUCCESS;
+return APR_EGENERAL;
 else
 return SERF_ERROR_ISSUE_IN_TESTSUITE;
 }
@@ -1206,8 +1206,8 @@

 create_new_request(tb, _ctx[0], "GET", "/", 1);

-test_helper_run_requests_expect_ok(tc, tb, num_requests, handler_ctx,
-   test_pool);
+test_helper_run_requests_expect_fail(tc, tb, num_requests, handler_ctx,
+ test_pool);
 }

 /* Set up the ssl context with the CA and root CA certificates needed for
@@ -1774,8 +1774,8 @@

 create_new_request(tb, _ctx[0], "GET", "/", 1);

-test_helper_run_requests_expect_ok(tc, tb, num_requests, handler_ctx,
-   test_pool);
+test_helper_run_requests_expect_fail(tc, tb, num_requests, handler_ctx,
+ test_pool);
 }

 /* Validate that the expired certificate is reported as failure in the
@@ -1820,8 +1820,8 @@

 create_new_request(tb, _ctx[0], "GET", "/", 1);

-test_helper_run_requests_expect_ok(tc, tb, num_requests, handler_ctx,
-   test_pool);
+test_helper_run_requests_expect_fail(tc, tb, num_requests, handler_ctx,
+ test_pool);
 }


Index: test/test_serf.h
===
--- test/test_serf.h (revision 1885525)
+++ test/test_serf.h (working copy)
@@ -239,6 +239,12 @@
int num_requests,
handler_baton_t handler_ctx[],
apr_pool_t *pool);
+void
+test_helper_run_requests_expect_fail(CuTest *tc, test_baton_t *tb,
+ int num_requests,
+ handler_baton_t handler_ctx[],
+ apr_pool_t *pool);
+
 serf_bucket_t* accept_response(serf_request_t *request,
serf_bucket_t *stream,
void *acceptor_baton,
Index: test/test_util.c
===
--- test/test_util.c (revision 1885525)
+++ test/test_util.c (working copy)
@@ -461,6 +461,19 @@
 CuAssertIntEquals(tc, num_requests, tb->handled_requests->nelts);
 }

+void
+test_helper_run_requests_expect_fail(CuTest *tc, test_baton_t *tb,
+ int num_requests,
+ handler_baton_t handler_ctx[],
+ apr_pool_t *pool)
+{
+apr_status_t status;
+
+status = test_helper_run_requests_no_check(tc, tb, num_requests,
+   handler_ctx, pool);
+CuAssertIntEquals(tc, APR_EGENERAL, status);
+}
+
 serf_bucket_t* accept_response(serf_request_t *request,
serf_bucket_t *stream,
void *acceptor_baton,

On Thu, Jan 14, 2021 at 11:35 PM James McCoy  wrote:

> Happy New Year!
>
> On Wed, Dec 30, 2020 at 10:39:28PM -0500, James McCoy wrote:
> > On Tue, Dec 29, 2020 at 02:35:11PM -0500, Justin Erenkrantz wrote:
> > > The OpenSSL devs intended this to be a breaking change - but it's not
> > > documented anywhere.  Sigh.
> > >
> > > I've got a WIP patch against trunk that causes test_ssl to pass - see
> below.
> > > It also seems to work with OpenSSL 1.1.1h as well as OpenSSL 1.1.1i /
> > > 1.1.1-stable, AFAICT.
> > >
> > > James: can you please give it a try as well?
> >
> > Yes, I can confirm this fixes test_ssl_handshake on trunk.  There's
> > enough difference between trunk and branches/1.3.x that it doesn't apply
> > cleanly there.
>
> Any chance you would be able to make a patch for 1.3.x?  Although a 1.4
> release would be nice, it's a bit late in the Debian release cycle to
> upload a major new version.
>
> A targeted fix for the test suite would address the immediate issue,
> though.
>
> Cheers,
> --
> James
> GPG Key: 4096R/91BF BF4D 6956 BD5D F7B7  2D23 DFE6 91AE 331B A3DB
>


Bug#978353: serf: FTBFS: test_ssl_handshake fails with OpenSSL 1.1.1i

2020-12-29 Thread Justin Erenkrantz
", 1);

-run_client_and_mock_servers_loops_expect_ok(tc, tb, num_requests,
-handler_ctx, tb->pool);
+run_client_and_mock_servers_loops_expect_fail(tc, tb, num_requests,
+  handler_ctx, tb->pool);
 #endif /* OPENSSL_NO_TLSEXT */
 }

Index: test/test_util.c
===
--- test/test_util.c (revision 1884847)
+++ test/test_util.c (working copy)
@@ -561,6 +561,19 @@
 CuAssertIntEquals(tc, num_requests, tb->handled_requests->nelts);
 }

+void
+run_client_and_mock_servers_loops_expect_fail(CuTest *tc, test_baton_t *tb,
+  int num_requests,
+  handler_baton_t
handler_ctx[],
+  apr_pool_t *pool)
+{
+apr_status_t status;
+
+status = run_client_and_mock_servers_loops(tb, num_requests,
handler_ctx,
+   pool);
+CuAssertIntEquals_Msg(tc, serf_error_string(status), APR_EGENERAL,
status);
+}
+
 void setup_test_mock_server(test_baton_t *tb)
 {
 if (!tb->mh)/* TODO: move this to test_setup */




On Mon, Dec 28, 2020 at 5:22 PM Justin Erenkrantz 
wrote:

> On Mon, Dec 28, 2020 at 5:00 PM Justin Erenkrantz 
> wrote:
>
>> It's not clear to me if OpenSSL authors intended to make this breaking
>> change.  On the serf side, we would need to think through what it would
>> mean to have our app callback return false upon failure in order to
>> short-circuit the check.
>>
>> I probably won't get a chance to open an upstream OpenSSL issue today (or
>> even tomorrow)...
>>
>
> I found the original issue where they changed the behavior and added a
> comment there:
>
> https://github.com/openssl/openssl/issues/11297
>
> Cheers.  -- justin
>


Bug#978353: serf: FTBFS: test_ssl_handshake fails with OpenSSL 1.1.1i

2020-12-28 Thread Justin Erenkrantz
On Mon, Dec 28, 2020 at 5:00 PM Justin Erenkrantz 
wrote:

> It's not clear to me if OpenSSL authors intended to make this breaking
> change.  On the serf side, we would need to think through what it would
> mean to have our app callback return false upon failure in order to
> short-circuit the check.
>
> I probably won't get a chance to open an upstream OpenSSL issue today (or
> even tomorrow)...
>

I found the original issue where they changed the behavior and added a
comment there:

https://github.com/openssl/openssl/issues/11297

Cheers.  -- justin


Bug#978353: serf: FTBFS: test_ssl_handshake fails with OpenSSL 1.1.1i

2020-12-28 Thread Justin Erenkrantz
As an update, I've been able to triage this a bit further.

It's definitely that last noted change (erroring out on expired self-signed
root) that broke it.  OpenSSL 1.1.1{g,h} are fine, but {i,-stable} are
not.  Reverting just x509_vfy.c to what is in 1.1.1h causes the test to
pass.

In this test case, Serf receives 2 verify callbacks in test_ssl_handshake.
The first failing test case is to not have a known CA - so, we are
intentionally trying to trigger a verify failure.  One of the app
callback received has the expected failure, the other doesn't.  Serf
basically flags the second (success) as an unexpected pass.

2020-12-28T16:51:34.045142-05 test/test_ssl.c: Cert failure received: 4 ;
expected failure mask: 4

2020-12-28T16:51:34.045159-05 test/test_ssl.c: Cert failure received: 0 ;
expected failure mask: 4

The upstream issues/commits appear to be:

https://github.com/openssl/openssl/issues/13427
https://github.com/openssl/openssl/commit/3bed88a3970605a2ff817065f93b08e965d89e5f#diff-2a76d0a7ddc5ae2646a6c183270a7b4d5302d8491acb0af0dfbd70643efdf431

The key difference is almost certainly this change:

https://github.com/openssl/openssl/blob/OpenSSL_1_1_1h/crypto/x509/x509_vfy.c#L1754

---
return verify_cb_cert(ctx, xi, 0,

X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE);
---

https://github.com/openssl/openssl/blob/OpenSSL_1_1_1-stable/crypto/x509/x509_vfy.c#L1755

---
if (!verify_cb_cert(ctx, xi, 0,
X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE))
return 0;

xs = xi;
goto check_cert_time;
---

Up to 1.1.1h, OpenSSL would stop processing the certificate after sending
along the leaf error to the app callback.  In -stable (1.1.1i+ and master),
if the app's callback doesn't return a failure, it will then proceed to the
date check portion (check_cert_time) - which then receives a successful
verification callback.

It's not clear to me if OpenSSL authors intended to make this breaking
change.  On the serf side, we would need to think through what it would
mean to have our app callback return false upon failure in order to
short-circuit the check.

I probably won't get a chance to open an upstream OpenSSL issue today (or
even tomorrow)...

Cheers.  -- justin

Index: test/test_ssl.c

===

--- test/test_ssl.c (revision 1884847)

+++ test/test_ssl.c (working copy)

@@ -465,6 +465,7 @@



 tb->result_flags |= TEST_RESULT_SERVERCERTCB_CALLED;



+test__log(TEST_VERBOSE, __FILE__, "Cert failure received: %d ;
expected failure mask: %d\n", failures, expected_failures);

 /* We expect an error from the certificate validation function. */

 if (failures & expected_failures)

 return APR_SUCCESS;



On Sun, Dec 27, 2020 at 11:22 AM James McCoy  wrote:

> On Sun, Dec 27, 2020 at 10:46:24AM -0500, Justin Erenkrantz wrote:
> > Thanks.  I expect that this might be due to the last change - erroring
> out on
> > an expired self-signed root cert.  Though I thought we didn’t check in a
> root
> > cert for our test chain...could Debian’s packaging be including a cert
> for
> > testing?
>
> I use create_certs.py from trunk to re-generate the test certificates
> every build, otherwise I was running into time bombs with the certs
> expiring.  Other than that, I don't do anything different than the
> normal test process.
>
> The Debian packaging does have some local patches[0] applied to address
> issues that have been fixed upstream but not yet released.
>
> [0]: https://sources.debian.org/patches/serf/1.3.9-8/
>
> > I will try to take a look this week with Debian sid...I assume it has
> 1.1.1i
> > already?  — justin
>
> Yes, it does.
>
> Cheers,
> --
> James
> GPG Key: 4096R/91BF BF4D 6956 BD5D F7B7  2D23 DFE6 91AE 331B A3DB
>


Bug#978353: serf: FTBFS: test_ssl_handshake fails with OpenSSL 1.1.1i

2020-12-27 Thread Justin Erenkrantz
Thanks.  I expect that this might be due to the last change - erroring out
on an expired self-signed root cert.  Though I thought we didn’t check in a
root cert for our test chain...could Debian’s packaging be including a cert
for testing?

I will try to take a look this week with Debian sid...I assume it has
1.1.1i already?  — justin

On Sun, Dec 27, 2020 at 10:39 AM James McCoy  wrote:

> On Sat, Dec 26, 2020 at 11:09:41PM +0100, Lucas Nussbaum wrote:
> > Source: serf
> > Version: 1.3.9-8
> > [...]
> > > Trailer-Test: f
> > > ...F..
> > >
> > > There was 1 failure:
> > > 1) test_ssl_handshake: test/test_util.c:456: expected <0> but was
> <120199>
>
> It looks like the change from libssl1.1 version 1.1.1h to 1.1.1i
> regressed this test.
>
> The documented changes between these two releases are:
>
>  Changes between 1.1.1h and 1.1.1i [8 Dec 2020]
>
>   *) Fixed NULL pointer deref in the GENERAL_NAME_cmp function
>  This function could crash if both GENERAL_NAMEs contain an
> EDIPARTYNAME.
>  If an attacker can control both items being compared  then this could
> lead
>  to a possible denial of service attack. OpenSSL itself uses the
>  GENERAL_NAME_cmp function for two purposes:
>  1) Comparing CRL distribution point names between an available CRL
> and a
> CRL distribution point embedded in an X509 certificate
>  2) When verifying that a timestamp response token signer matches the
> timestamp authority name (exposed via the API functions
> TS_RESP_verify_response and TS_RESP_verify_token)
>  (CVE-2020-1971)
>  [Matt Caswell]
>
>   *) Add support for Apple Silicon M1 Macs with the darwin64-arm64-cc
> target.
>  [Stuart Carnie]
>
>   *) The security callback, which can be customised by application code,
> supports
>  the security operation SSL_SECOP_TMP_DH. This is defined to take an
> EVP_PKEY
>  in the "other" parameter. In most places this is what is passed. All
> these
>  places occur server side. However there was one client side call of
> this
>  security operation and it passed a DH object instead. This is
> incorrect
>  according to the definition of SSL_SECOP_TMP_DH, and is inconsistent
> with all
>  of the other locations. Therefore this client side call has been
> changed to
>  pass an EVP_PKEY instead.
>  [Matt Caswell]
>
>   *) In 1.1.1h, an expired trusted (root) certificate was not anymore
> rejected
>  when validating a certificate path. This check is restored in 1.1.1i.
>  [David von Oheimb]
>
> The full diff is at
> https://github.com/openssl/openssl/compare/OpenSSL_1_1_1h...OpenSSL_1_1_1i
>
> Cheers,
> --
> James
> GPG Key: 4096R/91BF BF4D 6956 BD5D F7B7  2D23 DFE6 91AE 331B A3DB
>


Bug#954698: serf: FTBFS: 1) test_ssltunnel_basic_auth_server_has_keepalive_off: test/test_context.c:2138: expected <0> but was <120199>

2020-04-02 Thread Justin Erenkrantz
On Thu, Apr 2, 2020 at 8:06 AM James McCoy  wrote:

> Just as an FYI, the OpenSSL changes have been partially reverted[1]
> upstream for the 1.1.1f release.  The change will be re-introduced in
> 3.0, so using the ifdef, instead of OpenSSL version, was the prudent
> choice.
>

Thanks for the update.  I'm guessing Debian Sid will just jump to OpenSSL
1.1.1f and we'll all pretend that 1.1.1e never happened.  =)

I think this takes some of the pressing nature to get a 1.3.10 out the
door.  That said, I'm trying to go down the rabbit hole of becoming
friendly with all of the cmake environments...it'll probably take me a fair
bit of time to get comfortable enough to cut a 1.4.x release that uses
cmake as the primary build system.  (I finally got my VS 2019 in a state
tonight where it can now build the dependency chains...)

Cheers.  -- justin


Bug#954698: serf: FTBFS: 1) test_ssltunnel_basic_auth_server_has_keepalive_off: test/test_context.c:2138: expected <0> but was <120199>

2020-03-30 Thread Justin Erenkrantz
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.0 -0400
+++ test/server/test_sslserver.c2020-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 
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  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>
>>

Bug#954698: serf: FTBFS: 1) test_ssltunnel_basic_auth_server_has_keepalive_off: test/test_context.c:2138: expected <0> but was <120199>

2020-03-28 Thread Justin Erenkrantz
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  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  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
> > >
>


Bug#954698: serf: FTBFS: 1) test_ssltunnel_basic_auth_server_has_keepalive_off: test/test_context.c:2138: expected <0> but was <120199>

2020-03-27 Thread Justin Erenkrantz
Ah, yes, I didn’t re-run the cert creation...so, that might be why those
tests are failing.  I just tried to run the cert creation script and it
seems that pyOpenSSL’s interfaces have changed.

But, the test that the Debian builder flagged didn’t fail...

So, I will start from your package and patches and work my way back towards
upstream.

Cheers.  — justin

On Fri, Mar 27, 2020 at 5:27 PM James McCoy  wrote:

> On Fri, Mar 27, 2020 at 05:15:24PM -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:
>
> I haven't tested what happens with 1.4.x, since there hasn't been a
> release yet.
>
> Are the test certs expired?  I automatically run
> test/certs/create_certs.py as part of the build process to ensure that
> doesn't happen.
>
> Cheers,
> --
> James
> GPG Key: 4096R/91BF BF4D 6956 BD5D F7B7  2D23 DFE6 91AE 331B A3DB
>


Bug#954698: serf: FTBFS: 1) test_ssltunnel_basic_auth_server_has_keepalive_off: test/test_context.c:2138: expected <0> but was <120199>

2020-03-27 Thread Justin Erenkrantz
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  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
>


Bug#954698: serf: FTBFS: 1) test_ssltunnel_basic_auth_server_has_keepalive_off: test/test_context.c:2138: expected <0> but was <120199>

2020-03-25 Thread Justin Erenkrantz
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.  If not, I'll try again as time (and kiddo) permit.

Cheers.  -- justin

Index: buckets/ssl_buckets.c

===

--- buckets/ssl_buckets.c (revision 1875631)

+++ buckets/ssl_buckets.c (working copy)

@@ -807,6 +807,11 @@

 if (ctx->pending_err) {

 status = ctx->pending_err;

 ctx->pending_err = 0;

+} else if (ctx->decrypt.status ==
SSL_R_UNEXPECTED_EOF_WHILE_READING) {

+serf__log(SSL_VERBOSE, __FILE__,

+  "ssl_decrypt: SSL read error: server"

+  " shut down connection!\n");

+status = APR_EOF;

 } else {

 ctx->fatal_err = status = SERF_ERROR_SSL_COMM_FAILED;

 }



On Mon, Mar 23, 2020 at 8:08 PM James McCoy  wrote:

> Looping in upstream:
>
> On Sun, Mar 22, 2020 at 02:57:54PM +0100, Lucas Nussbaum wrote:
> > Version: 1.3.9-8
>
> This is the same version of the serf package that's been in Debian since
> 2019/12/31, so something else seems to have changed.
>
> > [...]
> > During a rebuild of all packages in sid, your package failed to build
> > on amd64.
> >
> > Relevant part (hopefully):
> > > [...]
> > > Trailer-Test: f
> > > 140691743627136:error:14095126:SSL routines:ssl3_read_n:unexpected eof
> while reading:../ssl/record/rec_layer_s3.c:302:
> > > ..F...
> > >
> > > There was 1 failure:
> > > 1) test_ssltunnel_basic_auth_server_has_keepalive_off:
> test/test_context.c:2138: expected <0> but was <120199>
>
> Running a bisect against what's changed in the archive, shows that the
> test started failing when OpenSSL's version changed from 1.1.1d-2 to
> 1.1.1e-1.
>
> Looking at OpenSSL's changelog, it seems this was a change on their end
> that's affecting serf.
>
>  Changes between 1.1.1d and 1.1.1e [17 Mar 2020]
>   *) Properly detect EOF while reading in libssl. Previously if we hit an
> EOF
>  while reading in libssl then we would report an error back to the
>  application (SSL_ERROR_SYSCALL) but errno would be 0. We now add
>  an error to the stack (which means we instead return SSL_ERROR_SSL)
> and
>  therefore give a hint as to what went wrong.
>  [Matt Caswell]
>
> I guess serf needs to adapt to this change in behavior.
>
> Cheers,
> --
> James
> GPG Key: 4096R/91BF BF4D 6956 BD5D F7B7  2D23 DFE6 91AE 331B A3DB
>


Bug#372179: AC_CANONICAL_SYSTEM overwrites $@

2006-06-19 Thread Justin Erenkrantz

On 6/19/06, Tollef Fog Heen [EMAIL PROTECTED] wrote:

* Ralf Wildenhues

| Hello Justin, Tollef,
|
| * Justin Erenkrantz wrote on Sun, Jun 18, 2006 at 11:40:12PM CEST:
|  On 6/18/06, Tollef Fog Heen [EMAIL PROTECTED] wrote:
|  Currently, if configure is passed --sbindir=, it just overrides
|  $sbindir without flagging that at all.  If it, in addition to changing
|  $sbindir, it'd set ac_param_sbindir either to 1 to show that sbindir
|  is derviced from a command line argument or to the value passed on the
|  command line.  If we had this, APR_LAYOUT could check if
|  ac_param_sbindir was set, and if so skip setting sbindir from the
|  layout.
|  
|  Would this be an acceptable solution for both the APR and the autoconf
|  people?
| 
|  That could work for our purposes, yes.  It'd be cleaner to boot, too.
|
| So what do you do with
|   --bindir=/foo --with-layout=bar --sbindir=/baz
|
| shouldn't the bar layout override bindir but not sbindir?  (Note I don't
| know the exact semantics --with-layout is supposed to have.)

IMO, command line ordering shouldn't matter, unless you do
--bindir=/foo --bindir=/bar.  --with-layout just changes the defaults,
it doesn't override the state you're currently in.


Correct.  Anything explicitly provided on the command-line is
literally respected - the layout file just allows the user to change
autoconf's defaults in a straightforward manner.  -- justin


--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]



Bug#372179: AC_CANONICAL_SYSTEM overwrites $@

2006-06-18 Thread Justin Erenkrantz

On 6/18/06, Tollef Fog Heen [EMAIL PROTECTED] wrote:

Currently, if configure is passed --sbindir=, it just overrides
$sbindir without flagging that at all.  If it, in addition to changing
$sbindir, it'd set ac_param_sbindir either to 1 to show that sbindir
is derviced from a command line argument or to the value passed on the
command line.  If we had this, APR_LAYOUT could check if
ac_param_sbindir was set, and if so skip setting sbindir from the
layout.

Would this be an acceptable solution for both the APR and the autoconf
people?


That could work for our purposes, yes.  It'd be cleaner to boot, too.  -- justin


--
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of unsubscribe. Trouble? Contact [EMAIL PROTECTED]



Bug#372179: AC_CANONICAL_SYSTEM overwrites $@

2006-06-16 Thread Justin Erenkrantz

On 6/15/06, Paul Eggert [EMAIL PROTECTED] wrote:

Tollef Fog Heen [EMAIL PROTECTED] writes:

 Apart from the «ewww» factor, why can't it do its work in a subshell
 and echo back the parameters to be set and those get eval-ed by
 configure?

Yes, something like that might work, given that you know the values
can't contain ' (though you should probably check this).

It's unlikely that we'll buy the change back into Autoconf, though.


The point of our autoconf macro is to allow 'shortcuts', such that the argument

--with-layout=Foo

rewrites prefix/libexec/etc to a specific set of values (dictated by
our file config.layout) and then have explicit passed parameters
override those 'layout' files.

It looks like you switched the docs to recomend using AC_ARG_ENABLE,
but I'm not sure how that would address our issue.

We might be able to delay the AC_CANONICAL_SYSTEM invocation until
after we process the layout options - but I seem to recall a reason
why we delayed invoking those macros.

Thanks.  -- justin