Commit Graph

13773 Commits

Author SHA1 Message Date
Amaury Denoyelle
c193823343 MEDIUM: h1: generate WebSocket key on response if needed
Add the Sec-Websocket-Accept header on a websocket handshake response.
This header may be missing if a h2 server is used with a h1 client.

The response key is calculated following the rfc6455. For this, the
handshake request key must be stored in the h1 session, as a new field
name ws_key. Note that this is only done if the message has been
prealably identified as a Websocket handshake request.
2021-01-28 16:37:14 +01:00
Amaury Denoyelle
18ee5c3eb0 MINOR: h1: reject websocket handshake if missing key
If a request is identified as a WebSocket handshake, it must contains a
websocket key header or else it can be reject, following the rfc6455.

A new flag H1_MF_UPG_WEBSOCKET is set on such messages.  For the request
te be identified as a WebSocket handshake, it must contains the headers:
  Connection: upgrade
  Upgrade: websocket

This commit is a compagnon of
"MEDIUM: h1: generate WebSocket key on response if needed" and
"MEDIUM: h1: add a WebSocket key on handshake if needed".

Indeed, it ensures that a WebSocket key is added only from a http/2 side
and not for a http/1 bogus peer.
2021-01-28 16:37:14 +01:00
Christopher Faulet
5b82cc5b5c MEDIUM: http-ana: Deal with L7 retries in HTTP analysers
The code dealing with the copy of requests in the L7-buffer and the
retransmits during L7 retries has been moved in the HTTP analysers. The copy
is now performed in the REQ_HTTP_XFER_BODY analyser and the L7 retries is
performed in the RES_WAIT_HTTP analyser. This way, si_cs_recv() and
si_cs_send() don't care of it anymore. It is much more natural to deal with
L7 retry in HTTP analysers.
2021-01-28 16:37:14 +01:00
Christopher Faulet
991febdfe0 MEDIUM: mux-h2: Don't emit DATA frame for bodyless responses
Some responses must not contain data. Reponses to HEAD requests and 204/304
responses. But there is no warranty that this will be really respected by
the senders or even if it is possible. For instance, the method may be
rewritten by an http-request rule (HEAD->GET). Thus, it is not really
possible to always strip these data from the response at the receive
stage. And the response may be emitted by an applet or an internal service
not strictly following the spec. All that to say that we may be prepared to
handle payload for bodyless responses on the sending path.

In addition, unlike the HTTP/1, it is not really clear that the trailers is
part of the payload or not. Thus, some clients may expect to have the
trailers, if any, in the response to a HEAD request. For instance, the GRPC
status is placed in a trailer and clients rely on it. But what happens for
204 responses then.  Read the following thread for details :

  https://lists.w3.org/Archives/Public/ietf-http-wg/2020OctDec/0040.html

So, thanks to previous patches, it is now possible to know on the sending
path if a response must be bodyless or not. So, for such responses, no DATA
frame is emitted, except eventually the last empty one carring the ES
flag. However, the TRAILERS frames are still emitted. The h2s_skip_data()
function is added to take care to remove HTX DATA blocks without emitting
any DATA frame expect the last one, if there is no trailers.
2021-01-28 16:37:14 +01:00
Christopher Faulet
7d247f0771 MINOR: h2/mux-h2: Add flags to notify the response is known to have no body
The H2 message flag H2_MSGF_BODYLESS_RSP is now used during the request or
the response parsing to notify the mux that, considering the parsed message,
the response is known to have no body. This happens during HEAD requests
parsing and during 204/304 responses parsing.

On the H2 multiplexer, the equivalent flag is set on H2 streams. Thus the
H2_SF_BODYLESS_RESP flag is set on a H2 stream if the H2_MSGF_BODYLESS_RSP
is found after a HEADERS frame parsing. Conversely, this flag is also set
when a HEADERS frame is emitted for HEAD requests and for 204/304 responses.

