Commit Graph

6382 Commits

Author SHA1 Message Date
Frdric Lcaille
9d1b95b591 MINOR: server: Make 'default-server' support 'cookie' keyword.
Before this patch, 'cookie' setting was only supported by 'server' directives.
This patch makes 'default-server' directive also support 'cookie' setting.
Should not break anything.
2017-03-27 14:37:01 +02:00
Frdric Lcaille
547356e484 MINOR: server: Make 'default-server' support 'observe' keyword.
Before this path, 'observe' setting was only supported by 'server' directives.
This patch makes 'default-server' directives also support 'observe' setting.
Should not break anything.
2017-03-27 14:37:01 +02:00
Frdric Lcaille
16186236dd MINOR: server: Make 'default-server' support 'redir' keyword.
Before this patch only 'server' directives could support 'redir' setting.
This patch makes also 'default-server' directives support 'redir' setting.
Should not break anything.
2017-03-27 14:37:01 +02:00
Frdric Lcaille
5e57643e09 MINOR: server: Make 'default-server' support 'ca-file', 'crl-file' and 'crt' settings.
This patch makes 'default-server' directives support 'ca-file', 'crl-file' and
'crt' settings.
2017-03-27 14:37:01 +02:00
Frdric Lcaille
67e0e61316 MINOR: server: Make 'default-server' support 'track' setting.
Before this patch only 'server' directives could support 'track' setting.
This patch makes 'default-server' directives also support this setting.
Should not break anything.
2017-03-27 14:37:01 +02:00
Frdric Lcaille
65aa356c0b MINOR: server: Make 'default-server' support 'check' keyword.
Before this patch 'check' setting was only supported by 'server' directives.
This patch makes also 'default-server' directives support this setting.
A new 'no-check' keyword parser has been implemented to disable this setting both
in 'default-server' and 'server' directives.
Should not break anything.
2017-03-27 14:37:01 +02:00
Frdric Lcaille
273f321404 MINOR: server: Make 'default-server' support 'verifyhost' setting.
This patch makes 'default-server' directive support 'verifyhost' setting.
Note: there was a little memory leak when several 'verifyhost' arguments were
supplied on the same 'server' line.
2017-03-27 14:37:01 +02:00
Frdric Lcaille
7c8cd587c2 MINOR: server: Make 'default-server' support 'verify' keyword.
This patch makes 'default-server' directive support 'verify' keyword.
2017-03-27 14:37:01 +02:00
Frdric Lcaille
18388c910c CLEANUP: server: code alignement.
Code alignement again.
2017-03-27 14:37:01 +02:00
Frdric Lcaille
e892c4c670 MINOR: server: Make 'default-server' support 'send-proxy-v2-ssl*' keywords.
This patch makes 'default-server' directive support 'send-proxy-v2-ssl'
(resp. 'send-proxy-v2-ssl-cn') setting.
A new keyword 'no-send-proxy-v2-ssl' (resp. 'no-send-proxy-v2-ssl-cn') has been
added to disable 'send-proxy-v2-ssl' (resp. 'send-proxy-v2-ssl-cn') setting both
in 'server' and 'default-server' directives.
2017-03-27 14:37:01 +02:00
Frdric Lcaille
e381d7699e MINOR: server: Make 'default-server' support 'ssl' keyword.
This patch makes 'default-server' directive support 'ssl' setting.
 A new keyword 'no-ssl' has been added to disable this setting both
 in 'server' and 'default-server' directives.
