Hello Ilya, hello Willy,
On Thu, 27 Jun 2019 at 14:04, Lukas Tribus <[email protected]> wrote: > > Hello, > > On Thu, 27 Jun 2019 at 13:19, Willy Tarreau <[email protected]> wrote: > > > > Hi guys, > > > > On Thu, Jun 27, 2019 at 03:55:54PM +0500, ???? ??????? wrote: > > > you are right, commit messages is not my best. > > > sorry about that > > > > > > anyway, if you think it is wrong, you are welcome to provide a fix > > > (sometimes, I do wrong things, I'm ok with that) > > > > Ilya, don't take it bad, bugs always happen and the problem with bugs > > is to detect them then to figure how to address them. > > Indeed, bugs will always happen, it doesn't matter who the author is, > and that was not my critique. My issue was merely with the amount of > changes and the description. > > I will provide some more technical context tomorrow. SSL_state() would return an int, not an enum, so the comparison would have to be a bitwise AND (but even OpenSSL got it wrong in their own server example code). Even fixing that, it does not work; neither OpenSSL nor LibreSSL recognizes the empty handshake. I assume this broke a long time ago in OpenSSL (before the forks). An alternative, even simpler call available in both OpenSSL and LibreSSL would be SSL_in_before(), but that's really just a: # define SSL_in_before(a) (SSL_state(a)&SSL_ST_BEFORE) so it fails just like it's parent function. When I say fail, I mean the comparison is never true and we do not recognize the empty handshake, which I simulated with and without proxy protocol like this: echo -ne "PROXY TCP4 172.30.1.153 172.30.1.153 3052 3443\r\n" | nc 127.0.0.1 443 echo -ne "" | nc 127.0.0.1 444 I did not find any way in LibreSSL to make it recognize an empty handshake, which is why I suggest we remove LibreSSL from the equation just like we did with BoringSSL. I will follow-up with an RFC patch shortly and I'd ask you, Ilya, to provide feedback. I have yet to verify a possible additional issue with empty handshake detection in 2.0 and -dev for OpenSSL 1.1.0 that came up during testing, but are unrelated to this specific change. Regards, Lukas