The H2_SF_BODYLESS_RESP flag will be used to ignore data payload from the
response but not the trailers.
2021-01-28 16:37:14 +01:00
Christopher Faulet
f3e7619041 MINOR: mux-h1: Don't add Connection close/keep-alive header for 1xx messages
No connection header must be added by the H1 mux in 1xx messages, including
101. Existing connection headers remains untouched, especially the "Connection:
upgrade" of 101 responses. This patch only avoids to add "Connection: close" or
"Connection: keep-alive" to 1xx responses.
2021-01-28 16:37:14 +01:00
Christopher Faulet
91fcf21e45 MINOR: mux-h1: Don't emit C-L and T-E headers for 204 and 1xx responses
204 and 1xx responses must not have any payload. Now, the H1 mux takes care
of that in last resort. But they also must not have any C-L or T-E
headers. Thus, if found on the sending path, these headers are ignored.
2021-01-28 16:37:14 +01:00
Christopher Faulet
e5596bf53f MEDIUM: mux-h1: Don't emit any payload for bodyless responses
Some responses must not contain data. Reponses to HEAD requests and 204/304
xresponses. But there is no warranty that this will be really respected by
the senders or even if it is possible. For instance, the method may be
rewritten by an http-request rule (HEAD->GET). Thus, it is not really
possible to always strip the payload from the response at the receive
stage. And the response may be emitted by an applet or an internal service
not strictly following the spec. All that to say that we may be prepared to
handle payload for bodyless responses on the sending path.

So, thanks to previous patches, it is now possible to know on the sending
path if a response must be bodyless or not. So, for such responses, no
payload is emitted, all HTX blocks after the EOH are silently removed
(including the trailers).
2021-01-28 16:37:14 +01:00
Christopher Faulet
5696f5450e MINOR: mux-h1: Add a flag on H1 streams with a response known to be bodyless
In HTTP/1, responses to HEAD requests and 204/304 must not have payload. The
H1S_F_BODYLESS_RESP flag is not set on streams that should handle such
responses, on the client side and the server side.

On the client side, this flag is set when a HEAD request is parsed and when
a 204/304 response is emitted. On the server side, this happends when a HEAD
request is emitted or a 204/304 response is parsed.
2021-01-28 16:37:14 +01:00
Christopher Faulet
d1ac2b90cd MAJOR: htx: Remove the EOM block type and use HTX_FL_EOM instead
The EOM block may be removed. The HTX_FL_EOM flags is enough. Most of time,
to know if the end of the message is reached, we just need to have an empty
HTX message with HTX_FL_EOM flag set. It may also be detected when the last
block of a message with HTX_FL_EOM flag is manipulated.

Removing EOM blocks simplifies the HTX message filling. Indeed, there is no
more edge problems when the message ends but there is no more space to write
the EOM block. However, some part are more tricky. Especially the
compression filter or the FCGI mux. The compression filter must finish the
compression on the last DATA block. Before it was performed on the EOM
block, an extra DATA block with the checksum was added. Now, we must detect
the last DATA block to be sure to finish the compression. The FCGI mux on
its part must be sure to reserve the space for the empty STDIN record on the
last DATA block while this record was inserted on the EOM block.

The H2 multiplexer is probably the part that benefits the most from this
change. Indeed, it is now fairly easier to known when to set the ES flag.

The HTX documentaion has been updated accordingly.
2021-01-28 16:37:14 +01:00
Christopher Faulet
789a472674 MINOR: htx: Add a function to know if a block is the only one in a message
The htx_is_unique_blk() function may now be used to know if a block is the
only one in an HTX message, excluding all unused blocks. Note the purpose of
this function is not to know if a block is the last one of an HTTP message.
This means no more data part from the message are expected, except tunneled
data. It only says if a block is alone in an HTX message.
2021-01-28 16:37:14 +01:00
Christopher Faulet
d8b44f0d9f REGTESTS: Don't run http_msg_full_on_eom script on the 2.4 anymore
The EOM block will be removed on the 2.4, thus this script will be broken on
this version. Now it is skipped for this version. It remains valid for 2.3
and 2.2.
2021-01-28 16:37:14 +01:00
Christopher Faulet
42432f347f MINOR: htx: Rename HTX_FL_EOI flag into HTX_FL_EOM
The HTX_FL_EOI flag is not well named. For now, it is not very used. But
that will change. It will replace the EOM block. Thus, it is renamed.
2021-01-28 16:37:14 +01:00
Christopher Faulet
5be651d4d7 BUG/MAJOR: mux-h1/mux-h2/htx: Fix HTTP tunnel management at the mux level
Tunnel management between the H1 and H2 multiplexers is a bit blurred. And
the HTX is not enough well defined on this point to make things clear. In
fact, Establishing a tunnel between an H2 client and an H1 server, or the
opposite is buggy because the both multiplexers don't handle the EOM block
the same way when a tunnel is established. In fact, the H2 multiplexer is
pretty strict and add an END_STREAM flag when an EOM block is found, while
the H1 multiplexer is more flexible.