2017-03-27 14:37:00 +02:00
Frdric Lcaille
2cfcdbe58d MINOR: server: Make 'default-server' support 'no-ssl*' and 'no-tlsv*' keywords.
This patch makes 'default-server' directive support 'no-sslv3' (resp. 'no-ssl-reuse',
'no-tlsv10', 'no-tlsv11', 'no-tlsv12', and 'no-tls-tickets') setting.
New keywords 'sslv3' (resp. 'ssl-reuse', 'tlsv10', 'tlsv11', 'tlsv12', and
'tls-no-tickets') have been added to disable these settings both in 'server' and
'default-server' directives.
2017-03-27 14:36:58 +02:00
Frdric Lcaille
ec16f0300a CLEANUP: server: code alignement.
Code alignement.
2017-03-27 14:36:12 +02:00
Frdric Lcaille
9698092ab6 MINOR: server: Make 'default-server' support 'force-sslv3' and 'force-tlsv1[0-2]' keywords.
This patch makes 'default-server' directive support 'force-sslv3'
and 'force-tlsv1[0-2]' settings.
New keywords 'no-force-sslv3' (resp. 'no-tlsv1[0-2]') have been added
to disable 'force-sslv3' (resp. 'force-tlsv1[0-2]') setting both in 'server' and
'default-server' directives.
2017-03-27 14:36:12 +02:00
Frdric Lcaille
340ae606af MINOR: server: Make 'default-server' support 'check-ssl' keyword.
This patch makes 'default-server' directive support 'check-ssl' setting
to enable SSL for health checks.
A new keyword 'no-check-ssl' has been added to disable this setting both in
'server' and 'default-server' directives.
2017-03-27 14:36:12 +02:00
Frdric Lcaille
31045e4c10 MINOR: server: Make 'default-server' support 'send-proxy' and 'send-proxy-v2 keywords.
This patch makes 'default-server' directive support 'send-proxy'
(resp. 'send-proxy-v2') setting.
A new keyword 'no-send-proxy' (resp. 'no-send-proxy-v2') has been added
to disable 'send-proxy' (resp. 'send-proxy-v2') setting both in 'server' and
'default-server' directives.
2017-03-27 14:36:12 +02:00
Frdric Lcaille
f9bc1d6a13 MINOR: server: Make 'default-server' support 'non-stick' keyword.
This patch makes 'default-server' directive support 'non-stick' setting.
A new keyword 'stick' has been added so that to disable
'non-stick' setting both in 'server' and 'default-server' directives.
2017-03-27 14:36:11 +02:00
Frdric Lcaille
1502cfd1a3 CLEANUP: server: code alignement.
Code alignement.
2017-03-27 14:36:11 +02:00
Frdric Lcaille
25df89066b MINOR: server: Make 'default-server' support 'check-send-proxy' keyword.
This patch makes 'default-server' directive support 'check-send-proxy' setting.
A new keyword 'no-check-send-proxy' has been added so that to disable
'check-send-proxy' setting both in 'server' and 'default-server' directives.
2017-03-27 14:36:11 +02:00
Frdric Lcaille
f5bf903be6 MINOR: server: Make 'default-server' support 'backup' keyword.
At this time, only 'server' supported 'backup' keyword.
This patch makes also 'default-server' directive support this keyword.
A new keyword 'no-backup' has been added so that to disable 'backup' setting
both in 'server' and 'default-server' directives.

For instance, provided the following sequence of directives:

default-server backup
server srv1
server srv2 no-backup

default-server no-backup
server srv3
server srv4 backup

srv1 and srv4 are declared as backup servers,
srv2 and srv3 are declared as non-backup servers.
2017-03-27 14:36:11 +02:00
Frdric Lcaille
8065b6d4f2 MINOR: server: irrelevant error message with 'default-server' config file keyword.
There is no reason to emit such an error message:
"'default-server' expects <name> and <addr>[:<port>] as arguments."
if less than two arguments are provided on 'default-server' lines.
This is a 'server' specific error message.
2017-03-27 14:33:58 +02:00
Frdric Lcaille
2efc649447 BUG/MINOR: cfgparse: loop in tracked servers lists not detected by check_config_validity().
There is a silly case where a loop is not detected in tracked servers lists:
when a server tracks itself.

Ex:
   server srv1 127.0.0.1:8000 track srv1

Well, this never happens and this does not prevent haproxy from working.

But with this next following configuration:

   server srv1 127.0.0.1:8000 track srv2
   server srv2 127.0.0.1:8000 track srv2
   server srv3 127.0.0.1:8000 track srv2

