GitHub Issue #1026 reported a crash during configuration check for the
following example config:
backend 0
server 0 0
server 0 0
HAProxy crashed in srv_set_addr_desc() due to a NULL pointer dereference
caused by `sa2str` returning NULL for an `AF_UNSPEC` address (`0`).
Check to make sure the address key is non-null before using it for
comparison or inserting it into the tree.
The crash was introduced in commit 92149f9a8 ("MEDIUM: stick-tables: Add
srvkey option to stick-table") which not in any released version so no
backport is needed.
Cc: Tim Duesterhus <tim@bastelstu.be>
Rephrase the message to no longer talk about something that "is no longer
supported", but about what actually *is* supported.
Adjustments include:
- Removal of rare targets to make it easier to find the proper one.
- Reformatting to be easier to read (more newlines)
- Explanation of common non-default feature flags.
When all rules of a tcpcheck ruleset are successfully evaluated, the right
check status must always be reported. It is true if the last evaluated rule
is an expect or a connect rule. But not if it is a send rule. In this
situation, nothing more is done until the check timeout expiration and a
L7TOUT is reported instead of a L7OK.
Now, by default, when all rules were successfully evaluated, a L7OK is
reported. When the last evaluated rule is an expect or a connect, the
behavior remains unchanged.
This patch should fix the issue #1027. It must be backported as far as 2.2.
This variable is only needed deeply nested in a single location and clang's
static analyzer complains about a dead initialization. Reduce the scope to
satisfy clang and the human that reads the function.
As reported in issue #1017, there are two harmless duplicate tests in
cfg_parse_program(), one made of a "if" using the same condition as the
loop it's in, and the other one being a null test before a free. This
just removes them. No backport is needed.
This patch fixes GitHub issue #1024.
I could track the `strdup` back to commit
3a1f5fda10 which is 1.9-dev8. It's probably not
worth the effort to backport it across this refactoring.
This patch should be backported to 1.9+.
This reverts commit 5e8c35da1b.
While the issue is being discussed with gcc folks, a reasonable workaround
could be found for the tls_keys_ref list usage which doesn't significantly
complicate the code. Since it was the only place triggering the warning and
I don't feel very comfortable leaving this one disabled for too long, let's
re-enable it right now. This definitely closes issue #1010.
As reported in issue #1010, gcc-11 as of 2021-01-05 is overzealous in
its -Warray-bounds check as it considers that a cast of a global struct
accesses the entire struct even if only one specific element is accessed.
This instantly breaks all lists making use of container_of() to build
their iterators as soon as the starting point is known if the next
element is retrieved from the list head in a way that is visible to the
compiler's optimizer, because it decides that accessing the list's next
element dereferences the list as a larger struct (which it does not).
The temporary workaround consisted in disabling -Warray-bounds, but this
warning is traditionally quite effective at spotting real bugs, and we
actually have is a single occurrence of this issue in the whole code.
By changing the tlskeys_list_get_next() function to take a list element
as the starting point instead of the current element, we can avoid
the starting point issue but this requires to change all call places
to write hideous casts made of &((struct blah*)ref)->list. At the
moment we only have two such call places, the first one being used to
initialize the list (which is the one causing the warning) and which
is thus easy to simplify, and the second one for which we already have
an aliased pointer to the reference that is still valid at the call
place, and given the original pointer also remained unchanged, we can
safely use this alias, and this is safer than leaving a cast there.
Let's make this change now while it's still easy.
The generated code only changed in function cli_io_handler_tlskeys_files()
due to register allocation and the change of variable scope between the
old one and the new one.
`getnext` was only used to fill `ref` at the beginning of the function. Both
have the same type. Replace the parameter name by `ref` to remove the useless
local variable.
After having re-read the RFC, we noticed there are two bugs in the STREAM
frame parser. When the OFF bit (0x04) in the frame type is not set
we must set the offset to 0 (it was not set at all). When the LEN bit (0x02)
is not set we must extend the length of the data field to the end of the packet
(it was not set at all).
This is issue is due to the fact that when we call the function
responsible of building CRYPTO frames to fill a buffer, the Length
field of this packet did not take into an account the trailing 16 bytes for
the AEAD tag. Furthermore, the remaining <room> available in this buffer
was not decremented by the CRYPTO frame length, but only by the CRYPTO data length
of this frame.
This patch fixes GitHub issue #1023.
The function was introduced in commit 99c453d ("MEDIUM: ring: new
section ring to declare custom ring buffers."), which first appeared
in 2.2-dev9. The fix should be backported to 2.2+.
Ilya reported in issue #1010 that gcc 11 (still in development phase 3)
sees out-of-bounds accesses to tlskeys_reference. After a deep analysis
it turns out that the warning is erroneous and Tim could set up a
reproducer that was further simplified to file a gcc bug report (98503).
In order not to break builds on this version, let's temporarily disable
the warning, since no acceptable workaround could be defined. Once gcc
gets it right, the warning should be re-enabled as it is likely to catch
real errors (it's the first false positive here for us even though
roughly one hundred issues were reported around this one alone).
This allows using the address of the server rather than the name of the
server for keeping track of servers in a backend for stickiness.
The peers code was also extended to support feeding the dictionary using
this key instead of the name.
Fixes#814
This patch fixes GitHub Issue #988. Commit ce9e7b2521
was not sufficient, because it fell back to a hash comparison if the bitmap
of known encodings was not acceptable instead of directly returning the the
cached response is not compatible.
This patch also extends the reg-test to test the hash collision that was
mentioned in #988.
Vary handling is 2.4, no backport needed.
The accept-encoding normalizer now explicitely manages a subset of
encodings which will all have their own bit in the encoding bitmap
stored in the cache entry. This way two requests with the same primary
key will be served the same cache entry if they both explicitely accept
the stored response's encoding, even if their respective secondary keys
are not the same and do not match the stored response's one.
The actual hash of the accept-encoding will still be used if the
response's encoding is unmanaged.
The encoding matching and the encoding weight parsing are done for every
subpart of the accept-encoding values, and a bitmap of accepted
encodings is built for every request. It is then tested upon any stored
response that has the same primary key until one with an accepted
encoding is found.
The specific "identity" and "*" accept-encoding values are managed too.
When storing a response in the key, we also parse the content-encoding
header in order to only set the response's corresponding encoding's bit
in its cache_entry encoding bitmap.
This patch fixes GitHub issue #988.
It does not need to be backported.
The accept-encoding part of the secondary key (vary) was only built out
of the first occurrence of the header. So if a client had two
accept-encoding headers, gzip and br for instance, the key would have
been built out of the gzip string. So another client that only managed
gzip would have been sent the cached resource, even if it was a br resource.
The http_find_header function is now called directly by the normalizers
so that they can manage multiple headers if needed.
A request that has more than 16 encodings will be considered as an
illegitimate request and its response will not be stored.
This fixes GitHub issue #987.
It does not need any backport.
If any of the secondary hash normalizing functions raises an error, the
secondary hash will be unusable. In this case, the response will not be
stored anymore.
Add traces to have an idea why this function may fail. In fact
in never fails when the passed parameters are correct, especially the
lengths. This is not the case when a packet is not correctly built
before being encrypted.
Even if the size of frames built by qc_build_frm() are computed so that
not to overflow a buffer, do not rely on this and always makes a packet
build fails if we could not build a frame.
Also add traces to have an idea where qc_build_frm() fails.
Fixes a memory leak in qc_build_phdshk_apkt().
This salt is ued at leat up to draft-32. At this date ngtcp2 always
uses this salt even if it started the draft-33 development.
Note that when the salt is not correct, we cannot remove the header
protection. In this case the packet number length is wrong.
At least displays the SSL alert error code passed to ->ssl_send_alert()
QUIC BIO method and the SSL encryption level. This function is newly called
when using picoquic client with a recent version of BoringSSL (Nov 19 2020).
This is not the case with OpenSSL with 32 as QUIC draft implementation.
Reorder by increasing type the switch/case in qc_parse_pkt_frms()
which is the high level frame parser.
Add new STREAM_X frame types to support some tests with ngtcp2 client.
Add ->flags to the QUIC frame parser as this has been done for the builder so
that to flag RX packets as ack-eliciting at low level. This should also be
helpful to maintain the code if we have to add new flags to RX packets.
Remove the statements which does the same thing as higher level in
qc_parse_pkt_frms().