The purpose of this patch is to make the EOM block usage pretty clear and to
fix the HTTP multiplexers to really handle HTTP tunnels in the right
way. Now, an EOM block is used to mark the end of an HTTP message,
semantically speaking. That means it may be followed by tunneled data. Thus,
CONNECT requests are now finished by an EOM block, just after the EOH block.

On the H1 multiplexer side, a tunnel is now only established on the response
path. So a CONNECT request remains in a DONE state waiting for the 2xx
response. On the H2 multiplexer side, a flag is used to know an HTTP tunnel
is requested, to not immediately add the END_STREAM flag on the EOM block.

All these changes are sensitives and not backportable because of recent
changes. The same problem exists on earlier versions and should be
addressed. But it will only be possible with a specific patchset.

This patch relies on the following ones :

  * MEDIUM: mux-h1: Properly handle tunnel establishments and aborts
  * MEDIUM: mux-h2: Close streams when processing data for an aborted tunnel
  * MEDIUM: mux-h2: Block client data on server side waiting tunnel establishment
  * MINOR: mux-h2: Add 2 flags to help to properly handle tunnel mode
  * MINOR: mux-h1: Split H1C_F_WAIT_OPPOSITE flag to separate input/output sides
  * MINOR: mux-h1/mux-fcgi: Don't set TUNNEL mode if payload length is unknown
2021-01-28 16:37:14 +01:00
Christopher Faulet
dea2474991 MEDIUM: mux-h1: Properly handle tunnel establishments and aborts
In the same way than the H2 mux, we now bloc data sending on the server side
if a tunnel is not fully established. In addition, if some data are still
pending for a aborted tunnel, an error is triggered and the server
connection is closed.