the code in charge of detecting such loops never returns (without any error message).
haproxy becomes stuck in an infinite loop because of this statement found
in check_config_validity():

for (loop = srv->track; loop && loop != newsrv; loop = loop->track);

Again, such a configuration is never accidentally used I guess.
This latter example seems silly, but as several 'default-server' directives may be used
in the same proxy section, and as 'default-server' settings are not resetted each a
new 'default-server' line is created, it will match the following configuration, in the future,
when 'track' setting will be supported by 'default-server':

   default-server track srv3
   server srv1 127.0.0.1:8000
   server srv2 127.0.0.1:8000
   .
   .
   .
   default-server check
   server srv3 127.0.0.1:8000
(cherry picked from commit 6528fc93d3c065fdac841f24e55cfe9674a67414)
2017-03-27 12:04:20 +02:00
Baptiste
fc72590544 MINOR: dns: improve DNS response parsing to use as many available records as possible
A "weakness" exist in the first implementation of the parsing of the DNS
responses: HAProxy always choses the first IP available matching family
preference, or as a failover, the first IP.

It should be good enough, since most DNS servers do round robin on the
records they send back to clients.
That said, some servers does not do proper round robin, or we may be
unlucky too and deliver the same IP to all the servers sharing the same
hostname.

Let's take the simple configuration below:

  backend bk
    srv s1 www:80 check resolvers R
    srv s2 www:80 check resolvers R

The DNS server configured with 2 IPs for 'www'.
If you're unlucky, then HAProxy may apply the same IP to both servers.

Current patch improves this situation by weighting the decision
algorithm to ensure we'll prefer use first an IP found in the response
which is not already affected to any server.

The new algorithm does not guarantee that the chosen IP is healthy,
neither a fair distribution of IPs amongst the servers in the farm,
etc...
It only guarantees that if the DNS server returns many records for a
hostname and that this hostname is being resolved by multiple servers in
the same backend, then we'll use as many records as possible.
If a server fails, HAProxy won't pick up an other record from the
response.
2017-03-24 11:54:51 +01:00
Cyril Bont
203ec5a2b5 MEDIUM: global: add a 'hard-stop-after' option to cap the soft-stop time
When SIGUSR1 is received, haproxy enters in soft-stop and quits when no
connection remains.
It can happen that the instance remains alive for a long time, depending
on timeouts and traffic. This option ensures that soft-stop won't run
for too long.

Example:
  global
    hard-stop-after 30s  # Once in soft-stop, the instance will remain
                         # alive for at most 30 seconds.
2017-03-23 23:03:57 +01:00
Andriy Palamarchuk
1a943c48fb DOC: Protocol doc: add noop TLV
Add definition of the PP2_TYPE_NOOP TLV which can be used for data
padding and alignment.
2017-03-23 22:22:43 +01:00
Andriy Palamarchuk
01105ac3a8 DOC: Protocol doc: add SSL TLVs, rename CHECKSUM
Add SSL-related TLV types PP2_SUBTYPE_SSL_CIPHER,
PP2_SUBTYPE_SSL_SIG_ALG and PP2_SUBTYPE_SSL_KEY_ALG. Rename
PP2_TYPE_CHECKSUM to PP2_TYPE_CRC32C to make it easier to add checksums
using other algorithms. Clarified encoding of the string fields.
Renamed ASCII to US-ASCII as recommended by
https://www.iana.org/assignments/character-sets/character-sets.xhtml.
2017-03-23 15:16:05 +01:00
Andriy Palamarchuk
ceae85ba4a DOC: Protocol doc: add checksum, TLV type ranges
Add the CRC32c checksum TLV PP2_TYPE_CHECKSUM. Reserve TLV type ranges
CUSTOM, EXPERIMENT and FUTURE. Clarify that only UNSPEC protocol byte is
mandatory to implement on the receiver.
2017-03-23 15:15:47 +01:00
Andriy Palamarchuk
f1eae4ec38 DOC/MINOR: Fix typos in proxy protocol doc 2017-03-23 15:15:47 +01:00
Willy Tarreau
fa0617660a MEDIUM: kqueue: only set FD_POLL_IN when there are pending data
Let's avoid setting FD_POLL_IN when there's no pending data. It will
save a useless recv() syscall on pure closes.
2017-03-21 16:35:17 +01:00
Willy Tarreau
19c4ab97c1 MEDIUM: kqueue: take care of EV_EOF to improve polling status accuracy
kevent() always sets EV_EOF with EVFILT_READ to notify of a read shutdown
and EV_EOF with EVFILT_WRITE to notify of a write error. Let's check this
flag to properly update the FD's polled status (FD_POLL_HUP and FD_POLL_ERR
respectively).

