BUG/MEDIUM: ssl: crt-list ciphers ignored with AWS-LC
Since AWS-LC 1.47.0, SSL_new() copies the cipher lists of the SSL_CTX into
the SSL object, and SSL_set_SSL_CTX() does not update this copy while the
server handshake uses it. The "ciphers" and "ciphersuites" options of a
crt-list line were thus silently ignored: a client could negotiate a cipher
excluded by the certificate as long as the bind line allowed it. The same
configuration built against OpenSSL rejects such a connection.
ssl_sock_switchctx_set_ciphers() now sets both lists again on the SSL
object after the context switch.
Below AWS-LC 1.46 both lists of an SSL object share the same field, so
setting the ciphersuites would erase the ciphers. AWSLC_API_VERSION cannot
tell these versions apart, it stays at 32 from 1.44 to 1.49.
ssl_sock_check_awslc_cipher_lists() therefore checks the behaviour once at
startup, and the ciphersuites are only set when the two lists are separate.
Those older versions do not need the fix anyway, their handshake reads the
lists of the SSL_CTX, which the switch already updated.
This adds the ssl_crt-list_ciphers reg-test for TLSv1.2 and the
ssl_crt-list_ciphersuites ones for TLSv1.3, over TCP and QUIC.
Thank you to @ygkat for having reported this issue in GH #3500
This should be backported as far as 3.0. Note the code is in ssl_sock.c
before 3.1.
SSL_new() copies the curves of the SSL_CTX into the SSL object, and
SSL_set_SSL_CTX() does not update this copy while the handshake uses it.
The "curves" and "ecdhe" options of a crt-list line were thus silently
ignored: a client could negotiate a curve excluded by the certificate as
long as the bind line allowed it.
ssl_sock_switchctx_set_curves() now sets them again on the SSL object
after the context switch, in the ClientHello callback used by OpenSSL and
AWS-LC. The value is picked in the same order as when the SSL_CTX is
prepared, so the curves of the bind line still win over the <ecdhe> of the
crt-list line.
The libraries without SSL_CLIENT_HELLO_CB are not covered. LibreSSL calls
the servername callback once the group is already chosen, so setting the
curves there has no effect on a TLS 1.3 handshake. wolfSSL has its own
callback, left untouched. The reg-tests skip both libraries.
This adds the ssl_crt-list_curves reg-tests for TCP and QUIC, and
ssl_crt-list_ecdhe for the "ecdhe" option. The latter is skipped in FIPS
mode, where the bind lines get a default curves list which wins over the
"ecdhe" of a crt-list line.
Thank you to @ygkat for having reported this issue in GH #3500
This should be backported to all stable versions. Note the code is in
ssl_sock.c before 3.1.
DOC: internals: Update the HTX API documentation for tunneled data
The HTX API documentation was updated to reflect the addition of the RAW_DATA
block type:
* The new block type was added in the list of supported types and in the
typical composition of an HTX message, the tunneled data being always
located after the end of the message.
* The section about the end of the message was rewritten. Tunneled data are
no longer stored in a separate buffer, they have their own block type. It
is now stated that testing the tail block is enough because a RAW_DATA
block may only be found after the end of the message. It is also stated
that the block carrying the flag may already have been consumed, an HTX
message being a partial and stateless representation of an HTTP message.
* htx_add_raw_data(), htx_add_data_type(), htx_add_data_type_atonce(),
htx_is_data_type() and htx_msg_ended() were documented, the last one
replacing htx_has_eom(). It is also stated that htx_add_last_data() is
dedicated to regular DATA blocks and that htx_defrag() merges consecutive
blocks of the same payload type.
MINOR: mux-h1: Only handle RAW_DATA blocks when emitting tunneled data
As expected, now the changes to properly handle the RAW_DATA blocs were
performed, the h1_make_tunnel() function is updated to only handle RAW_DATA
blocks.
MINOR: http-htx: Report the payload of RAW_DATA blocks in internal.htx_blk.data
The "internal.htx_blk.data" sample fetch only reported the payload of DATA
blocks. Because tunneled data are now stored in RAW_DATA blocks, and because
"internal.htx_blk.type" reports this new type, an empty string was returned
for such block while its type was properly displayed.
These sample fetches are debugging helpers. So both types are now handled,
htx_is_data_type() being used to test it.
MINOR: http-htx: Really test the EOM flag presence in internal.htx.has_eom
The "internal.htx.has_eom" sample fetch is documented to return true if the
HTX message contains the end-of-message flag. It relied on htx_msg_ended()
to do so. But this function now also reports the end of the message when the
block carrying the flag was already consumed, which is the case when the
tail block is a RAW_DATA block.
Because this sample fetch is a debugging helper, it must really answer the
question it is documented for. So the HTX message is now scanned, looking for
a block carrying the flag. Note the scan is performed from the head on
purpose. This way, no assumption is made about the position of the block
carrying the flag and an unexpected one would be reported.
The documentation was updated to mention the flag may already have been
consumed, in which case false is returned while the end of the message was
already reached.
Revert "MEDIUM: mux-h2: Don't mix HTTP and tunneled data in the same buffer"
This reverts commit b6433eb62 (the rename of htx_has_eom() into
htx_msg_ended() was taken into account).
Because the end of the message is carried by the tail block of the HTX
message, HTTP data and tunneled data could not be stored in the same
buffer. Adding tunneled data after the block carrying the EOM flag would
have hidden the end of the message. So the H2 multiplexer had to store them
in a separate buffer and to delay their transfer to the upper layer until
the HTTP part was fully consumed.
Tunneled data are now stored in RAW_DATA blocks. They can no longer be
confused with HTTP data and htx_msg_ended() is able to report the end of the
message when the tail block is such block. So this workaround is no longer
necessary. The original behavior is restored: only one rxbuf is used and the
transfer of the tunneled data is no longer delayed.
MEDIUM: mux-h2: Use RAW_DATA blocks to store tunneled data
For a tunneled H2 stream, the end of the HTTP message is reported just after
the headers, when H2_SF_BODY_TUNNEL flag is set. So any DATA frame received
after that carries tunneled data and must be stored in a RAW_DATA block.
h2_frt_transfer_data() was updated accordingly.
Thanks to this change, HTTP data and tunneled data are no longer mixed on
the H2 side. The early end of the message is thus preserved when tunneled
data are received, and the next commit is able to remove the workaround that
was necessary to keep both in separate buffers.
MEDIUM: mux-h1: Use RAW_DATA blocks to store tunneled data
When a H1 stream is switched in tunnel mode, the transfer length is unknown
and the data are read till the shutdown. This is exactly the same code path
than a message without any transfer length information. So the block type to
use is chosen depending on the parser state: in H1_MSG_TUNNEL state, the
data are tunneled data and a RAW_DATA block is used. Otherwise it is still an
HTTP payload and a DATA block is used.
h1_copy_msg_data() takes the block type as extra argument for that purpose.
Thanks to this change, HTTP data and tunneled data are no longer mixed on
the H1 side. It also means the end of the message, reported just after the
headers for a tunneled stream, is now preserved when tunneled data are
received.
MINOR: mux-h2: Handle RAW_DATA blocks when sending data
Tunneled data are now stored in RAW_DATA blocks. The H2 multiplexer must be
able to send them, to be able to forward a tunnel established on the other
side.
For the H2 multiplexer, there is no difference with HTTP data: both are sent
in DATA frames. Only the ability to handle the new type is added here. No
producer emits such block yet.
MINOR: mux-h1: Handle RAW_DATA blocks when sending data
Tunneled data are now stored in RAW_DATA blocks. The H1 multiplexer must be
able to send them. h1_make_tunnel() is responsible to do so.
For the H1 multiplexer, there is no difference with HTTP data: both are sent
verbatim. Only the ability to handle the new type is added here. No producer
emits such block yet so h1_make_tunnel() relies on htx_is_data_type() for
now. But once all changes done, this will have to be modified to accept
RAW_DATA blocks only.
MEDIUM: htx: Allow tunneled data to be added in a message already ended
For a tunneled stream, the end of the HTTP message is reported just after
the headers. It is an "early" EOM: from the HTTP point of view the message
is finished and only tunneled data may be exchanged then. But nothing may be
added in a message already ended, otherwise the end of the message would no
longer be reported on the tail block.
Now tunneled data have their own block type, they can be added in a message
already ended without breaking this rule. htx_add_raw_data() was added for
that purpose. It is the only function able to add a block in such message,
and the corresponding BUG_ON() were relaxed accordingly.
htx_msg_ended() was updated to also report the end of the message when the
tail block is a RAW_DATA block. Indeed, tunneled data may only be found
after the end of the message. So, if there are some, the end of the message
was necessarily seen. Either the block carrying the flag is still in the HTX
message, or it was already consumed. In this last case, it is the consumer
responsibility to remember it.
Finally, the test on the end of the message was removed from htx_xfer()
because blocks are transferred one by one, using the safe functions. So
invalid additions are detected there, with a better accuracy.
MINOR: htx: Handle RAW_DATA blocks like DATA blocks in the HTX API
From the HTX API point of view, a RAW_DATA block is exactly the same as a
DATA block. It carries a payload that may be partially consumed. So the
functions dealing with this property must handle both types the same way:
* htx_truncate() must be able to truncate such block
* htx_drain() must be able to partially drain it
* htx_xfer() must be able to partially transfer it and must of course take
care to preserve its type
There is no change of behavior because no producer emits RAW_DATA blocks
yet.
MINOR: htx: Add the block type as argument of the data insertion functions
htx_add_data() was renamed into htx_add_data_type() and takes the block type
as extra argument. htx_add_data() is now a thin wrapper on it, passing
HTX_BLK_DATA. The tests on the tail block type were changed accordingly to
be performed against the type of the block to add, instead of hardcoding
HTX_BLK_DATA.
The same was done for the "atonce" version. __htx_add_data_atonce() was
renamed into __htx_add_data_type_atonce() and also takes the block type as
extra argument. A public and guarded version, htx_add_data_type_atonce(), was
added on top of it and htx_add_data_atonce() is now a thin wrapper on this
one, passing HTX_BLK_DATA. Note htx_add_last_data() still relies on the
private and unguarded version because it must be able to insert a block after
the one carrying the end of the message, before moving it at the right
place. It remains dedicated to regular DATA blocks.
Finally, htx_defrag() was updated to pass the block type. This way, RAW_DATA
blocks are merged when a message is defragmented, exactly like DATA blocks.
Otherwise they would be copied one by one, keeping a block per received
chunk.
This way, RAW_DATA blocks will be able to be inserted using the same
functions, and thus the same logic to append data to the tail block or to
deal with the free space in the message.
There is no change of behavior. Only HTTP data can be added for now.
An HTX message is a partial and stateless representation of an HTTP
message. Tunneled data are not part of the HTTP message. However, for now,
they are stored in DATA blocks, exactly like the HTTP payload. So there is
no way to make the difference between both.
A dedicated block type, HTX_BLK_RAW_DATA, is added to fix this. For now, no
producer emits such block. Only the HTX API is able to handle it. From this
point of view, a RAW_DATA block is exactly the same as a DATA block. Both
carry a payload that may be partially consumed. htx_is_data_type() function
was added to test this property on a block type.
It is important to add the new type in htx_get_blk_value() and
htx_set_blk_value_len() switches. Both have a default case silently doing
nothing. So a missing entry would lead to an empty payload on one side and
to a size never updated on the other side.
MINOR: htx: Rename htx_has_eom() into htx_msg_ended()
htx_has_eom() name suggests the HTX message contains a block carrying the
HTX_BLK_FL_EOM flag. It is true for now, because the flag can only be found
on the tail block. But this will change. The block carrying this flag may be
consumed while the end of the message must still be reported. So the name is
misleading and htx_msg_ended() is more accurate: it states the end of the
message was reached, regardless of the presence of the flag in the message.
Only a rename, there is no change of behavior. Note the sample fetch
"internal.htx.has_eom" was not renamed because it is part of the
configuration.
The HTX API documentation was not up-to-date anymore. It still described the
end of the message as a flag on the HTX message itself and said nothing
about the flags on the HTX blocks. Let's update it to reflect the recent
changes:
* A block's metadata is now composed of 3 fields. The flags field was
added and the address field is now a 28 bits field, limiting the
payloads part to 256 MB.
* The end of the message is now marked by the HTX_BLK_FL_EOM flag, set on
the last block of the message. htx_set_eom() and htx_has_eom() must be
used to set it and to test it. Because the flag is carried by the tail
block, nothing must be added in a message already ended. Only headers
and trailers may still be added because they are moved at the right
place, before the corresponding end-of marker.
* htx_get_first_blk(), htx_get_prev_blk() and htx_get_next_blk() now skip
the unused blocks and htx_remove_blk() never leaves an unused block on
the head or on the tail of the message. So there is no longer any need
to test the block's type against HTX_BLK_UNUSED when looping on an HTX
message.
* htx_is_unique_blk() was removed.
* The restrictions on htx_xfer() and htx_append_msg() were added and
htx_skip_msg_payload(), which was not documented at all, was added.
In addition, few pre-existing errors were fixed. htx_move_blk_before() moves
a block before the pivot and not after it. The function used to add a
trailer is htx_add_trailer() and not htx_add_trailers(). And a duplicated
paragraph about htx_remove_blk() was removed.
MEDIUM: htx: Remove usless htx_is_unique_blk() function
Tagged as MEDIUM while it is only a code cleanup, but this function may
still be used by external code. It is an API change.
This function is no longer used but it is also useless. Indeed, thanks to
recent changes, it is not possible to have unused HTX blocks on head and
tail of the message. So, htx_is_unique_blk() can be replaced by a test on
the number of blocks in the message.
In addition, this function was almost always used to know if the EOM flag on
the message can be processed. Now, the info is on the block itself. So no
reason to still use this function. It should disappear naturally with
modifications to handle all recent changes.
MEDIUM: htx: Harden HTX API to avoid invalid uses when EOM was reached
With recent changes, the info about the end of the message is now carried by
a block only. In addition, it is really important to never mix HTTP blocks
with tunneled data because we must be able at any time to test the tail
block to know if the EOM flag is set or not. To avoid any invalid uses, the
API was reinforced. BUG_ONs were added to trigger errors when such uses are
detected.
Note it should not be a huge change because mixing http data and tunneled
data was already not supported.
MEDIUM: mux-h2: Don't mix HTTP and tunneled data in the same buffer
For a tunneled stream, the end of the HTTP message is reported just after
the headers while the tunneled data are received after. Because the
information about the end of the message was a flag on the HTX message
itself, both could be stored in the same buffer without any issue.
This is no longer true. The end of the message is now carried by the tail
block of the HTX message. So nothing must be added after this block,
otherwise the message would no longer be seen as ended. It means HTTP data
and tunneled data must never be mixed in the same buffer.
Two changes are required for that:
* h2s_may_append_to_rxbuf() now refuses to append data to an rxbuf
carrying a message already ended. h2_frt_transfer_data() thus gets an
extra rxbuf to store the tunneled data. If no buffer is available, the
demux is blocked, as usual.
* h2_rcv_buf() no longer transfers an rxbuf to the upper layer when the
HTX message in the channel buffer was already ended. SE_FL_RCV_MORE and
SE_FL_WANT_ROOM are reported instead, so the transfer is retried once
the HTTP part was consumed.
Note the H1 multiplexer does not have this issue because the demux is
blocked, thanks to H1S_F_RX_BLK flag, until the tunnel is established.
This patch is a prerequisite to be able to harden the HTX API and forbid
any addition in a message already ended.
This flag can now be removed because it is no longer used. This commit is
pretty light but it is marked as MAJOR because it is an API change. All
external code relying on this flag will have to be updated.
MAJOR: tree-wide: No longer use HTX_FL_HAS_EOM to detect end of message
HTX_FL_HAS_EOM flag on the HTX message is no longer used to mark or detect
the end of the message. htx_has_eom() function is used to detect it and
htx_set_eom() is used to set it (except inside the HTX API). Thus we now
rely on flags on the tail block to deal with the end of the message.
Most of time, the change is quite light and easy to review. However there
are 3 parts no so obvious:
* The decompression filter: DECOMP_STATE_EOM_SEEN flag is now set only when
a block is removed. It is the only way to loose the EOM. This part was
placed in a dedicated function to not duplicate the code. At the end,
before exiting and when the decompression is finished, the EOM flag is
added on the tail block if necessary.
* The compression filter: As for the decompression filter, the end of
message is only evaluated when an HTX block is removed. It is the only
way to loose it. And we try to restore it only on the last DATA block.
* The lua filter API: The internal function used to remove part of the
payload was updated to be sure to now loos the EOM on the tail block if
it is removed.
In the HTX API, HTX_FL_HAS_EOM flag is now longet set.
MINOR: htx: Add function to know if the tail block carry EOM flag
htx_has_eom() function can now be used to know if the tail block of a
message carry the HTX_BLK_FL_EOM flag, so to know if the current HTX message
contains the end of the message.
MINOR: hlua: Use flags of HTX blocks to detect EOM from an HTTP applet
When we try to get request's data from an HTTP applet, instead of testing
HTX_FL_HAS_EOM flag on the message, we now rely on the flags of HTX blocks.
The logic remains the same, when all DATA blocks were consumed, we consider
the request as fully received. It may be when the blocks with the EOM flag
was removed or because a trailer was seen.
MINOR: stats-html: Detect a complete request by testing flags on the tail bloc
In the HTTP stats applet, when a POST is received, instead of testing the
HTX_FL_HAS_EOM flag on the message itself, we are now relying on the tail
bloc to know if the end of the request was received or not, by testing its
flags.
MEDIUM: h3: Use flags of HTX blocks to detect end of message during sending
When a message is sent, the end of the message is now detected by testing
flags of each HTX blocs. To do the API was slightly changed. The "fin" flag
is not longer set in the main loop in h3_snd_buf() function. Instead, the
flag is set in functions consumming the HTX blocks (h3_req_headers_send,
h3_resp_headers_send...).
H3_SF_SENT_INTERIM flag is no longer tested. An interim message cannot have
EOM set. A BUG_ON() may possibly be added. In addition, the HTX message is
not check for emptiness. Here again a BUG_ON() may be added.
MINOR: mux-fcgi: Use flags of HTX blocks to detect end of message during sending
When a message is sent, the end of the message is now detected by testing
flags of each HTX blocs. Otherwise, the logic remains mostly unchanged. The
way the end of the message is handled in fcgi_strm_send_stdin() was modified
a bit but the result is the same. Now, we rely on a variable to decide, at
the end, if an empty STDIN record should be sent or not. This avoid an extra
test on the HTX message to be sure it is empty.
MINOR: mux-h1: Use flags of HTX blocks to detect end of message during sending
When a message is sent, the end of the message is now detected by testing
flags of each HTX blocs. Otherwise, the logic remains mostly unchanged. A
test in h1_make_eoh() was slightly changed but it should be harmless.
Especially because HTX_BLK_FL_EOM cannot be set on the EOH block if it is
followed by something else than tunneled data, which is unexpected and
invalid in that case. All conditions to establish a tunnel at this stage
were already evaluated. And if any, the error will be detected in
h1_make_trailers() function.
MINOR: mux-h2: Use flags of HTX blocks to detect end of message during sending
When a message is sent, the end of the message is now detected by testing
flags of each HTX blocs. Otherwise, most of the logic remains unchanged. The
only difference is the detection of the end of the message when headers are
sent (back or front). Now, it is be possible to detect the EOM when the
stream is already closed and H2_SF_MORE_HTX_DATA may be removed while it was
untouched before. But it is not a big deal because this flag is only tested
when a shutdown is performed on the stream. So removing it for an already
closed stream is a no-op.
MINOR: fcgi-app: Stop on last HTX block when getting the payload size
In the loop counting all DATA block to be able to add a "Content-length:"
header in the FCGI response, we now check flags on the HTX block to know if
it is the last one of the message or not. If HTX_BLK_FL_EOM flag is set, it
means we can stop.
MEDIUM: htx: Add HTX_BLK_FL_EOM flag and set it on last block of the message
HTX_BLK_FL_EOM flag was introduced to be set on the last block of an HTX
message. It must be set on the tail block when the HTX_FL_HAS_EOM is set.
Now, only the HTX API must set these flags by hand. From outside of the HTX
API, htx_set_eom() function must be used.
A BUG_ON() was added to unsure the flag is added on valid blocks. It is possible
to add it on EOH, EOT and DATA blocks only.
It is just a flag renaming but it is an API breakage and this could generate
compilation errors for all external modules relying on the HTX API. This
change was performed to force to review all code using HTX to be sure this
flag is never set by hand from outside. Indeed, htx_set_eom() function must
always be used now.
MAJOR: htx: Use htx_set_eom() instead of setting HTX_FL_EOM by hand
The function htx_set_eom() takes care to the HTX_FL_EOM on the HTX message,
by first adding a EOT block if the message is empty. So let's use it
wherever possible.
This commit is tagged as MAJOR because it is an API change.
In the commit 32b51cdf3 ("BUG/MINOR: config: reject configs using HTTP with
bufsize >= 256 MB"), the maximum possible size for buffers is limited to 256
MB. So, on HTX side, it means a bloc cannot have a relative address higher
than this value. So on the 32-bits integer used to store the relative block
address, only 28 bits can be used, and 4 bits remains unused.
So these 4 bits were reused to store flags on block themselves. In this
patch, the support for these flags is introduced and the HTX API was adapted
to deal with it.
MEDIUM: htx: Insert headers and trailers before corresponding end-of block
htx_add_header() and htx_add_trailer() were updated to automatically insert
the block at the right place, just before the corresponding end-of block.
Indeed, it is invalid, unexepected and probably unsupported to insert a
header after the EOH block without moving it. So it is easier to handle it
in HTX function directly.
MEDIUM: htx/tree-wide: Remove tests on HTX_BLK_UNUSED when possible
Thanks to previous changes, it is possible to remove all tests on
HTX_BLK_UNUSED outside of the HTX API. Indeed, because htx_get_*_blk() and
htx_remove_blk() functions are now skipping automatically unused blocks, we
are sure to never get such blocks from outside.
MEDIUM: htx: Skip UNUSED blocs when getting previous,next and first blocks
htx_get_prev_blk(), htx_get_next_blk() and htx_get_first_blk() functions
were modified to automatically skip UNUSED blocks. So now, a valid block or
NULL is returned. In addition, htx_remove_blk() was adapted accordingly to
also skip UNUSED blocks.
Thanks to this change, the loops on HTX messages could be simplified by
removing test on UNUSED blocks, while it remains harmless.
BUG/MINOR: mux-h2: Reject trailers received on a tunneled stream
For a tunneled stream, the end of the HTTP message is reported just after
the headers. It is an "early" EOM: from the HTTP point of view, the message
is finished and only tunneled data may be exchanged then.
However, a HEADERS frame received after the initial one on such a stream
was still handled as a trailers block. The trailers were thus pushed in an
HTTP message already ended, leading to an invalid HTX message where blocks
are inserted after the end of the message. The stream is then aborted when
the outgoing multiplexer handles the unexpected block. So a client is able
to kill its own tunnel by sending a bogus HEADERS frame.
Such a frame is now rejected with a stream error (PROTOCOL_ERROR). The test
is performed in h2c_dec_hdrs(), at the only place where a HEADERS frame is
identified as a trailers block. This way, both directions are covered. The
aborted tunnel attempts are also covered because the early EOM was reported
in that case too.
This patch should be backported to all stable versions. Note that
h2c_report_glitch() only exists since 3.0. The corresponding line must be
removed for older versions.
MINOR: htx: Add macros for the header/trailer name and value max lengths
The maximum length of the name and the value of a header or a trailer were
hardcoded in the different places where they are tested. Let's define
HTX_HDR_NAME_MAX_LEN and HTX_HDR_VALUE_MAX_LEN to avoid these magic values
and document why these limits exist.
BUG/MAJOR: htx: Check the header/trailer length limits when one is updated
The name and the value lengths of a header or a trailer are respectively
encoded on 8 and 20 bits in the block descriptor. So the name must not
exceed 255 bytes and the value must not exceed 1048575 bytes. This is
properly tested when a header or a trailer is added, since commit 3b69886f7
("BUG/MAJOR: htx: fix missing header name length check in
htx_add_header/trailer"). But no test was performed when one is updated.
htx_replace_blk_value(), used to update a part of a header or a trailer
value, may grow it above the limit. And htx_replace_header(), used to
replace a full header, does not check its arguments at all. In both cases,
the length silently overflows on the block type, changing it. For instance,
a HDR block becomes an EOH block while its value length wraps to a small
value.
Only the value length limit is reachable from the configuration, via a
"replace-header" or a "replace-value" rule, and only if "tune.bufsize" was
set above 1MB, because the block must first fit in the buffer. In that case,
a client sending a large enough header is enough to hit the issue.
So, perform the same tests than those performed when a header or a trailer
is added, instead of corrupting the message. In addition, a BUG_ON() was
added in htx_change_blk_value_len() and htx_set_blk_value_len() to catch any
other caller missing the test.
Reported by: Adam Crawford
This patch must be backported to all stable versions.
BUG/MINOR: tools: Don't try to anonymize a NULL string
hash_anon() function is used to anonymize a string. It is called at several
places (via HA_ANON_* macros), especially from debugging functions. It is
hard to be sure the string to hash is always defined. So let's return
immediately if it is NULL.
Among other things, this fix avoids to get a crash when the server name
created by a LUA CO-SOCKET is anonymized. Indeed, this server is created
with no name.
This patch should fix the issue #3491. It must be backported as far as 2.8.
BUG/MEDIUM: htx: Fix the position returned by htx_defrag()
In htx_defrag(), the <new> counter was used to compute the new position of
the start-line and of the referenced block, if any. But this counter was
incremented for each block of the source message while consecutive DATA
blocks may be merged when they are copied. In this case, no new block is
created in the temporary message, so <new> was desynchronized from the real
position.
As a consequence, htx_defrag() could return a block located after the tail
of the message, thus pointing on the payload part instead of a block
descriptor. The caller then used it to copy data, at a pseudo-random offset,
corrupting the message.
htx_replace_blk_value() is the only concerned caller because it is the only
one referencing a block that may be a DATA block. htx_replace_header() and
htx_replace_stline() reference a header or a start-line, always located
before any DATA block. So, to trigger the bug, an expansion requiring a
defragmentation must be performed on a DATA block preceded by at least two
mergeable DATA blocks.
The positions are now computed from the block really created or updated. The
<new> counter is thus no longer necessary and was removed.
The flaw exists since the HTX was introduced. However the only known way to
expand a DATA block this way is the decompression filter, added in 3.5-dev.
So no released version should be affected. This patch may be backported to
all stable versions as a precaution.
BUG/MINOR: ssl/ocsp: Report an error when an empty OCSP response is received
When the response of the OCSP update request has no payload, no OCSP
response is loaded but a successful update is reported anyway. The number of
successes is incremented, the last update date is refreshed and the entry is
reinserted in the update tree with the regular delay, as if everything was
fine.
The "Content-Type" header of the response is already checked to be
"application/ocsp-response". So an empty payload is an error. It is now
reported as an OCSP response check failure.
This patch depends on:
* "MINOR: ssl/ocsp: Remove the useless res_payload callback of the http-client".
MINOR: ssl/ocsp: Remove the useless res_payload callback of the http-client
The OCSP update task does not process the response payload on the fly. The
whole payload is accumulated in the http-client response buffer and only
retrieved when the response is fully received, on the res_end callback. So
there is no reason to define a res_payload callback. It was only used to set
the HC_OCSP_RES_BODY flag, to know if a payload was received or not. Testing
the content of the response buffer is enough to do so.
The res_payload callback and the HC_OCSP_RES_BODY flag are thus removed.
This way, the OCSP update task now works the same way than the ACME one.
BUG/MEDIUM: http-client/cli: Notify the http-client when the response is consumed
In HTX mode, the "httpclient" CLI command does not use httpclient_res_xfer()
to consume the response. It dumps the HTX message stored in the http-client
response buffer and resets it by hand.
Thus, since commit b24260ec9 ("BUG/MEDIUM: http-client: Only consume input
buffer when hc one is empty"), the applet stops to consume data and is never
woken up again. The command is stuck just after the response headers, till
the server timeout expiration.
httpclient_res_consumed() is now called when the response buffer is reset.
This patch depends on :
* MINOR: http-client: Add a function to notify some data were consumed
It must be backported as far as 3.3 with commit b24260ec9.
MINOR: http-client: Add a function to notify some data were consumed
Since commit b24260ec9 ("BUG/MEDIUM: http-client: Only consume input buffer
when hc one is empty"), the http-client applet stops to consume data as soon
as its response buffer is not empty. Only httpclient_res_xfer() is able to
unblock it. However, some callers are directly accessing the httpclient
response buffer. These ones have no way to notify the applet it can fill
the response buffer again.
httpclient_res_consumed() is added to do so and is now called from
httpclient_res_xfer(). In addition, the applet is no longer only woken up
when the response buffer is emptied, but as soon as some room was released.
Because the response buffer is no longer necessarily released when data are
consumed, the I/O handler must now rely on b_data() instead of b_is_null()
to decide to stop the reception. Otherwise, an emptied but still allocated
buffer would block the applet for good.
This patch is required to fix a bug and must be backported as far as 3.3
with the next commit.
BUG/MEDIUM: acme: Set HTTPCLIENT_O_RES_ACCUM option on the http-client
The ACME task does not consume the response payload on the fly. It waits for
the end of the response to parse the JSON object stored in the http-client
response buffer.
Thus, since commit b24260ec9 ("BUG/MEDIUM: http-client: Only consume input
buffer when hc one is empty"), the http-client applet stops to consume data
as soon as the first part of the payload is stored in its response buffer,
waiting for the caller to consume it. But it never happens. So, when the
response is not received in one time, the ACME task is stuck till the server
timeout expiration.
The HTTPCLIENT_O_RES_ACCUM option is now set to instruct the http-client to
continue to fill its response buffer.
This patch depends on :
* BUG/MEDIUM: http-client: Don't use the httpclient context if it was released
* MINOR: http-client: Add an option to not stop the reception of the response
It must be backported as far as 3.3 with commit b24260ec9.
BUG/MEDIUM: ssl/ocsp: Set HTTPCLIENT_O_RES_ACCUM option on the http-client
The OCSP update task does not consume the response payload on the fly. It
waits for the end of the response to check and load the OCSP response stored
in the http-client response buffer.
Thus, since commit b24260ec9 ("BUG/MEDIUM: http-client: Only consume input
buffer when hc one is empty"), the http-client applet stops to consume data
as soon as the first part of the payload is stored in its response buffer,
waiting for the caller to consume it. But it never happens. So, when the
OCSP response is not received in one time, the update is stuck till the
server timeout expiration and always fails.
The HTTPCLIENT_O_RES_ACCUM option is now set to instruct the http-client to
continue to fill its response buffer.
This patch depends on :
* BUG/MEDIUM: http-client: Don't use the httpclient context if it was released
* MINOR: http-client: Add an option to not stop the reception of the response
It should fix issue #3498. It must be backported as far as 3.3 with commit b24260ec9.
MINOR: http-client: Add an option to not stop the reception of the response
Since commit b24260ec9 ("BUG/MEDIUM: http-client: Only consume input buffer
when hc one is empty"), the http-client applet stops to consume data as soon
as its response buffer is not empty. The applet is only unblocked when the
caller consumes these data by calling httpclient_res_xfer().
However, some callers are not consuming the response payload on the
fly. They are waiting for the end of the response to retrieve the whole
payload from the httpclient response buffer. For these callers,
httpclient_res_xfer() is never called. So the applet remains blocked and the
response is never fully received. The transfer is stuck till the server
timeout expiration.
To fix this, the HTTPCLIENT_O_RES_ACCUM option is added. It must be set by
the callers accumulating the whole response payload in the httpclient
response buffer. In that case, the applet must not stop the reception. Of
course, it means the whole payload must fit in the response buffer for these
callers. But it was already the case.
This patch depends on:
* "BUG/MEDIUM: http-client: Don't use the httpclient context if it was released".
It is required to fix a bug and must be backported as far as 3.3 with the
next commits.
BUG/MEDIUM: http-client: Don't use the httpclient context if it was released
The I/O handler of the http-client applet may be called after the applet
release. In this case, the httpclient context is no longer available and the
I/O handler must not use it. It is properly handled at the beginning of the
function. However, the code added at the end, on the "out" label, to stop
the data reception is evaluated in all cases, including when the httpclient
context is NULL. It leads to a NULL-pointer dereference.
So, now, <hc> is tested first.
This patch must be backported as far as 3.3 with commit b24260ec9
("BUG/MEDIUM: http-client: Only consume input buffer when hc one is empty").
BUG/MEDIUM: task: Do not destroy a task that is not ours in task_destroy
In task_destroy(), if the task is not exclusively ours, then just use
task_kill(). If it is a shared task, then there is a race condition if
another thread tries to schedule it at the same time, which could result
in the task being scheduled after been freed.
Released version 3.5-dev7 with the following main changes :
- BUG/MINOR: quic: leak of the NEW_TOKEN frame on token generation failure
- MINOR: quic: implement the AEAD integrity limit
- DOC: config: clarify tune tune.bufsize doc regarding various limits
- BUG/MEDIUM: hlua: Never release a cosocket applet from the GC
- BUG/MINOR: mux-h1: Return an error on trailers if last chunk is unfinished
- BUG/MINOR: hlua: Always return nil if there is no data to dup for HTTP messages
- BUG/MEDIUM: htx: Don't reset flags when source must be preserved during transfer
- BUG/MINOR: flt-http-comp: Rely on htx_set_eom() to restore EOM flag
- MINOR: httpclient: Don't manually set HTX_FL_EOM flag after calling htx_xfer()
- MINOR: htx: Remove htx_xfer_blks function
- CLEANUP: htx: Remove ref to EOM block on HTX_FL_EOM flag comment
- DOC: htx: Fix comment of htx_expect_more() function
- REORG: htx: Move functions to deal with first block after ones about next block
- MINOR: mux-h1: loop on HTX blocks using _blk() functions to look for WS key
- BUG/MINOR: stream/debug: harden the stream dump function regarding signals
- MINOR: stconn: prefer to replace endpoints rather than detaching them
- BUILD: fd: unshare is not available on NetBSD
- MINOR: ring: add "-r" to "show events" to dump in raw mode (undoes "-s")
- MINOR: ring: permit to combine options on "show events"
- MINOR: ring/cli: improve the "show events" parser regarding options
- MINOR: ring/cli: also support "show events -h" to display the help
- MINOR: http: define new HTTP status codes for parsers
- MINOR: http-htx: add a new generic trailers parser
- REGTESTS: http-messaging: add tests for the H2 and H3 trailers
- MEDIUM: mux-h2: rely on the generic trailers parser
- MINOR: mux-h2: report more accurate traces and glitches on trailers errors
- CLEANUP: h2: get rid of the now unused h2_make_htx_trailers()
- MEDIUM: h3: rely on the generic trailers parser
- BUG/MINOR: h3: reject truncated frames with mandatory payload
- BUG/MEDIUM: h3: do not block FIN on empty DATA frame
- BUG/MINOR: h3: handle empty HEADERS frame as specified
- BUG/MINOR: h3: handle unknown frame type on request stream as specified
- BUG/MINOR: h3: only increment frame type counter on new header
- BUILD: fd: Make sure to include sched.h
- BUG/MEDIUM: http-ana: apply tunnel timeout with data filters
- SCRIPTS: run-regtests: Increase internal buffer size used for logs
- REGTESTS: specify explicit chroot in confs using unix-stream servers
- MAJOR: startup: use chroot auto by default when available
- BUG/MINOR: ssl: don't pass the address of a size_t as an int* to OpenSSL
- BUG/MEDIUM: map: do not overwrite the fetch context in pat_match_regm()
- REGTESTS: map: check map_regm inside an ACL over an iterating fetch
- BUG/MINOR: cli: Make sure two instances of _getsocks don't run
- BUG/MEDIUM: listener: Don't drop suspend/resume deferred requests
- MEDIUM: twork: Introduce a new cross-thread/thread-group deferred work API
- MEDIUM: listener: make the rx_agent use the generic twork API
- MINOR: fd: add fd_tables_are_split() to test for split FD tables
- BUG/MEDIUM cli: dump other thread groups' FDs from their own threads
- BUG/MINOR: stream: resolve foreign groups' addresses for "show sess"
- REGTESTS: http-messaging: add tests for the H1 trailers
- MINOR: http-htx: pass extra flags to the HTTP trailers parser
- MINOR: http-htx: condition uppercase acceptance in trailer names to HTTP_PF_UPCASE_OK
- MEDIUM: h1: switch to the generic trailers parser
- REGTESTS: update trailers tests to cover H1's refusal of forbidden names
- BUG/MEDIUM: http_act: fix normalize-uri normalizer selection with a condition
- REGTESTS: uri_normalizer: test a normalizer used with a condition
- BUG/MEDIUM: cli: Don't parse next command if input buffer is empty
- BUG/MEDIUM: cli: Don't read a command or payload line into a full buffer
- BUG/MINOR: cli: Reject payload with no command
- BUG/MINOR: pattern: do not match a string with an embedded \0 on its prefix
- REGTESTS: http-rules: check "-m str" on a sample with an embedded \0
- BUILD: ssl: ASN1_STRING_length() is deprecated since OpenSSL 4.1
- IMPORT: uslz: reject codes from an empty distance table
- MINOR: htx: don't reserve a block when there is no free space left
- MEDIUM: jwt: Reject "none" alg in jwt_verify by default
- BUG/MINOR: ssl: Fix leak of ca_list in cafile_entry
- BUG/MINOR: sample: reject a \0 byte in url_dec, json_query and jwt_*_query
- DOC: configuration: clarify the validity ranges of sample types
- BUG/MEDIUM: ssl: don't set a 0 timeout on an expired TLS session
- BUG/MEDIUM: htx: Reserve a block descriptor for zero-sized blocks
- CLEANUP: stream: Remove an excess newline in the stream dump
- BUG/MEDIUM: tcp-rules: Don't restart tcp-request content evaluation on yield
- BUG/MINOR: proxy: pass PR_FL_DEF_EXPLICIT_MODE through cascaded defaults
- MAJOR: filters: Revert the filter-sequence directive
- MINOR: promex: properly use alt_name and val instead of hard-coded values
- MINOR: mworker: assign a unique ID to the master process
- MINOR: config: add a "worker-id" global keyword to identify the worker
- MINOR: sample: add the "master_id" and "worker_id" sample fetches
- MINOR: stats: report the master and worker IDs in "show info"
- MINOR: promex: expose the master and worker IDs as labels
- MINOR: extcheck: pass the master and worker IDs to external checks
- MINOR: stats: report the process IDs on the HTML page with "stats show-node"
- REGTESTS: mcli: check the master and worker unique identifiers
- DOC: config: fix indent issue introduced with worker id
- MINOR: sample: add the "be_sort" converter
- REGTESTS: converter: add a test for the "be_sort" converter
REGTESTS: converter: add a test for the "be_sort" converter
Check the 4 supported element sizes, the unsigned ordering (an element
made of 0xff bytes must be sorted last), a single-element input and an
input whose length is not a multiple of the element size, for which the
conversion must fail.
This converter sorts in ascending numerical order the big-endian unsigned
integers of <elem_size> bytes stored in a binary sample. <elem_size> must
be 1, 2, 4 or 8, and the input length must be a multiple of <elem_size>,
otherwise the conversion fails.
Since the elements are unsigned and stored in network byte order, comparing
their bytes starting from the most significant one gives the numerical
order, so a simple memcmp() over the element width is enough and no byte
swapping is needed.
It is mainly intended to normalize a list of binary values whose order may
vary from one client to another before hashing it, for instance the cipher
list or the extension list of a TLS client hello:
DOC: config: fix indent issue introduced with worker id
A paragraph break and poor indent was left over by recent commit 3639aac31f ("MINOR: config: add a "worker-id" global keyword to
identify the worker"), let's fix it. No backport needed.
REGTESTS: mcli: check the master and worker unique identifiers
The test forces the worker's ID from the configuration so that it can be
matched exactly, both from the "worker_id" sample fetch function, from
$HAPROXY_WORKER_ID and from "show info", while the master's one, which is
generated, is matched against the UUID version 7 format, in the master's
environment, in "show info" and through the "master_id" sample fetch
function.
A second instance runs without a master and lets its worker ID be
generated, which permits to check that "master_id" then finds nothing and
that the generated identifier matches its exported copy.
Both instances also verify what "stats show-node" reports on the HTML
page, with and without a master.
MINOR: stats: report the process IDs on the HTML page with "stats show-node"
This adds the worker-id and (optionally) master-id on the output next to
the node when "stats show-node" is present, since these generally go
together. No escaping is needed since guid_is_valid_fmt() only allows
alphanumerical values and '.', ':', '-' and '_'.
MINOR: extcheck: pass the master and worker IDs to external checks
By default, i.e. unless "external-check preserve-env" is used, the
environment of an external check is not inherited but entirely rebuilt
from the curated list in extcheck_envs[], so the two new identifiers
would not be visible from a check script. Let's add them there, next to
PATH, as a new process-scoped group before the proxy and server ones.
It's also cleaner because we pass the real directives, not the env
variables which could have been modified.
MINOR: promex: expose the master and worker IDs as labels
This adds support for emitting the master id when defined (when master
is running) and the worker id. Note that we add a check for the value
being a non-empty string, which remains valid for the few other field
that is always defined (node), so no impact on existing fields.
MINOR: sample: add the "master_id" and "worker_id" sample fetches
These two new sample fetch functions return the unique identifiers of the
master and of the current worker process, so that they can be placed in
logs, in response headers, or be used in any other expression.
The "master_id" one fails when there is no master, i.e. when not running
in master-worker mode, so that a log or a header will simply not report
anything there instead of an inaccurate value.
Note that these functions rely on internal storage, not environment
variables, so they are unaffected by "setenv" directives.
MINOR: config: add a "worker-id" global keyword to identify the worker
The worker process now also has a unique identifier. Contrary to the
master's one, it may be replaced using the new "worker-id" keyword in
the global section. Like for the master, a time-ordered UUID (version 7)
is generated at boot (allowing to compare older vs newer process).
The identifier is immediately exported as HAPROXY_WORKER_ID so that it
may be referenced from the worker's configuration and be inherited by
the processes we may fork (e.g. external checks). It may even be
referenced by the worker-id directive itself to combine a static part
with the dynamically generated one.
MINOR: mworker: assign a unique ID to the master process
The master process now has a unique identifier, either generated at boot
as a time-ordered UUID (version 7), or inherited from the
HAPROXY_MASTER_ID environment variable when a valid one is already set
there (inherited from a previous master instance). An invalid value is
simply replaced by a generated one, since there is generally nothing the
user can do about it. The identifier is exported to the environment so
that it is both preserved by the next master upon reload (re-exec) and
visible from the worker, where it may be referenced as
"$HAPROXY_MASTER_ID" from the configuration.
The format is validated using guid_is_valid_fmt() so that it is exactly
the same as the one of the proxies' and servers' "guid": at most 127
characters among letters, digits, '.', ':', '-' and '_'.
Note that both mworker_reexec() and the worker restore their initial
environment before going further, so the variable has to be set again
after that point, hence the two extra setenv() calls, just like for a
few other such variables.
MINOR: promex: properly use alt_name and val instead of hard-coded values
Some string-based info from the "show info" output are returned as gauges
in promex, and the way it's done consists in forcing their label's name
and value to the desired name and values, then faking a value of 1 on the
gauge. Since 3.2 with commit b03e05cd36 ("MINOR: stats: add alt name info
to stat_cols_info where relevant"), this label's name appears in the stats
info's alt_name field. Thus there's no need to hard-code it again in the
promex code. Let's first do this for the "node" label.
MAJOR: filters: Revert the filter-sequence directive
The filter-sequence directive was introduced to be able to define a
different evaluation order of filters for the request and the response. The
first usage should be to mix the compression and the decompression, when a
specific order must be defined.
However, there were several bugs. And working on fixing those bugs, it
appeared that the feature was underdesigned. To fix the bugs, the syntax of
the directive must be changed. And even this way, it will be difficult to
use it properly.
In fact, we really underestimate the changes to perform. So we decided to
revert the directive for now, especially to remove it from the 3.4. It is
not really mandatory on this version. Because of all current bugs, it was
probably not used yet on production. On 3.5, the feature will be fully and
deeply redesigned because it is mandatory.
This patch reverts the following commits:
* efe1099c3 ("BUG/MEDIUM: filters: check the filter name before comparing it")
* 3eebd5f67 ("REGTESTS: add a test for "filter-sequence" directive")
* 8d28c0e37 ("MEDIUM: filters: add "filter-sequence" directive")
BUG/MINOR: proxy: pass PR_FL_DEF_EXPLICIT_MODE through cascaded defaults
Basha Mougamadou reported that in the following setup:
defaults http
mode http
timeout client 10s
timeout server 10s
defaults fast-http from http
timeout server 1s
Trying to add a backend using "from fast-http" would be rejected because
the "fast-http" section has no explicit "mode" directive. This is indeed
true, but not correct in that it is inherited from the "http" section
where it is explicit, so there it must be considered as explicit as well.
The reason is that the flag PR_FL_DEF_EXPLICIT_MODE is not inherited from
defaults sections in proxy_defproxy_cpy(). Let's just add it to the set
of such inherited flags to fix the problem.
BUG/MEDIUM: tcp-rules: Don't restart tcp-request content evaluation on yield
The commit 9f1e9ee0e ("DEBUG: stream: Display the currently running rule in
stream dump") introduced a regression. The currently evaluated "tcp-request
content" rule was reset on resume while it should not. If it yielded again,
the rules evaluation was restarted to the first rule.
The "tcp-response content" rule evaluation was properly modified by the
commit above. The same must be performed gor the "tcp-request content" rule
evaluation.
This patch should fix the issue #3496. It must be backported as far as 3.2.
CLEANUP: stream: Remove an excess newline in the stream dump
When the function dumping a stream was update by the commit c044fdbf4
("BUG/MINOR: stream/debug: harden the stream dump function regarding
signals"), an extra newline character was forgotten. Let's remove it.
This patch must be backported with the commit above.
BUG/MEDIUM: htx: Reserve a block descriptor for zero-sized blocks
htx_reserve_nxblk() only refused to reserve a new block when
<blksz> was greater than the free space available for the payload, as
reported by htx_free_data_space(). However this function returns 0 in two
different cases: when there is exactly enough room left for a block
descriptor and no payload, but also when there is not even enough room for
a block descriptor. Both cases cannot be distinguished when a zero-sized
block is added, because the test is then always false.
In that case, the function goes on and hands out a block descriptor that
does not fit in the blocks table. Concretely, with <head> set to 0 and less
than sizeof(struct htx_blk) bytes left in the message, the defragmentation
has nothing to reclaim and the descriptor is placed inside the payload
part, silently overwriting the end of the last block. In addition, the tail
is incremented, so htx_used_space() becomes greater than the message size
and htx_free_space() wraps around to a huge value. None of the sanity
checks at the end of the function is able to detect it.
The free space is now explicitly tested against the size of a block
descriptor. This way, a zero-sized block is refused when there is no room
left for its descriptor. It is fixed in htx_reserve_nxblk() to also cover
htx_add_data_atonce(), which has the same flaw in its own test, and
htx_add_header()/htx_add_trailer(), which compute a null size when both the
name and the value are empty.
The only known caller able to request a zero-sized block is the
decompression filter, when there is no room left to drain the decompressed
data. It was introduced in 3.5-dev, so no released version should be
affected. However the flaw exists in the HTX API since its introduction in
1.9, so this patch may be backported as hardening to all stable versions.
BUG/MEDIUM: ssl: don't set a 0 timeout on an expired TLS session
Commit 9f1d590999 ("BUG/MEDIUM: ssl: enforce tune.ssl.lifetime across
TLS1.3 session renewals") clamps a resumed session's timeout to what is
left of its original lifetime. Once that deadline is reached there is
nothing left, and the session was given a timeout of 0.
OpenSSL encodes the session timeout as an optional ASN.1 field which it
omits when the value is zero, and the decoder then substitutes its own
default (3 seconds) instead. construct_stateless_ticket() first measures
the ticket, then re-encodes it, so the two lengths no longer match and
an internal consistency check fails. The resumed handshake is aborted
with an SSL_AD_INTERNAL_ERROR alert (80) instead of gracefully falling
back to a full handshake:
Only OpenSSL 3.1 and older are affected, newer versions don't compare
session ages in whole seconds anymore.
Fix this by never handing out a zero timeout: when nothing is left of
the original lifetime, clear the session's id context instead. The
session then no longer matches any SSL_CTX, so OpenSSL drops it and
performs a full handshake. This also makes the expired session really
disappear from the stateful cache, which a 0 timeout failed to do since
it was read back as the 3 seconds default.
This fixes issue #3499.
This must be backported as far as 2.8, where 9f1d590999 was backported.
DOC: configuration: clarify the validity ranges of sample types
Sample types are not super clear about what they contain nor how they
cast. It's trivial for developers because they deal with that all the
day but the user-facing doc is much less obvious about that. Let's
add a descriptive list of each types in 7.3. This could be backported.
BUG/MINOR: sample: reject a \0 byte in url_dec, json_query and jwt_*_query
A string sample cannot contain a \0 byte by design: this is what
distinguishes it from a binary sample, and every consumer relies on it.
Yet url_dec decoded "%00" to a \0, json_query decoded "\u0000" to one,
and jwt_header_query/jwt_payload_query could return one from the base64
decoded JSON. Such a sample was then matched by "-m str" on its prefix
only, so "/public%00/admin" matched "/public" and an allow-list could be
bypassed.
Commit 442f583f4 ("BUG/MINOR: pattern: do not match a string with an
embedded \0 on its prefix") worked around it in pat_match_str(), which
is the wrong place since such a string must not exist at all.
This commit reverts it and fixes the producers instead: url_decode() now
fails on "%00" like it does on an invalid sequence, and json_query and
jwt_*_query fail on a decoded \0. This is also cheaper, as the string
is checked once where it is produced instead of on every call to
pat_match_str().
This must be backported to all stable branches, in place of 442f583f4
where it was not yet backported.
MEDIUM: jwt: Reject "none" alg in jwt_verify by default
In RFC8725 section 3.2, they specify that "JWT libraries SHOULD NOT
consume JWTs using "none" unless explicitly requested by the caller".
This patch disables the support for the "none" algorithm by default in
jwt_verify converter and adds a global option that explicitely allows
it.
MINOR: htx: don't reserve a block when there is no free space left
htx_reserve_max_data() computed the available room up front but tested
the empty-message case before testing that room:
int32_t len = htx_free_data_space(htx);
if (htx->head == -1)
goto rsv_new_block;
if (!len)
return (struct htx_ret){.ret = 0, .blk = NULL};
So a message which is both empty and has no room at all went to
rsv_new_block with len == 0. That matters because htx_reserve_nxblk()
only rejects a reservation when "blksz > htx_free_data_space(htx)", so
a zero-sized one passes even when the free space is zero, and the
"Empty message" path then evaluates htx_get_blk(htx, 0), that is
htx->blocks + (htx->size - sizeof(struct htx_blk)). For any size below
sizeof(struct htx_blk) this subtraction underflows the uint32_t and
yields a pointer roughly 4 GB away, which is immediately written to.
The case that is trivial to reach in theory is the shared empty HTX
(htx_empty, size 0) that htxbuf() and htx_from_buf() return for an
unallocated buffer. No caller can do it today: the only caller,
h1_parse_full_contig_chunks(), returns earlier when the maximum is not
larger than sizeof(struct htx_blk), and the H1 mux allocates the
destination buffer before parsing into it. So this is not a fix for a
reachable bug, just one less trap for whoever adds the next caller.
Swapping the two tests is transparent for every other case: when the
message is empty but its buffer is allocated, the free space is
necessarily non-zero, so the early return cannot trigger and we still
reach rsv_new_block as before.
IMPORT: uslz: reject codes from an empty distance table
gen_huffman_table() may be asked to accept a table with no symbol at
all (allow_no_symbols, the distance table). It used to return success
without initializing the table, but that storage aliases the persistent
header-detection buffer (hdr_detect), so gethuff() could walk header
bytes out of bounds. Make the two root entries terminal with an
out-of-range symbol so any code decoded against an empty table is
rejected.
BUILD: ssl: ASN1_STRING_length() is deprecated since OpenSSL 4.1
OpenSSL 4.1.0-alpha1 deprecates ASN1_STRING_length() in favor of
ASN1_STRING_get_length(), which only differs by returning a size_t
instead of an int. This breaks the build when warnings are fatal:
src/ssl_sock.c:4655:9: error: 'ASN1_STRING_length' is deprecated: Since OpenSSL 4.1; use ASN1_STRING_get_length() [-Werror=deprecated-declarations]
Let's simply remap the old name on the new function when built with
OpenSSL 4.1 and above, all the call places being compatible with both
return types.
This could be backported to all versions where we want to support
OpenSSL 4.1.
REGTESTS: http-rules: check "-m str" on a sample with an embedded \0
Check that a case-sensitive exact match does not match a longer string
carrying an embedded \0, from url_dec or json_query, that the
case-insensitive match still works, and that "%00" in the raw path can
be matched as text.
BUG/MINOR: pattern: do not match a string with an embedded \0 on its prefix
pat_match_str() looks the sample up in the pattern tree with
ebst_lookup(), which reads it as a C string and stops at the first \0.
A sample "/public\0/admin" with <data> = 14 was thus matched as
"/public" by the default case-sensitive "-m str" exact match, while the
case-insensitive match, which uses the list, compares the length and
rejected it.
Such samples are produced by converters that decode client data, e.g.
url_dec ("%00") or json_query ("\u0000"), so an allow-list written as
"http-request deny unless { path,url_dec -m str /public }" could be
bypassed with "/public%00/admin". Maps (map, map_str) use the same
matcher and were affected too.
Patterns are loaded as C strings and never contain a \0, so such a
sample cannot be an exact match. Check it once at the top of
pat_match_str() and return no match.
Note that a deny-list such as "-m str /admin" used to reject
"/admin%00zz" by accident and now lets it through, since it is not an
exact match. The has_ctl() converter can be used to reject any decoded
\0.
This is the same issue as the one fixed for the ssl_*_dn() fetches in
commit 449133cba ("BUG/MINOR: ssl: reject an embedded \0 in the
ssl_*_dn(entry) fetches"), fixed here in the matcher so that all
producers are covered.
No error is reported during command line parsing when a payload pattern is
found with no command. It is not really a big deal because when the command
is processed, because there is no command, nothing is executed. It is
harmless.
But, accordingly to the doc, the payload is the last argument of a
command. So a command is required. Thus, an error is now reported when a
payload is received with no command.
This patch could be backported to 3.4. Older versions could benefit of this
patch. If we decide to backport it farther, the patch will have to be
adapted.
BUG/MEDIUM: cli: Don't read a command or payload line into a full buffer
Commit e3c175871 ("BUG/MEDIUM: cli: Don't parse next command if input buffer
is empty") fixed a crash triggered when a command line exactly filling the
buffer was finished by a payload pattern, by no longer trying to parse a
next command when the input buffer was empty.
However this only addressed the case where no more data was available. When
a command line exactly fills the buffer and ends with a payload pattern, the
payload buffer is created with a size of 0 (no room left). If payload data
is then received on a subsequent read, the loop was not interrupted and
b_getline() was called with a length of 'b_room(buf) - 1'. Because b_room()
returns a size_t, an empty buffer made this length wrap around to SIZE_MAX,
and the line was copied past the end of the trash chunk, leading to a buffer
overflow.
To fix the issue, no attempt to read a command or a payload line is
performed when there is no room left in the destination buffer. In that
case, we fall back to the existing path that reallocates a bigger (dynamic)
payload buffer, which is guaranteed to be given a non-zero size, or reports
an error for the command line. This way b_getdelim() and b_getline() are
never called on a full buffer and the size_t underflow can no longer happen.
On older versions, 1-byte OOB write is also possible, but with a lower
severity.
This patch must be backported to 3.4, along with commit e3c175871. And it
should be adapted and backported to all other stable versions.
BUG/MEDIUM: cli: Don't parse next command if input buffer is empty
In the loop of the command line parser, we always tried to get the next
command, even when the input buffer was empty. However it was an issue if
the size of command line was exactly the buffer size and it was finished by
a payload pattern. In that case, an attempt to allocate a dynamic buffer for
the payload was performed with a size of 0, which led to a crash.
To fix the issue, the loop is now interrupted with there is no more data to
parse in the input buffer. This way, when an dynamic buffer must be
(re)allocated, we are sure its size will never be 0. To be sure it is true,
a BUG_ON() was added on the new dynamic buffer size.
To mitigate the issue, "tune.cli.max-payload-size" parameter can be set to
0.
Youngkwang Lee [Wed, 9 Sep 2026 08:31:04 +0000 (17:31 +0900)]
REGTESTS: uri_normalizer: test a normalizer used with a condition
The existing tests always use the normalizers unconditionally, so the
selection of the normalizer when a condition directly follows a
normalizer whose sub-option is omitted was not covered.
The request uses both a percent sequence and duplicate slashes so that
the expected result differs depending on which normalizer is picked:
percent-decode-unreserved leaves the duplicate slashes alone, while
path-merge-slashes, which was wrongly selected before the previous
commit, would merge them and leave the percent sequence encoded.
Youngkwang Lee [Wed, 9 Sep 2026 08:31:03 +0000 (17:31 +0900)]
BUG/MEDIUM: http_act: fix normalize-uri normalizer selection with a condition
The normalizers that accept an optional sub-option ("strict" for
percent-to-uppercase and percent-decode-unreserved, "full" for
path-strip-dotdot) only set rule->action in the branch handling the
sub-option and in the branch handling the end of the arguments. When
the sub-option is omitted and an "if" or "unless" keyword directly
follows the normalizer name, none of these branches is taken and
rule->action is left untouched.
As a result the rule ends up with action 0, which is
ACT_NORMALIZE_URI_PATH_MERGE_SLASHES, and a configuration such as:
http-request normalize-uri percent-decode-unreserved if { ... }
silently merges duplicate slashes instead of decoding percent
sequences. This is easily missed because the rule still does
something, and for a URI without duplicate slashes the request is
left unchanged, which looks like the rule was simply skipped.
Set the default action right after the normalizer name is matched and
let the sub-option overwrite it. The check rejecting unknown arguments
is kept, guarded by a test on the argument being non-empty.
This should fix GitHub issue #3493.
This may be backported to all stable versions where the affected
normalizers exist. The three of them were introduced in 2.4-dev by
commits 560e1a635 ("MINOR: uri_normalizer: Add support for supressing
leading `../` for dotdot normalizer"), a40719337 ("MINOR:
uri_normalizer: Add a `percent-upper` normalizer") and 2e4a18e04
("MINOR: uri_normalizer: Add a `percent-decode-unreserved`
normalizer").
Signed-off-by: Youngkwang Lee <ykglory0216@gmail.com>
Let's now use http_trailers_to_htx() instead of htx_add_all_trailers()
to parse HTTP/1 trailers. Upper case is permitted and thats the only
difference with H2/H3 for now.
The parser is stricter than the previous function, so the following
names are now rejected as it is for other versions:
MINOR: http-htx: condition uppercase acceptance in trailer names to HTTP_PF_UPCASE_OK
The trailers parser currently rejects uppercase chars in the name.
Let's add a new flag to indicate that they are OK. This will be used
by HTTP/1. However this is not sufficient, we now need to use isteqi()
instead of isteq() to compare header names.
Neither H2 nor H3 need to pass that new flag, hence why the change is
minimal.
MINOR: http-htx: pass extra flags to the HTTP trailers parser
Let's introduce HTTP parser flags (HTTP_PF_*) and rely on them for the
trailers parser. This one will need them to decide to enable upper case
characters or not in header names in order to reuse it for H1 parsing.
For now we only pass the direction. The current parser doesn't use it
but the goal is to generalize the mechanism to other parsers in the
future.
REGTESTS: http-messaging: add tests for the H1 trailers
The Transfer-Encoding test is extended to cover the HTTP/1 trailers,
both on the request and on the response path.
Valid trailers are checked to be forwarded in both directions. vtest is
not able to parse HTTP/1 trailers (it stops on the CRLF following the
last chunk), so they are read using "recv" and only the number of bytes
emitted by haproxy is verified. This is sufficient to prove that the
spaces surrounding the values were trimmed, since it changes the length,
and a second trailer-less message is exchanged over the same connection
to make sure nothing was emitted after the trailers.
The trailers rejected by the H1 parser are checked to result in a 400 on
the request path and in a 502 on the response path: invalid token in the
field name, missing colon, pseudo-header, empty field name and a CR not
followed by an LF in the value. Note that each message must be emitted
at once, otherwise haproxy has already forwarded the beginning of the
message when the invalid trailers are parsed and can only abort the
stream instead of responding with an error.
BUG/MINOR: stream: resolve foreign groups' addresses for "show sess"
A stream in another group can't have its addresses resolved from the CLI
thread when using per-thread group file descriptor tables, as
getsockname/getpeername need the owner group's FD table. Ask the owning thread
to resolve a batch and cache them in the connections, picked up when the dump
resumes.
BUG/MEDIUM cli: dump other thread groups' FDs from their own threads
"show fd" data (getsockname/getpeername) is only valid in the FD's owner
group. With split FD tables, dump each foreign group's FDs from one of
its own threads via twork, one buffer at a time, forwarded to the client.
Olivier Houchard [Thu, 20 Aug 2026 16:34:45 +0000 (18:34 +0200)]
MEDIUM: twork: Introduce a new cross-thread/thread-group deferred work API
Introduce twork, a way to execute code on another thread, or another
thread group. The main motivation for that was to be able to obtain
information about file descriptor belonging to other thread groups, when
running with per-thread-group file descriptor tables.
The functions are :
int twork_call_on(struct twork *tw, int tid,
union twork_arg (*fn)(union twork_arg),
union twork_arg arg,
void (*done)(union twork_arg arg, union twork_arg ret));
Runs function fn on thread tid, providing arg as an argument, and
calling done from the caller thread when the work is complete.
int twork_call_tgroup(struct twork *tw, uint tgid,
union twork_arg (*fn)(union twork_arg),
union twork_arg arg,
void (*done)(union twork_arg arg, union twork_arg ret));
Same thing, but running on thread group tgid instead.
"done" is optional, if there is no need to be notified that the work is
complete.
"tw" is optional, if none is provided, one will be allocated
dynamically. Note that you can only cancel a twork if you're using one
that you provided by yourself.
int twork_cancel(struct twork *tw);
Cancels the tw, assuming it is not already running. It will return 1 on
success, and 0 on failure, in which case the work will be completed, and
the "done" function will be called.
BUG/MEDIUM: listener: Don't drop suspend/resume deferred requests
When using per-thread group file descriptors, we have to send messages
to other thread groups to tell them to suspend or resume listeners. Add
a current status for each thread group, instead of looking for the
global listener state, to know the current state for each thread group,
otherwise a rapid succession of suspend/resume could lead to an
inconsistant state.
BUG/MINOR: cli: Make sure two instances of _getsocks don't run
It should never happen, but two instances of _getsocks could run at the
same time, and cause a deadlock, so make sure that never happens by only
allowing one at the same time.
REGTESTS: map: check map_regm inside an ACL over an iterating fetch
This test covers the bug fixed by this commit:
7646c657a BUG/MEDIUM: map: do not overwrite the fetch context in pat_match_regm()
It uses map_regm in an ACL over req.hdr() with no occurrence. The first
value does not match, so the ACL runs a second iteration of the fetch,
which crashed the process before the fix. A request with two header
values checks that the fetch cursor survives the first iteration.
Thank you to @Michael-JRead for having produced the first version of this reg
test.
BUG/MEDIUM: map: do not overwrite the fetch context in pat_match_regm()
pat_match_regm() stored the address of the thread-local <pmatch> array
into <smp->ctx.a[0]> so that sample_conv_map() could read the capture
offsets back for exp_replace(). But this slot belongs to the sample
fetch, which uses it to keep an iteration cursor.
When map_regm is used inside an ACL over an iterating fetch (for example
req.hdr() with no occurrence), acl_exec_cond() re-runs the fetch to get
the next occurrence without clearing <smp->ctx>. On the second pass the
fetch reads back the <pmatch> pointer as its own cursor and dereferences
regex offsets as a pointer, which crashes the process. A single client
request is enough to trigger it.
Do not alias the slot: sample_conv_map() reads the thread-local <pmatch>
directly, since nothing runs another regex between pattern_exec_match()
filling it and this read. This is how http_replace_hdrs() and the
replace-path action already use <pmatch>.
Thank you to @Michael-JRead for having reported this issue.
This bug was introduced in 1.7 by commit 8feaa661b ("MINOR: map: Add
regex matching replacement").
BUG/MINOR: ssl: don't pass the address of a size_t as an int* to OpenSSL
aes_process() passes &out->data, which is a size_t, to the EVP update and
final routines through an (int*) cast. OpenSSL writes an int there, so on
a 64-bit big-endian platform (s390x, ppc64) the resulting length lands in
the high half of the size_t, and the buffer's data count becomes the real
length shifted left by 32 bits. The subsequent write at out->area +
out->data then targets a wild address.
Concretely, on such builds the "aes_gcm_dec" / "aes_cbc_dec" converters
and the JWE decryption paths corrupt memory as soon as they process any
input, so any client able to submit a token to them can crash the worker.
Little-endian builds, which are the vast majority, happen to work by
chance since the count starts at zero and only the low half is written.
Let's use a proper local int for the OpenSSL output length and assign it
back to the buffer's data count.
The shared helper comes from commit f0e64de75 ("MINOR: ssl: Factorize AES
GCM data processing") in 3.4 but the same cast existed in the AES
converters before that. It must be backported to all stable versions.
projects / haproxy.git / log