7204 Commits

Author SHA1 Message Date
Christopher Faulet
eb3e276d39 BUG/MEDIUM: threads/vars: Fix deadlock in register_name
In register_name, before locking the var_names array, we check the variable name
validity. So if we try to register an invalid or empty name, we need to return
without unlocking it (because it was never locked).

This patch must be backported in 1.8.
2017-12-08 10:37:24 +01:00
PiBa-NL
1714b9f286 BUG/MEDIUM: email-alert: don't set server check status from a email-alert task
This avoids possible 100% cpu usage deadlock on a EMAIL_ALERTS_LOCK and
avoids sending lots of emails when 'option log-health-checks' is used.
It is avoided to change the server state and possibly queue a new email
while processing the email alert by setting check->status to
HCHK_STATUS_UNKNOWN which will exit the set_server_check_status(..) early.

This needs to be backported to 1.8.
2017-12-08 05:58:56 +01:00
Aleksandar Lazic
f2b5d75ae2 CONTRIB: halog: Add help text for -s switch in halog program
It was not documented. May be backported to older releases.
2017-12-07 19:27:47 +01:00
Tim Duesterhus
d16f450c98 MINOR: mworker: Improve wording in void mworker_wait()
Replace "left" / "leaving" with "exit" / "exiting".

This should be backported to haproxy 1.8.
2017-12-07 19:21:25 +01:00
Tim Duesterhus
c578d9acfa MINOR: mworker: Update messages referencing exit-on-failure
Commit 4cfede87a313456fcbce7a185312460b4e1d05b7 removed
`exit-on-failure` in favor of `no-exit-on-failure`, but failed
to update references to the former in user facing messages.