It's worth noting that this one can be coupled with a regular read event
to notify about a pending read followed by a shutdown, but for now we only
use this to set the relevant flags (HUP and ERR).

The poller now exhibits the flag HAP_POLL_F_RDHUP to indicate this new
capability.

An improvement may consist in not setting FD_POLL_IN when the "data"
field is null since it normally only reflects the amount of pending
data.
2017-03-21 16:35:17 +01:00
Willy Tarreau
dd437d9a4c MINOR: kqueue: exclusively rely on the kqueue returned status
After commit e852545 ("MEDIUM: polling: centralize polled events processing")
all pollers stopped to explicitly check the FD's polled status, except the
kqueue poller which is constructed a bit differently. It doesn't seem possible
to cause any issue such as missing an event however, but anyway it's better to
definitely get rid of this since the event filter already provides the event
direction.
2017-03-21 16:35:17 +01:00
Willy Tarreau
3c8a89642d OPTIM: poll: enable support for POLLRDHUP
On Linux since 2.6.17 poll() supports POLLRDHUP to notify of an upcoming
hangup after pending data. Making use of it allows us to avoid a useless
recv() after short responses on keep-alive connections. Note that we
automatically enable the feature once this flag has been met first in a
poll() status. Till now it was only enabled on epoll.
2017-03-21 16:30:44 +01:00
Willy Tarreau
68128710d0 BUG/MINOR: raw_sock: always perfom the last recv if RDHUP is not available
Curu Wong reported a case where haproxy used to send RST to a server
after receiving its FIN. The problem is caused by the fact that being
a server connection, its fd is marked with linger_risk=1, and that the
poller didn't report POLLRDHUP, making haproxy unaware of a pending
shutdown that came after the data, so it used to resort to nolinger
for closing.

However when pollers support RDHUP we're pretty certain whether or not
a shutdown comes after the data and we don't need to perform that extra
recv() call.

Similarly when we're dealing with an inbound connection we don't care
and don't want to perform this extra recv after a request for a very
unlikely case, as in any case we'll have to deal with the client-facing
TIME_WAIT socket.

So this patch ensures that only when it's known that there's no risk with
lingering data, as well as in cases where it's known that the poller would
have detected a pending RDHUP, we perform the fd_done_recv() otherwise we
continue, trying a subsequent recv() to try to detect a pending shutdown.

This effectively results in an extra recv() call for keep-alive sockets
connected to a server when POLLRDHUP isn't known to be supported, but
it's the only way to know whether they're still alive or closed.

This fix should be backported to 1.7, 1.6 and 1.5. It relies on the
previous patch bringing support for the HAP_POLL_F_RDHUP flag.
2017-03-21 16:30:44 +01:00
Willy Tarreau
5a767693b5 MINOR: fd: add a new flag HAP_POLL_F_RDHUP to struct poller
We'll need to differenciate between pollers which can report hangup at
the same time as read (POLL_RDHUP) from the other ones, because only
these ones may benefit from the fd_done_recv() optimization. Epoll has
had support for EPOLLRDHUP since Linux 2.6.17 and has always been used
this way in haproxy, so now we only set the flag once we've observed it
once in a response. It means that some initial requests may try to
perform a second recv() call, but after the first closed connection it
will be enough to know that the second call is not needed anymore.

