Hi Mateusz,

I'm CCing Emeric (SSL maintainer) and Olivier (who added early-data support),
and responding to some points below.

On Sat, Feb 10, 2018 at 06:26:42PM +0100, Mateusz Malek wrote:
> Hi everyone,
> 
> I've narrowed down my problem down to the same commit as Tomek Gacek -
> c2aae74f010f97a3415542fe649198a5d3be1ea8 (MEDIUM: ssl: Handle early data
> with OpenSSL 1.1.1), so I guess it may be related. In my case, since upgrade
> to 1.8, some responses from some backends (not sure what exactly triggers
> the bug) do not have their headers modified (despite http-response
> add-header and http-response del-header being set).
> 
> Applying patch part-by-part, I got to a point where it seems that that was
> caused by changes to ssl_sock_to_buf function in src/ssl_sock.c (lines
> 396-431):
> https://gist.github.com/mkwm/13dd32fe2b5ec21182f8a06a304228df#file-break-patch-L396-L431
> 
> Code at out_error label behave a bit differently from part removed in this
> commit - namely, it sets conn->flags |= CO_FL_ERROR unconditionally, while
> previously there was an additional check (skipping error flag setting if
> errno was set to EAGAIN). My problems went straight away when I've changed
> out_error to match old code.

Interesting, it would be possible that sometimes an SSL error remains on
the stack, and that it randomly gets trapped here. It used to happen in
the past and we went through a great pain trying to always clean all of
them.

I have no idea why this specific condition was removed. It's not mentionned
at all in Olivier's commit message and suspiciously looks like an accident.
Olivier, do you have an idea on this one ? It's commit c2aae74f. It may be
related to an issue Emeric was chasing recently where some server-side
connections would occasionally fail if my memory serves me right.

> There is also another issue with this commit - it seems that one "1" got
> lost in OPENSSL_VERSION_NUMBER comparison (line 267):
> https://gist.github.com/mkwm/13dd32fe2b5ec21182f8a06a304228df#file-break-patch-L267
>
> Throughout this commit all additions of similar ifdefs use 0x10101000L,
> which translates to OpenSSL 1.1.1 - and this one oddly translates to version
> 0.1.1.

Yes but this one was fixed in 1.8-rc3 by commit cfdef2e3 so it cannot be
this one. But the removed part above definitely is an interesting one.

> Hope this helps!

Sure it does!

Thanks!
Willy

Reply via email to