maskit commented on PR #13761:
URL: https://github.com/apache/trafficserver/pull/13761#issuecomment-5935812930
I wanted to check whether this is working around an OpenSSL bug, since there
are a lot of `BIO_eof` issues and PRs in the OpenSSL repository. It isn't.
OpenSSL 4.0 changed this on purpose.
The change is OpenSSL commit `f17230ae6c` ("Fix of EOF and retry handling in
BIO implementations", OpenSSL PR 29401), first released in 4.0.0. Before it,
`BIO_CTRL_EOF` on a mem BIO returned `bm->length == 0` no matter what
eof-return was set. That disagreed with `mem_read()`. With
`BIO_set_mem_eof_return(b, -1)`, an empty BIO's read returns -1 and sets the
retry flag ("try again"), while `BIO_eof()` said end-of-file. Now `BIO_eof()`
reports EOF only if eof-return is 0, or if the BIO still has the default legacy
behaviour.
For compatibility, reviewers asked for an internal flag,
`BIO_FLAGS_MEM_LEGACY_EOF`. `mem_init()` sets it on every new R/W mem BIO, and
`BIO_set_mem_eof_return()` clears it. The updated `BIO_s_mem(3)` describes it:
> The default behaviour for read-only BIOs is as if
BIO_set_mem_eof_return(0) were called. The default behaviour for read-write
BIOs is special: BIO_eof() returns EOF for an empty buffer, while the
BIO_read() behaviour remains identical to the case BIO_set_mem_eof_return(-1).
This default behaviour is maintained for backward compatibility.
`BIO_ctrl(3)` now says:
> BIO_eof() returns 1 if the BIO has reached end-of-file as a result of the
most recent read operation. The precise meaning of "EOF" varies according to
the BIO type. The function reports the result of the previous read attempt and
does not update this state based on subsequent operations.
The flag doesn't help ATS, because we opt out of it. We create the handshake
rbio with `BIO_new_mem_buf()` and then call `BIO_set_mem_eof_return(rbio, -1)`.
That clears the flag, so on 4.x `BIO_eof()` always returns 0 for that BIO, and
`update_rbio()` never sees the buffer as used up. The new `test_eof` in
`test/membio_test.c` asserts exactly this: after `BIO_set_mem_eof_return(bio,
-1)`, `BIO_eof()` is false. The other recent `BIO_eof` reports are different
problems: OpenSSL issue 30348 and PR 30395 (NonStop `feof`), issue 32179 and PR
32382 (Windows file BIO), and issue 30545 and PR 30547 (socket EOF in
`SSL_read`).
So checking `BIO_ctrl_pending() == 0` is the right fix. What we want to know
is whether the buffer is empty, not whether the last read hit EOF. It works on
BoringSSL too. There, the mem BIO's `BIO_CTRL_EOF` and `BIO_CTRL_PENDING` both
read `b->length` and ignore eof-return. In both changed places the rbio is
always a mem BIO, because the only switch to a socket BIO is followed right
away by `free_handshake_buffers()`, which clears `handShakeReader`. BoringSSL
exports `BIO_eof()` as a function rather than a macro, so the removed `#ifndef
BIO_eof` fallback was defining a macro nothing needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