Later we may extend these flags to designate event-triggered pollers.
2017-03-21 16:30:35 +01:00
Hongbo Long
e39683c4d4 BUG/MEDIUM: stream: fix client-fin/server-fin handling
A tcp half connection can cause 100% CPU on expiration.

First reproduced with this haproxy configuration :

  global
      tune.bufsize 10485760
  defaults
      timeout server-fin 90s
      timeout client-fin 90s
  backend node2
      mode tcp
      timeout server 900s
      timeout connect 10s
      server def 127.0.0.1:3333
  frontend fe_api
      mode  tcp
      timeout client 900s
      bind :1990
      use_backend node2

Ie timeout server-fin shorter than timeout server, the backend server
sends data, this package is left in the cache of haproxy, the backend
server continue sending fin package, haproxy recv fin package. this
time the session information is as follows:

  time the session information is as follows:
      0x2373470: proto=tcpv4 src=127.0.0.1:39513 fe=fe_api be=node2
      srv=def ts=08 age=1s calls=3 rq[f=848000h,i=0,an=00h,rx=14m58s,wx=,ax=]
      rp[f=8004c020h,i=0,an=00h,rx=,wx=14m58s,ax=] s0=[7,0h,fd=6,ex=]
      s1=[7,18h,fd=7,ex=] exp=14m58s

  rp has set the CF_SHUTR state, next, the client sends the fin package,

  session information is as follows:
      0x2373470: proto=tcpv4 src=127.0.0.1:39513 fe=fe_api be=node2
      srv=def ts=08 age=38s calls=4 rq[f=84a020h,i=0,an=00h,rx=,wx=,ax=]
      rp[f=8004c020h,i=0,an=00h,rx=1m11s,wx=14m21s,ax=] s0=[7,0h,fd=6,ex=]
      s1=[9,10h,fd=7,ex=] exp=1m11s

  After waiting 90s, session information is as follows:
      0x2373470: proto=tcpv4 src=127.0.0.1:39513 fe=fe_api be=node2
      srv=def ts=04 age=4m11s calls=718074391 rq[f=84a020h,i=0,an=00h,rx=,wx=,ax=]
      rp[f=8004c020h,i=0,an=00h,rx=?,wx=10m49s,ax=] s0=[7,0h,fd=6,ex=]
      s1=[9,10h,fd=7,ex=] exp=? run(nice=0)

  cpu information:
      6899 root      20   0  112224  21408   4260 R 100.0  0.7   3:04.96 haproxy

Buffering is set to ensure that there is data in the haproxy buffer, and haproxy
can receive the fin package, set the CF_SHUTR flag, If the CF_SHUTR flag has been
set, The following code does not clear the timeout message, causing cpu 100%:

stream.c:process_stream:

        if (unlikely((res->flags & (CF_SHUTR|CF_READ_TIMEOUT)) == CF_READ_TIMEOUT)) {
            if (si_b->flags & SI_FL_NOHALF)
            si_b->flags |= SI_FL_NOLINGER;
            si_shutr(si_b);
        }

If you have closed the read, set the read timeout does not make sense.
With or without cf_shutr, read timeout is set:

       if (tick_isset(s->be->timeout.serverfin)) {
           res->rto = s->be->timeout.serverfin;
           res->rex = tick_add(now_ms, res->rto);
       }

After discussion on the mailing list, setting half-closed timeouts the
hard way here doesn't make sense. They should be set only at the moment
the shutdown() is performed. It will also solve a special case which was
already reported of some half-closed timeouts not working when the shutw()
is performed directly at the stream-interface layer (no analyser involved).
Since the stream interface layer cannot know the timeout values, we'll have
to store them directly in the stream interface so that they are used upon
shutw(). This patch does this, fixing the problem.

