When compiled with Openssl >= 1.1.1, before attempting to do the handshake,
try to read any early data. If any early data is present, then we'll create
the session, read the data, and handle the request before we're doing the
handshake.
For this, we add a new connection flag, CO_FL_EARLY_SSL_HS, which is not
part of the CO_FL_HANDSHAKE set, allowing to proceed with a session even
before an SSL handshake is completed.
As early data do have security implication, we let the origin server know
the request comes from early data by adding the "Early-Data" header, as
specified in this draft from the HTTP working group :
Emmanuel Hocdet [Wed, 16 Aug 2017 09:33:17 +0000 (11:33 +0200)]
MINOR: ssl: support Openssl 1.1.1 early callback for switchctx
Use Openssl-1.1.1 SSL_CTX_set_client_hello_cb to mimic BoringSSL early callback.
Native multi certificate and SSL/TLS method per certificate is now supported by
Openssl >= 1.1.1.
Emmanuel Hocdet [Wed, 16 Aug 2017 09:28:44 +0000 (11:28 +0200)]
MEDIUM: ssl: convert CBS (BoringSSL api) usage to neutral code
switchctx early callback is only supported for BoringSSL. To prepare
the support of openssl 1.1.1 early callback, convert CBS api to neutral
code to work with any ssl libs.
This adds a new status code for use with the "http-request deny" ruleset.
The use case for this code is currently handled by this draft dedicated
to 0-RTT processing :
Thierry FOURNIER [Wed, 25 Oct 2017 10:59:51 +0000 (12:59 +0200)]
MINOR: hlua: Add regex class
This patch simply brings HAProxy internal regex system to the Lua API.
Lua doesn't embed regexes, now it inherits from the regexes compiled
with haproxy.
Dragan Dosen [Thu, 26 Oct 2017 09:25:10 +0000 (11:25 +0200)]
BUG/MEDIUM: prevent buffers being overwritten during build_logline() execution
Calls to build_logline() are audited in order to use dynamic trash buffers
allocated by alloc_trash_chunk() instead of global trash buffers.
This is similar to commits 07a0fec ("BUG/MEDIUM: http: Prevent
replace-header from overwriting a buffer") and 0d94576 ("BUG/MEDIUM: http:
prevent redirect from overwriting a buffer").
This patch should be backported in 1.7, 1.6 and 1.5. It relies on commit b686afd ("MINOR: chunks: implement a simple dynamic allocator for trash
buffers") for the trash allocator, which has to be backported as well.
Willy Tarreau [Thu, 26 Oct 2017 13:26:17 +0000 (15:26 +0200)]
MINOR: buffer: add the buffer input manipulation functions
We used to have bo_{get,put}_{chr,blk,str} to retrieve/send data to
the output area of a buffer, but not the equivalent ones for the input
area. This will be needed to copy uploaded data frames in HTTP/2.
Olivier Houchard [Tue, 24 Oct 2017 17:03:30 +0000 (19:03 +0200)]
BUG/MINOR: checks: Don't forget to release the connection on error case.
When switching the check code to a non-permanent connection, the new code
forgot to free the connection if an error happened and was returned by
connect_conn_chk(), leading to the check never be ran again.
Willy Tarreau [Wed, 25 Oct 2017 07:32:15 +0000 (09:32 +0200)]
MINOR: ssl_sock: make use of CO_FL_WILL_UPDATE
Now when ssl_sock_{to,from}_buf are called, if the connection doesn't
feature CO_FL_WILL_UPDATE, they will first retrieve the updated flags
using conn_refresh_polling_flags() before changing any flag, then call
conn_cond_update_sock_polling() before leaving, to commit such changes.
Willy Tarreau [Wed, 25 Oct 2017 07:30:13 +0000 (09:30 +0200)]
MINOR: raw_sock: make use of CO_FL_WILL_UPDATE
Now when raw_sock_{to,from}_{pipe,buf} are called, if the connection
doesn't feature CO_FL_WILL_UPDATE, they will first retrieve the updated
flags using conn_refresh_polling_flags() before changing any flag, then
call conn_cond_update_sock_polling() before leaving, to commit such
changes. Note that the only real call to one of the __conn_* functions
is in fact in conn_sock_read0() which is called from here.
Willy Tarreau [Wed, 25 Oct 2017 07:59:22 +0000 (09:59 +0200)]
MEDIUM: connection: make use of CO_FL_WILL_UPDATE in conn_sock_shutw()
This one may be called by upper layers (eg: si_shutw()) or lower layers
(si_shutw() as well during stream_int_notify()) so we want it to take
care of updating the connection's flags if it's not going to be done
by the caller.
Willy Tarreau [Wed, 25 Oct 2017 07:22:43 +0000 (09:22 +0200)]
MINOR: connection: add flag CO_FL_WILL_UPDATE to indicate when updates are granted
In transport-layer functions (snd_buf/rcv_buf), it's very problematic
never to know if polling changes made to the connection will be propagated
or not. This has led to some conn_cond_update_polling() calls being placed
at a few places to cover both the cases where the function is called from
the upper layer and when it's called from the lower layer. With the arrival
of the MUX, this becomes even more complicated, as the upper layer will not
have to manipulate anything from the connection layer directly and will not
have to push such updates directly either. But the snd_buf functions will
need to see their updates committed when called from upper layers.
The solution here is to introduce a connection flag set by the connection
handler (and possibly any other similar place) indicating that the caller
is committed to applying such changes on return. This way, the called
functions will be able to apply such changes by themselves before leaving
when the flag is not set, and the upper layer will not have to care about
that anymore.
Willy Tarreau [Wed, 25 Oct 2017 08:28:45 +0000 (10:28 +0200)]
MINOR: connection: move the cleanup of flag CO_FL_WAIT_ROOM
This flag is only used when reading using splicing for now, and is only
set when a pipe full condition is met, so we can simplify its reset
condition in conn_refresh_polling_flags so that it's cleared at the
same time as the other ones, only when the control layer is ready.
This flag could be used more, to mark that a buffer full condition was
met with any receive method in order to simplify polling management.
This should probably be revisited after 1.8.
Willy Tarreau [Wed, 25 Oct 2017 13:34:39 +0000 (15:34 +0200)]
MINOR: ssl: don't abort after sending 16kB
SSL records are 16kB max. When trying to send larger data chunks at once,
SSL_read() only processes 16kB and ssl_sock_from_buf() believes it means
the system buffers are full, which is not the case, contrary to raw_sock.
This is particularly noticeable with HTTP/2 when using a 64kB buffer with
multiple streams, as the mux buffer can start to fill up pretty quickly
in this situation, slowing down the data delivery.
Willy Tarreau [Wed, 25 Oct 2017 12:22:28 +0000 (14:22 +0200)]
MINOR: stream-int: stop checking for useless connection flags in chk_snd_conn
We've been keep this test for a connection being established since 1.5-dev14
when the stream-interface was still accessing the FD directly. The test on
CO_FL_HANDSHAKE and L{4,6}_CONN is totally useless here, and can even be
counter-productive on pure TCP where it could prevent a request from being
sent on a connection still attempting to complete its establishment. And it
creates an abnormal dependency between the layers that will complicate the
implementation of the mux, so let's get rid of it now.
Dragan Dosen [Tue, 24 Oct 2017 06:48:25 +0000 (08:48 +0200)]
IMPORT: sha1: import SHA1 functions
This is based on the git SHA1 implementation and optimized to do word
accesses rather than byte accesses, and to avoid unnecessary copies into
the context array.
Lukas Tribus [Tue, 24 Oct 2017 10:26:31 +0000 (12:26 +0200)]
BUG/MINOR: cli: restore "set ssl tls-key" command
in 32af203b75 ("REORG: cli: move ssl CLI functions to ssl_sock.c")
"set ssl tls-key" was accidentally replaced with "set ssl tls-keys"
(keys instead of key). This is undocumented and breaks upgrades from
1.6 to 1.7.
This patch restores "set ssl tls-key" and also registers a helptext.
Emmanuel Hocdet [Tue, 24 Oct 2017 12:57:16 +0000 (14:57 +0200)]
BUG/MINOR: ssl: OCSP_single_get0_status can return -1
Commit 872085ce "BUG/MINOR: ssl: ocsp response with 'revoked' status is correct"
introduce a regression. OCSP_single_get0_status can return -1 and haproxy must
generate an error in this case.
Thanks to Sander Hoentjen who have spotted the regression.
This patch should be backported in 1.7, 1.6 and 1.5 if the patch above is
backported.
Emmanuel Hocdet [Mon, 2 Oct 2017 15:12:06 +0000 (17:12 +0200)]
MINOR: ssl: build with recent BoringSSL library
BoringSSL switch OPENSSL_VERSION_NUMBER to 1.1.0 for compatibility.
Fix BoringSSL call and openssl-compat.h/#define occordingly.
This will not break openssl/libressl compat.
Willy Tarreau [Tue, 24 Oct 2017 08:58:20 +0000 (10:58 +0200)]
CONTRIB: trace: try to display the function's return value on exit
On x86_64, when gcc instruments functions and compiles at -O0, it saves
the function's return value in register rbx before calling the trace
callback. It provides a nice opportunity to display certain useful
values (flags, booleans etc) during trace sessions. It's absolutely
not guaranteed that it will always work but it provides a considerable
help when it does so it's worth activating it. When building on a
different architecture, the value 0 is always reported as the return
value. On x86_64 with optimizations (-O), the RBX register will not
necessarily match and random values will be reported, but since it's
not the primary target it's not a problem.
Willy Tarreau [Tue, 24 Oct 2017 08:54:08 +0000 (10:54 +0200)]
CONTRIB: trace: add the possibility to place trace calls in the code
Now any call to trace() in the code will automatically appear interleaved
with the call sequence and timestamped in the trace file. They appear with
a '#' on the 3rd argument (caller's pointer) in order to make them easy to
spot. If the trace functionality is not used, a dmumy weak function is used
instead so that it doesn't require to recompile every time traces are
enabled/disabled.
The trace decoder knows how to deal with these messages, detects them and
indents them similarly to the currently traced function. This can be used
to print function arguments for example.
Note that we systematically flush the log when calling trace() to ensure we
never miss important events, so this may impact performance.
The trace() function uses the same format as printf() so it should be easy
to setup during debugging sessions.
Willy Tarreau [Sun, 22 Oct 2017 08:13:45 +0000 (10:13 +0200)]
[RELEASE] Released version 1.8-dev3
Released version 1.8-dev3 with the following main changes :
- REORG: ssl: move defines and methodVersions table upper
- MEDIUM: ssl: ctx_set_version/ssl_set_version func for methodVersions table
- MINOR: ssl: support ssl-min-ver and ssl-max-ver with crt-list
- MEDIUM: ssl: disable SSLv3 per default for bind
- BUG/MAJOR: ssl: fix segfault on connection close using async engines.
- BUG/MAJOR: ssl: buffer overflow using offloaded ciphering on async engine
- BUG/MINOR: ssl: do not call directly the conn_fd_handler from async_fd_handler
- BUG/MINOR: haproxy/cli : fix for solaris/illumos distros for CMSG* macros
- BUG/MEDIUM: build without openssl broken
- BUG/MINOR: warning: need_resend may be used uninitialized
- BUG/MEDIUM: misplaced exit and wrong exit code
- BUG/MINOR: Makefile: fix compile error with USE_LUA=1 in ubuntu16.04
- BUILD: scripts: make publish-release support bare repositories
- BUILD: scripts: add an automatic mode for publish-release
- BUILD: scripts: add a "quiet" mode to publish-release
- BUG/MAJOR: http: call manage_client_side_cookies() before erasing the buffer
- BUG/MINOR: buffers: Fix bi/bo_contig_space to handle full buffers
- CONTRIB: plug qdiscs: Plug queuing disciplines mini HOWTO.
- BUG/MINOR: acls: Set the right refflag when patterns are loaded from a map
- BUG/MINOR: ssl: Be sure that SSLv3 connection methods exist for openssl < 1.1.0
- BUG/MINOR: http/filters: Be sure to wait if a filter loops in HTTP_MSG_ENDING
- BUG/MEDIUM: peers: Peers CLOSE_WAIT issue.
- BUG/MAJOR: server: Segfault after parsing server state file.
- BUG/MEDIUM: unix: never unlink a unix socket from the file system
- scripts: create-release pass -n to tail
- SCRIPTS: create-release: enforce GIT_COMMITTER_{NAME|EMAIL} validity
- BUG/MEDIUM: fix segfault when no argument to -x option
- MINOR: warning on multiple -x
- MINOR: mworker: don't copy -x argument anymore in copy_argv()
- BUG/MEDIUM: mworker: don't reuse PIDs passed to the master
- BUG/MINOR: Wrong peer task expiration handling during synchronization processing.
- BUG/MINOR: cfgparse: Check if tune.http.maxhdr is in the range 1..32767
- BUG/MINOR: log: pin the front connection when front ip/ports are logged
- DOC: fix references to the section about the unix socket
- BUG/MINOR: stream: flag TASK_WOKEN_RES not set if task in runqueue
- MAJOR: task: task scheduler rework.
- MINOR: task/stream: tasks related to a stream must be init by the caller.
- MINOR: queue: Change pendconn_get_next_strm into private function
- MINOR: backends: Change get_server_sh/get_server_uh into private function
- MINOR: queue: Change pendconn_from_srv/pendconn_from_px into private functions
- MEDIUM: stream: make stream_new() always set the target and analysers
- MINOR: frontend: initialize HTTP layer after the debugging code
- MINOR: connection: add a .get_alpn() method to xprt_ops
- MINOR: ssl: add a get_alpn() method to ssl_sock
- MINOR: frontend: retrieve the ALPN name when available
- MINOR: frontend: report the connection's ALPN in the debug output
- MINOR: stream: don't set backend's nor response analysers on SF_TUNNEL
- MINOR: connection: send data before receiving
- MAJOR: applet: applet scheduler rework.
- BUG/MAJOR: frontend: don't dereference a null conn on outgoing connections
- BUG/MAJOR: cli: fix custom io_release was crushed by NULL.
- BUG/MAJOR: map: fix segfault during 'show map/acl' on cli.
- BUG/MAJOR: compression: Be sure to release the compression state in all cases
- MINOR: compression: Use a memory pool to allocate compression states
- BUG/MAJOR: applet: fix a freeze if data is immedately forwarded.
- DOC: fix references to the section about time format.
- BUG/MEDIUM: map/acl: fix unwanted flags inheritance.
- BUG/MAJOR: http: fix buffer overflow on loguri buffer.
- MINOR: ssl: compare server certificate names to the SNI on outgoing connections
- BUG/MINOR: stream: Don't forget to remove CF_WAKE_ONCE flag on response channel
- BUG/MINOR: http: Don't reset the transaction if there are still data to send
- BUG/MEDIUM: filters: Be sure to call flt_end_analyze for both channels
- MINOR: peers: Add additional information to stick-table definition messages.
- BUG/MINOR: http: properly handle all 1xx informational responses
- OPTIM: ssl: don't consider a small ssl_read() as an indication of end of buffer
- BUG/MINOR: peers: peer synchronization issue (with several peers sections).
- CLEANUP: hdr_idx: make some function arguments const where possible
- BUG/MINOR: Prevent a use-after-free on error scenario on option "-x".
- BUG/MINOR: lua: In error case, the safe mode is not removed
- BUG/MINOR: lua: executes the function destroying the Lua session in safe mode
- BUG/MAJOR: lua/socket: resources not detroyed when the socket is aborted
- BUG/MEDIUM: lua: bad memory access
- BUG/MINOR: Lua: variable already initialized
- DOC: update CONTRIBUTING regarding optional parts and message format
- DOC: update the list of OpenSSL versions in the README
- BUG/MINOR: http: Set the response error state in http_sync_res_state
- MINOR: http: Reorder/rewrite checks in http_resync_states
- MINOR: http: Switch requests/responses in TUNNEL mode only by checking txn flags
- BUG/MEDIUM: http: Switch HTTP responses in TUNNEL mode when body length is undefined
- MINOR: http: Rely on analyzers mask to end processing in forward_body functions
- BUG/MINOR: http: Fix bug introduced in previous patch in http_resync_states
- BUG/MINOR: contrib/modsecurity: BSD build fix
- BUG/MINOR: contrib/mod_defender: build fix
- BUG/MINOR: ssl: remove haproxy SSLv3 support when ssl lib have no SSLv3
- MINOR: ssl: remove an unecessary SSL_OP_NO_* dependancy
- BUILD: ssl: fix compatibility with openssl without TLSEXT_signature_*
- MINOR: tools: add a portable timegm() alternative
- BUILD: lua: replace timegm() with my_timegm() to fix build on Solaris 10
- DOC: Updated 51Degrees git URL to point to a stable version.
- BUG/MAJOR: http: Fix possible infinity loop in http_sync_(req|res)_state
- MINOR: memory: remove macros
- BUG/MINOR: lua: Fix Server.get_addr() port values
- BUG/MINOR: lua: Correctly use INET6_ADDRSTRLEN in Server.get_addr()
- MINOR: samples: Handle the type SMP_T_METH when we duplicate a sample in smp_dup
- MINOR: samples: Handle the type SMP_T_METH in smp_is_safe and smp_is_rw
- MINOR: samples: Don't allocate memory for SMP_T_METH sample when method is known
- BUG/MINOR: lua: always detach the tcp/http tasks before freeing them
- MINOR: task: always preinitialize the task's timeout in task_init()
- CLEANUP: task: remove all initializations to TICK_ETERNITY after task_new()
- BUG/MAJOR: lua: properly dequeue hlua_applet_wakeup() for new scheduler
- MINOR: lua: Add proxy as member of proxy object.
- DOC: lua: Proxy class doc update
- MINOR: lua: Add lists of frontends and backends
- BUG/MINOR: ssl: Fix check against SNI during server certificate verification
- BUG/MINOR: ssl: make use of the name in SNI before verifyhost
- MINOR: ssl: add a new error codes for wrong server certificates
- BUG/MEDIUM: stream: don't retry SSL connections which fail the SNI name check
- MINOR: ssl: add "no-ca-names" parameter for bind
- BUG/MINOR: lua: Fix bitwise logic for hlua_server_check_* functions.
- DOC: fix alphabetical order of "show commands" in management.txt
- MINOR: listener: add a function to return a listener's state as a string
- MINOR: cli: add a new "show fd" command
- BUG/MEDIUM: ssl: Fix regression about certificates generation
- MINOR: Add server port field to server state file.
- MINOR: ssl: allow to start without certificate if strict-sni is set
- MINOR: dns: Cache previous DNS answers.
- MINOR: obj: Add a new type of object, OBJ_TYPE_SRVRQ.
- Add a few functions to do unaligned access.
- MINOR: dns: Handle SRV records.
- MINOR: check: Fix checks when using SRV records.
- MINOR: doc: Document SRV label usage.
- BUILD/MINOR: cli: shut a minor gcc warning in "show fd"
- BUILD: ssl: replace SSL_CTX_get0_privatekey for openssl < 1.0.2
- BUILD/MINOR: build without openssl still broken
- BUG/MAJOR: stream: in stream_free(), close the front endpoint and not the origin
- CLEANUP: raw_sock: Use a better name for the constructor than __ssl_sock_deinit()
- MINOR: init: Fix CPU affinity setting on FreeBSD.
- MINOR: dns: Update analysis of TRUNCATED response for SRV records
- MINOR: dns: update record dname matching for SRV query types
- MINOR: dns: update dns response buffer reading pointer due to SRV record
- MINOR: dns: duplicate entries in resolution wait queue for SRV records
- MINOR: dns: make debugging function dump_dns_config() compatible with SRV records
- MINOR: dns: ability to use a SRV resolution for multiple backends
- MINOR: dns: enable caching of responses for server set by a SRV record
- MINOR: dns: new dns record type (RTYPE) for OPT
- MINOR: dns: enabled edns0 extension and make accpeted payload size tunable
- MINOR: dns: default "hold obsolete" timeout set to 0
- MINOR: chunks: add chunk_memcpy() and chunk_memcat()
- MINOR: session: add a streams field to the session struct
- MINOR: stream: link the stream to its session
- MEDIUM: session: do not free a session until no stream references it
- MINOR: ist: implement very simple indirect strings
- TESTS: ist: add a test file for the functions
- MINOR: http: export some of the HTTP parser macros
- BUG/MINOR: Wrong type used as argument for spoe_decode_buffer().
- BUG/MINOR: dns: server set by SRV records stay in "no resolution" status
- MINOR: dns: Maximum DNS udp payload set to 8192
- MINOR: dns: automatic reduction of DNS accpeted payload size
- MINOR: dns: make SRV record processing more verbose
- CLEANUP: dns: remove duplicated code in dns_resolve_recv()
- CLEANUP: dns: remove duplicated code in dns_validate_dns_response()
- BUG/MINOR: dns: wrong resolution interval lead to 100% CPU
- BUG/MEDIUM: dns: fix accepted_payload_size parser to avoid integer overflow
- BUG/MAJOR: lua: fix the impact of the scheduler changes again
- BUG/MEDIUM: lua: HTTP services must take care of body-less status codes
- MINOR: lua: properly process the contents of the content-length field
- BUG/MEDIUM: stream: properly set the required HTTP analysers on use-service
- OPTIM: lua: don't use expensive functions to parse headers in the HTTP applet
- OPTIM: lua: don't add "Connection: close" on the response
- REORG/MEDIUM: connection: introduce the notion of connection handle
- BUG/MINOR: stream-int: don't check the CO_FL_CURR_WR_ENA flag
- MEDIUM: connection: get rid of data->init() which was not for data
- MEDIUM: stream: make stream_new() allocate its own task
- CLEANUP: listener: remove the unused handler field
- MEDIUM: session: add a pointer to a struct task in the session
- MINOR: stream: provide a new stream creation function for connections
- MEDIUM: connection: remove useless flag CO_FL_DATA_RD_SH
- CLEANUP: connection: remove the unused conn_sock_shutw_pending()
- MEDIUM: connection: remove useless flag CO_FL_DATA_WR_SH
- DOC: add CLI info on privilege levels
- DOC: Refer to Mozilla TLS info / config generator
- MINOR: ssl: remove duplicate ssl_methods in struct bind_conf
- BUG/MEDIUM: http: Fix a regression bug when a HTTP response is in TUNNEL mode
- DOC: Add note about "* " prefix in CSV stats
- CLEANUP: memory: Remove unused function pool_destroy
- MINOR: listeners: Change listener_full and limit_listener into private functions
- MINOR: listeners: Change enable_listener and disable_listener into private functions
- MINOR: fd: Don't forget to reset fdtab[fd].update when a fd is added/removed
- MINOR: fd: Set owner and iocb field before inserting a new fd in the fdtab
- MINOR: backends: Make get_server_* functions explicitly static
- MINOR: applet: Check applets_active_queue before processing applets queue
- MINOR: chunks: Use dedicated function to init/deinit trash buffers
- MEDIUM: chunks: Realloc trash buffers only after the config is parsed and checked
- MINOR: logs: Use dedicated function to init/deinit log buffers
- MINOR: logs: Realloc log buffers only after the config is parsed and checked
- MINOR: buffers: Move swap_buffer into buffer.c and add deinit_buffer function
- MINOR: stick-tables: Make static_table_key a struct variable instead of a pointer
- MINOR: http: Use a trash chunk to store decoded string of the HTTP auth header
- MINOR: fd: Add fd_active function
- MINOR: fd: Use inlined functions to check fd state in fd_*_send/recv functions
- MINOR: fd: Move (de)allocation of fdtab and fdinfo in (de)init_pollers
- MINOR: freq_ctr: Return the new value after an update
- MEDIUM: check: server states and weight propagation re-work
- BUG/MEDIUM: epoll: ensure we always consider HUP and ERR
- MINOR: fd: Add fd_update_events function
- MINOR: polling: Use fd_update_events to update events seen for a fd
- BUG/MINOR: server: Remove FQDN requirement for using init-addr and state file
- Revert "BUG/MINOR: server: Remove FQDN requirement for using init-addr and state file"
- MINOR: ssl: rework smp_fetch_ssl_fc_cl_str without internal ssl use
- BUG/MEDIUM: http: Close streams for connections closed before a redirect
- BUG/MINOR: Lua: The socket may be destroyed when we try to access.
- MINOR: xref: Add a new xref system
- MEDIUM: xref/lua: Use xref for referencing cosocket relation between stream and lua
- MINOR: tasks: Move Lua notification from Lua to tasks
- MINOR: net_helper: Inline functions meant to be inlined.
- MINOR: cli: add socket commands and config to prepend informational messages with severity
- MINOR: add severity information to cli feedback messages
- BUILD: Makefile: add a function to detect support by the compiler of certain options
- BUILD: Makefile: shut certain gcc/clang stupid warnings
- BUILD: Makefile: improve detection of support for compiler warnings
- MINOR: peers: don't reference the incoming listener on outgoing connections
- MINOR: frontend: don't retrieve ALPN on the critical path
- MINOR: protocols: always pass a "port" argument to the listener creation
- MINOR: protocols: register the ->add function and stop calling them directly
- MINOR: unix: remove the now unused proto_uxst.h file
- MINOR: listeners: new function create_listeners
- MINOR: listeners: make listeners count consistent with reality
- MEDIUM: session: take care of incrementing/decrementing jobs
- MINOR: listener: new function listener_release
- MINOR: session: small cleanup of conn_complete_session()
- MEDIUM: session: factor out duplicated code for conn_complete_session
- MEDIUM: session: count the frontend's connections at a single place
- BUG/MEDIUM: compression: Fix check on txn in smp_fetch_res_comp_algo
- BUG/MINOR: compression: Check response headers before http-response rules eval
- BUG/MINOR: spoe: Don't rely on SPOE ctx in debug message when its creation failed
- BUG/MINOR: dns: Fix check on nameserver in snr_resolution_cb
- MINOR: ssl: Remove useless checks on bind_conf or bind_conf->is_ssl
- BUG/MINOR: contrib/mod_defender: close the va_list argp before return
- BUG/MINOR: contrib/modsecurity: close the va_list ap before return
- MINOR: tools: make my_htonll() more efficient on x86_64
- MINOR: buffer: add b_del() to delete a number of characters
- MINOR: buffer: add b_end() and b_to_end()
- MINOR: net_helper: add functions to read from vectors
- MINOR: net_helper: add write functions
- MINOR: net_helper: add 64-bit read/write functions
- MINOR: connection: adjust CO_FL_NOTIFY_DATA after removal of flags
- MINOR: ist: add a macro to ease const array initialization
- BUG/MEDIUM: server: unwanted behavior leaving maintenance mode on tracked stopping server
- BUG/MEDIUM: server: unwanted behavior leaving maintenance mode on tracked stopping server (take2)
- BUG/MINOR: log: fixing small memory leak in error code path.
- BUG/MINOR: contrib/halog: fixing small memory leak
- BUG/MEDIUM: tcp/http: set-dst-port action broken
- CLEANUUP: checks: don't set conn->handle.fd to -1
- BUG/MEDIUM: tcp-check: properly indicate polling state before performing I/O
- BUG/MINOR: tcp-check: don't quit with pending data in the send buffer
- BUG/MEDIUM: tcp-check: don't call tcpcheck_main() from the I/O handlers!
- BUG/MINOR: unix: properly check for octal digits in the "mode" argument
- MINOR: checks: make chk_report_conn_err() take a check, not a connection
- CLEANUP: checks: remove misleading comments and statuses for external process
- CLEANUP: checks: don't report report the fork() error twice
- CLEANUP: checks: do not allocate a connection for process checks
- TESTS: checks: add a simple test config for external checks
- BUG/MINOR: tcp-check: don't initialize then break a connection starting with a comment
- TESTS: checks: add a simple test config for tcp-checks
- MINOR: tcp-check: make tcpcheck_main() take a check, not a connection
- MINOR: checks: don't create then kill a dummy connection before tcp-checks
- MEDIUM: checks: make tcpcheck_main() indicate if it recycled a connection
- MEDIUM: checks: do not allocate a permanent connection anymore
- BUG/MEDIUM: cli: fix "show fd" crash when dumping closed FDs
- BUG/MEDIUM: http: Return an error when url_dec sample converter failed
- BUG/MAJOR: stream-int: don't re-arm recv if send fails
- BUILD/MINOR: 51d: fix warning when building with 51Degrees release version 3.2.12.12
- DOC: 51d: add 51Degrees git URL that points to release version 3.2.12.12
- DOC: 51d: Updated git URL and instructions for getting Hash Trie data files.
- MINOR: compiler: restore the likely() wrapper for gcc 5.x
- MINOR: session: remove the list of streams from struct session
- DOC: fix some typos
- MINOR: server: add the srv_queue() sample fetch method
- MINOR: payload: add new sample fetch functions to process distcc protocol
- MAJOR: servers: propagate server status changes asynchronously.
- BUG/MEDIUM: ssl: fix OCSP expiry calculation
- BUG/MINOR: stream-int: don't set MSG_MORE on SHUTW_NOW without AUTO_CLOSE
- MINOR: server: Handle weight increase in consistent hash.
- MINOR: checks: Add a new keyword to specify a SNI when doing SSL checks.
- BUG/MINOR: tools: fix my_htonll() on x86_64
- BUG/MINOR: stats: Clear a bit more counters with in cli_parse_clear_counters().
- BUG/MAJOR: lua: scheduled task is freezing.
- MINOR: buffer: add bo_del() to delete a number of characters from output
- MINOR: buffer: add a function to match against string patterns
- MINOR: buffer: add two functions to inject data into buffers
- MINOR: buffer: add buffer_space_wraps()
- REORG: channel: finally rename the last bi_* / bo_* functions
- MINOR: buffer: add bo_getblk() and bo_getblk_nc()
- MINOR: channel: make use of bo_getblk{,_nc} for their channel equivalents
- MINOR: channel: make the channel be a const in all {ci,co}_get* functions
- MINOR: ist: add ist0() to add a trailing zero to a string.
- BUG/MEDIUM: log: check result details truncated.
- MINOR: buffer: make bo_getblk_nc() not return 2 for a full buffer
- REORG: http: move some very http1-specific parts to h1.{c,h}
- REORG: http: move the HTTP/1 chunk parser to h1.{c,h}
- REORG: http: move the HTTP/1 header block parser to h1.c
- MEDIUM: http: make the chunk size parser only depend on the buffer
- MEDIUM: http: make the chunk crlf parser only depend on the buffer
- MINOR: h1: add struct h1m for basic HTTP/1 messages
- MINOR: http: add very simple header management based on double strings
- MEDIUM: h1: reimplement the http/1 response parser for the gateway
- REORG: connection: rename CO_FL_DATA_* -> CO_FL_XPRT_*
- MEDIUM: connection: make conn_sock_shutw() aware of lingering
- MINOR: connection: ensure conn_ctrl_close() also resets the fd
- MINOR: connection: add conn_stop_tracking() to disable tracking
- MINOR: tcp: use conn_full_close() instead of conn_force_close()
- MINOR: unix: use conn_full_close() instead of conn_force_close()
- MINOR: checks: use conn_full_close() instead of conn_force_close()
- MINOR: session: use conn_full_close() instead of conn_force_close()
- MINOR: stream: use conn_full_close() instead of conn_force_close()
- MINOR: stream: use conn_full_close() instead of conn_force_close()
- MINOR: backend: use conn_full_close() instead of conn_force_close()
- MINOR: stream-int: use conn_full_close() instead of conn_force_close()
- MINOR: connection: remove conn_force_close()
- BUG/MINOR: ssl: ocsp response with 'revoked' status is correct
Emmanuel Hocdet [Tue, 10 Oct 2017 13:18:52 +0000 (15:18 +0200)]
BUG/MINOR: ssl: ocsp response with 'revoked' status is correct
ocsp_status can be 'good', 'revoked', or 'unknown'. 'revoked' status
is a correct status and should not be dropped.
In case of certificate with OCSP must-stapling extension, response with
'revoked' status must be provided as well as 'good' status.
Willy Tarreau [Thu, 5 Oct 2017 15:49:56 +0000 (17:49 +0200)]
MINOR: connection: remove conn_force_close()
Now only conn_full_close() will be used. It will become more obvious
when the tracking is in place or not and will make it easier to
convert remaining call places to conn_streams.
Willy Tarreau [Thu, 5 Oct 2017 15:24:42 +0000 (17:24 +0200)]
MEDIUM: connection: make conn_sock_shutw() aware of lingering
Instead of having to manually handle lingering outside, let's make
conn_sock_shutw() check for it before calling shutdown(). We simply
don't want to emit the FIN if we're going to reset the connection
due to lingering. It's particularly important for silent-drop where
it's absolutely mandatory that no packet leaves the machine.
These flags are not exactly for the data layer, they instead indicate
what is expected from the transport layer. Since we're going to split
the connection between the transport and the data layers to insert a
mux layer, it's important to have a clear idea of what each layer does.
All function conn_data_* used to manipulate these flags were renamed to
conn_xprt_*.
MEDIUM: h1: reimplement the http/1 response parser for the gateway
The HTTP/2->HTTP/1 gateway will need to process HTTP/1 responses. We
cannot sanely rely on the HTTP/1 txn to parse a response because :
1) responses generated by haproxy such as error messages, redirects,
stats or Lua are neither parsed nor indexed ; this could be
addressed over the long term but will take time.
2) the http txn is useless to parse the body : the states present there
are only meaningful to received bytes (ie next bytes to parse) and
not at all to sent bytes. Thus chunks cannot be followed at all.
Even when implementing this later, it's unsure whether it will be
possible when dealing with compression.
So using the HTTP txn is now out of the equation and the only remaining
solution is to call an HTTP/1 message parser. We already have one, it was
slightly modified to avoid keeping states by benefitting from the fact
that the response was produced by haproxy and this is entirely available.
It assumes the following rules are true, or that incuring an extra cost
to work around them is acceptable :
- the response buffer is read-write and supports modifications in place
- headers sent through / by haproxy are not folded. Folding is still
implemented by replacing CR/LF/tabs/spaces with spaces if encountered
- HTTP/0.9 responses are never sent by haproxy and have never been
supported at all
- haproxy will not send partial responses, the whole headers block will
be sent at once ; this means that we don't need to keep expensive
states and can afford to restart the parsing from the beginning when
facing a partial response ;
- response is contiguous (does not wrap). This was already the case
with the original parser and ensures we can safely dereference all
fields with (ptr,len)
The parser replaces all of the http_msg fields that were necessary with
local variables. The parser is not called on an http_msg but on a string
with a start and an end. The HTTP/1 states were reused for ease of use,
though the request-specific ones have not been implemented for now. The
error position and error state are supported and optional ; these ones
may be used later for bug hunting.
The parser issues the list of all the headers into a caller-allocated
array of struct ist.
The content-length/transfer-encoding header are checked and the relevant
info fed the h1 message state (flags + body_len).
MINOR: http: add very simple header management based on double strings
This will be used initially by the hpack table and hopefully later by a
new native http processor. These headers are made of name and value, both
an immediate string (ie: pointer and length).
MEDIUM: http: make the chunk crlf parser only depend on the buffer
The chunk crlf parser used to depend on the channel and on the HTTP
message, eventhough it's not really needed. Let's remove this dependency
so that it can be used within the H2 to H1 gateway.
As part of this small API change, it was renamed to h1_skip_chunk_crlf()
to mention that it doesn't depend on http_msg anymore.
MEDIUM: http: make the chunk size parser only depend on the buffer
The chunk parser used to depend on the channel and on the HTTP message
but it's not really needed as they're only used to retrieve the buffer
as well as to return the number of bytes parsed and the chunk size.
Here instead we pass the (few) relevant information in arguments so that
the function may be reused without a channel nor an HTTP message (ie
from the H2 to H1 gateway).
As part of this API change, it was renamed to h1_parse_chunk_size() to
mention that it doesn't depend on http_msg anymore.
REORG: http: move the HTTP/1 chunk parser to h1.{c,h}
Functions http_parse_chunk_size(), http_skip_chunk_crlf() and
http_forward_trailers() were moved to h1.h and h1.c respectively so
that they can be called from outside. The parts that were inline
remained inline as it's critical for performance (+41% perf
difference reported in an earlier test). For now the "http_" prefix
remains in their name since they still depend on the http_msg type.
REORG: http: move some very http1-specific parts to h1.{c,h}
Certain types and enums are very specific to the HTTP/1 parser, and we'll
need to share them with the HTTP/2 to HTTP/1 translation code. Let's move
them to h1.c/h1.h. Those with very few occurrences or only used locally
were renamed to explicitly mention the relevant HTTP version :
Willy Tarreau [Fri, 20 Oct 2017 16:21:49 +0000 (18:21 +0200)]
MINOR: buffer: make bo_getblk_nc() not return 2 for a full buffer
Thus function returns the number of blocks. When a buffer is full and
properly aligned, buf->p loops back the beginning, and the test in the
code doesn't cover that specific case, so it returns two chunks, a full
one and an empty one. It's harmless but can sometimes have a small impact
on performance and definitely makes the code hard to debug.
Willy Tarreau [Thu, 19 Oct 2017 04:28:23 +0000 (06:28 +0200)]
MINOR: ist: add ist0() to add a trailing zero to a string.
This function modifies the string to add a zero after the end, and returns
the start pointer. The purpose is to use it on strings extracted by parsers
from larger strings cut with delimiters that are not important and can be
destroyed. It allows any such string to be used with regular string
functions. It's also convenient to use with printf() to show data extracted
from writable areas.
Willy Tarreau [Thu, 19 Oct 2017 12:58:40 +0000 (14:58 +0200)]
MINOR: channel: make the channel be a const in all {ci,co}_get* functions
There's no point having the channel marked writable as these functions
only extract data from the channel. The code was retrieved from their
ci/co ancestors.
Willy Tarreau [Thu, 19 Oct 2017 12:32:15 +0000 (14:32 +0200)]
REORG: channel: finally rename the last bi_* / bo_* functions
For HTTP/2 we'll need some buffer-only equivalent functions to some of
the ones applying to channels and still squatting the bi_* / bo_*
namespace. Since these names have kept being misleading for quite some
time now and are really getting annoying, it's time to rename them. This
commit will use "ci/co" as the prefix (for "channel in", "channel out")
instead of "bi/bo". The following ones were renamed :
Willy Tarreau [Mon, 16 Oct 2017 12:01:18 +0000 (14:01 +0200)]
MINOR: buffer: add buffer_space_wraps()
This function returns true if the available buffer space wraps. This
will be used to detect if it's worth realigning a buffer when it lacks
contigous space.
MINOR: buffer: add two functions to inject data into buffers
bi_istput() injects the ist string into the input region of the buffer,
it will be used to feed small data chunks into the conn_stream. bo_istput()
does the same into the output region of the buffer, it will be used to send
data via the transport layer and assumes there's no input data.
MINOR: buffer: add a function to match against string patterns
In order to match known patterns in wrapping buffer, we'll introduce new
string manipulation functions for buffers. The new function b_isteq()
relies on an ist string for the pattern and compares it against any
location in the buffer relative to <p>. The second function bi_eat()
is specially designed to match input contents.
Willy Tarreau [Wed, 18 Oct 2017 06:32:12 +0000 (08:32 +0200)]
MINOR: buffer: add bo_del() to delete a number of characters from output
This simply reduces the amount of output data from the buffer after
they have been transferred, in a way that is more natural than by
fiddling with buf->o. b_del() was renamed to bi_del() to avoid any
ambiguity (it's not yet used).
Olivier Houchard [Tue, 17 Oct 2017 17:23:25 +0000 (19:23 +0200)]
BUG/MINOR: stats: Clear a bit more counters with in cli_parse_clear_counters().
Clear MaxSslRate, SslFrontendMaxKeyRate and SslBackendMaxKeyRate when
clear counters is used, it was probably forgotten when those counters were
added.
[wt: this can probably be backported as far as 1.5 in dumpstats.c]
Willy Tarreau [Wed, 18 Oct 2017 09:39:33 +0000 (11:39 +0200)]
BUG/MINOR: tools: fix my_htonll() on x86_64
Commit 36eb3a3 ("MINOR: tools: make my_htonll() more efficient on x86_64")
brought an incorrect asm statement missing the input constraints, causing
the input value not necessarily to be placed into the same register as the
output one, resulting in random output. It happens to work when building at
-O0 but not above. This was only detected in the HTTP/2 parser, but in
mainline it could only affect the integer to binary sample cast.
No backport is needed since this bug was only introduced in the development
branch.
Willy Tarreau [Tue, 17 Oct 2017 14:33:46 +0000 (16:33 +0200)]
BUG/MINOR: stream-int: don't set MSG_MORE on SHUTW_NOW without AUTO_CLOSE
Since around 1.5-dev12, we've been setting MSG_MORE on send() on various
conditions, including the fact that SHUTW_NOW is present, but we don't
check that it's accompanied with AUTO_CLOSE. The result is that on requests
immediately followed by a close (where AUTO_CLOSE is not set), the request
gets delayed in the TCP stack before being sent to the server. This is
visible with the H2 code where the end-of-stream flag is set on requests,
but probably happens when a POLL_HUP is detected along with the request.
The (lack of) presence of option abortonclose has no effect here since we
never send the SHUTW along with the request.
The hour part of the timezone offset was multiplied by 60 instead of
3600, resulting in an inaccurate expiry. This bug was introduced in
1.6-dev1 by commit 4f3c87a ("BUG/MEDIUM: ssl: Fix to not serve expired
OCSP responses."), so this fix must be backported into 1.7 and 1.6.
Emeric Brun [Tue, 3 Oct 2017 12:46:45 +0000 (14:46 +0200)]
MAJOR: servers: propagate server status changes asynchronously.
In order to prepare multi-thread development, code was re-worked
to propagate changes asynchronoulsy.
Servers with pending status changes are registered in a list
and this one is processed and emptied only once 'run poll' loop.
Operational status changes are performed before administrative
status changes.
In a case of multiple operational status change or admin status
change in the same 'run poll' loop iteration, those changes are
merged to reach only the targeted status.
Willy Tarreau [Fri, 13 Oct 2017 09:03:15 +0000 (11:03 +0200)]
MINOR: payload: add new sample fetch functions to process distcc protocol
When using haproxy in front of distccd, it's possible to provide significant
improvements by only connecting when the preprocessing is completed, and by
selecting different farms depending on the payload size. This patch provides
two new sample fetch functions :
Willy Tarreau [Fri, 13 Oct 2017 09:46:26 +0000 (11:46 +0200)]
MINOR: server: add the srv_queue() sample fetch method
srv_queue([<backend>/]<server>) : integer
Returns an integer value corresponding to the number of connections currently
pending in the designated server's queue. If <backend> is omitted, then the
server is looked up in the current backend. It can sometimes be used together
with the "use-server" directive to force to use a known faster server when it
is not much loaded. See also the "srv_conn", "avg_queue" and "queue" sample
fetch methods.
Willy Tarreau [Sun, 8 Oct 2017 19:32:53 +0000 (21:32 +0200)]
MINOR: session: remove the list of streams from struct session
Commit bcb86ab ("MINOR: session: add a streams field to the session
struct") added this list of streams that is not needed anymore. Let's
get rid of it now.
Willy Tarreau [Sun, 8 Oct 2017 20:26:03 +0000 (22:26 +0200)]
MINOR: compiler: restore the likely() wrapper for gcc 5.x
After some tests, gcc 5.x produces better code with likely()
than without, contrary to gcc 4.x where it was better to disable
it. Let's re-enable it for 5 and above.
BUILD/MINOR: 51d: fix warning when building with 51Degrees release version 3.2.12.12
The warning appears when building with 51Degrees release that uses a new
Hash Trie algorithm (release version 3.2.12.12):
src/51d.c: In function init_51degrees:
src/51d.c:566:2: warning: enumeration value DATA_SET_INIT_STATUS_TOO_MANY_OPEN_FILES not handled in switch [-Wswitch]
switch (_51d_dataset_status) {
^
Bin Wang [Fri, 15 Sep 2017 06:56:40 +0000 (14:56 +0800)]
BUG/MAJOR: stream-int: don't re-arm recv if send fails
When
1) HAProxy configured to enable splice on both directions
2) After some high load, there are 2 input channels with their socket buffer
being non-empty and pipe being full at the same time, sitting in `fd_cache`
without any other fds.
The 2 channels will repeatedly be stopped for receiving (pipe full) and waken
for receiving (data in socket), thus getting out and in of `fd_cache`, making
their fd swapping location in `fd_cache`.
There is a `if (entry < fd_cache_num && fd_cache[entry] != fd) continue;`
statement in `fd_process_cached_events` to prevent frequent polling, but since
the only 2 fds are constantly swapping location, `fd_cache[entry] != fd` will
always hold true, thus HAProxy can't make any progress.
The root cause of the issue is dual :
- there is a single fd_cache, for next events and for the ones being
processed, while using two distinct arrays would avoid the problem.
- the write side of the stream interface wakes the read side up even
when it couldn't write, and this one really is a bug.
Due to CF_WRITE_PARTIAL not being cleared during fast forwarding, a failed
send() attempt will still cause ->chk_rcv() to be called on the other side,
re-creating an entry for its connection fd in the cache, causing the same
sequence to be repeated indefinitely without any opportunity to make progress.
CF_WRITE_PARTIAL used to be used for what is present in these tests : check
if a recent write operation was performed. It's part of the CF_WRITE_ACTIVITY
set and is tested to check if timeouts need to be updated. It's also used to
detect if a failed connect() may be retried.
What this patch does is use CF_WROTE_DATA() to check for a successful write
for connection retransmits, and to clear CF_WRITE_PARTIAL before preparing
to send in stream_int_notify(). This way, timeouts are still updated each
time a write succeeds, but chk_rcv() won't be called anymore after a failed
write.
It seems the fix is required all the way down to 1.5.
Without this patch, the only workaround at this point is to disable splicing
in at least one direction. Strictly speaking, splicing is not absolutely
required, as regular forwarding could theorically cause the issue to happen
if the timing is appropriate, but in practice it appears impossible to
reproduce it without splicing, and even with splicing it may vary.
The following config manages to reproduce it after a few attempts (haproxy
going 100% CPU and having to be killed) :
BUG/MEDIUM: http: Return an error when url_dec sample converter failed
url_dec sample converter uses url_decode function to decode an URL. This
function fails by returning -1 when an invalid character is found. But the
sample converter never checked the return value and it used it as length for the
decoded string. Because it always succeeded, the invalid sample (with a string
length set to -1) could be used by other sample fetches or sample converters,
leading to undefined behavior like segfault.
The fix is pretty simple, url_dec sample converter just needs to return an error
when url_decode fails.
Willy Tarreau [Wed, 4 Oct 2017 18:24:54 +0000 (20:24 +0200)]
BUG/MEDIUM: cli: fix "show fd" crash when dumping closed FDs
I misplaced the "if (!fdt.owner)" test so it can occasionally crash
when dumping an fd that's already been closed but still appears in
the table. It's not critical since this was not pushed into any
release nor backported though.
Willy Tarreau [Wed, 4 Oct 2017 16:05:01 +0000 (18:05 +0200)]
MEDIUM: checks: do not allocate a permanent connection anymore
Health check currently cheat, they allocate a connection upon startup and never
release it, it's only recycled. The problem with doing this is that this code
is preventing the connection code from evolving towards multiplexing.
This code ensures that it's safe for the checks to run without a connection
all the time. Given that the code heavily relies on CO_FL_ERROR to signal
check errors, it is not trivial but in practice this is the principle adopted
here :
- the connection is not allocated anymore on startup
- new checks are not supposed to have a connection, so an attempt is made
to allocate this connection in the check task's context. If it fails,
the check is aborted on a resource error, and the rare code on this path
verifying the connection was adjusted to check for its existence (in
practice, avoid to close it)
- returning checks necessarily have a valid connection (which may possibly
be closed).
- a "tcp-check connect" rule tries to allocate a new connection before
releasing the previous one (but after closing it), so that if it fails,
it still keeps the previous connection in a closed state. This ensures
a connection is always valid here
Now it works well on all tested cases (regular and TCP checks, even with
multiple reconnections), including when the connection is forced to NULL or
randomly allocated.
Willy Tarreau [Wed, 4 Oct 2017 16:41:00 +0000 (18:41 +0200)]
MEDIUM: checks: make tcpcheck_main() indicate if it recycled a connection
The tcp-checks are very fragile. They can modify a connection's FD by
closing and reopening a socket without informing the connection layer,
which may then possibly touch the wrong fd. Given that the events are
only cleared and that the fd is just created, there should be no visible
side effect because the old fd is deleted so even if its flags get cleared
they were already, and the new fd already has them cleared as well so it's
a NOP. Regardless, this is too fragile and will not resist to threads.
In order to address this situation, this patch makes tcpcheck_main()
indicate if it closed a connection and report it to wake_srv_chk(), which
will then report it to the connection's fd handler so that it refrains
from updating the connection polling and the fd. Instead the connection
polling status is updated in the wake() function.
Willy Tarreau [Wed, 4 Oct 2017 14:21:19 +0000 (16:21 +0200)]
MINOR: checks: don't create then kill a dummy connection before tcp-checks
When tcp-checks are in use, a connection starts to be created, then it's
destroyed so that tcp-check can recreate its own. Now we directly move
to tcpcheck_main() when it's detected that tcp-check is in use.
Willy Tarreau [Wed, 4 Oct 2017 13:58:52 +0000 (15:58 +0200)]
BUG/MINOR: tcp-check: don't initialize then break a connection starting with a comment
The following config :
backend tcp9000
option tcp-check
tcp-check comment "this is a comment"
tcp-check connect port 10000
server srv 127.0.0.1:9000 check inter 1s
will result in a connection being first made to port 9000 then immediately
destroyed and re-created on port 10000, because the first rule is a comment
and doesn't match the test for the first rule being a connect(). It's
mostly harmless (unless the server really must not receive empty
connections) and the workaround simply consists in removing the comment.
Let's proceed like in other places where we simply skip leading comments.
A new function was made to make this lookup les boring. The fix should be
backported to 1.7 and 1.6.
Willy Tarreau [Wed, 4 Oct 2017 13:25:38 +0000 (15:25 +0200)]
CLEANUP: checks: do not allocate a connection for process checks
Since this connection is not used at all anymore, do not allocate it.
It was verified that check successes and failures (both synchronous
and asynchronous) continue to be properly reported.
Willy Tarreau [Wed, 4 Oct 2017 13:19:26 +0000 (15:19 +0200)]
CLEANUP: checks: don't report report the fork() error twice
Upon fork() error, a first report is immediately made by connect_proc_chk()
via set_server_check_status(), then process_chk_proc() detects the error
code and makes up a dummy connection error to call chk_report_conn_err(),
which tries to retrieve the errno code from the connection, fails, then
saves the status message from the check, fails all "if" tests on its path
related to the connection then resets the check's state to the current one
with the current status message. All this useless chain is the only reason
why process checks require a connection! Let's simply get rid of this second
useless call.
Willy Tarreau [Wed, 4 Oct 2017 13:07:02 +0000 (15:07 +0200)]
CLEANUP: checks: remove misleading comments and statuses for external process
The external process check code abused a little bit from copy-pasting to
the point of making think it requires a connection... The initialization
code only returns SF_ERR_NONE and SF_ERR_RESOURCE, so the other one can
be folded there. The code now only uses the connection to report the
error status.
Willy Tarreau [Wed, 4 Oct 2017 12:47:29 +0000 (14:47 +0200)]
MINOR: checks: make chk_report_conn_err() take a check, not a connection
Amazingly, this function takes a connection to report an error and is used
by process checks, placing a hard dependency between the connection and the
check preventing the mux from being completely implemented. Let's first get
rid of this.
Willy Tarreau [Wed, 4 Oct 2017 12:43:44 +0000 (14:43 +0200)]
BUG/MINOR: unix: properly check for octal digits in the "mode" argument
A config containing "stats socket /path/to/socket mode admin" used to
silently start and be unusable (mode 0, level user) because the "mode"
parser doesn't take care of non-digits. Now it properly reports :
This can probably be backported to 1.7, 1.6 and 1.5, though reporting
parsing errors in very old versions probably isn't a good idea if the
feature was left unused for years.
Willy Tarreau [Wed, 4 Oct 2017 09:58:22 +0000 (11:58 +0200)]
BUG/MEDIUM: tcp-check: don't call tcpcheck_main() from the I/O handlers!
This function can destroy a socket and create a new one, resulting in a
change of FD on the connection between recv() and send() for example,
which is absolutely not permitted, and can result in various funny games
like polling not being properly updated (or with the flags from a previous
fd) etc.
Let's only call this from the wake() callback which is more tolerant.
Ideally the operations should be made even more reliable by returning
a specific value to indicate that the connection was released and that
another one was created. But this is hasardous for stable releases as
it may reveal other issues.
Willy Tarreau [Wed, 4 Oct 2017 09:38:08 +0000 (11:38 +0200)]
BUG/MINOR: tcp-check: don't quit with pending data in the send buffer
In the rare case where the "tcp-check send" directive is the last one in
the list, it leaves the loop without sending the data. Fortunately, the
polling is still enabled on output, resulting in the connection handler
calling back to send what remains, but this is ugly and not very reliable.
Willy Tarreau [Wed, 4 Oct 2017 06:45:19 +0000 (08:45 +0200)]
BUG/MEDIUM: tcp-check: properly indicate polling state before performing I/O
While porting the connection to use the mux layer, it appeared that
tcp-checks wouldn't receive anymore because the polling is not enabled
before attempting to call xprt->rcv_buf() nor xprt->snd_buf(), and it
is illegal to call these functions with polling disabled as they
directly manipulate the FD state, resulting in an inconsistency where
the FD is enabled and the connection's polling flags disabled.
Till now it happened to work only because when recv() fails on EAGAIN
it calls fd_cant_recv() which enables polling while signaling the
failure, so that next time the message is received. But the connection's
polling is never enabled, and any tiny change resulting in a call to
conn_data_update_polling() immediately disables reading again.
It's likely that this problem already happens on some corner cases
such as multi-packet responses. It definitely breaks as soon as the
response buffer is full but we don't support consuming more than one
response buffer.
This fix should be backported to 1.7 and 1.6. In order to check for the
proper behaviour, this tcp-check must work and clearly show an SSH
banner in recvfrom() as observed under strace, otherwise it's broken :
Willy Tarreau [Wed, 4 Oct 2017 05:48:56 +0000 (07:48 +0200)]
CLEANUUP: checks: don't set conn->handle.fd to -1
This used to be needed to know whether there was a check in progress a
long time ago (before tcp_checks) but this is not true anymore and even
becomes wrong after the check is reused as conn_init() initializes it
to DEAD_FD_MAGIC.
projects / thirdparty / haproxy.git / log
