When sending RST_STREAM in response to a frame delivered on an already
closed stream, we used not to be able to update the error code and
deliver an RST_STREAM with a wrong code (e.g. H2_ERR_CANCEL). Let's
always allow to update the code so that RST_STREAM is always sent
with the appropriate error code (most often H2_ERR_STREAM_CLOSED).
This should be backported to 1.9 and possibly to 1.8.
There are incompatible MUST statements in the HTTP/2 specification. Some
require a stream error and others a connection error for the same situation.
As discussed in the thread below, let's always apply the connection error
when relevant (headers-like frame in half-closed(remote)) :
https://mailarchive.ietf.org/arch/msg/httpbisa/pOIWRBRBdQrw5TDHODZXp8iblcE
This must be backported to 1.9, possibly to 1.8 as well.
Since we now support CONTINUATION frames, we must take care of properly
aborting the connection when they are sent on a closed stream. By default
we'd get a stream error which is not sufficient since the compression
context is modified and unrecoverable.
More info in this discussion :
https://mailarchive.ietf.org/arch/msg/httpbisa/azZ1jiOkvM3xrpH4jX-Q72KoH00
This needs to be backported to 1.9 and possibly to 1.8 (less important there).
This patch adds H2_FT_HDR_MASK to group all frame types carrying headers
information, and H2_FT_LATE_MASK to group frame types allowed to arrive
after a stream was closed.
There was an incomplete test in h2c_frt_handle_headers() resulting
in negative return values from h2c_decode_headers() not being taken
as errors. The effect is that the stream is then aborted on timeout
only.
This fix must be backported to 1.9.
The current test consists in removing muxes which report that they're going
to assign their last available stream, but a mux may already be saturated
without having passed in this situation at all. This is what happens with
mux_h2 when receiving a GOAWAY frame informing the mux about the ID of the
last stream the other end is willing to process. The limit suddenly changes
from near infinite to 0. Currently what happens is that such a mux remains
in the idle list for a long time and refuses all new streams. Now at least
it will only fail a single stream in a retryable way. A future improvement
should consist in trying to pick another connection from the idle list.
This fix must be backported to 1.9.
In case we cannot allocate a stream ID for an outgoing stream, the stream
will be aborted. The problem is that we also release it and it will be
destroyed again by the application detecting the error, leading to a NULL
dereference in h2_shutr() and h2_shutw(). Let's only mark the error on the
CS and let the rest of the code handle the close.
This should be backported to 1.9.
It's almost funny but one side effect of the latest zero-copy changes made
to mux-h1 resulted in the temporary buffer being copied over itself at the
exact same location. This has no impact except slowing down operations and
irritating valgrind. The cause is an incorrect pointer check after the
alignment optimizations were made.
This needs to be backported to 1.9.
Reported-by: Tim Duesterhus <tim@bastelstu.be>
There is no reason to use the reserve to limit the amount of data of the input
buffer that we can parse at a time. The only important thing is to keep the
reserve free in the channel's buffer.
This patch should be backported to 1.9.
When data are pushed in the channel's buffer, in h2_rcv_buf(), the mux-h2 must
respect the reserve if the flag CO_RFL_KEEP_RSV is set. In HTX, because the
stream-interface always sees the buffer as full, there is no other way to know
the reserve must be respected.
This patch must be backported to 1.9.
When an HTX stream is waiting for a request or a response, it reports an error
(400 for the request or 502 for the response) if a parsing error is reported by
the mux (HTX_FL_PARSING_ERROR). The mux-h1 uses this error, among other things,
when the headers are too big to be analyzed at once. But the mux-h2 doesn't. So
the stream must also report an error if the multiplexer is unable to emit all
headers at once. The multiplexers must always emit all the headers at once
otherwise it is an error.
There are 2 ways to detect this error:
* The end-of-headers marker was not received yet _AND_ the HTX message is not
empty.
* The end-of-headers marker was not received yet _AND_ the multiplexer have
some data to emit but it is waiting for more space in the channel's buffer.
Note the mux-h2 is buggy for now when HTX is enabled. It does not respect the
reserve. So there is no way to hit this bug.
This patch must be backported to 1.9.
In OpenSSL 1.1.1 TLS 1.3 KeyUpdate messages will trigger the callback
that is used to verify renegotiation is disabled. This means that these
KeyUpdate messages fail. In OpenSSL 1.1.1 a better mechanism is
available with the SSL_OP_NO_RENEGOTIATION flag that disables any TLS
1.2 and earlier negotiation.
So if this SSL_OP_NO_RENEGOTIATION flag is available, instead of having
a manual check, trust OpenSSL and disable the check. This means that TLS
1.3 KeyUpdate messages will work properly.
Reported-By: Adam Langley <agl@imperialviolet.org>
With tcp-check, the result of the check is set by the function tcpcheck_main()
from the I/O layer. So it is important to wake up the check task to handle the
result and finish the check. Otherwise, we will wait the task timeout to handle
the result of a tcp-check, delaying the next check by as much.
This patch also fixes a problem about email alerts reported by PiBa-NL (Pieter)
on the ML [1] on all versions since the 1.6. So this patch must be backported
from 1.9 to 1.6.
[1] https://www.mail-archive.com/haproxy@formilux.org/msg32190.html
When we load health values from a server state file, make sure what we assign
to srv->check.health actually matches the state we restore.
This should be backported as far as 1.6.
In order to address the mailers issues, we needed to store the proxy
into the checks struct, which was done by commit c98aa1f18 ("MINOR:
checks: Store the proxy in checks."). However this one did it only for
the health checks and not for the agent checks, resulting in an immediate
crash when the agent is enabled on a random config like this one :
listen agent
bind :8000
server s1 255.255.255.255:1 agent-check agent-port 1
Thanks to Seri Kim for reporting it and providing a reproducer in
issue #20. This fix must be backported to 1.9.
If we fail to initialize pollers due to fdtab/fdinfo/polled_mask
not getting allocated, we free any of those that were allocated
and exit. However the ordering was incorrect, and there was an old
unused and unreachable "fail_cache" path as well, which needs to
be taken when no poller works.
This was introduced with this commit during 1.9-dev :
cb92f5c ("MINOR: pollers: move polled_mask outside of struct fdtab.")
It needs to be backported to 1.9 only.
Make "bind" keywork be supported in "peers" sections.
All "bind" settings are supported on this line.
Add "default-bind" option to parse the binding options excepted the bind address.
Do not parse anymore the bind address for local peers on "server" lines.
Do not use anymore list_for_each_entry() to set the "peers" section
listener parameters because there is only one listener by "peers" section.
May be backported to 1.5 and newer.
This patch adds pointer to a struct server to peer structure which
is initialized after having parsed a remote "peer" line.
After having parsed all peers section we run ->prepare_srv to initialize
all SSL/TLS stuff of remote perr (or server).
Remaining thing to do to completely support peer protocol over SSL/TLS:
make "bind" keyword be supported in "peers" sections to make SSL/TLS
incoming connections to local peers work.
May be backported to 1.5 and newer.
With this patch "default-server" lines are supported in "peers" sections
to setup the default settings of peers which are from now setup
when parsing both "peer" and "server" lines.
May be backported to 1.5 and newer.
Even if not already the case, we suppose that the frontend "peers" section
may have been already initialized outside of "peer" line, we seperate
their initializations from their binding initializations.
May be backported to 1.5 and newer.
Use ->local "peers" struct member to flag a "peers" section frontend
has being initialized. This is to be able to initialize the frontend
of "peers" sections on lines different from "peer" lines.
May be backported to 1.5 and newer.
Create init_peers_frontend() function to allocate and initialize
the frontend of "peers" sections (->peers_fe) so that to reuse it later.
May be backported to 1.5 and newer.
If a send succeeded, add the CO_FL_CONNECTED flag, the send may have been
called by the upper layers before we even realized we were connected, and we
may even read the response before we get the information, and si_cs_recv()
has to know if we were connected or not.
This should be backported to 1.9.
If an ALPN is set on the server line, then when we reach assign_tproxy_address,
the stream_interface's endpoint will be a connection, not a conn_stream,
so make sure assign_tproxy_address() handles both cases.
This should be backported to 1.9.
This regtest verifies that the stats webpage can be used to change a
server state to maintenance or drain, and that filtering the page scope
will result in a filtered page.
For HTX streams, the scope pointer is relative to the URI in the start-line. But
for streams using the legacy HTTP representation, the scope pointer is relative
to the beginning of output data in the channel's buffer. So we must be careful
to use the right one depending on the HTX is used or not.
Because the start-line is used to get de scope pointer, it is important to keep
it after the parsing of post paramters. So now, instead of removing blocks when
read in the function stats_process_http_post(), we just move on next, leaving it
in the HTX message.
Thanks to Pieter (PiBa-NL) to report this bug.
This patch must be backported to 1.9.
The code using the deprecated 'buf->p' has been updated to use
'ci_head(buf)' as described in section 5 of
'doc/internals/buffer-api.txt'.
A compile time switch on 'BUF_NULL' has also been added to enable the
same source code to be used with pre and post API change HAProxy.
This should be backported to 1.9, and is compatible with previous
versions.
When using the peers feature a race condition could prevent
a connection from being properly counted. When this connection
exits it is being "uncounted" nonetheless, leading to a possible
underflow (-1) of the conn_curr stick table entry in the following
scenario :
- Connect to peer A (A=1, B=0)
- Peer A sends 1 to B (A=1, B=1)
- Kill connection to A (A=0, B=1)
- Connect to peer B (A=0, B=2)
- Peer A sends 0 to B (A=0, B=0)
- Peer B sends 0/2 to A (A=?, B=0)
- Kill connection to B (A=?, B=-1)
- Peer B sends -1 to A (A=-1, B=-1)
This fix may be backported to all supported branches.
The most significant change from 1.8 to >=1.9 is the buffer
data structure, using the new field and fixing along side
a little hidden compilation warning.
This must be backported to 1.9.
When an argument <draws> is present, it must be an integer value one
or greater, indicating the number of draws before selecting the least
loaded of these servers. It was indeed demonstrated that picking the
least loaded of two servers is enough to significantly improve the
fairness of the algorithm, by always avoiding to pick the most loaded
server within a farm and getting rid of any bias that could be induced
by the unfair distribution of the consistent list. Higher values N will
take away N-1 of the highest loaded servers at the expense of performance.
With very high values, the algorithm will converge towards the leastconn's
result but much slower. The default value is 2, which generally shows very
good distribution and performance. This algorithm is also known as the
Power of Two Random Choices and is described here :
http://www.eecs.harvard.edu/~michaelm/postscripts/handbook2001.pdf
Since all of them are exclusive, let's move them to an union instead
of eating memory with the sum of all of them. We're using a transparent
union to limit the code changes.
Doing so reduces the struct lbprm from 392 bytes to 372, and thanks
to these changes, the struct proxy is now down to 6480 bytes vs 6624
before the changes (144 bytes saved per proxy).
This one is a proxy option which can be inherited from defaults even
if the LB algo changes. Move it out of the lb_chash struct so that we
don't need to keep anything separate between these structs. This will
allow us to merge them into an union later. It even takes less room
now as it fills a hole and removes another one.
The algo-specific settings move from the proxy to the LB algo this way :
- uri_whole => arg_opt1
- uri_len_limit => arg_opt2
- uri_dirs_depth1 => arg_opt3
Some algorithms require a few extra options (up to 3). Let's provide
some room in lbprm to store them, and make sure they're passed from
defaults to backends.
These ones used to rely on separate variables called hh_name/hh_len
but they are exclusive with the former. Let's use the same variable
which becomes a generic argument name and length for the LB algorithm.
There are a few instances where the lookup algo is tested against
BE_LB_LKUP_CHTREE using a binary "AND" operation while this macro
is a value among a set, and not a bit. The test happens to work
because the value is exactly 4 and no bit overlaps with the other
possible values but this is a latent bug waiting for a new LB algo
to appear to strike. At the moment the only other algo sharing a bit
with it is the "first" algo which is never supported in the same code
places.
This fix should be backported to maintained versions for safety if it
passes easily, otherwise it's not important as it will not fix any
visible issue.
The "balance uri" options "whole", "len" and "depth" were not properly
inherited from the defaults sections. In addition, "whole" and "len"
were not even reset when parsing "uri", meaning that 2 subsequent
"balance uri" statements would not have the expected effect as the
options from the first one would remain for the second one.
This may be backported to all maintained versions.
At a few places in the code we used to rely on this variable to guess
what LB algo was in place. This is wrong because if the defaults section
presets "balance url_param foo" and a backend uses "balance roundrobin",
these locations will still see this url_param_name set and consider it.
The harm is limited, as this only causes the beginning of the request
body to be buffered. And in general this is a bad practice which prevents
us from cleaning the lbprm stuff. Let's explicitly check the LB algo
instead.
This may be backported to all currently maintained versions.
Openssl switched from aes128 to aes256 since may 2016 to compute
tls ticket secrets used by default. But Haproxy still handled only
128 bits keys for both tls key file and CLI.
This patch permit the user to set aes256 keys throught CLI or
the key file (80 bytes encoded in base64) in the same way that
aes128 keys were handled (48 bytes encoded in base64):
- first 16 bytes for the key name
- next 16/32 bytes for aes 128/256 key bits key
- last 16/32 bytes for hmac 128/256 bits
Both sizes are now supported (but keys from same file must be
of the same size and can but updated via CLI only using a key of
the same size).
Note: This feature need the fix "dec func ignores padding for output
size checking."
This patch fixes missing allocation checks loading tls key file
and avoid memory leak in some error cases.
This patch should be backport on branches 1.9 and 1.8
Decode function returns an error even if the ouptut buffer is
large enought because the padding was not considered. This
case was never met with current code base.