An easier reproducer to validate the fix is to keep the huge buffer and
shorten all timeouts, then call it under tcploop server and client, and
wait 3 seconds to see haproxy run at 100% CPU :

  global
      tune.bufsize 10485760

  listen px
      bind :1990
      timeout client 90s
      timeout server 90s
      timeout connect 1s
      timeout server-fin 3s
      timeout client-fin 3s
      server def 127.0.0.1:3333

  $ tcploop 3333 L W N20 A P100 F P10000 &
  $ tcploop 127.0.0.1:1990 C S10000000 F
2017-03-21 15:04:43 +01:00
Lukas Tribus
aa83a3144b MINOR: doc: 2.4. Examples should be 2.5. Examples
Guillaume Michaud reported against Cyril's haproxy-dconv project
that the index for the Examples section should be 2.5 instead of
2.4.

Should be backported to 1.7 and 1.6, so that the example section
can be linked to:
https://cbonte.github.io/haproxy-dconv/1.6/configuration.html#2.5
2017-03-21 11:22:43 +01:00
Christopher Faulet
014e39c0b6 BUG/MAJOR: http: fix typo in http_apply_redirect_rule
Because of this typo, AN_RES_FLT_END was never called when a redirect rule is
applied on a keep-alive connection. In almost all cases, this bug has no
effect. But, it leads to a memory leak if a redirect is done on a http-response
rule when the HTTP compression is enabled.

This patch should be backported in 1.7.
2017-03-21 11:21:18 +01:00
Emmanuel Hocdet
9490cedb4e BUILD: ssl: fix OPENSSL_NO_SSL_TRACE for boringssl and libressl
"sample-fetch which captures the cipherlist" patch introduce #define
do deal with trace functions only available in openssl > 1.0.2.
Add this #define to libressl and boringssl environment.
Thanks to Piotr Kubaj for postponing and testing with libressl.
2017-03-20 12:01:44 +01:00
Emmanuel Hocdet
a52bb15cc7 BUILD: ssl: simplify SSL_CTX_set_ecdh_auto compatibility
SSL_CTX_set_ecdh_auto is declared (when present) with #define. A simple #ifdef
avoid to list all cases of ssllibs. It's a placebo in new ssllibs. It's ok with
openssl 1.0.1, 1.0.2, 1.1.0, libressl and boringssl.
Thanks to Piotr Kubaj for postponing and testing with libressl.
2017-03-20 12:01:34 +01:00
Felipe Guerreiro Barbosa Ruiz
00f55524e0 BUG: payload: fix payload not retrieving arbitrary lengths
This fixes a regression introduced in d7bdcb874b, that removed the
ability to use req.payload(0,0) to read the whole buffer content. The
offending commit is present starting in version 1.6, so the patch
should be backported to versions 1.6 and 1.7.
2017-03-20 07:25:37 +01:00
Willy Tarreau
de40d798de CLEANUP: connection: completely remove CO_FL_WAKE_DATA
Since it's only set and never tested anymore, let's remove it.
2017-03-19 12:18:27 +01:00
Willy Tarreau
9fa1ee61cc MEDIUM: connection: don't test for CO_FL_WAKE_DATA
This flag is always set when we end up here, for each and every data
layer (idle, stream-interface, checks), and continuing to test it
leaves a big risk of forgetting to set it as happened once already
before 1.5-dev13.

It could make sense to backport this into stable branches as part of
the connection flag fixes, after some cool down period.
2017-03-19 12:17:35 +01:00
Willy Tarreau
3c0cc49d30 BUG/MEDIUM: connection: ensure to always report the end of handshakes
Despite the previous commit working fine on all tests, it's still not
sufficient to completely address the problem. If the connection handler
is called with an event validating an L4 connection but some handshakes
remain (eg: accept-proxy), it will still wake the function up, which
will not report the activity, and will not detect a change once the
handshake it complete so it will not notify the ->wake() handler.

In fact the only reason why the ->wake() handler is still called here
is because after dropping the last handshake, we try to call ->recv()
and ->send() in turn and change the flags in order to detect a data
activity. But if for any reason the data layer is not interested in
reading nor writing, it will not get these events.

A cleaner way to address this is to call the ->wake() handler only
on definitive status changes (shut, error), on real data activity,
and on a complete connection setup, measured as CONNECTED with no
more handshake pending.

