create-release shows the next steps at the end and suggest to use
"git push origin master" but on my machine it's not "origin" so let's
determine it using git config and only use origin as a fall back.
Christopher found another issue in the H2 backend implementation that
results from a miss in the H2 spec: the processing of a HEADERS frame
is always permitted in IDLE state, but this doesn't make sense on the
response path! And here when facing such a frame, we try to decode it
while we didn't allocate any stream, so we end up trying to fill the
idle stream's buffer (read-only) and crash.
What we're doing here is that if we get a HEADERS frame in IDLE state
from a server, we terminate the connection with a PROTOCOL_ERROR. No
such transition seems to be permitted by the spec but it seems to be
the only sane solution.
This fix must be backported as far as 1.9. Note that in 2.0 and earlier
there's no h2_frame_check_vs_state() function, instead the check is
inlined in h2_process_demux().
Tim D�sterhus found that the amount of sanitization we perform on HTTP
header field names received in H2 is insufficient. Currently we reject
upper case letters as mandated by RFC7540#8.1.2, but section 10.3 also
requires that intermediaries translating streams to HTTP/1 further
refine the filtering to also reject invalid names (which means any name
that doesn't match a token). There is a small trick here which is that
the colon character used to start pseudo-header names doesn't match a
token, so pseudo-header names fall into that category, thus we have to
swap the pseudo-header name lookup with this check so that we only check
from the second character (past the ':') in case of pseudo-header names.
Another possibility could have been to perform this check only in the
HTX-to-H1 trancoder but doing would still expose the configured rules
and logs to such header names.
This fix must be backported as far as 1.8 since this bug could be
exploited and serve as the base for an attack. In 2.0 and earlier,
functions h2_make_h1_request() and h2_make_h1_trailers() must also
be adapted to sanitize requests coming in legacy mode.
Tim D�sterhus reported an annoying problem in the H2 decoder related to
an ambiguity in the H2 spec. The spec says in section 10.3 that HTTP/2
allows header field values that are not valid (since they're binary) and
at the same time that an H2 to H1 gateway must be careful to reject headers
whose values contain \0, \r or \n.
Till now, and for the sake of the ability to maintain end-to-end binary
transparency in H2-to-H2, the H2 mux wouldn't reject this since it does
not know what version will be used on the other side.
In theory we should in fact perform such a check when converting an HTX
header to H1. But this causes a problem as it means that all our rule sets,
sample fetches, captures, logs or redirects may still find an LF in a header
coming from H2. Also in 2.0 and older in legacy mode, the frames are instantly
converted to H1 and HTX couldn't help there. So this means that in practice
we must refrain from delivering such a header upwards, regardless of any
outgoing protocol consideration.
Applying such a lookup on all headers leaving the mux comes with a
significant performance hit, especially for large ones. A first attempt
was made at placing this into the HPACK decoder to refrain from learning
invalid literals but error reporting becomes more complicated. Additional
tests show that doing this within the HTX transcoding loop benefits from
the hot L1 cache, and that by skipping up to 8 bytes per iteration the
CPU cost remains within noise margin, around ~0.5%.
This patch must be backported as far as 1.8 since this bug could be
exploited and serve as the base for an attack. In 2.0 and earlier the
fix must also be added to functions h2_make_h1_request() and
h2_make_h1_trailers() to handle legacy mode. It relies on previous patch
"MINOR: ist: add ist_find_ctl()" to speed up the control bytes lookup.
All credits go to Tim for his detailed bug report and his initial patch.
This new function looks for the first control character in a string (a
char whose value is between 0x00 and 0x1F included) and returns it, or
NULL if there is none. It is optimized for quickly evicting non-matching
strings and scans ~0.43 bytes per cycle. It can be used as an accelerator
when it's needed to look up several of these characters (e.g. CR/LF/NUL).
gcc complains rightfully:
src/ssl_sock.c: In function ‘ssl_sock_prepare_all_ctx’:
src/ssl_sock.c:5507:3: warning: format not a string literal and no format arguments [-Wformat-security]
ha_warning(errmsg);
^
src/ssl_sock.c:5509:3: warning: format not a string literal and no format arguments [-Wformat-security]
ha_alert(errmsg);
^
src/ssl_sock.c: In function ‘cli_io_handler_commit_cert’:
src/ssl_sock.c:10208:3: warning: format not a string literal and no format arguments [-Wformat-security]
chunk_appendf(trash, err);
Introduced in 8b453912ce.
Since commit 9a1ab08 ("CLEANUP: ssl-sock: use HA_OPENSSL_VERSION_NUMBER
instead of OPENSSL_VERSION_NUMBER") we restrict LibreSSL to the OpenSSL
1.0.1 API, to avoid breaking LibreSSL every minute. We set
HA_OPENSSL_VERSION_NUMBER to 0x1000107fL if LibreSSL is detected and
only allow curves to be configured if HA_OPENSSL_VERSION_NUMBER is at
least 0x1000200fL.
However all relevant LibreSSL releases actually support settings curves,
which is now broken. Fix this by always allowing curve configuration when
using LibreSSL.
Reported on GitHub in issue #366.
Fixes: 9a1ab08 ("CLEANUP: ssl-sock: use HA_OPENSSL_VERSION_NUMBER instead
of OPENSSL_VERSION_NUMBER").
recent commit 8b453912ce ("MINOR: ssl: ssl_sock_prepare_ctx() return an error code")
converted all errors handling; in this patch we always test `err`, but
three of them are missing. I did not found a plausible explanation about
it.
this should fix issue #374
Fixes: 8b453912ce ("MINOR: ssl: ssl_sock_prepare_ctx() return an error code")
Reported-by: Илья Шипицин <chipitsine@gmail.com>
Signed-off-by: William Dauchy <w.dauchy@criteo.com>
The use-service action may be used in tcp-request and http-request rules. It was
added to customize HAproxy reply to a client using an applet (initially a lua
applet). But the documentation was missing.
This patch may be backported as far as 1.6.
The link to the known bugs page for the current version is built and
reported there. When it is a development version (less than 2 dots),
instead a link to github open issues is reported as there's no way to
be sure about the current situation in this case and it's better that
users report their trouble there.
As discussed on Discourse here:
https://discourse.haproxy.org/t/haproxy-branch-support-lifetime/4466
it's not always easy for end users to know the lifecycle of the version
they are using. This patch introduces a "Status" line in the output of
"haproxy -vv" indicating whether it's a development, stable, long-term
supported version, possibly with an estimated end of life for the branch
when it can be anticipated (e.g. for stable versions). This field should
be adjusted when creating a major release to reflect the new status.
It may make sense to backport this to other branches to clarify the
situation.
Apply the configuration of the ssl_bind_conf on the generated SSL_CTX.
It's a little bit hacky at the moment because the ssl_sock_prepare_ctx()
function was made for the configuration parsing, not for being using at
runtime. Only the 'verify' bind keyword seems to cause a file access so
we prevent it before calling the function.
Rework ssl_sock_prepare_ctx() so it fills a buffer with the error
messages instead of using ha_alert()/ha_warning(). Also returns an error
code (ERR_*) instead of the number of errors.
It was noted in #48 that there are times when a configuration
may use the server-template directive with SRV records and
simultaneously want to control weights using an agent-check or
through the runtime api. This patch adds a new option
"ignore-weight" to the "resolve-opts" directive.
When specified, any weight indicated within an SRV record will
be ignored. This is for both initial resolution and ongoing
resolution.
INITCALLs are used a lot in the code now and were not documented, resulting
in each user having to grep for functions in other files. This doc is not
perfect but aims at improving the situation. It documents what's been
available since 1.9 and may be backported there if it helps though it's
unlikely to be needed as it's mostly aimed at developers.
The previous patch on this function (36b536d6c "BUG/MEDIUM: stream-int: Don't
loose events on the CS when an EOS is reported") contains a bug. The return
value is based on the conn-stream's flags. But it may be reset if the CS is
closed. Ironically it was exactly the purpose of this patch...
This patch must be backported to 2.0 and 1.9.
The limitation on the first chunk for chunked requests was true for the legacy
HTTP mode. But, it does not exist with the HTX. Becaue, the legacy HTTP mode was
removed in 2.1, this limitation does not exist anymore.
When no data filter are registered on a channel, if the message length is known,
the HTTP payload is infinitely forwarded to save calls to process_stream(). When
we finally fall back again in XFER_BODY analyzers, we detect the end of the
message by checking channel flags. If CF_EOI or CF_SHUTR is set, we switch the
message in DONE state. For CF_EOI, it is relevant. But not for CF_SHUTR. a
shutdown for reads without the end of input must be interpreted as an abort for
messages with a known length.
Because of this bug, some aborts are not properly handled and reported. Instead,
we interpret it as a legitimate shutdown.
This patch must be backported to 2.0.
There are two issues with the way tunnel mode is detected on the response
path. First, when a response with an unknown content length is handled, the
request is also switched in tunnel mode. It is obviously wrong. Because it was
done on the server side only (so not during the request parsing), it is no
noticeable effects.
The second issue is about the way protocol upgrades are handled. The request is
switched in tunnel mode from the time the 101 response is processed. So an
unfinished request may be switched in tunnel mode too early. It is not a common
use, but a protocol upgrade on a POST is allowed. Thus, parsing of the payload
may be hijacked. It is especially bad for chunked payloads.
Now, conditions to switch the request in tunnel mode reflect what should be
done. Especially for the second issue. We wait the end of the request to switch
it in tunnel mode.
This patch must be backported to 2.0 and 1.9. Note that these versions are only
affected by the second issue but the patch cannot be easily splitted.
Some BUG_ON() tests emit a warning because of a potential null pointer
dereference on an HTX block. In fact, it should never happen, but now, GCC is
happy.
This patch must be backported to 2.0.
By passing the parameter "no-maint" in the query-string, it is now possible to
ignore servers in maintenance. It means that the metrics for servers in this
state will not be exported.
Now, the prometheus exporter parses the HTTP query-string to filter or to adapt
the exported metrics. In this first version, it is only possible select the
scopes of metrics to export. To do so, one or more parameters with "scope" as
name must be passed in the query-string, with one of those values: global,
frontend, backend, server or '*' (means all). A scope parameter with no value
means to filter out all scopes (nothing is returned). The scope parameters are
parsed in their appearance order in the query-string. So an empty scope will
reset all scopes already parsed. But it can be overridden by following scope
parameters in the query-string. By default everything is exported.
The filtering can also be done on prometheus scraping configuration, but general
aim is to optimise the source of data to improve load and scraping time. This is
particularly true for huge configuration with thousands of backends and servers.
Also note that this configuration was possible on the previous official haproxy
exporter but with even more parameters to select the needed metrics. Here we
thought it was sufficient to simply avoid a given type of metric. However, more
filters are still possible.
Thanks to William Dauchy. This patch is based on his work.
In si_cs_recv(), when a shutdown for reads is handled, the conn-stream may be
closed. It happens when the ouput channel is closed for writes or if
SI_FL_NOHALF is set on the stream-interface. In this case, conn-stream's flags
are reset. Thus, if an error (CS_FL_ERROR) or an end of input (CS_FL_EOI) is
reported by the mux, the event is lost. si_cs_recv() does not report these
events by itself. It relies on si_cs_process() to report them to the
stream-interface and/or the channel.
For instance, if CS_FL_EOS and CS_FL_EOI are set by the H1 multiplexer during a
call to si_cs_recv() on the server side, if the conn-stream is closed (read0 +
SI_FL_NOHALF), the CS_FL_EOI flag is lost. Thus, this may lead the stream to
interpret it as a server abort.
Now, conn-stream's flags are processed at the end of si_cs_recv(). The function
is responsible to set the right flags on the stream-interface and/or the
channel. Due to this patch, the function is now almost linear. Except some early
checks at the beginning, there is only one return statement. It also fixes a
potential bug because of an inconsistency between the splicing and the buffered
receipt. On the first case, CS_FL_EOS if handled before errors on the connection
or the conn-stream. On the second one, it is the opposite.
This patch must be backported to 2.0 and 1.9.
There is a compiler warning after commit a9363eb6 ("BUG/MEDIUM: ssl:
'tune.ssl.default-dh-param' value ignored with openssl > 1.1.1"):
src/ssl_sock.c: In function 'ssl_sock_prepare_ctx':
src/ssl_sock.c:4481:4: error: statement with no effect [-Werror=unused-value]
Fix it by adding a (void)
The peer flags (->flags member of peer struct) are reset by __peer_session_deinit()
function. PEER_F_ALIVE flag which is used by the heartbeat part of the peer protocol
to mark a peer as being alive was not reset by this function. This simple patch adds
add the statement to this.
Note that, at this time, there was no identified issue due to this missing reset.
Must be backported to 2.0.
Upon a reexec_on_failure, if the process tried to exit after the
initialization of the process structure but before it was filled with a
PID, the PID in the mworker_proc structure is set to -1.
In this particular case the -sf argument is filled with -1 and haproxy
will exit with the usage message because of that argument.
Should be backported in 2.0.
This patch introduces the new CLI command 'abort ssl cert' which abort
an on-going transaction and free its content.
This command takes the name of the filename of the transaction as an
argument.
As the peers protocol expects to parse at least one encoded integer value for
each stick-table data field even when not configured on the local side,
about the "server_name" data field we must emit something even if it has
not been set (no server was configured for instance).
As this data field is made of first one encoded integer which is the length
of the remaining data (the dictionary cache entry), we encode the length 0
when emitting such an absent dictionary cache entry.
On the remote side, when we decode such an integer with 0 as value, we stop
parsing the data field and that's it.
Must be backported to 2.0.
This patch adds three counters to help in debugging peers protocol issues
to "peer" struct:
->no_hbt counts the number of reconnection period without receiving heartbeat
->new_conn counts the number of reconnections after ->reconnect timeout expirations.
->proto_err counts the number of protocol errors.
Add RX/TX heartbeat counters to "peer" struct to have an idead about which
peer is alive or not.
Dump these counters values on the CLI via "show peers" command.
This patch enable us to dump the stick-table information of remote or local peers
without already opened peer session. This may be the case also for the local peer
during synchronizations with an old processus (reload).
Certificate selection in client_hello_cb (openssl >= 1.1.1) correctly
handles crt-list neg filter. Certificate selection for openssl < 1.1.1
has not been touched for a while: crt-list neg filter is not the same
than his counterpart and is wrong. Fix it to mimic the same behavior
has is counterpart.
It should be backported as far as 1.6.
With CLI cert update, sni_ctx can be removed at runtime. ssl_pkey_info_index
ex_data is filled with one of sni_ctx.kinfo pointer but SSL_CTX can be shared
between sni_ctx. Remove and free a sni_ctx can lead to a segfault when
ssl_pkey_info_index ex_data is used (in ssl_sock_get_pkey_algo). Removing the
dependency on ssl_pkey_info_index ex_data is the easiest way to fix the issue.
since the introduction of mworker, the setuid/setgid was duplicated in
two places; try to improve that by creating a dedicated function.
this patch does not introduce any functional change.
Signed-off-by: William Dauchy <w.dauchy@criteo.com>
in mworker mode used with uid/gid settings, it was not possible to get
a coredump despite the set-dumpable option.
indeed prctl(2) manual page specifies the dumpable attribute is reverted
to `/proc/sys/fs/suid_dumpable` in a few conditions such as process
effective user and group are changed.
this patch moves the whole set-dumpable logic before the polling code in
order to catch all possible cases where we could have changed the
uid/gid. It however does not cover the possible segfault at startup.
this patch should be backported in 2.0.
Signed-off-by: William Dauchy <w.dauchy@criteo.com>
Released version 2.1-dev5 with the following main changes :
- BUG/MEDIUM: ssl/cli: don't alloc path when cert not found
- BUG/MINOR: ssl/cli: unable to update a certificate without bundle extension
- BUG/MINOR: ssl/cli: fix an error when a file is not found
- MINOR: ssl/cli: replace the default_ctx during 'commit ssl cert'
- DOC: fix date and http_date keywords syntax
- MINOR: peers: Add "log" directive to "peers" section.
- BUG/MEDIUM: mux-h1: Disable splicing for chunked messages
- BUG/MEDIUM: stream: Be sure to support splicing at the mux level to enable it
- MINOR: flt_trace: Rename macros to print trace messages
- MINOR: trace: Add a set of macros to trace events if HA is compiled with debug
- MEDIUM: stream/trace: Register a new trace source with its events
- MINOR: doc: http-reuse connection pool fix
- BUG/MEDIUM: stream: Be sure to release allocated captures for TCP streams
- MINOR: http-ana: Remove the unused function http_reset_txn()
- BUG/MINOR: action: do-resolve now use cached response
- BUG: dns: timeout resolve not applied for valid resolutions
- DOC: management: fix typo on "cache_lookups" stats output
- BUG/MINOR: stream: init variables when the list is empty
- BUG/MEDIUM: tasks: Make tasklet_remove_from_tasklet_list() no matter the tasklet.
- BUG/MINOR: queue/threads: make the queue unlinking atomic
- BUG/MEDIUM: Make sure we leave the session list in session_free().
- CLEANUP: session: slightly simplify idle connection cleanup logic
- MINOR: memory: also poison the area on freeing
- CLEANUP: cli: use srv_shutdown_streams() instead of open-coding it
- CLEANUP: stats: use srv_shutdown_streams() instead of open-coding it
- BUG/MEDIUM: listeners: always pause a listener on out-of-resource condition
- BUILD: contrib/da: remove an "unused" warning
- BUG/MEDIUM: filters: Don't call TCP callbacks for HTX streams
- MEDIUM: filters: Adapt filters API to allow again TCP filtering on HTX streams
- MINOR: freq_ctr: Make the sliding window sums thread-safe
- MINOR: stream: Remove the lock on the proxy to update time stats
- MINOR: counters: Add fields to store the max observed for {q,c,d,t}_time
- MINOR: stats: Report max times in addition of the averages for sessions
- MINOR: contrib/prometheus-exporter: Report metrics about max times for sessions
- BUG/MINOR: contrib/prometheus-exporter: Rename some metrics
- MINOR: contrib/prometheus-exporter: report the number of idle conns per server
- DOC: Add missing stats fields in the management manual
- BUG/MINOR: mux-h1: Properly catch parsing errors on payload and trailers
- BUG/MINOR: mux-h1: Don't set CS_FL_EOS on a read0 when receiving data to pipe
- MINOR: mux-h1: Set EOI on the conn-stream when EOS is reported in TUNNEL state
- MINOR: sink: Set the default max length for a message to BUFSIZE
- MINOR: ring: make the parse function automatically set the handler/release
- BUG/MINOR: log: make "show startup-log" use a ring buffer instead
- MINOR: stick-table: allow sc-set-gpt0 to set value from an expression
Allow the sc-set-gpt0 action to set GPT0 to a value dynamically evaluated from
its <expr> argument (in addition to the existing static <int> alternative).
The copy of the startup logs used to rely on a re-allocated memory area
on the fly, that would attempt to be delivered at once over the CLI. But
if it's too large (too many warnings) it will take time to start up, and
may not even show up on the CLI as it doesn't fit in a buffer.
The ring buffer infrastructure solves all this with no more code, let's
switch to this instead. It simply requires a parsing function to attach
the ring via ring_attach_cli() and all the rest is automatically handled.
Initially this was imagined as a code cleanup, until a test with a config
involving 100k backends and just one occurrence of
"load-server-state-from-file global" in the defaults section took approx
20 minutes to parse due to the O(N^2) cost of concatenating the warnings
resulting in ~1 TB of data to be copied, while it took only 0.57s with
the ring.
Ideally this patch should be backported to 2.0 and 1.9, though it relies
on the ring infrastructure which will then also need to be backported.
Configs able to trigger the bug are uncommon, so another workaround for
older versions without backporting the rings would consist in simply
limiting the size of the error message in print_message() to something
always printable, which will only return the first errors.
ring_attach_cli() is called by the keyword parsing function to dump a
ring to the CLI. It can only work with a specific handler and release
function. Let's make it set them appropriately instead of having the
caller know these functions. This way adding a command to dump a ring
is as simple as declaring a parsing function calling ring_attach_cli().
It was set to MAX_SYSLOG_LEN (1K). It is a bit short to print debug
traces. Especially when part of a buffers is dump. Now, the maximum length is
set to BUFSIZE (16K).
This is mandatory to process input one more time to add the EOM in the HTX
message and to set CS_FL_EOI on the conn-stream. Otherwise, in the stream, a
SHUTR will be reported on the corresponding channel without the EOI. It may be
erroneously interpreted as an abort.
This patch must be backported to 2.0 and 1.9.
Errors during the payload or the trailers parsing are reported with the
HTX_FL_PARSING_ERROR flag on the HTX message and not a negative return
value. This change was introduced when the fonctions to convert an H1 message to
HTX one were moved to a dedicated file. But the h1 mux was not fully updated
accordingly.
No backport needed except if the commits about file h1_htx.c are backported.