This should be backported to haproxy 1.8.
2017-12-07 19:21:14 +01:00
Willy Tarreau
0249219be8 BUG/MEDIUM: h2: fix handling of end of stream again
Commit 9470d2c ("BUG/MINOR: h2: try to abort closed streams as
soon as possible") tried to address the situations where a stream
is closed by the client, but caused a side effect which is that in
some cases, a regularly closed stream reports an error to the stream
layer. The reason is that we purposely matched H2_SS_CLOSED in the
test for H2_SS_ERROR to report this so that we can check for RST,
but it accidently catches certain end of transfers as well. This
results in valid requests to report flags "CD" in the logs.

Instead, let's roll back to detecting H2_SS_ERROR and explicitly check
for a received RST. This way we can correctly abort transfers without
mistakenly reporting errors in normal situations.

This fix needs to be backported to 1.8 as the fix above was merged into
1.8.1.
2017-12-07 19:20:35 +01:00
Willy Tarreau
dbd026792a BUG/MEDIUM: peers: set NOLINGER on the outgoing stream interface
Since peers were ported to an applet in 1.5, an issue appeared which
is that certain attempts to close an outgoing connection are a bit
"too nice". Specifically, protocol errors and stream timeouts result
in a clean shutdown to be sent, waiting for the other side to confirm.
This is particularly problematic in the case of timeouts since by
definition the other side will not confirm as it has disappeared.

As found by Fred, this issue was further emphasized in 1.8 by commit
f9ce57e ("MEDIUM: connection: make conn_sock_shutw() aware of
lingering") which causes clean shutdowns not to be sent if the fd is
marked as linger_risk, because now even a clean timeout will not be
sent on an idle peers session, and the other one will have nothing
to respond to.

The solution here is to set NOLINGER on the outgoing stream interface
to ensure we always close whenever we attempt a simple shutdown.

However it is important to keep in mind that this also underlines
some weaknesses of the shutr/shutw processing inside process_stream()
and that all this part needs to be reworked to clearly consider the
abort case, and to stop the confusion between linger_risk and NOLINGER.

This fix needs to be backported as far as 1.5 (all versions are affected).
However, during testing of the backport it was found that 1.5 never tries
to close the peers connection on timeout, so it suffers for another issue.
2017-12-06 17:48:36 +01:00
Emeric Brun
8f29829e24 BUG/MEDIUM: checks: a down server going to maint remains definitely stucked on down state.
The new admin state was not correctly commited in this case.
Checks were fully disabled but the server was not marked in MAINT state.
It results with a server definitely stucked on the DOWN state.

This patch should be backported on haproxy 1.8
2017-12-06 17:01:00 +01:00
Emeric Brun
ece0c334bd BUG/MEDIUM: ssl engines: Fix async engines fds were not considered to fix fd limit automatically.
The number of async fd is computed considering the maxconn, the number
of sides using ssl and the number of engines using async mode.

This patch should be backported on haproxy 1.8
2017-12-06 14:17:41 +01:00
Willy Tarreau
473cf5d0cd BUG/MEDIUM: mworker: also close peers sockets in the master
There's a nasty case related to signaling all processes via SIGUSR1.
Since the master process still holds the peers sockets, the old process
trying to connect to the new one to teach it its tables has a risk to
connect to the master instead, which will not do anything, causing the
old process to hang instead of quitting.

This patch ensures we correctly close the peers in the master process
on startup, just like it is done for proxies. Ultimately we would rather
have a complete list of listeners to avoid such issues. But that's a bit
trickier as it would require using unbind_all() and avoiding side effects
the master could cause to other processes (like unlinking unix sockets).

To be backported to 1.8.
2017-12-06 11:14:08 +01:00
William Lallemand
c61c0b371b BUG/MINOR: ssl: support tune.ssl.cachesize 0 again
Since the split of the shctx and the ssl cache, we lost the ability to
disable the cache with tune.ssl.cachesize 0.

Worst than that, when using this configuration, haproxy segfaults during
the configuration parsing.

Must be backported to 1.8.
2017-12-04 18:48:26 +01:00
Willy Tarreau
6c71e4696b BUG/MAJOR: hpack: don't pretend large headers fit in empty table
In hpack_dht_make_room(), we try to fulfill this rule form RFC7541#4.4 :

 "It is not an error to attempt to add an entry that is larger than the
  maximum size; an attempt to add an entry larger than the maximum size
  causes the table to be emptied of all existing entries and results in
  an empty table."

Unfortunately it is not consistent with the way it's used in
hpack_dht_insert() as this last one will consider a success as a
confirmation it can copy the header into the table, and a failure as
an indexing error. This results in the two following issues :
  - if a client sends too large a header into an empty table, this
    header may overflow the table. Fortunately, most clients send
    small headers like :authority first, and never mark headers that
    don't fit into the table as indexable since it is counter-productive ;

  - if a client sends too large a header into a populated table, the
    operation fails after the table is totally flushed and the request
    is not processed.

This patch fixes the two issues at once :
  - a header not fitting into an empty table is always a sign that it
    will never fit ;
  - not fitting into the table is not an error

Thanks to Yves Lafon for reporting detailed traces demonstrating this
issue. This fix must be backported to 1.8.
2017-12-04 18:06:51 +01:00
Christopher Faulet
fd608dd2d2 BUG/MINOR: action: Don't check http capture rules when no id is defined
This is a regression in the commit 29730ba5 ("MINOR: action: Add a functions to
check http capture rules"). We must check the capture id only when an id is
defined.

This patch must be backported in 1.8.
2017-12-04 10:39:56 +01:00
Willy Tarreau
7912781a30 BUG/MINOR: h2: use the H2_F_DATA_* macros for DATA frames
A typo resulted in H2_F_HEADERS_* being used there, but it's harmless
as they are equal. Better fix the confusion though.

Should be backported to 1.8.
2017-12-03 21:09:38 +01:00
Willy Tarreau
637f64d565 BUG/MEDIUM: h2: do not accept upper case letters in request header names
This is explicitly forbidden by 7540#8.1.2, and may be used to bypass
some of the other filters, so they must be blocked early. It removes
another issue reported by h2spec.

To backport to 1.8.
2017-12-03 21:09:38 +01:00
Willy Tarreau
fe7c356be6 BUG/MEDIUM: h2: remove connection-specific headers from request
h2spec rightfully outlines that we used not to reject these ones, and
they may cause trouble if presented, especially "upgrade".

Must be backported to 1.8.
2017-12-03 21:09:18 +01:00
Willy Tarreau
520886990f BUG/MINOR: h2: reject response pseudo-headers from requests
At the moment there's only ":status". Let's block it early when parsing
the request. Otherwise it would be blocked by the HTTP/1 code anyway.
This silences another h2spec issue.

To backport to 1.8.
2017-12-03 21:08:43 +01:00
Willy Tarreau
92153fccd3 BUG/MINOR: h2: properly check PRIORITY frames
We don't use them right now but it's better to ensure they're properly
checked. This removes another 3 warnings in h2spec.

To backport to 1.8.
2017-12-03 21:08:43 +01:00
Willy Tarreau
18b86cd074 BUG/MINOR: h2: reject incorrect stream dependencies on HEADERS frame
We currently don't use stream dependencies, but as reported by h2spec,
the spec requires that we reject streams that depend on themselves in
HEADERS frames.

To backport to 1.8.
2017-12-03 21:08:42 +01:00
Willy Tarreau
1b38b46ab7 BUG/MINOR: h2: do not accept SETTINGS_ENABLE_PUSH other than 0 or 1
We don't use yet it but for correctness, let's enforce the check.

To backport to 1.8.
2017-12-03 21:08:42 +01:00
Willy Tarreau
497456154e BUG/MEDIUM: h2: enforce the per-connection stream limit
h2spec reports that we unfortunately didn't enforce the per-connection
stream limit that we advertise. It's important to ensure it's never
crossed otherwise it's cheap for a client to create many streams. This
requires the addition of a stream count. The h2c struct could be cleaned
up a bit, just like the h2_detach() function where an "if" block doesn't
make sense anymore since it's always true.

To backport to 1.8.
2017-12-03 21:08:42 +01:00
Willy Tarreau
d8d2ac75e8 BUG/MINOR: h2: the TE header if present may only contain trailers
h2spec reports this issue which has no side effect for now, but is
better cleared.

To backport to 1.8.
2017-12-03 21:08:42 +01:00
Willy Tarreau
68ed64148a BUG/MINOR: h2: fix a typo causing PING/ACK to be responded to
The ACK flag was tested on the frame type instead of the frame flag.

To backport to 1.8.
2017-12-03 21:08:41 +01:00
Willy Tarreau
cd4fe17a26 BUG/MINOR: h2: ":path" must not be empty
As reported by h2spec, the h2->h1 gateway doesn't verify that ":path"
is not empty. This is harmless since the H1 parser will reject such a
request, but better fix it anyway.

To backport to 1.8.
2017-12-03 21:08:41 +01:00
Willy Tarreau
9470d2cd35 BUG/MINOR: h2: try to abort closed streams as soon as possible
The purpose here is to be able to signal receipt of RST_STREAM to
streams when they start to provide a response so that the response
can be aborted ASAP. Given that RST_STREAM immediately switches the
stream to the CLOSED state, we must check for CLOSED in addition to
the existing ERROR check.

To be backported to 1.8.
2017-12-03 21:08:41 +01:00
Willy Tarreau
11cc2d6031 BUG/MINOR: h2: immediately close if receiving GOAWAY after the last stream
The h2spec test suite reveals that a GOAWAY frame received after the
last stream doesn't cause an immediate close, because we count on the
last stream to quit to do so. By simply setting the last_sid to the
received value in case it was not set, we can ensure to properly close
an idle connection during h2_wake().

To be backported to 1.8.
2017-12-03 21:08:40 +01:00
Willy Tarreau
811ad12414 BUG/MAJOR: h2: correctly check the request length when building an H1 request
Due to a typo in the request maximum length calculation, we count the
request path twice instead of counting it added to the method's length.
This has two effects, the first one being that a path cannot be larger
than half a buffer, and the second being that the method's length isn't
properly checked. Due to the way the temporary buffers are used internally,
it is quite difficult to meet this condition. In practice, the only
situation where this can cause a problem is when exactly one of either
the method or the path are compressed and the other ones is sent as a
literal.

Thanks to Yves Lafon for providing useful traces exhibiting this issue.

To be backported to 1.8.
2017-12-03 21:08:40 +01:00
Willy Tarreau
c611e6681b BUG/MINOR: hpack: dynamic table size updates are only allowed before headers
h2spec reports that we used to support a dynamic table size update
anywhere in the header block but it's only allowed before other
headers (cf RFC7541#4.2.1). In practice we don't use these for now
since we only use literals in responses.

To backport to 1.8.
2017-12-03 21:08:40 +01:00
Willy Tarreau
d85ba4e092 BUG/MINOR: hpack: reject invalid header index
If the hpack decoder sees an invalid header index, it emits value
"### ERR ###" that was used during debugging instead of rejecting the
block. This is harmless, and was detected by h2spec.

To backport to 1.8.
2017-12-03 21:08:39 +01:00
Willy Tarreau
4235d18214 BUG/MINOR: hpack: must reject huffman literals padded with more than 7 bits
h2spec reported that we didn't check that no more than 7 bits of padding
were left after decoding an huffman-encoded literal. This is harmless but
better fix it now.

To backport to 1.8.
2017-12-03 21:08:39 +01:00
Willy Tarreau
9e28f459b4 BUG/MINOR: hpack: fix debugging output of pseudo header names
When a pseudo header is used, name.ptr is NULL and we must replace it
with hpack_idx_to_name(). This only affects code built with DEBUG_HPACK.

To be backported to 1.8.
2017-12-03 09:43:38 +01:00
Olivier Houchard
6377a0004f BUG/MEDIUM: checks: Be sure we have a mux if we created a cs.
In connect_conn_chk(), there were one case we could return with a new
conn_stream created, but no mux attached. With no mux, cs_destroy() would
segfault. Fix that by setting the mux before we can fail.

This should be backported to 1.8.
2017-12-03 08:13:22 +01:00
Christian Ruppert
57dc283014 BUILD: Fix LDFLAGS vs. LIBS re linking order in various makefiles
Libraries should always be listed last. Should be backported to 1.8.

Signed-off-by: Christian Ruppert <idl0r@qasl.de>
2017-12-02 14:36:15 +01:00
Christopher Faulet
81991d3285 BUG/MAJOR: thread: Be sure to request a sync between threads only once at a time
The first thread requesting a synchronization is responsible to write in the
"sync" pipe to notify all others. But we must write only once in the pipe
between two synchronizations to have exactly one character in the pipe. It is
important because we only read 1 character in return when the last thread exits
from the sync-point.

Here there is a bug. If two threads request a synchronization, only the first
writes in the pipe. But, if the same thread requests several times a
synchronization before entering in the sync-point (because, for instance, it
detects many servers down), it writes as many as characters in the pipe. And
only one of them will be read. Repeating this bug many times will block HAProxy
on the write because the pipe is full.

To fix the bug, we just check if the current thread has already requested a
synchronization before trying to notify all others.

The patch must be backported in 1.8
2017-12-02 14:31:01 +01:00
Olivier Houchard
829aa24459 MINOR: threads: Fix pthread_setaffinity_np on FreeBSD.
As with the call to cpuset_setaffinity(), FreeBSD expects the argument to
pthread_setaffinity_np() to be a cpuset_t, not an unsigned long, so the call
was silently failing.

This should probably be backported to 1.8.
2017-12-02 14:23:12 +01:00
PiBa-NL
baf6ea4bd5 BUG/MINOR: mworker: detach from tty when in daemon mode
This allows a calling script to show the first startup output and
know when to stop reading from stdout so haproxy can daemonize.

To be backpored to 1.8.
2017-12-02 14:13:40 +01:00
PiBa-NL
4763ffdf04 BUG/MINOR: mworker: fix validity check for the pipe FDs
Check if master-worker pipe getenv succeeded, also allow pipe fd 0 as
valid. On FreeBSD in quiet mode the stdin/stdout/stderr are closed
which lets the mworker_pipe to use fd 0 and fd 1. Additionally exit()
upon failure to create or get the master-worker pipe.

This needs to be backported to 1.8.
2017-12-02 13:24:47 +01:00
Willy Tarreau
721d8e0286 MINOR: config: report when "monitor fail" rules are misplaced
"monitor-uri" may rely on "monitor fail" rules, which are processed
very early, immediately after the HTTP request is parsed and before
any http rulesets. It's not reported by the config parser when this
ruleset is misplaces, causing some configurations not to work like
users would expect. Let's just add the warning for a misplaced rule.
2017-12-01 18:25:08 +01:00
David Carlier
7e351eefe5 BUILD/MINOR: haproxy: compiling config cpu parsing handling when needed
parse_cpu_set is only relevant where there is cpu affinity,
avoiding in the process compilation warning as well.
2017-12-01 16:05:35 +01:00
Emeric Brun
088c9b73ca BUG/MAJOR: thread/peers: fix deadlock on peers sync.
Table lock was not released on an error path (if there is no
enough room to write table switch message).

[wt: needs to be backported to 1.8]
2017-12-01 15:06:43 +01:00
Emeric Brun
0fed0b0a38 BUG/MEDIUM: peers: fix some track counter rules dont register entries for sync.
This BUG was introduced with:
'MEDIUM: threads/stick-tables: handle multithreads on stick tables'

The API was reviewed to handle stick table entry updates
asynchronously and the caller must now call a 'stkable_touch_*'
function each time the content of an entry is modified to
register the entry to be synced.

There was missing call to stktable_touch_* resulting in
not propagated entries to remote peers (or local one during reload)
2017-11-29 19:16:22 +01:00
Willy Tarreau
872855998b BUG/MEDIUM: h2: don't report an error after parsing a 100-continue response
Yves Lafon reported a breakage with 100-continue. In fact the problem
is caused when an 1xx is the last response in the buffer (which commonly
is the case). We loop back immediately into the parser with what remains
of the input buffer (ie: nothing), while it is not expected to be called
with an empty response, so it fails.

Let's simply get back to the caller to decide whether or not more data
are expected to be sent.

This fix needs to be backported to 1.8.
2017-11-29 15:41:32 +01:00
Willy Tarreau
cea8537efd BUG/MEDIUM: threads/peers: decrement, not increment jobs on quitting
Commit 8d8aa0d ("MEDIUM: threads/listeners: Make listeners thread-safe")
mistakenly placed HA_ATOMIC_ADD(job, 1) to replace a job--, so it maintains
the job count too high preventing the process from cleanly exiting on
reload.

This needs to be backported to 1.8.
2017-11-29 14:51:20 +01:00
Emmanuel Hocdet
cebd7962e2 BUG/MINOR: ssl: CO_FL_EARLY_DATA removal is managed by stream
Manage BoringSSL early_data as it is with openssl 1.1.1.
2017-11-29 14:34:47 +01:00
David Carlier
903ddfd9c8 BUILD/MINOR: Makefile : enabling USE_CPU_AFFINITY
FreeBSD can handle cpuset matters just fine, we can hence enable it
by default as linux2628 TARGET.
2017-11-29 14:31:10 +01:00
David Carlier
6d5c841d24 BUILD/MINOR: haproxy : FreeBSD/cpu affinity needs pthread_np header
for pthread_*_np calls, pthread_np.h is needed under FreeBSD.
2017-11-29 14:30:38 +01:00
Willy Tarreau
5bcfd56519 BUG/MEDIUM: stream: fix session leak on applet-initiated connections
Commit 3e13cba ("MEDIUM: session: make use of the connection's destroy
callback") ensured that connections could be autonomous to destroy the
session they initiated, but it didn't take care of doing the same for
applets. Such applets are used for peers, Lua and SPOE outgoing
connections. In this case, once the stream ends, it closes everything
and nothing takes care of releasing the session. The problem is not
immediately obvious since the only visible effect is that older
processes will not quit on reload after having leaked one such session.

For now we check in stream_free() if the session's origin is the applet
we're releasing, and then free the session as well. Something more
uniform should probably be done once we manage to unify applets and
connections a bit more.

This fix needs to be backported to 1.8. Thanks to Emmanuel Hocdet for
reporting the problem.
2017-11-29 14:26:11 +01:00
Willy Tarreau
ec7464726f BUILD: checks: don't include server.h
server.h needs checks.h since it references the struct check, but depending
on the include order it will fail if check.h is included first due to this
one including server.h in turn while it doesn't need it.
2017-11-29 10:54:05 +01:00
William Lallemand
bcd9101a66 BUG/MEDIUM: cache: bad computation of the remaining size
The cache was not setting the hdrs_len to zero when we are called
in the http_forward_data with headers + body.

The consequence is to always try to store a size - the size of headers,
during the calls to http_forward_data even when it has already forwarded
the headers.

Thanks to Cyril Bont for reporting this bug.

Must be backported to 1.8.
2017-11-28 12:06:06 +01:00
William Lallemand
c3cd35f96c BUG/MEDIUM: ssl: don't allocate shctx several time
The shctx_init() function does not check anymore if the pointer is not
NULL, this check must be done is the caller.

The consequence was to allocate one shctx per ssl bind.

Bug introduced by 4f45bb9 ("MEDIUM: shctx: separate ssl and shctx")

Thanks to Maciej Zdeb for reporting this bug.

Must be backported to 1.8.
2017-11-28 12:04:16 +01:00