It could be argued that the handshake flags have to be made part of
the condition to set CO_FL_CONNECTED but that would currently break
a part of the health checks. Also a handshake could appear at any
moment even after a connection is established so we'd lose the
ability to detect a second end of handshake.

For now the situation around CO_FL_CONNECTED is not clean :
  - session_accept() only sets CO_FL_CONNECTED if there's no pending
    handshake ;

  - conn_fd_handler() will set it once L4 and L6 are complete, which
    will do what session_accept() above refrained from doing even if
    an accept_proxy handshake is still pending ;

  - ssl_sock_infocbk() and ssl_sock_handshake() consider that a
    handshake performed with CO_FL_CONNECTED set is a renegociation ;
    => they should instead filter on CO_FL_WAIT_L6_CONN

  - all ssl_fc_* sample fetch functions wait for CO_FL_CONNECTED before
    accepting to fetch information
    => they should also get rid of any pending handshake

  - smp_fetch_fc_rcvd_proxy() uses !CO_FL_CONNECTED instead of
    CO_FL_ACCEPT_PROXY

  - health checks (standard and tcp-checks) don't check for HANDSHAKE
    and may report a successful check based on CO_FL_CONNECTED while
    not yet done (eg: send buffer full on send_proxy).

This patch aims at solving some of these side effects in a backportable
way before this is reworked in depth :
  - we need to call ->wake() to report connection success, measure
    connection time, notify that the data layer is ready and update
    the data layer after activity ; this has to be done either if
    we switch from pending {L4,L6}_CONN to nothing with no handshakes
    left, or if we notice some handshakes were pending and are now
    done.

  - we document that CO_FL_CONNECTED exactly means "L4 connection
    setup confirmed at least once, L6 connection setup confirmed
    at least once or not necessary, all this regardless of any
    possibly remaining handshakes or future L6 negociations".

This patch also renames CO_FL_CONN_STATUS to the more explicit
CO_FL_NOTIFY_DATA, and works around the previous flags trick consiting
in setting an impossible combination of flags to notify the data layer,
by simply clearing the current flags.