To do so, we rely on the H1C_F_WAIT_INPUT flag to bloc the output
processing. This patch contributes to fix the tunnel mode between the H1 and
the H2 muxes.
2021-01-28 16:37:14 +01:00
Christopher Faulet
91b21dc8d8 MEDIUM: mux-h2: Close streams when processing data for an aborted tunnel
In the previous patch ("MEDIUM: mux-h2: Block client data on server side
waiting tunnel establishment"), we added a way to block client data for not
fully established tunnel on the server side. This one closes the stream with
an ERR_CANCEL erorr if there are some pending tunneled data while the tunnel
was aborted. This may happen on the client side if a non-empty DATA frame or
an empty DATA frame without the ES flag is received. This may also happen on
the server side if there is a DATA htx block. However in this last case, we
first wait the response is fully forwarded.

This patch contributes to fix the tunnel mode between the H1 and the H2
muxes.
2021-01-28 16:37:14 +01:00
Christopher Faulet
f95f87650f MEDIUM: mux-h2: Block client data on server side waiting tunnel establishment
On the server side, when a tunnel is not fully established, we must block
tunneled data, waiting for the server response. It is mandatory because the
server may refuse the tunnel. This happens when a DATA htx block is
processed in tunnel mode (H2_SF_BODY_TUNNEL flag set) but before the
response HEADERS frame is received (H2_SF_HEADERS_RCVD flag no set). In this
case, the H2_SF_BLK_MBUSY flag is set to mark the stream as busy. This flag
is removed when the tunnel is fully established or aborted.

This patch contributes to fix the tunnel mode between the H1 and the H2
muxes.
2021-01-28 16:37:14 +01:00
Christopher Faulet
d0db42326d MINOR: mux-h2: Add 2 flags to help to properly handle tunnel mode
H2_SF_BODY_TUNNEL and H2_SF_TUNNEL_ABRT flags are added to properly handle
the tunnel mode in the H2 mux. The first one is used to detect tunnel
establishment or fully established tunnel. The second one is used to abort a
tunnel attempt. It is the first commit having as a goal to fix tunnel
establishment between H1 and H2 muxes.

There is a subtlety in h2_rcv_buf(). CS_FL_EOS flag is added on the
conn-stream when ES is received on a tunneled stream. It really reflects the
conn-stream state and is mandatory for next commits.
2021-01-28 16:37:14 +01:00
Christopher Faulet
b385b50fbb MINOR: mux-h1: Split H1C_F_WAIT_OPPOSITE flag to separate input/output sides
The H1C_F_WAIT_OPPOSITE flag is now splitted in 2 flags, H1C_F_WAIT_INPUT
and H1C_F_WAIT_OUTPUT, depending on the side is waiting. The change is a
prerequisite to fix the tunnel mode management in HTTP muxes.

H1C_F_WAIT_INPUT must be used to bloc the output side and to wait for an
event from the input side. H1C_F_WAIT_OUTPUT does the opposite. It bloc the
input side and wait for an event from the output side.
2021-01-28 16:37:14 +01:00
Christopher Faulet
1e857785e9 MINOR: mux-h1/mux-fcgi: Don't set TUNNEL mode if payload length is unknown
Responses with no C-L and T-E headers are no longer switched in TUNNEL mode
and remains in DATA mode instead. The H1 and FCGI muxes are updated
accordingly. This change reflects the real message state. It is not a true
tunnel. Data received are still part of the message.

It is not a bug. However, this message may be backported after some
observation period (at least as far as 2.2).
2021-01-28 16:37:14 +01:00
Christopher Faulet
8989942cfc BUG/MINOR: h2/mux-h2: Reject 101 responses with a PROTOCOL_ERROR h2s error
As stated in the RFC7540, section 8.1.1, the HTTP/2 removes support for the
101 informational status code. Thus a PROTOCOL_ERROR is now returned to the
server if a 101-switching-protocols response is received. Thus, the server
connection is aborted.

This patch may be backported as far as 2.0.
2021-01-28 16:36:40 +01:00
Christopher Faulet
6e6c7b1284 MEDIUM: http-ana: Refuse invalid 101-switching-protocols responses
A 101-switching-protocols response must contain a Connection header with the
Upgrade option. And this response must only be received from a server if the
client explicitly requested a protocol upgrade. Thus, the request must also
contain a Connection header with the Upgrade option. If not, a
502-bad-gateway response is returned to the client. This way, a tunnel is
only established if both sides are agree.

It is closer to what the RFC says, but it remains a bit flexible because
there is no check on the Upgrade header itself. However, that's probably
enough to ensure a tunnel is not established when not requested.

This one is not tagged as a bug. But it may be backported, at least to
2.3. It relies on :

  * MINOR: htx/http-ana: Save info about Upgrade option in the Connection header
2021-01-28 16:27:48 +01:00
Christopher Faulet
576c358508 MINOR: htx/http-ana: Save info about Upgrade option in the Connection header
Add an HTX start-line flag and its counterpart into the HTTP message to
track the presence of the Upgrade option into the Connection header. This
way, without parsing the Connection header again, it will be easy to know if
a client asks for a protocol upgrade and if the server agrees to do so. It
will also be easy to perform some conformance checks when a
101-switching-protocols is received.
2021-01-28 16:27:48 +01:00
Christopher Faulet
0f9395d81e BUG/MAJOR: mux-h1: Properly handle TCP to H1 upgrades
It is the second part and the most important of the fix.

Since the mux-h1 refactoring, and more specifically since the commit
c4bfa59f1 ("MAJOR: mux-h1: Create the client stream as later as possible"),
the upgrade from a TCP client connection to H1 is broken. Indeed, now the H1
mux is responsible to create the frontend conn-stream once the request
headers are fully received. But, to properly support TCP to H1 upgrades, we
must inherit from the existing conn-stream. To do so, if the conn-stream
already exists when the client H1 connection is created, we create a H1
stream in ST_ATTACHED state, but not ST_READY, and the conn-stream is
attached to it. Because the ST_READY state is not set, no data are xferred
to the data layer when h1_rcv_buf() is called and shutdowns are inhibited
except on client aborts. This way, the request is parsed the same way than
for a classical H1 connection. Once the request headers are fully received
and parsed, the data stream is upgraded and the ST_READY state is set.

A tricky case appears when an H2 upgrade is performed because the H2 preface
is matched. In this case, the conn-stream must be detached and destroyed
before switching to the H2 mux and releasing the current H1 mux. We must
also take care to detach and destroy the conn-stream when a timeout
occurres.

This patch relies on the following series of patches :

* BUG/MEDIUM: stream: Don't immediatly ack the TCP to H1 upgrades
* MEDIUM: http-ana: Do nothing in wait-for-request analyzer if not htx
* MINOR: stream: Add a function to validate TCP to H1 upgrades
* MEDIUM: mux-h1: Add ST_READY state for the H1 connections
* MINOR: mux-h1: Wake up instead of subscribe for reads after H1C creation
* MINOR: mux-h1: Try to wake up data layer first before calling its wake callback
* MINOR: stream-int: Take care of EOS in the SI wake callback function
* BUG/MINOR: stream: Don't update counters when TCP to H2 upgrades are performed

This fix is specific for 2.4. No backport needed.
2021-01-28 16:27:48 +01:00
Christopher Faulet
cdd1e2a44b BUG/MEDIUM: stream: Don't immediatly ack the TCP to H1 upgrades
Instead of switching the stream to HTX mode, the request channel is only
reset (the request buffer is xferred to the mux) and the SF_IGNORE flag is
set on the stream. This flag prevent any processing in case of abort. Once
the upgrade confirmed, the flag is removed, in stream_upgrade_from_cs().

It is only the first part of the fix. The next one ("BUG/MAJOR: mux-h1:
Properly handle TCP to H1 upgrades") is also required. Both rely on the
following series of patches :

* MEDIUM: http-ana: Do nothing in wait-for-request analyzer if not htx
* MINOR: stream: Add a function to validate TCP to H1 upgrades
* MEDIUM: mux-h1: Add ST_READY state for the H1 connections
* MINOR: mux-h1: Wake up instead of subscribe for reads after H1C creation
* MINOR: mux-h1: Try to wake up data layer first before calling its wake callback
* MINOR: stream-int: Take care of EOS in the SI wake callback function
* BUG/MINOR: stream: Don't update counters when TCP to H2 upgrades are performed

This fix is specific for 2.4. No backport needed.
2021-01-28 16:27:48 +01:00
Christopher Faulet
da46a0dca7 MEDIUM: http-ana: Do nothing in wait-for-request analyzer if not htx
If http_wait_for_request() analyzer is called with a non-htx stream, nothing
is performed and we return immediatly. For now, it is totally unexpected.
But it will be true during TCP to H1 upgrades, once fixed. Indeed, there
will be a transition period during these upgrades. First the mux will be
upgraded and the not the stream, and finally the stream will be upgraded by
the mux once ready. In the meantime, the stream will still be in raw
mode. Nothing will be performed in wait-for-request analyzer because it will
be the mux responsibility to handle errors.

This patch is required to fix the TCP to H1 upgrades.
2021-01-28 16:27:48 +01:00
Christopher Faulet
4ef84c9c41 MINOR: stream: Add a function to validate TCP to H1 upgrades
TCP to H1 upgrades are buggy for now. When such upgrade is performed, a
crash is experienced. The bug is the result of the recent H1 mux
refactoring, and more specifically because of the commit c4bfa59f1 ("MAJOR:
mux-h1: Create the client stream as later as possible"). Indeed, now the H1
mux is responsible to create the frontend conn-stream once the request
headers are fully received. Thus the TCP to H1 upgrade is a problem because
the frontend conn-stream already exists.

To fix the bug, we must keep this conn-stream and the associate stream and
use it in the H1 mux. To do so, the upgrade will be performed in two
steps. First, the mux is upgraded from mux-pt to mux-h1. Then, the mux-h1
performs the stream upgrade, once the request headers are fully received and
parsed. To do so, stream_upgrade_from_cs() must be used. This function set
the SF_HTX flags to switch the stream to HTX mode, it removes the SF_IGNORE
flags and eventually it fills the request channel with some input data.

This patch is required to fix the TCP to H1 upgrades and is intimately
linked with the next commits.
2021-01-28 16:27:48 +01:00
Christopher Faulet
39c7b6b09d MEDIUM: mux-h1: Add ST_READY state for the H1 connections
An alive H1 connection may be in one of these 3 states :

  * ST_IDLE : not active and is waiting to be reused (no h1s and no cs)
  * ST_EMBRYONIC : active with a h1s but without any cs
  * ST_ATTACHED : active with a h1s and a cs

ST_IDLE and ST_ATTACHED are possible for frontend and backend
connection. ST_EMBRYONIC is only possible on the client side, when we are
waiting for the request headers. The last one is the expected state for an
active connection processing data. These states are mutually exclusives.

Now, there is a new state, ST_READY. It may only be set if ST_ATTACHED is
also set and when the CS is considered as fully active. For now, ST_READY is
set in the same time of ST_ATTACHED. But it will be used to fix TCP to H1
upgrades. Idea is to have an H1 connection in ST_ATTACHED state but not
ST_READY yet and have more or less the same behavior than an H1 connection
in ST_EMBRYONIC state. And when the upgrade is fully achieved, the ST_READY
state may be set and the data layer may be notified accordingly.

So for now, this patch should not change anything. TCP to H1 upgrades are
still buggy. But it is mandatory to make it work properly.
2021-01-28 16:27:48 +01:00
Christopher Faulet
d9ee788b7a MINOR: mux-h1: Wake up H1C after its creation if input buffer is not empty
When a H1 connection is created, we now wakeup the H1C tasklet if there are
some data in the input buffer. If not we only subscribe for reads.

This patch is required to fix the TCP to H1 upgrades.
2021-01-28 16:27:15 +01:00
Christopher Faulet
ad4daf629e MINOR: mux-h1: Try to wake up data layer first before calling its wake callback
Instead of calling the data layer wake callback function, we now first try
to wake it up. If the data layer is subscribed for receives or for sends,
its tasklet is woken up. The wake callback function is only called as the
last chance to notify the data layer.
2021-01-28 16:22:53 +01:00
Christopher Faulet
89e34c261b MEDIUM: stream-int: Take care of EOS if the SI wake callback function
Because si_cs_process() is also the SI wake callback function, it may be
called from the mux layer. Thus, in such cases, it is performed outside any
I/O event and si_cs_recv() is not called. If a read0 is reported by the mux,
via the CS_FL_EOS flag, the event is not handled, because only si_cs_recv()
take care of this flag for now.

It is not a bug, because this does not happens for now. All muxes set this
flag when the data layer retrieve data (via mux->rcv_buf()). But it is safer
to be prepared to handle it from the wake callback. And in fact, it will be
useful to fix the HTTP upgrades of TCP connections (especially TCP>H1>H2
upgrades).

To be sure to not handle the same event twice, it is only handled if the
shutr is not already set on the input channel.
2021-01-28 16:22:04 +01:00
William Lallemand
7b79424c05 REGTESTS: set_ssl_server_cert.vtc: check the sha1 from the server
Check the sha1 from the server side with the sample ssl_c_sha1 sample
fetch in order to evict a possible problem with "show/set ssl cert".
2021-01-28 16:00:22 +01:00
William Lallemand
7e69637ac5 REGTESTS: set_ssl_server_cert.vtc: check the Sha1 Fingerprint
Check the sha1 fingerprint once the certificate was changed with "show
ssl cert". This way the test is more reliable.
2021-01-28 15:11:59 +01:00
William Lallemand
8788c6ff13 REGTESTS: set_ssl_server_cert.vtc: remove the abort command
Temporarily remove the abort command as it seems to cause problems when
trying to do a "show ssl cert" after it.
2021-01-28 15:04:03 +01:00
Amaury Denoyelle
08d87b3f49 BUG/MEDIUM: backend: never reuse a connection for tcp mode
The reuse of idle connections should only happen for a proxy with the
http mode. In case of a backend with the tcp mode, the reuse selection
and insertion in session list are skipped.

This behavior is present since commit :
MEDIUM: connection: Add private connections synchronously in session server list
It could also be further exagerated by :
MEDIUM: backend: add reused conn to sess if mux marked as HOL blocking

It can be backported up to 2.3.
2021-01-28 14:18:33 +01:00
Amaury Denoyelle
3f07c20fab BUG/MEDIUM: session: only retrieve ready idle conn from session
A bug was introduced by the early insertion of idle connections at the
end of connect_server. It is possible to reuse a connection not yet
ready waiting for an handshake (for example with proxy protocol or ssl).
A wrong duplicate xprt_handshake_io_cb tasklet is thus registered as a
side-effect.

This triggers the BUG_ON statement of xprt_handshake_subscribe :
    BUG_ON(ctx->subs && ctx->subs != es);

To counter this, a check is now present in session_get_conn to only
return a connection without the flag CO_FL_WAIT_XPRT. This might cause
sometimes the creation of dedicated server connections when in theory
reuse could have been used, but probably only occurs rarely in real
condition.

This behavior is present since commit :
    MEDIUM: connection: Add private connections synchronously in session server list
It could also be further exagerated by :
    MEDIUM: backend: add reused conn to sess if mux marked as HOL blocking

It can be backported up to 2.3.

NOTE : This bug seems to be only reproducible with mode tcp, for an
unknown reason. However, reuse should never happen when not in http
mode. This improper behavior will be the subject of a dedicated patch.

This bug can easily be reproducible with the following config (a
webserver is required to accept proxy protocol on port 31080) :

    global

    defaults
      mode tcp
      timeout connect 1s
      timeout server 1s
      timeout client 1s

    listen li
      bind 0.0.0.0:4444
      server bla1 127.0.0.1:31080 check send-proxy-v2

with the inject client :
    $ inject -u 10000 -d 10 -G 127.0.0.1:4444

This should fix the github issue #1058.
2021-01-28 14:16:27 +01:00
William Lallemand
8d67394f69 BUG/MINOR: ssl: init tmp chunk correctly in ssl_sock_load_sctl_from_file()
Use chunk_inistr() for a chunk initialisation in
ssl_sock_load_sctl_from_file() instead of a manual initialisation which
was not initialising head.

Fix issue #1073.

Must be backported as far as 2.2
2021-01-27 14:58:51 +01:00
William Lallemand
b8868498ed CLEANUP: ssl: remove dead code in ckch_inst_new_load_srv_store()
The new ckch_inst_new_load_srv_store() function which mimics the
ckch_inst_new_load_store() function includes some dead code which was
used only in the former function.

Fix issue #1081.
2021-01-27 14:44:59 +01:00
Christopher Faulet
3888b8cd7b BUG/MINOR: stats: Add a break after filling ST_F_MODE field for servers
The previous patch was pushed too quickly (399bf72f6 "BUG/MINOR: stats:
Remove a break preventing ST_F_QCUR to be set for servers"). It was not an
extra break but a misplaced break statement. Thus, now a break statement
must be added after filling the ST_F_MODE field in stats_fill_sv_stats().

No backport needed except if the above commit is backported.
2021-01-27 13:32:26 +01:00
Christopher Faulet
399bf72f66 BUG/MINOR: stats: Remove a break preventing ST_F_QCUR to be set for servers
There is an extra break statement wrongly placed in stats_fill_sv_stats()
function, just before filling the ST_F_QCUR field. It prevents this field to
be set to the right value for servers.

No backport needed except if commit 3a9a4992 ("MEDIUM: stats: allow to
select one field in `stats_fill_sv_stats`") is backported.
2021-01-27 12:48:38 +01:00
Tim Duesterhus
7f0f4786d1 CI: Fix DEBUG_STRICT definition for Coverity
The DEBUG_STRICT define needs to be passed as part of `DEBUG`, not as a bare
parameter.
2021-01-27 12:45:07 +01:00
Tim Duesterhus
491be54cf1 BUILD: Include stdlib.h in compiler.h if DEBUG_USE_ABORT is set
Building with `"DEBUG=-DDEBUG_STRICT=1 -DDEBUG_USE_ABORT=1"` previously emitted the warning:

    In file included from include/haproxy/api.h:35:0,
                     from src/mux_pt.c:13:
    include/haproxy/buf.h: In function ‘br_init’:
    include/haproxy/bug.h:42:90: warning: implicit declaration of function ‘abort’ [-Wimplicit-function-declaration]
     #define ABORT_NOW() do { extern void ha_backtrace_to_stderr(); ha_backtrace_to_stderr(); abort(); } while (0)
                                                                                              ^
    include/haproxy/bug.h:56:21: note: in expansion of macro ‘ABORT_NOW’
     #define CRASH_NOW() ABORT_NOW()
                         ^
    include/haproxy/bug.h:68:4: note: in expansion of macro ‘CRASH_NOW’
        CRASH_NOW();                                           \
        ^
    include/haproxy/bug.h:62:35: note: in expansion of macro ‘__BUG_ON’
     #define _BUG_ON(cond, file, line) __BUG_ON(cond, file, line)
                                       ^
    include/haproxy/bug.h:61:22: note: in expansion of macro ‘_BUG_ON’
     #define BUG_ON(cond) _BUG_ON(cond, __FILE__, __LINE__)
                          ^
    include/haproxy/buf.h:875:2: note: in expansion of macro ‘BUG_ON’
      BUG_ON(size < 2);
      ^

This patch fixes that issue. The `DEBUG_USE_ABORT` option exists for use with
static analysis tools. No backport needed.
2021-01-27 12:44:39 +01:00
William Lallemand
db26e2b00e CLEANUP: ssl: make load_srv_{ckchs,cert} match their bind counterpart
This patch makes things more consistent between the bind_conf functions
and the server ones:

- ssl_sock_load_srv_ckchs() loads the SSL_CTX in the server
  (ssl_sock_load_ckchs() load the SNIs in the bind_conf)

- add the server parameter to ssl_sock_load_srv_ckchs()

- changes made to the ckch_inst are done in
  ckch_inst_new_load_srv_store()
2021-01-26 15:19:36 +01:00
William Lallemand
795bd9ba3a CLEANUP: ssl: remove SSL_CTX function parameter
Since the server SSL_CTX is now stored in the ckch_inst, it is not
needed anymore to pass an SSL_CTX to ckch_inst_new_load_srv_store() and
ssl_sock_load_srv_ckchs().
2021-01-26 15:19:36 +01:00
William Lallemand
1dedb0a82a CLEANUP: ssl/cli: rework free in cli_io_handler_commit_cert()
The new feature allowing the change of server side certificates
introduced duplicated free code. Rework the code in
cli_io_handler_commit_cert() to be more consistent.
2021-01-26 15:19:36 +01:00
Remi Tricot-Le Breton
bb470aa327 MINOR: ssl: Remove client_crt member of the server's ssl context
The client_crt member is not used anymore since the server's ssl context
initialization now behaves the same way as the bind lines one (using
ckch stores and instances).
2021-01-26 15:19:36 +01:00
Remi Tricot-Le Breton
f3eedfe195 MEDIUM: ssl: Enable backend certificate hot update
When trying to update a backend certificate, we should find a
server-side ckch instance thanks to which we can rebuild a new ssl
context and a new ckch instance that replace the previous ones in the
server structure. This way any new ssl session will be built out of the
new ssl context and the newly updated certificate.

This resolves a subpart of GitHub issue #427 (the certificate part)
2021-01-26 15:19:36 +01:00
Remi Tricot-Le Breton
d817dc733e MEDIUM: ssl: Load client certificates in a ckch for backend servers
In order for the backend server's certificate to be hot-updatable, it
needs to fit into the implementation used for the "bind" certificates.
This patch follows the architecture implemented for the frontend
implementation and reuses its structures and general function calls
(adapted for the server side).
The ckch store logic is kept and a dedicated ckch instance is used (one
per server). The whole sni_ctx logic was not kept though because it is
not needed.
All the new functions added in this patch are basically server-side
copies of functions that already exist on the frontend side with all the
sni and bind_cond references removed.
The ckch_inst structure has a new 'is_server_instance' flag which is
used to distinguish regular instances from the server-side ones, and a
new pointer to the server's structure in case of backend instance.
Since the new server ckch instances are linked to a standard ckch_store,
a lookup in the ckch store table will succeed so the cli code used to
update bind certificates needs to be covered to manage those new server
side ckch instances.
2021-01-26 15:19:36 +01:00
Remi Tricot-Le Breton
ec805a32b9 MINOR: ssl: Certificate chain loading refactorization
Move the certificate chain loading code into a dedicated function that
will then be useable elsewhere.
2021-01-26 15:19:36 +01:00
Remi Tricot-Le Breton
442b7f2238 MINOR: ssl: Server ssl context prepare function refactoring
Split the server's ssl context initialization into the general ssl
related initializations and the actual initialization of a single
SSL_CTX structure. This way the context's initialization will be
usable by itself from elsewhere.
2021-01-26 15:19:36 +01:00