William Dauchy [Sun, 27 Oct 2019 19:08:11 +0000 (20:08 +0100)]
MINOR: init: always fail when setrlimit fails
this patch introduces a strict-limits parameter which enforces the
setrlimit setting instead of a warning. This option can be forcingly
disable with the "no" keyword.
The general aim of this patch is to avoid bad surprises on a production
environment where you change the maxconn for example, a new fd limit is
calculated, but cannot be set because of sysfs setting. In that case you
might want to have an explicit failure to be aware of it before seeing
your traffic going down. During a global rollout it is also useful to
explictly fail as most progressive rollout would simply check the
general health check of the process.
As discussed, plan to use the strict by default mode starting from v2.3.
Signed-off-by: William Dauchy <w.dauchy@criteo.com>
William Dauchy [Sun, 27 Oct 2019 19:08:10 +0000 (20:08 +0100)]
MINOR: config: allow no set-dumpable config option
in global config parsing, we currently expect to have a possible no
keyword (KWN_NO), but we never allow it in config parsing.
another patch could have been to simply remove the code handling a
possible KWN_NO.
take this opportunity to update documentation of set-dumpable.
Signed-off-by: William Dauchy <w.dauchy@criteo.com>
Olivier Houchard [Fri, 25 Oct 2019 15:00:54 +0000 (17:00 +0200)]
BUG/MEDIUM: servers: Only set SF_SRV_REUSED if the connection if fully ready.
In connect_server(), if we're reusing a connection, only use SF_SRV_REUSED
if the connection is fully ready. We may be using a multiplexed connection
created by another stream that is not yet ready, and may fail.
If we set SF_SRV_REUSED, process_stream() will then not wait for the timeout
to expire, and retry to connect immediately.
Olivier Houchard [Fri, 25 Oct 2019 14:25:20 +0000 (16:25 +0200)]
BUG/MEDIUM: stream_interface: Only use SI_ST_RDY when the mux is ready.
In si_connect(), only switch the strema_interface status to SI_ST_RDY if
we're reusing a connection and if the connection's mux is ready. Otherwise,
maybe we're reusing a connection that is not fully established yet, and may
fail, and setting SI_ST_RDY would mean we would not be able to retry to
connect.
Olivier Houchard [Fri, 25 Oct 2019 14:19:26 +0000 (16:19 +0200)]
MINOR: mux: Add a new method to get informations about a mux.
Add a new method, ctl(), to muxes. It uses a "enum mux_ctl_type" to
let it know which information we're asking for, and can output it either
directly by returning the expected value, or by using an optional argument.
"output" argument.
Right now, the only known mux_ctl_type is MUX_STATUS, that will return 0 if
the mux is not ready, or MUX_STATUS_READY if the mux is ready.
Willy Tarreau [Tue, 29 Oct 2019 12:06:21 +0000 (13:06 +0100)]
Revert "MINOR: istbuf: add b_fromist() to make a buffer from an ist"
This reverts commit 9e46496d45ff06317ae8f4f785e6117e5b786f6f. It was
wrong and is not reliable, depending on the compiler's version and
optimization, as the struct is assigned inside a statement, thus on
its own stack. It's not needed anymore now so let's remove this.
Willy Tarreau [Tue, 29 Oct 2019 12:02:15 +0000 (13:02 +0100)]
MINOR: chunk: add chunk_istcat() to concatenate an ist after a chunk
We previously relied on chunk_cat(dst, b_fromist(src)) for this but it
is not reliable as the allocated buffer is inside the expression and
may be on a temporary stack. While it's possible to allocate stack space
for a struct and return a pointer to it, it's not possible to initialize
it form a temporary variable to prevent arguments from being evaluated
multiple times. Since this is only used to append an ist after a chunk,
let's instead have a chunk_istcat() function to perform exactly this
from a native ist.
The only call place (URI computation in the cache) was updated.
Willy Tarreau [Tue, 29 Oct 2019 09:54:24 +0000 (10:54 +0100)]
BUILD: do not disable -Wformat-truncation anymore
There were very few entries to fix and this warning, while often
wrong, can actually spot future issues. If it can help developers
adjust their code in the future to make it more robust, it's not
necessarily that bad. Let's re-enable it and see how it goes.
Willy Tarreau [Tue, 29 Oct 2019 09:48:50 +0000 (10:48 +0100)]
BUILD/MINOR: ssl: shut up a build warning about format truncation
Actually gcc believes it has detected a possible truncation but it
cannot since the output string is necessarily at least one char
shorter than what it expects. However addressing it is easy and
removes the need for an intermediate copy so let's do it.
Willy Tarreau [Tue, 29 Oct 2019 09:25:49 +0000 (10:25 +0100)]
BUG/MINOR: spoe: fix off-by-one length in UUID format string
The per-thread UUID string produced by generate_pseudo_uuid() could be
off by one character due to too small of size limit in snprintf(). In
practice the UUID remains large enough to avoid any collision though.
Willy Tarreau [Tue, 29 Oct 2019 09:16:11 +0000 (10:16 +0100)]
BUILD/MINOR: tools: shut up the format truncation warning in get_gmt_offset()
The gcc warning about format truncation in get_gmt_offset() is annoying
since we always call it with a valid time thus it cannot fail. However
it's true that nothing guarantees that future code reuses this function
incorrectly in the future, so better enforce the modulus on one day and
shut the warning.
Tim Duesterhus [Mon, 28 Oct 2019 23:05:13 +0000 (00:05 +0100)]
DOC: Improve documentation of http-re(quest|sponse) replace-(header|value|uri)
- Clarify that everything and not only the matched part is replaced (GitHub #328)
- Reduce duplication and inconsistencies by referring to a single canonical directive
that includes everything one needs to know about.
- Fix indentation
Ilya Shipitsin [Sun, 27 Oct 2019 15:16:29 +0000 (20:16 +0500)]
BUILD: CI: comment out cygwin build, upgrade various ssl libraries
cirrus ci builds are now limited to branches master, next
travis-ci images are upgraded to ubuntu bionic
cygwin builds are temporarily disabled on travis-ci
(maybe someone will figure out how to fix them, here's link
https://travis-ci.community/t/cygwin-issue-cygheap-base-mismatch-detected/5359/2 )
openssl upgraded to 1.0.2t, 1.1.0l, 1.1.1d
libressl are upgraded (2.9.2, 2.8.3, 2.7.5) --> (3.0.2, 2.9.2, 2.8.3)
BUG/MINOR: ssl/cli: cleanup on cli_parse_set_cert error
Since commit 90b098c ("BUG/MINOR: cli: don't call the kw->io_release if
kw->parse failed"), the io_release() callback is not called anymore when
the parse() failed. Call it directly on the error path of the
cli_parse_set_cert() function.
BUG/MAJOR: stream-int: Don't receive data from mux until SI_ST_EST is reached
This bug is pretty pernicious and have serious consequences : In 2.1, an
infinite loop in process_stream() because the backend stream-interface remains
in the ready state (SI_ST_RDY). In 2.0, a call in loop to process_stream()
because the stream-interface remains blocked in the connect state
(SI_ST_CON). In both cases, it happens after a connection retry attempt. In 1.9,
it seems to not happen. But it may be just by chance or just because it is
harder to get right conditions to trigger the bug. However, reading the code,
the bug seems to exist too.
Here is how the bug happens in 2.1. When we try to establish a new connection to
a server, the corresponding stream-interface is first set to the connect state
(SI_ST_CON). When the underlying connection is known to be connected (the flag
CO_FL_CONNECTED set), the stream-interface is switched to the ready state
(SI_ST_RDY). It is a transient state between the connect state (SI_ST_CON) and
the established state (SI_ST_EST). It must be handled on the next call to
process_stream(), which is responsible to operate the transition. During all
this time, errors can occur. A connection error or a client abort. The transient
state SI_ST_RDY was introduced to let a chance to process_stream() to catch
these errors before considering the connection as fully established.
Unfortunatly, if a read0 is catched in states SI_ST_CON or SI_ST_RDY, it is
possible to have a shutdown without transition to SI_ST_DIS (in fact, here,
SI_ST_CON is swichted to SI_ST_RDY). This happens if the request was fully
received and analyzed. In this case, the flag SI_FL_NOHALF is set on the backend
stream-interface. If an error is also reported during the connect, the behavior
is undefined because an error is returned to the client and a connection retry
is performed. So on the next connection attempt to the server, if another error
is reported, a client abort is detected. But the shutdown for writes was already
done. So the transition to the state SI_ST_DIS is impossible. We stay in the
state SI_ST_RDY. Because it is a transient state, we loop in process_stream() to
perform the transition.
It is hard to understand how the bug happens reading the code and even harder to
explain. But there is a trivial way to hit the bug by sending h2 requests to a
server only speaking h1. For instance, with the following config :
listen tst
bind *:80
server www 127.0.0.1:8000 proto h2 # in reality, it is a HTTP/1.1 server
It is a configuration error, but it is an easy way to observe the bug. Note it
may happen with a valid configuration.
So, after a careful analyzis, it appears that si_cs_recv() should never be
called for a not fully established stream-interface. This way the connection
retries will be performed before reporting an error to the client. Thus, if a
shutdown is performed because a read0 is handled, the stream-interface is
inconditionnaly set to the transient state SI_ST_DIS.
This patch must be backported to 2.0 and 1.9. However on these versions, this
patch reveals a design flaw about connections and a bad way to perform the
connection retries. We are working on it.
BUG/MINOR: mux-h2: Don't pretend mux buffers aren't full anymore if nothing sent
In h2_send(), when something is sent, we remove the flags
(H2_CF_MUX_MFULL|H2_CF_DEM_MROOM) on the h2 connection. This way, we are able to
wake up all streams waiting to send data. Unfortunatly, these flags are
unconditionally removed, even when nothing was sent. So if the h2c is blocked
because the mux buffers are full and we are unable to send anything, all streams
in the send_list are woken up for nothing. Now, we only remove these flags if at
least a send succeeds.
BUG/MINOR: cli: don't call the kw->io_release if kw->parse failed
The io_release() callback of the cli_kw is supposed to be used to clean
what an io_handler() has made. It is called once the work in the IO
handler is finished, or when the connection was aborted by the client.
This patch fixes a bug where the io_release callback was called even
when the parse() callback failed. Which means that the io_release() could
called even if the io_handler() was not called.
Should be backported in every versions that have a cli_kw->release().
(as far as 1.7)
Willy Tarreau [Fri, 25 Oct 2019 13:48:53 +0000 (15:48 +0200)]
[RELEASE] Released version 2.1-dev3
Released version 2.1-dev3 with the following main changes :
- MINOR: mux-h2/trace: missing conn pointer in demux full message
- MINOR: mux-h2: add a per-connection list of blocked streams
- BUILD: ebtree: make eb_is_empty() and eb_is_dup() take a const
- BUG/MEDIUM: mux-h2: do not enforce timeout on long connections
- BUG/MEDIUM: tasks: Don't forget to decrement tasks_run_queue.
- BUG/MINOR: peers: crash on reload without local peer.
- BUG/MINOR: mux-h2/trace: Fix traces on h2c initialization
- MINOR: h1-htx: Update h1_copy_msg_data() to ease the traces in the mux-h1
- MINOR: htx: Adapt htx_dump() to be used from traces
- MINOR: mux-h1/trace: register a new trace source with its events
- MINOR: proxy: Store http-send-name-header in lower case
- MINOR: http: Remove headers matching the name of http-send-name-header option
- BUG/MINOR: mux-h1: Adjust header case when the server name is add to a request
- BUG/MINOR: mux-h1: Adjust header case when chunked encoding is add to a message
- MINOR: mux-h1: Try to wakeup the stream on output buffer allocation
- MINOR: fcgi: Add function to get the string representation of a record type
- MINOR: mux-fcgi/trace: Register a new trace source with its events
- BUG/MEDIUM: cache: make sure not to cache requests with absolute-uri
- DOC: clarify some points around http-send-name-header's behavior
- MEDIUM: mux-h2: support emitting CONTINUATION frames after HEADERS
- BUG/MINOR: mux-h1/mux-fcgi/trace: Fix position of the 4th arg in some traces
- DOC: fix typo in Prometheus exporter doc
- MINOR: h2: clarify the rules for how to convert an H2 request to HTX
- MINOR: htx: Add 2 flags on the start-line to have more info about the uri
- MINOR: http: Add a function to get the authority into a URI
- MINOR: h1-htx: Set the flag HTX_SL_F_HAS_AUTHORITY during the request parsing
- MEDIUM: http-htx: Keep the Host header and the request start-line synchronized
- MINOR: h1-htx: Only use the path of a normalized URI to format a request line
- MEDIUM: h2: make the request parser rebuild a complete URI
- MINOR: h2: report in the HTX flags when the request has an authority
- MEDIUM: mux-h2: do not map Host to :authority on output
- MEDIUM: h2: use the normalized URI encoding for absolute form requests
- MINOR: stats: mention in the help message support for "json" and "typed"
- MINOR: stats: get rid of the ST_CONVDONE flag
- MINOR: stats: replace the ST_* uri_auth flags with STAT_*
- MINOR: stats: always merge the uri_auth flags into the appctx flags
- MINOR: stats: set the appctx flags when initializing the applet only
- MINOR: stats: get rid of the STAT_SHOWADMIN flag
- MINOR: stats: make stats_dump_fields_json() directly take flags
- MINOR: stats: uniformize the calling convention of the dump functions
- MINOR: stats: support the "desc" output format modifier for info and stat
- MINOR: stats: prepare to add a description with each stat/info field
- MINOR: stats: make "show stat" and "show info"
- MINOR: stats: fill all the descriptions for "show info" and "show stat"
- BUG/MEDIUM: applet: always check a fast running applet's activity before killing
- BUILD: stats: fix missing '=' sign in array declaration
- MINOR: lists: add new macro LIST_SPLICE_END_DETACHED
- MINOR: list: add new macro MT_LIST_BEHEAD
- MEDIUM: task: Split the tasklet list into two lists.
- MINOR: h2: Document traps to be avoided on multithread.
- MINOR: lists: Try to use local variables instead of macro arguments.
- MINOR: lists: Fix alignement of \ when relevant.
- MINOR: mux-h2: also support emitting CONTINUATION on trailers
- MINOR: ssl: crt-list do ckchn_lookup
- REORG: ssl: rename ckch_node to ckch_store
- REORG: ssl: move structures to ssl_sock.h
- MINOR: ssl: initialize the sni_keytypes_map as EB_ROOT
- MINOR: ssl: initialize explicitly the sni_ctx trees
- BUG/MINOR: ssl: abort on sni allocation failure
- BUG/MINOR: ssl: free the sni_keytype nodes
- BUG/MINOR: ssl: abort on sni_keytypes allocation failure
- MEDIUM: ssl: introduce the ckch instance structure
- MEDIUM: ssl: split ssl_sock_add_cert_sni()
- MINOR: ssl: ssl_sock_load_ckchn() can properly fail
- MINOR: ssl: ssl_sock_load_multi_ckchs() can properly fail
- MEDIUM: ssl: ssl_sock_load_ckchs() alloc a ckch_inst
- MINOR: ssl: ssl_sock_load_crt_file_into_ckch() is filling from a BIO
- MEDIUM: ssl/cli: 'set ssl cert' updates a certificate from the CLI
- MINOR: ssl: load the sctl in/from the ckch
- MINOR: ssl: load the ocsp in/from the ckch
- BUG/MEDIUM: ssl: NULL dereference in ssl_sock_load_cert_sni()
- BUG/MINOR: ssl: fix build without SSL
- BUG/MINOR: ssl: fix build without multi-cert bundles
- BUILD: ssl: wrong #ifdef for SSL engines code
- BUG/MINOR: ssl: fix OCSP build with BoringSSL
- BUG/MEDIUM: htx: Catch chunk_memcat() failures when HTX data are formatted to h1
- BUG/MINOR: chunk: Fix tests on the chunk size in functions copying data
- BUG/MINOR: mux-h1: Mark the output buffer as full when the xfer is interrupted
- MINOR: mux-h1: Xfer as much payload data as possible during output processing
- CLEANUP: h1-htx: Move htx-to-h1 formatting functions from htx.c to h1_htx.c
- BUG/MINOR: mux-h1: Capture ignored parsing errors
- MINOR: h1: Reject requests with different occurrences of the header host
- MINOR: h1: Reject requests if the authority does not match the header host
- REGTESTS: Send valid URIs in peers reg-tests and fix HA config to avoid warnings
- REGTESTS: Adapt proxy_protocol_random_fail.vtc to match normalized URI too
- BUG/MINOR: WURFL: fix send_log() function arguments
- BUG/MINOR: ssl: fix error messages for OCSP loading
- BUG/MINOR: ssl: can't load ocsp files
- MINOR: version: make the version strings variables, not constants
- BUG/MINOR: http-htx: Properly set htx flags on error files to support keep-alive
- MINOR: htx: Add a flag on HTX to known when a response was generated by HAProxy
- MINOR: mux-h1: Force close mode for proxy responses with an unfinished request
- BUILD: travis-ci: limit build to branches "master" and "next"
- BUILD/MEDIUM: threads: rename thread_info struct to ha_thread_info
- BUILD/SMALL: threads: enable threads on osx
- BUILD/MEDIUM: threads: enable cpu_affinity on osx
- MINOR: istbuf: add b_fromist() to make a buffer from an ist
- BUG/MINOR: cache: also cache absolute URIs
- BUG/MINOR: mworker/ssl: close openssl FDs unconditionally
- BUG/MINOR: tcp: Don't alter counters returned by tcp info fetchers
- BUG/MEDIUM: lists: Handle 1-element-lists in MT_LIST_BEHEAD().
- BUG/MEDIUM: mux_pt: Make sure we don't have a conn_stream before freeing.
- BUG/MEDIUM: tasklet: properly compute the sleeping threads mask in tasklet_wakeup()
- BUG/MAJOR: idle conns: schedule the cleanup task on the correct threads
- BUG/MEDIUM: task: make tasklets either local or shared but not both at once
- Revert e8826ded5fea3593d89da2be5c2d81c522070995.
- BUG/MEDIUM: mux_pt: Don't destroy the connection if we have a stream attached.
- BUG/MEDIUM: mux_pt: Only call the wake emthod if nobody subscribed to receive.
- REGTEST: mcli/mcli_show_info: launch a 'show info' on the master CLI
- CLEANUP: ssl: make ssl_sock_load_cert*() return real error codes
- CLEANUP: ssl: make ssl_sock_load_ckchs() return a set of ERR_*
- CLEANUP: ssl: make cli_parse_set_cert handle errcode and warnings.
- CLEANUP: ssl: make ckch_inst_new_load_(multi_)store handle errcode/warn
- CLEANUP: ssl: make ssl_sock_put_ckch_into_ctx handle errcode/warn
- CLEANUP: ssl: make ssl_sock_load_dh_params handle errcode/warn
- CLEANUP: bind: handle warning label on bind keywords parsing.
- BUG/MEDIUM: ssl: 'tune.ssl.default-dh-param' value ignored with openssl > 1.1.1
- BUG/MINOR: mworker/cli: reload fail with inherited FD
- BUG/MINOR: ssl: Fix fd leak on error path when a TLS ticket keys file is parsed
- BUG/MINOR: stick-table: Never exceed (MAX_SESS_STKCTR-1) when fetching a stkctr
- BUG/MINOR: cache: alloc shctx after check config
- BUG/MINOR: sample: Make the `field` converter compatible with `-m found`
- BUG/MINOR: server: check return value of fopen() in apply_server_state()
- REGTESTS: make seamless-reload depend on 1.9 and above
- REGTESTS: server/cli_set_fqdn requires version 1.8 minimum
- BUG/MINOR: dns: allow srv record weight set to 0
- BUG/MINOR: ssl: fix memcpy overlap without consequences.
- BUG/MINOR: stick-table: fix an incorrect 32 to 64 bit key conversion
- BUG/MEDIUM: pattern: make the pattern LRU cache thread-local and lockless
- BUG/MINOR: mux-h2: do not emit logs on backend connections
- CLEANUP: ssl: remove old TODO commentary
- CLEANUP: ssl: fix SNI/CKCH lock labels
- MINOR: ssl: OCSP functions can load from file or buffer
- MINOR: ssl: load sctl from buf OR from a file
- MINOR: ssl: load issuer from file or from buffer
- MINOR: ssl: split ssl_sock_load_crt_file_into_ckch()
- BUG/MINOR: ssl/cli: fix looking up for a bundle
- MINOR: ssl/cli: update ocsp/issuer/sctl file from the CLI
- MINOR: ssl: update ssl_sock_free_cert_key_and_chain_contents
- MINOR: ssl: copy a ckch from src to dst
- MINOR: ssl: new functions duplicate and free a ckch_store
- MINOR: ssl/cli: assignate a new ckch_store
- MEDIUM: cli/ssl: handle the creation of SSL_CTX in an IO handler
- BUG/MINOR: ssl/cli: fix build of SCTL and OCSP
- BUG/MINOR: ssl/cli: out of bounds when built without ocsp/sctl
- BUG/MINOR: ssl: fix build with openssl < 1.1.0
- BUG/MINOR: ssl: fix build of X509_chain_up_ref() w/ libreSSL
- MINOR: tcp: avoid confusion in time parsing init
- MINOR: debug: add a new "debug dev stream" command
- MINOR: cli/debug: validate addresses using may_access() in "debug dev stream"
- REORG: move CLI access level definitions to cli.h
- MINOR: cli: add an expert mode to hide dangerous commands
- MINOR: debug: make most debug CLI commands accessible in expert mode
- MINOR: stats/debug: maintain a counter of debug commands issued
- BUG/MEDIUM: debug: address a possible null pointer dereference in "debug dev stream"
Willy Tarreau [Fri, 25 Oct 2019 08:06:55 +0000 (10:06 +0200)]
BUG/MEDIUM: debug: address a possible null pointer dereference in "debug dev stream"
As reported in issue #343, there is one case where a NULL stream can
still be dereferenced, when getting &s->txn->flags. Let's protect all
assignments to stay on the safe side for future additions.
Willy Tarreau [Thu, 24 Oct 2019 16:18:02 +0000 (18:18 +0200)]
MINOR: stats/debug: maintain a counter of debug commands issued
Debug commands will usually mark the fate of the process. We'd rather
have them counted and visible in a core or in stats output than trying
to guess how a flag combination could happen. The counter is only
incremented when the command is about to be issued however, so that
failed attempts are ignored.
Willy Tarreau [Thu, 24 Oct 2019 16:03:39 +0000 (18:03 +0200)]
MINOR: debug: make most debug CLI commands accessible in expert mode
Instead of relying on DEBUG_DEV for most debugging commands, which is
limiting, let's condition them to expert mode. Only one ("debug dev exec")
remains conditionned to DEBUG_DEV because it can have a security implication
on the system. The commands are not listed unless "expert-mode on" was first
entered on the CLI :
> expert-mode on
> help
debug dev close <fd> : close this file descriptor
debug dev delay [ms] : sleep this long
debug dev exec [cmd] ... : show this command's output
debug dev exit [code] : immediately exit the process
debug dev hex <addr> [len]: dump a memory area
debug dev log [msg] ... : send this msg to global logs
debug dev loop [ms] : loop this long
debug dev panic : immediately trigger a panic
debug dev stream ... : show/manipulate stream flags
debug dev tkill [thr] [sig] : send signal to thread
Willy Tarreau [Thu, 24 Oct 2019 15:55:53 +0000 (17:55 +0200)]
MINOR: cli: add an expert mode to hide dangerous commands
Some commands like the debug ones are not enabled by default but can be
useful on some production environments. In order to avoid the temptation
of using them incorrectly, let's introduce an "expert" mode for a CLI
connection, which allows some commands to appear and be used. It is
enabled by command "expert-mode on" which is not listed by default.
Willy Tarreau [Thu, 24 Oct 2019 16:28:23 +0000 (18:28 +0200)]
MINOR: cli/debug: validate addresses using may_access() in "debug dev stream"
This function adds some control by verifying that the target address is
really readable. It will not protect against writing to wrong places,
but will at least protect against a large number of mistakes such as
incorrectly copy-pasted addresses.
Willy Tarreau [Wed, 23 Oct 2019 15:23:25 +0000 (17:23 +0200)]
MINOR: debug: add a new "debug dev stream" command
This new "debug dev stream" command allows to manipulate flags, timeouts,
states for streams, channels and stream interfaces, as well as waking a
stream up. These may be used to help reproduce certain bugs during
development. The operations are performed to the stream assigned by
"strm" which defaults to the CLI's stream. This stream pointer can be
chosen from one of those reported in "show sess". Example:
William Dauchy [Wed, 23 Oct 2019 17:31:36 +0000 (19:31 +0200)]
MINOR: tcp: avoid confusion in time parsing init
We never enter val_fc_time_value when an associated fetcher such as `fc_rtt` is
called without argument. meaning `type == ARGT_STOP` will never be true and so
the default `data.sint = TIME_UNIT_MS` will never be set. remove this part to
avoid thinking default data.sint is set to ms while reading the code.
Signed-off-by: William Dauchy <w.dauchy@criteo.com>
[Cf: This patch may safely backported as far as 1.7. But no matter if not.]
8c1cddef ("MINOR: ssl: new functions duplicate and free a ckch_store")
use some OpenSSL refcount functions that were introduced in OpenSSL
1.0.2 and OpenSSL 1.1.0.
Fix the problem by introducing them in openssl-compat.h.
BUG/MINOR: ssl/cli: out of bounds when built without ocsp/sctl
Commit 541a534 ("BUG/MINOR: ssl/cli: fix build of SCTL and OCSP")
introduced a bug in which we iterate outside the array durint a 'set ssl
cert' if we didn't built with the ocsp or sctl.
When updating a certificate from the CLI, it is not possible to revert
some of the changes if part of the certicate update failed. We now
creates a copy of the ckch_store for the changes so we can revert back
if something goes wrong.
Even if the ckch_store was affected before this change, it wasn't
affecting the SSL_CTXs used for the traffic. It was only a problem if we
try to update a certificate after we failed to do it the first time.
The new ckch_store is also linked to the new sni_ctxs so it's easy to
insert the sni_ctxs before removing the old ones.
MINOR: ssl/cli: update ocsp/issuer/sctl file from the CLI
It is now possible to update new parts of a CKCH from the CLI.
Currently you will be able to update a PEM (by default), a OCSP response
in base64, an issuer file, and a SCTL file.
Each update will creates a new CKCH and new sni_ctx structure so we will
need a "commit" command later to apply several changes and create the
sni_ctx only once.
Split the ssl_sock_load_crt_file_into_ckch() in two functions:
- ssl_sock_load_files_into_ckch() which is dedicated to opening every
files related to a filename during the configuration parsing (PEM, sctl,
ocsp, issuer etc)
- ssl_sock_load_pem_into_ckch() which is dedicated to opening a PEM,
either in a file or a buffer
Willy Tarreau [Wed, 23 Oct 2019 09:06:35 +0000 (11:06 +0200)]
BUG/MINOR: mux-h2: do not emit logs on backend connections
The logs were added to the H2 mux so that we can report logs in case
of errors that prevent a stream from being created, but as a side effect
these logs are emitted twice for backend connections: once by the H2 mux
itself and another time by the upper layer stream. It can even happen
more with connection retries.
This patch makes sure we do not emit logs for backend connections.
Willy Tarreau [Wed, 23 Oct 2019 04:59:31 +0000 (06:59 +0200)]
BUG/MEDIUM: pattern: make the pattern LRU cache thread-local and lockless
As reported in issue #335, a lot of contention happens on the PATLRU lock
when performing expensive regex lookups. This is absurd since the purpose
of the LRU cache was to have a fast cache for expressions, thus the cache
must not be shared between threads and must remain lockless.
This commit makes the LRU cache thread-local and gets rid of the PATLRU
lock. A test with 7 threads on 4 cores climbed from 67kH/s to 369kH/s,
or a scalability factor of 5.5.
Given the huge performance difference and the regression caused to
users migrating from processes to threads, this should be backported at
least to 2.0.
Thanks to Brian Diekelman for his detailed report about this regression.
Willy Tarreau [Wed, 23 Oct 2019 04:21:05 +0000 (06:21 +0200)]
BUG/MINOR: stick-table: fix an incorrect 32 to 64 bit key conversion
As reported in issue #331, the code used to cast a 32-bit to a 64-bit
stick-table key is wrong. It only copies the 32 lower bits in place on
little endian machines or overwrites the 32 higher ones on big endian
machines. It ought to simply remove the wrong cast dereference.
This bug was introduced when changing stick table keys to samples in
1.6-dev4 by commit bc8c404449 ("MAJOR: stick-tables: use sample types
in place of dedicated types") so it the fix must be backported as far
as 1.6.
Emeric Brun [Tue, 8 Oct 2019 16:27:37 +0000 (18:27 +0200)]
BUG/MINOR: ssl: fix memcpy overlap without consequences.
A trick is used to set SESSION_ID, and SESSION_ID_CONTEXT lengths
to 0 and avoid ASN1 encoding of these values.
There is no specific function to set the length of those parameters
to 0 so we fake this calling these function to a different value
with the same buffer but a length to zero.
But those functions don't seem to check the length of zero before
performing a memcpy of length zero but with src and dst buf on the
same pointer, causing valgrind to bark.
So the code was re-work to pass them different pointers even
if buffer content is un-used.
In a second time, reseting value, a memcpy overlap
happened on the SESSION_ID_CONTEXT. It was re-worked and this is
now reset using the constant global value SHCTX_APPNAME which is a
different pointer with the same content.
This patch should be backported in every version since ssl
support was added to haproxy if we want valgrind to shut up.
This is tracked in github issue #56.
Baptiste Assmann [Mon, 21 Oct 2019 13:13:48 +0000 (15:13 +0200)]
BUG/MINOR: dns: allow srv record weight set to 0
Processing of SRV record weight was inaccurate and when a SRV record's
weight was set to 0, HAProxy enforced it to '1'.
This patch aims at fixing this without breaking compability with
previous behavior.
Willy Tarreau [Tue, 22 Oct 2019 08:42:10 +0000 (10:42 +0200)]
REGTESTS: make seamless-reload depend on 1.9 and above
Since latest updates, vtest requires the master CLI when running in
master-worker mode, and this one is only available starting with 1.9.
The seamless reload test is the only one depending on this and now
fails on 1.8, so let's adjust it accordingly.
When running haproxy -c, the cache parser is trying to allocate the size
of the cache. This can be a problem in an environment where the RAM is
limited.
This patch moves the cache allocation in the post_check callback which
is not executed during a -c.
This patch may be backported at least to 2.0 and 1.9. In 1.9, the callbacks
registration mechanism is not the same. So the patch will have to be adapted. No
need to backport it to 1.8, the code is probably too different.
BUG/MINOR: stick-table: Never exceed (MAX_SESS_STKCTR-1) when fetching a stkctr
When a stick counter is fetched, it is important that the requested counter does
not exceed (MAX_SESS_STKCTR -1). Actually, there is no bug with a default build
because, by construction, MAX_SESS_STKCTR is defined to 3 and we know that we
never exceed the max value. scN_* sample fetches are numbered from 0 to 2. For
other sample fetches, the value is tested.
But there is a bug if MAX_SESS_STKCTR is set to a lower value. For instance
1. In this case the counters sc1_* and sc2_* may be undefined.
This patch fixes the issue #330. It must be backported as far as 1.7.
BUG/MINOR: ssl: Fix fd leak on error path when a TLS ticket keys file is parsed
When an error occurred in the function bind_parse_tls_ticket_keys(), during the
configuration parsing, the opened file is not always closed. To fix the bug, all
errors are catched at the same place, where all ressources are released.
This patch fixes the bug #325. It must be backported as far as 1.7.
BUG/MINOR: mworker/cli: reload fail with inherited FD
When using the master CLI with 'fd@', during a reload, the master CLI
proxy is stopped. Unfortunately if this is an inherited FD it is closed
too, and the master CLI won't be able to bind again during the
re-execution. It lead the master to fallback in waitpid mode.
This patch forbids the inherited FDs in the master's listeners to be
closed during a proxy_stop().
This patch is mandatory to use the -W option in VTest versions that contain the
-mcli feature.
(https://github.com/vtest/VTest/commit/86e65f1024453b1074d239a88330b5150d3e44bb)
Emeric Brun [Thu, 17 Oct 2019 12:53:03 +0000 (14:53 +0200)]
BUG/MEDIUM: ssl: 'tune.ssl.default-dh-param' value ignored with openssl > 1.1.1
If openssl 1.1.1 is used, c2aae74f0 commit mistakenly enables DH automatic
feature from openssl instead of ECDH automatic feature. There is no impact for
the ECDH one because the feature is always enabled for that version. But doing
this, the 'tune.ssl.default-dh-param' was completely ignored for DH parameters.
This patch fix the bug calling 'SSL_CTX_set_ecdh_auto' instead of
'SSL_CTX_set_dh_auto'.
Currently some users may use a 2048 DH bits parameter, thinking they're using a
1024 bits one. Doing this, they may experience performance issue on light hardware.
This patch warns the user if haproxy fails to configure the given DH parameter.
In this case and if openssl version is > 1.1.0, haproxy will let openssl to
automatically choose a default DH parameter. For other openssl versions, the DH
ciphers won't be usable.
A commonly case of failure is due to the security level of openssl.cnf
which could refuse a 1024 bits DH parameter for a 2048 bits key:
Emeric Brun [Thu, 17 Oct 2019 11:27:40 +0000 (13:27 +0200)]
CLEANUP: ssl: make ssl_sock_load_dh_params handle errcode/warn
ssl_sock_load_dh_params used to return >0 or -1 to indicate success
or failure. Make it return a set of ERR_* instead so that its callers
can transparently report its status. Given that its callers only used
to know about ERR_ALERT | ERR_FATAL, this is the only code returned
for now. An error message was added in the case of failure and the
comment was updated.
Emeric Brun [Thu, 17 Oct 2019 11:25:14 +0000 (13:25 +0200)]
CLEANUP: ssl: make ssl_sock_put_ckch_into_ctx handle errcode/warn
ssl_sock_put_ckch_into_ctx used to return 0 or >0 to indicate success
or failure. Make it return a set of ERR_* instead so that its callers
can transparently report its status. Given that its callers only used
to know about ERR_ALERT | ERR_FATAL, this is the only code returned
for now. And a comment was updated.
Emeric Brun [Thu, 17 Oct 2019 11:16:58 +0000 (13:16 +0200)]
CLEANUP: ssl: make ckch_inst_new_load_(multi_)store handle errcode/warn
ckch_inst_new_load_store() and ckch_inst_new_load_multi_store used to
return 0 or >0 to indicate success or failure. Make it return a set of
ERR_* instead so that its callers can transparently report its status.
Given that its callers only used to know about ERR_ALERT | ERR_FATAL,
his is the only code returned for now. And the comment was updated.
Willy Tarreau [Wed, 16 Oct 2019 15:06:25 +0000 (17:06 +0200)]
CLEANUP: ssl: make ssl_sock_load_ckchs() return a set of ERR_*
ssl_sock_load_ckchs() used to return 0 or >0 to indicate success or
failure even though this was not documented. Make it return a set of
ERR_* instead so that its callers can transparently report its status.
Given that its callers only used to know about ERR_ALERT | ERR_FATAL,
this is the only code returned for now. And a comment was added.
Willy Tarreau [Wed, 16 Oct 2019 14:42:19 +0000 (16:42 +0200)]
CLEANUP: ssl: make ssl_sock_load_cert*() return real error codes
These functions were returning only 0 or 1 to mention success or error,
and made it impossible to return a warning. Let's make them return error
codes from ERR_* and map all errors to ERR_ALERT|ERR_FATAL for now since
this is the only code that was set on non-zero return value.
In addition some missing comments were added or adjusted around the
functions' return values.
REGTEST: mcli/mcli_show_info: launch a 'show info' on the master CLI
This test launches a HAProxy process in master worker with 'nbproc 4'.
It sends a "show info" to the process 3 and verify that the right
process replied.
This regtest depends on the support of the master CLI for VTest.
Olivier Houchard [Fri, 18 Oct 2019 12:18:29 +0000 (14:18 +0200)]
BUG/MEDIUM: mux_pt: Only call the wake emthod if nobody subscribed to receive.
In mux_pt_io_cb(), instead of always calling the wake method, only do so
if nobody subscribed for receive. If we have a subscription, just wake the
associated tasklet up.
Olivier Houchard [Fri, 18 Oct 2019 11:56:40 +0000 (13:56 +0200)]
BUG/MEDIUM: mux_pt: Don't destroy the connection if we have a stream attached.
There's a small window where the mux_pt tasklet may be woken up, and thus
mux_pt_io_cb() get scheduled, and then the connection is attached to a new
stream. If this happen, don't do anything, and just let the stream know
by calling its wake method. If the connection had an error, the stream should
take care of destroying it by calling the detach method.
This reverts commit "BUG/MEDIUM: mux_pt: Make sure we don't have a
conn_stream before freeing.".
mux_pt_io_cb() is only used if we have no associated stream, so we will
never have a cs, so there's no need to check that, and we of course have to
destroy the mux in mux_pt_detach() if we have no associated session, or if
there's an error on the connection.
Willy Tarreau [Fri, 18 Oct 2019 04:43:53 +0000 (06:43 +0200)]
BUG/MEDIUM: task: make tasklets either local or shared but not both at once
Tasklets may be woken up to run on the calling thread or by a specific thread
(the owner). But since we use a non-thread safe mechanism when the calling
thread is also the for the owner, there may sometimes be collisions when two
threads decide to wake the same tasklet up at the same time and one of them
is the owner.
This is more of a matter of usage than code, in that a tasklet usually is
designed to be woken up and executed on the calling thread only (most cases)
or on a specific thread. Thus it is a property of the tasklet itself as this
solely depends how the code is constructed around it.
This patch performs a small change to address this. By default tasklet_new()
creates a "local" tasklet, which will run on the calling thread, like in 2.0.
This is done by setting tl->tid to a negative value. If the caller wants the
tasklet to run exclusively on a specific thread, it just has to set tl->tid,
which is already what shared tasklet callers do anyway.
Willy Tarreau [Fri, 18 Oct 2019 06:50:49 +0000 (08:50 +0200)]
BUG/MAJOR: idle conns: schedule the cleanup task on the correct threads
The idle cleanup tasks' masks are wrong for threads 32 to 64, which
causes the wrong thread to wake up and clean the connections that it
does not own, with a risk of crash or infinite loop depending on
concurrent accesses. For thread 32, any thread between 32 and 64 will
be woken up, but for threads 33 to 64, in fact threads 1 to 32 will
run the task instead.
This issue only affects deployments enabling more than 32 threads. While
is it not common in 1.9 where this has to be explicit, and can easily be
dealt with by lowering the number of threads, it can be more common in
2.0 since by default the thread count is determined based on the number
of available processors, hence the MAJOR tag which is mostly relevant
to 2.x.
The problem was first introduced into 1.9-dev9 by commit 0c18a6fe3
("MEDIUM: servers: Add a way to keep idle connections alive.") and was
later moved to cfgparse.c by commit 980855bd9 ("BUG/MEDIUM: server:
initialize the orphaned conns lists and tasks at the end").
This patch needs to be backported as far as 1.9, with care as 1.9 is
slightly different there (uses idle_task[] instead of idle_conn_cleanup[]
like in 2.x).
Willy Tarreau [Fri, 18 Oct 2019 06:45:41 +0000 (08:45 +0200)]
BUG/MEDIUM: tasklet: properly compute the sleeping threads mask in tasklet_wakeup()
The use of ~(1 << tid) to compute the sleeping_mask in tasklet_wakeup()
will result in breakage above 32 threads, because (1<<31) = 0xFFFFFFFF8000000,
and upper values will lead to theorically undefined results, but practically
will wrap over 0x1 to 0x80000000 again and indicate wrong sleeping masks. It
seems that the main visible effect maybe extra latency on some threads or
short CPU loops on others.
Olivier Houchard [Thu, 17 Oct 2019 16:02:53 +0000 (18:02 +0200)]
BUG/MEDIUM: mux_pt: Make sure we don't have a conn_stream before freeing.
On error, make sure we don't have a conn_stream before freeing the connection
and the associated mux context. Otherwise a stream will still reference
the connection, and attempt to use it.
If we still have a conn_stream, it will properly be free'd when the detach
method is called, anyway.
Olivier Houchard [Thu, 17 Oct 2019 15:46:01 +0000 (17:46 +0200)]
BUG/MEDIUM: lists: Handle 1-element-lists in MT_LIST_BEHEAD().
In MT_LIST_BEHEAD(), explicitely set the next element of the prev to NULL,
instead of setting it to the prev of the next. If we only had one element,
then we'd set the next and the prev to the element itself, and thus it would
make the element appear to be outside any list.
BUG/MINOR: tcp: Don't alter counters returned by tcp info fetchers
There are 2 kinds of tcp info fetchers. Those returning a time value (fc_rtt and
fc_rttval) and those returning a counter (fc_unacked, fc_sacked, fc_retrans,
fc_fackets, fc_lost, fc_reordering). Because of a bug, the counters were handled
as time values, and by default, were divided by 1000 (because of an invalid
conversion from us to ms). To work around this bug and have the right value, the
argument "us" had to be specified.
So now, tcp info fetchers returning a counter don't support any argument
anymore. To not break old configurations, if an argument is provided, it is
ignored and a warning is emitted during the configuration parsing.
In addition, parameter validiation is now performed during the configuration
parsing.
BUG/MINOR: mworker/ssl: close openssl FDs unconditionally
Patch 56996da ("BUG/MINOR: mworker/ssl: close OpenSSL FDs on reload")
fixes a issue where the /dev/random FD was leaked by OpenSSL upon a
reload in master worker mode. Indeed the FD was not flagged with
CLOEXEC.
The fix was checking if ssl_used_frontend or ssl_used_backend were set
to close the FD. This is wrong, indeed the lua init code creates an SSL
server without increasing the backend value, so the deinit is never
done when you don't use SSL in your configuration.
To reproduce the problem you just need to build haproxy with openssl and
lua with an openssl which does not use the getrandom() syscall. No
openssl nor lua configuration are required for haproxy.
Willy Tarreau [Thu, 17 Oct 2019 07:28:28 +0000 (09:28 +0200)]
BUG/MINOR: cache: also cache absolute URIs
The recent changes to address URI issues mixed with the recent fix to
stop caching absolute URIs have caused the cache not to cache H2 requests
anymore since these ones come with a scheme and authority. Let's unbreak
this by using absolute URIs all the time, now that we keep host and
authority in sync. So what is done now is that if we have an authority,
we take the whole URI as it is as the cache key. This covers H2 and H1
absolute requests. If no authority is present (most H1 origin requests),
then we prepend "https://" and the Host header. The reason for https://
is that most of the time we don't care about the scheme, but since about
all H2 clients use this scheme, at least we can share the cache between
H1 and H2.
No backport is needed since the breakage only affects 2.1-dev.
Willy Tarreau [Thu, 17 Oct 2019 08:38:10 +0000 (10:38 +0200)]
MINOR: istbuf: add b_fromist() to make a buffer from an ist
A lot of our chunk-based functions are able to work on a buffer pointer
but not on an ist. Instead of duplicating all of them to also take an
ist as a source, let's have a macro to make a temporary dummy buffer
from an ist. This will only result in structure field manipulations
that the compiler will quickly figure to eliminate them with inline
functions, and in other cases it will just use 4 words in the stack
before calling a function, instead of performing intermediary
conversions.
Willy Tarreau [Thu, 17 Oct 2019 04:53:55 +0000 (06:53 +0200)]
BUILD: travis-ci: limit build to branches "master" and "next"
Occasionally some short-lived branches are pushed to help developers
rebase their work, these ones do not need to be built. This patch
explicitly lists "master" and "next" as the two only branches of
interest. It also adds a comment with the URL for the build status.
MINOR: mux-h1: Force close mode for proxy responses with an unfinished request
When a response generated by HAProxy is handled by the mux H1, if the
corresponding request has not fully been received, the close mode is
forced. Thus, the client is notified the connection will certainly be closed
abruptly, without waiting the end of the request.
MINOR: htx: Add a flag on HTX to known when a response was generated by HAProxy
The flag HTX_FL_PROXY_RESP is now set on responses generated by HAProxy,
excluding responses returned by applets and services. It is an informative flag
set by the applicative layer.
BUG/MINOR: http-htx: Properly set htx flags on error files to support keep-alive
When an error file was loaded, the flag HTX_SL_F_XFER_LEN was never set on the
HTX start line because of a bug. During the headers parsing, the flag
H1_MF_XFER_LEN is never set on the h1m. But it was the condition to set
HTX_SL_F_XFER_LEN on the HTX start-line. Instead, we must only rely on the flags
H1_MF_CLEN or H1_MF_CHNK.
Because of this bug, it was impossible to keep a connection alive for a response
generated by HAProxy. Now the flag HTX_SL_F_XFER_LEN is set when an error file
have a content length (chunked responses are unsupported at this stage) and the
connection may be kept alive if there is no connection header specified to
explicitly close it.
Willy Tarreau [Wed, 16 Oct 2019 07:44:55 +0000 (09:44 +0200)]
MINOR: version: make the version strings variables, not constants
It currently is not possible to figure the exact haproxy version from a
core file for the sole reason that the version is stored into a const
string and as such ends up in the .text section that is not part of a
core file. By turning them into variables we move them to the data
section and they appear in core files. In order to help finding them,
we just prepend an extra variable in front of them and we're able to
immediately spot the version strings from a core file:
(These are haproxy_version and haproxy_date respectively). This may be
backported to 2.0 since this part is not support to impact anything but
the developer's time spent debugging.
246c024 ("MINOR: ssl: load the ocsp in/from the ckch") broke the loading
of OCSP files. The function ssl_sock_load_ocsp_response_from_file() was
not returning 0 upon success which lead to an error after the .ocsp was
read.
BUG/MINOR: ssl: fix error messages for OCSP loading
The error messages for OCSP in ssl_sock_load_crt_file_into_ckch() add a
double extension to the filename, that can be confusing. The messages
reference a .issuer.issuer file.
Miroslav Zagorac [Mon, 14 Oct 2019 15:15:56 +0000 (17:15 +0200)]
BUG/MINOR: WURFL: fix send_log() function arguments
If the user agent data contains text that has special characters that
are used to format the output from the vfprintf() function, haproxy
crashes. String "%s %s %s" may be used as an example.
% curl -A "%s %s %s" localhost:10080/index.html
curl: (52) Empty reply from server
REGTESTS: Send valid URIs in peers reg-tests and fix HA config to avoid warnings
Absolute path must be used, otherwise, the requests are rejected by HAProxy
because of the recent changes. In addition, the configuration has been slightly
updated to remove warnings at startup.
MINOR: h1: Reject requests if the authority does not match the header host
As stated in the RCF7230#5.4, a client must send a field-value for the header
host that is identical to the authority if the target URI includes one. So, now,
by default, if the authority, when provided, does not match the value of the
header host, an error is triggered. To mitigate this behavior, it is possible to
set the option "accept-invalid-http-request". In that case, an http error is
captured without interrupting the request parsing.
MINOR: h1: Reject requests with different occurrences of the header host
There is no reason for a client to send several headers host. It even may be
considered as a bug. However, it is totally invalid to have different values for
those. So now, in such case, an error is triggered during the request
parsing. In addition, when several headers host are found with the same value,
only the first instance is kept and others are skipped.
When the option "accept-invalid-http-request" is enabled, some parsing errors
are ignored. But the position of the error is reported. In legacy HTTP mode,
such errors were captured. So, we now do the same in the H1 multiplexer.
If required, this patch may be backported to 2.0 and 1.9.
MINOR: mux-h1: Xfer as much payload data as possible during output processing
When an outgoing HTX message is formatted to a raw message, DATA blocks may be
splitted to not tranfser more data than expected. But if the buffer is almost
full, the formatting is interrupted, leaving some unused free space in the
buffer, because data are too large to be copied in one time.
Now, we transfer as much data as possible. When the message is chunked, we also
count the size used to encode the data.
BUG/MINOR: mux-h1: Mark the output buffer as full when the xfer is interrupted
When an outgoing HTX message is formatted to a raw message, if we fail to copy
data of an HTX block into the output buffer, we mark it as full. Before it was
only done calling the function buf_room_for_htx_data(). But this function is
designed to optimize input processing.
BUG/MINOR: chunk: Fix tests on the chunk size in functions copying data
When raw data are copied or appended in a chunk, the result must not exceed the
chunk size but it can reach it. Unlike functions to copy or append a string,
there is no terminating null byte.
This patch must be backported as far as 1.8. Note in 1.8, the functions
chunk_cpy() and chunk_cat() don't exist.