This fix should be backported to 1.7, 1.6 and 1.5.
2017-03-19 12:06:18 +01:00
Willy Tarreau
52821e2737 BUG/MAJOR: stream-int: do not depend on connection flags to detect connection
Recent fix 7bf3fa3 ("BUG/MAJOR: connection: update CO_FL_CONNECTED before
calling the data layer") marked an end to a fragile situation where the
absence of CO_FL_{CONNECTED,L4,L6}* flags is used to mark the completion
of a connection setup. The problem is that by setting the CO_FL_CONNECTED
flag earlier, we can indeed call the ->wake() function from conn_fd_handler
but the stream-interface's wake function needs to see CO_FL_CONNECTED unset
to detect that a connection has just been established, so if there's no
pending data in the buffer, the connection times out. The other ->wake()
functions (health checks and idle connections) don't do this though.

So instead of trying to detect a subtle change in connection flags,
let's simply rely on the stream-interface's state and validate that the
connection is properly established and that handshakes are completed
before reporting the WRITE_NULL indicating that a pending connection was
just completed.

This patch passed all tests of handshake and non-handshake combinations,
with synchronous and asynchronous connect() and should be safe for backport
to 1.7, 1.6 and 1.5 when the fix above is already present.
2017-03-19 12:00:04 +01:00
Willy Tarreau
d78c0fa0fb TESTS: add a test configuration to stress handshake combinations
This config tries to involve the various possible combinations of connection
handshakes, on the accept side and on the connect side. It also produces logs
indicating the handshake time.

May be tested with tcploop as the server, both for TCP and HTTP mode :
   - accept new connection
   - pause 100ms
   - send what looks like an HTTP response
   - wait 500ms and close

Starting log server (mainly to check timers) :
   $ socat udp-recvfrom:5514,fork -

Starting server :
   $ tcploop 8000 L N A W P100 S:"HTTP/1.0 200 OK\r\nConnection: close\r\n\r\n" P500

Testing all combinations with server-speaks-first (tcp) :
   $ nc 0 8007

Testing all combinations with client-speaks-first (tcp) :
   $ (printf "GET / HTTP/1.0\r\n\r\n";sleep 1) | nc 0 8007

Testing all combinations with client-speaks-first after pause (tcp) :
   $ (usleep 0.05 ; printf "GET / HTTP/1.0\r\n\r\n";sleep 1) | nc 0 8007

Testing all combinations with client-speaks-first (http) :
   $ (printf "GET / HTTP/1.0\r\n\r\n";sleep 1) | nc 0 8017

Testing all combinations with client-speaks-first after pause (http) :
   $ (usleep 0.05 ; printf "GET / HTTP/1.0\r\n\r\n";sleep 1) | nc 0 8017

Same tests must be redone after surrounding connect() in tcp_connect_server()
with fcntl(fd, F_SETFL, 0) and fcntl(fd, F_SETFL, O_NONBLOCK) for sycnhronous
connect().
2017-03-19 11:59:47 +01:00
Christopher Faulet
e6006245de BUG/MEDIUM: filters: Fix channels synchronization in flt_end_analyze
When a filter is used, there are 2 channel's analyzers to surround all the
others, flt_start_analyze and flt_end_analyze. This is the good place to acquire
and release resources used by filters, when needed. In addition, the last one is
used to synchronize the both channels, especially for HTTP streams. We must wait
that the analyze is finished for the both channels for an HTTP transaction
before restarting it for the next one.

But this part was buggy, leading to unexpected behaviours. First, depending on
which channel ends first, the request or the response can be switch in a
"forward forever" mode. Then, the HTTP transaction can be cleaned up too early,
while a processing is still in progress on a channel.

To fix the bug, the flag CF_FLT_ANALYZE has been added. It is set on channels in
flt_start_analyze and is kept if at least one filter is still analyzing the
channel. So, we can trigger the channel syncrhonization if this flag was removed
on the both channels. In addition, the flag TX_WAIT_CLEANUP has been added on
the transaction to know if the transaction must be cleaned up or not during
channels syncrhonization. This way, we are sure to reset everything once all the
processings are finished.

This patch should be backported in 1.7.
2017-03-15 19:09:06 +01:00
Olivier Houchard
a5938f71e4 CLEANUP: config: Typo in comment.
This is for the recently merged dynamic cookie patch set.
2017-03-15 16:01:51 +01:00
Olivier Houchard
2cb49ebbc4 BUG/MEDIUM server: Fix crash when dynamic is defined, but not key is provided.
Wait until we're sure we have a key before trying to calculate its length.

[wt: no backport needed, was just merged]
2017-03-15 16:01:33 +01:00
Willy Tarreau
3569df3fcf BUG/MEDIUM: listener: do not try to rebind another process' socket
When the "process" setting of a bind line limits the processes a
listening socket is enabled on, a "disable frontend" operation followed
by an "enable frontend" triggers a bug because all declared listeners
are attempted to be bound again regardless of their assigned processes.
This can at minima create new sockets not receiving traffic, and at worst
prevent from re-enabling a frontend if it's bound to a privileged port.

This bug was introduced by commit 1c4b814 ("MEDIUM: listener: support
rebinding during resume()") merged in 1.6-dev1, trying to perform the
bind() before checking the process list instead of after.

Just move the process check before the bind() operation to fix this.
This fix must be backported to 1.7 and 1.6.

Thanks to Pavlos for reporting this one.
2017-03-15 12:47:46 +01:00
Willy Tarreau
a84a2dba94 CONTRIB: tcploop: use the trash instead of NULL for recv()
NULL is Linux-centric and we're not focused on performance here but
portability and reproducibility. Don't use NULL and use the trash
instead. It may lead to multiple recv() calls for large blocks but
as a benefit it will be possible to see the contents with strace.
2017-03-15 11:48:46 +01:00