Commit Graph

10014 Commits

Author SHA1 Message Date
Christopher Faulet
5adbeeb336 MINOR: stats/htx: Call channel_add_input instead of updating channel state by hand
This way we are sure the channel state is always correctly upadated, especially
the amount of data directly forwarded. For the stats applet, it is not a bug
because the fast forwarding is never used (the response is chunked and the HTX
extra field is always set to 0).

This patch must be backported to 1.9.
2019-01-02 20:12:47 +01:00
Christopher Faulet
e64582929f MINOR: channel: Add the function channel_add_input
This function must be called when new incoming data are pushed in the channel's
buffer. It updates the channel state and take care of the fast forwarding by
consuming right amount of data and decrementing "->to_forward" accordingly when
necessary. In fact, this patch just moves a part of ci_putblk in a dedicated
function.

This patch must be backported to 1.9.
2019-01-02 20:12:44 +01:00
Willy Tarreau
b1d7b700bb BUG/MEDIUM: log: don't mark log FDs as non-blocking on terminals
With the new ability to log to a terminal, it's convenient to be able
to use "log stdout" in a config file, except that it now results in
setting the terminal to non-blocking mode, breaking every utility
relying on stdin afterwards. Since the only reason for logging to a
terminal is to debug, do not set the FD to non-blocking mode when it's
a terminal.

This fix must be backported to 1.9.
2019-01-02 20:12:02 +01:00
Alex Zorin
4afdd13842 MINOR: payload: add sample fetch for TLS ALPN
Application-Layer Protocol Negotiation (ALPN, RFC7301) is a TLS
extension which allows a client to present a preference for which
protocols it wishes to connect to, when a single port supports multiple
multiple application protocols.
It allows a transparent proxy to take a decision based on the beginning
of an SSL/TLS stream without deciphering it.

The new fetch "req.ssl_alpn" extracts the ALPN protocol names that may
be present in the ClientHello message.
2019-01-01 09:15:01 +01:00
Olivier Houchard
a2dbeb22fc MEDIUM: sessions: Keep track of which connections are idle.
Instead of keeping track of the number of connections we're responsible for,
keep track of the number of connections we're responsible for that we are
currently considering idling (ie that we are not using, they may be in use
by other sessions), that way we can actually reuse connections when we have
more connections than the max configured.
2018-12-28 19:16:03 +01:00
Olivier Houchard
c685d700fd MEDIUM: servers: Be smarter when switching connections.
When connecting to a server, and reusing a connection, always attempt to give
the owner of the previous session one of its own connections, so that one
session won't be responsible for too many connections.

This should be backported to 1.9.
2018-12-28 16:34:03 +01:00
Olivier Houchard
4f41751ad2 BUG/MEDIUM: servers: Flag the stream_interface on handshake error.
When creating a new outgoing connection, if we're using ALPN and waiting
for the handshake completion to choose the mux, and for some reason the
handshake failed, add the SI_FL_ERR flag to the stream_interface, so that
process_streams() knows the connection failed, and can attempt to retry,
instead of just hanging.

This should be backported to 1.9.
2018-12-28 16:33:22 +01:00
Olivier Houchard
351411facd BUG/MAJOR: sessions: Use an unlimited number of servers for the conn list.
When a session adds a connection to its connection list, we used to remove
connections for an another server if there were not enough room for our
server. This can't work, because those lists are now the list of connections
we're responsible for, not just the idle connections.
To fix this, allow for an unlimited number of servers, instead of using
an array, we're now using a linked list.
2018-12-28 16:33:13 +01:00
Olivier Houchard
5f7de56a08 BUG/MAJOR: servers: Correctly use LIST_ELEM().
To access the first element of the list, correctly use LIST_ELEM(), or we
end up getting the head of the list, instead of getting the first connection.

This should be backported to 1.9.
2018-12-28 16:33:06 +01:00
Olivier Houchard
c3fa638b4c BUG/MAJOR: servers: Use the list api correctly to avoid crashes.
In connect_server(), if we looked for an usable connection and failed to
find one, srv_conn won't be NULL at the end of list_for_each_entry(), but
will point to the head of a list, which is not a pointer to a struct
connection, so explicitely set it to NULL.

This should be backported to 1.9.
2018-12-28 16:33:00 +01:00
Olivier Houchard
134a2045bb BUG/MEDIUM: servers: Fail if we fail to allocate a conn_stream.
If, for some reason we failed to allocate a conn_stream when reusing an
existing connection, set srv_conn to NULL, so that we fail later, instead
of pretending all is right. This ends up giving a stream_interface with
no endpoint, and so the stream will never end.

This should be backported to 1.9.
2018-12-28 15:49:24 +01:00
Olivier Houchard
855ac25d82 BUG/MEDIUM: mux_h2: Don't add to the idle list if we're full.
In h2_detach(), don't add the connection to the idle list if nb_streams
is at the max. This can happen if we already closed that stream before, so
its slot became available and was used by another stream.

This should be backported to 1.9.
2018-12-28 15:48:52 +01:00
Jérôme Magnin
86cef23266 BUG/MINOR: htx: send the proper authenticate header when using http-request auth
When we use htx and http-request auth rules, we need to send WWW-Authenticate
with a 401 and Proxy-Authenticate with a 407. We only sent Proxy-Authenticate
regardless of status, with htx enabled.

To be backported to 1.9.
2018-12-28 15:48:12 +01:00
Olivier Houchard
09e498f1a1 BUG/MEDIUM: tasks: Decrement tasks_run_queue in tasklet_free().
If the tasklet is in the list, don't forget to decrement tasks_run_queue
in tasklet_free().

This should be backported to 1.9.
2018-12-24 14:04:55 +01:00
Olivier Houchard
bb3dac37a2 BUG/MEDIUM: servers: Don't try to reuse connection if we switched server.
In connect_server(), don't attempt to reuse the old connection if it's
targetting a different server than the one we're supposed to access, or
we will never be able to connect to a server if the first one we tried failed.

This should be backported to 1.9.
2018-12-24 13:45:43 +01:00
Willy Tarreau
48507ef558 CLEANUP: mux-h2: remove misleading comments about CONTINUATION
These ones were left-over from copy-pastes that are unrelated to
CONTINUATION frames.
2018-12-24 11:45:00 +01:00
Willy Tarreau
ea18f86364 MEDIUM: mux-h2: handle decoding of CONTINUATION frames
Now that the HEADERS frame decoding is retryable, we can safely try to
fold CONTINUATION frames into a HEADERS frame when the END_OF_HEADERS
flag is missing. In order to do this, h2c_decode_headers() moves the
frames payloads in-situ and leaves a hole that is plugged when leaving
the function. There is no limit to the number of CONTINUATION frames
handled this way provided that all of them fit into the buffer. The
error reported when meeting isolated CONTINUATION frames has now changed
from INTERNAL_ERROR to PROTOCOL_ERROR.

Now there is only one (unrelated) remaining failure in h2spec.
2018-12-24 11:45:00 +01:00
Willy Tarreau
a4428bd531 MINOR: mux-h2: make h2_peek_frame_hdr() support an offset
This function will be used to parse multiple subsequent frames so it
needs to support an offset.
2018-12-24 11:45:00 +01:00
Willy Tarreau
f48919aafb MINOR: buffers: add a new b_move() function
This function will be used to move parts of a buffer to another place
in the same buffer, even if the parts overlap. In order to keep things
under reasonable control, it only uses a length and absolute offsets
for the source and destination, and doesn't consider head nor data.
2018-12-24 11:45:00 +01:00
Willy Tarreau
96a10c24cf MINOR: mux-h2: fail stream creation more cleanly using RST_STREAM
The H2 demux only checks for too many streams in h2c_frt_stream_new(),
then refuses to create a new stream and causes the connection to be
aborted by sending a GOAWAY frame. This will also happen if any error
happens during the stream creation (e.g. memory allocation).

RFC7540#5.1.2 says that attempts to create streams in excess should
instead be dealt with using an RST_STREAM frame conveying either the
PROTOCOL_ERROR or REFUSED_STREAM reason (the latter being usable only
if it is guaranteed that the stream was not processed). In theory it
should not happen for well behaving clients, though it may if we
configure a low enough h2.max_concurrent_streams limit. This error
however may definitely happen on memory shortage.

Previously it was not possible to use RST_STREAM due to the fact that
the HPACK decompressor would be desynchronized. But now we first decode
and only then try to allocate the stream, so the decompressor remains
synchronized regardless of policy or resources issues.

With this patch we enforce stream termination with RST_STREAM and
REFUSED_STREAM if this protocol violation happens, as well as if there
is a temporary condition like a memory allocation issue. It will allow
a client to recover cleanly.

This could possibly be backported to 1.9. Note that this requires that
these five previous patches are merged as well :

    MINOR: h2: add a bit-based frame type representation
    MEDIUM: mux-h2: remove padlen during headers phase
    MEDIUM: mux-h2: decode HEADERS frames before allocating the stream
    MINOR: mux-h2: make h2c_send_rst_stream() use the dummy stream's error code
    MINOR: mux-h2: add a new dummy stream for the REFUSED_STREAM error code
2018-12-24 11:45:00 +01:00
Willy Tarreau
8d0d58bf6a MINOR: mux-h2: add a new dummy stream for the REFUSED_STREAM error code
This patch introduces a new dummy stream, h2_refused_stream, in CLOSED
status with the aforementioned error code. It will be usable to reject
unexpected extraneous streams.
2018-12-24 11:45:00 +01:00
Willy Tarreau
e6888fff75 MINOR: mux-h2: make h2c_send_rst_stream() use the dummy stream's error code
We currently have 2 dummy streams allowing us to send an RST_STREAM
message with an error code matching this one. However h2c_send_rst_stream()
still enforces the STREAM_CLOSED error code for these dummy streams,
ignoring their respective errcode fields which however are properly
set.

Let's make the function always use the stream's error code. This will
allow to create other dummy streams for different codes.
2018-12-24 11:45:00 +01:00
Willy Tarreau
5c8cafae39 MEDIUM: mux-h2: decode HEADERS frames before allocating the stream
It's hard to recover from a HEADERS frame decoding error after having
already created the stream, and it's not possible to recover from a
stream allocation error without dropping the connection since we can't
maintain the HPACK context, so let's decode it before allocating the
stream, into a temporary buffer that will then be offered to the newly
created stream.
2018-12-24 11:45:00 +01:00
Willy Tarreau
6fa380dbba MINOR: mux-h2: remove useless check for empty frame length in h2s_decode_headers()
This test for an empty frame was already performed in the callers, there
is no need for checking it again.
2018-12-24 11:45:00 +01:00
Willy Tarreau
deab244dc1 MINOR: h2: add a bit-based frame type representation
This will ease checks among sets of frames.
2018-12-24 11:45:00 +01:00
Willy Tarreau
3bf6918cb2 MEDIUM: mux-h2: remove padlen during headers phase
Three types of frames may be padded : DATA, HEADERS and PUSH_PROMISE.
Currently, each of these independently deals with padding and needs to
wait for and skip the initial padlen byte. Not only this complicates
frame processing, but it makes it very hard to process CONTINUATION
frames after a padded HEADERS frame, and makes it complicated to perform
atomic calls to h2s_decode_headers(), which are needed if we want to be
able to maintain the HPACK decompressor's context even when dropping
streams.

This patch takes a different approach : the padding is checked when
parsing the frame header, the padlen byte is waited for and parsed,
and the dpl value is updated with this padlen value. This will allow
the frame parsers to decide to overwrite the padding if needed when
merging adjacent frames.
2018-12-24 11:45:00 +01:00
Willy Tarreau
a875466243 BUG/MEDIUM: mux-h2: mark that we have too many CS once we have more than the max
Since commit f210191 ("BUG/MEDIUM: h2: don't accept new streams if
conn_streams are still in excess") we're refraining from reading input
frames if we've reached the limit of number of CS. The problem is that
it prevents such situations from working fine. The initial purpose was
in fact to prevent from reading new HEADERS frames when this happens,
and causes some occasional transfer hiccups and pauses with large
concurrencies.

Given that we now properly reject extraneous streams before checking
this value, we can be sure never to have too many streams, and that
any higher value is only caused by a scheduling reason and will go
down after the scheduler calls the code.

This fix must be backported to 1.9 and possibly to 1.8. It may be
tested using h2spec this way with an h2spec config :

  while :; do
    h2spec -o 5 -v -t -S -k -h 127.0.0.1 -p 4443 http2/5.1.2
  done
2018-12-24 08:13:16 +01:00
Willy Tarreau
c4ea04c2b6 BUG/MINOR: mux-h2: make empty HEADERS frame return a connection error
We were returning a stream error of type PROTOCOL_ERROR on empty HEADERS
frames, but RFC7540#4.2 stipulates that we should instead return a
connection error of type FRAME_SIZE_ERROR.

This may be backported to 1.9 and 1.8 though it's unlikely to have any
real life effect.
2018-12-23 10:02:38 +01:00
Willy Tarreau
9832a37b16 REGTESTS: remove the expected window updates from H2 handshakes
These ones are not needed anymore since commit 97aaa67 ("MINOR: mux-h2:
only increase the connection window with the first update"). The tests
should now be more reliable. It might be worth simply removing all the
explicit handshake though it doesn't hurt and still serves as documentation.
2018-12-23 09:58:41 +01:00
Willy Tarreau
97aaa67658 MINOR: mux-h2: only increase the connection window with the first update
Commit dc57236 ("BUG/MINOR: mux-h2: advertise a larger connection window
size") caused a WINDOW_UPDATE message to be sent early with the connection
to increase the connection's window size. It turns out that it causes some
minor trouble that need to be worked around :
  - varnishtest cannot transparently cope with the WU frames during the
    handshake, forcing all tests to explicitly declare the handshake
    sequence ;
  - some vtc scripts randomly fail if the WU frame is sent after another
    expected response frame, adding uncertainty to some tests ;
  - h2spec doesn't correctly identify these WU at the connection level
    that it believes are the responses to some purposely erroneous frames
    it sends, resulting in some errors being reported

None of these are a problem with real clients but they add some confusion
during troubleshooting.

Since the fix above was intended to increase the upload bandwidth, we
have another option which is to increase the window size with the first
WU frame sent for the connection. This way, no WU frame is sent until
one is really needed, and this first frame will adjust the window to
the maximum value. It will make the window increase slightly later, so
the client will experience the first round trip when uploading data,
but this should not be perceptible, and is not worth the extra hassle
needed to maintain our debugging abilities. As an extra bonus, a few
extra bytes are saved for each connection until the first attempt to
upload data.

This should possibly be backported to 1.9 and 1.8.
2018-12-23 09:49:04 +01:00
Willy Tarreau
fba74ea7b0 [RELEASE] Released version 2.0-dev0
Released version 2.0-dev0 with the following main changes :
    - BUG/MAJOR: connections: Close the connection before freeing it.
    - REGTEST: Require the option LUA to run lua tests
    - REGTEST: script: Process script arguments before everything else
    - REGTEST: script: Evaluate the varnishtest command to allow quoted parameters
    - REGTEST: script: Add the option --clean to remove previous log direcotries
    - REGTEST: script: Add the option --debug to show logs on standard ouput
    - REGTEST: script: Add the option --keep-logs to keep all log directories
    - REGTEST: script: Add the option --use-htx to enable the HTX in regtests
    - REGTEST: script: Print only errors in the results report
    - REGTEST: Add option to use HTX prefixed by the macro 'no-htx'
    - REGTEST: Make reg-tests target support argument.
    - REGTEST: Fix a typo about barrier type.
    - REGTEST: Be less Linux specific with a syslog regex.
    - REGTEST: Missing enclosing quotes for ${tmpdir} macro.
    - REGTEST: Exclude freebsd target for some reg tests.
    - BUG/MEDIUM: h2: Don't forget to quit the sending_list if SUB_CALL_UNSUBSCRIBE.
    - BUG/MEDIUM: mux-h2: Don't forget to quit the send list on error reports
    - BUG/MEDIUM: dns: Don't prevent reading the last byte of the payload in dns_validate_response()
    - BUG/MEDIUM: dns: overflowed dns name start position causing invalid dns error
    - BUG/MINOR: compression/htx: Don't compress responses with unknown body length
    - BUG/MINOR: compression/htx: Don't add the last block of data if it is empty
    - MEDIUM: mux_h1: Implement h1_show_fd.
    - REGTEST: script: Add support of alternatives in requited options list
    - REGTEST: Add a basic test for the compression
    - BUG/MEDIUM: mux-h2: don't needlessly wake up the demux on short frames
    - REGTEST: A basic test for "http-buffer-request"
    - BUG/MEDIUM: server: Also copy "check-sni" for server templates.
    - MINOR: ssl: Add ssl_sock_set_alpn().
    - MEDIUM: checks: Add check-alpn.
2018-12-22 11:20:35 +01:00
Olivier Houchard
921501443b MEDIUM: checks: Add check-alpn.
Add a way to configure the ALPN used by check, with a new "check-alpn"
keyword. By default, the checks will use the server ALPN, but it may not
be convenient, for instance because the server may use HTTP/2, while checks
are unable to do HTTP/2 yet.
2018-12-21 19:54:16 +01:00
Olivier Houchard
ab28a320aa MINOR: ssl: Add ssl_sock_set_alpn().
Add a new function, ssl_sock_set_alpn(), to be able to change the ALPN
for a connection, instead of relying of the one defined in the SSL_CTX.
2018-12-21 19:53:30 +01:00
Olivier Houchard
21944019ca BUG/MEDIUM: server: Also copy "check-sni" for server templates.
When using server templates, if "check-sni" is used, make sure it shows up
in all the created servers.

This should be backported to 1.8 and 1.9.
2018-12-21 19:53:28 +01:00
Frédéric Lécaille
02a16690d7 REGTEST: A basic test for "http-buffer-request" 2018-12-21 18:33:36 +01:00
Willy Tarreau
47b515a462 BUG/MEDIUM: mux-h2: don't needlessly wake up the demux on short frames
In some situations, if too short a frame header is received, we may leave
h2_process_demux() waking up the task again without checking that we were
already subscribed.

In order to avoid this once for all, let's introduce an h2_restart_reading()
function which performs the control and calls the task up. This way we won't
needlessly wake the task up if it's already waiting for I/O.

Must be backported to 1.9.
2018-12-21 16:12:33 +01:00
Christopher Faulet
e98ce25b61 REGTEST: Add a basic test for the compression 2018-12-21 15:33:26 +01:00
Christopher Faulet
7e245dfaca REGTEST: script: Add support of alternatives in requited options list
It is now possible to specify a list of "alternatives" for a required
option. This must be done by separating options by a pipe ('|'). A test will be
executed if at least one of them is available. For instance:

  #REQUIRED_OPTIONS=ZLIB|SLZ,LUA,OPENSSL

The function _findtest() has also been sligthly simplified.
2018-12-21 15:33:26 +01:00
Olivier Houchard
a8f6b43b07 MEDIUM: mux_h1: Implement h1_show_fd.
In the mux_h1, implement h1_show_fd, to give a bit more information about
each H1 stream, when using "show fd".
2018-12-21 15:33:26 +01:00
Christopher Faulet
d238ae3a9b BUG/MINOR: compression/htx: Don't add the last block of data if it is empty
In HTX, when the compression filter analyze the EOM, it flushes the compression
context and add the last block of compressed data. But, this block can be
empty. In this case, we must ignore it.
2018-12-21 15:33:26 +01:00
Christopher Faulet
c963eb2a1d BUG/MINOR: compression/htx: Don't compress responses with unknown body length
In HTX, if the body length of a response cannot be determined, we must not try
to compress it.
2018-12-21 15:33:16 +01:00
Nikhil Agrawal
2fa66c3b93 BUG/MEDIUM: dns: overflowed dns name start position causing invalid dns error
In dns_read_name() when dns name is used with compression and start position of
name is greater than 255 name read is incorrect and causes invalid dns error.
eg: 0xc11b c specifies name compression being used. 11b represent the start
position of name but currently we are using only 1b for start position.

This should be backported as far as 1.7.
2018-12-21 11:36:44 +01:00
Jérôme Magnin
8d4e7dc880 BUG/MEDIUM: dns: Don't prevent reading the last byte of the payload in dns_validate_response()
A regression was introduced with efbbdf72 BUG: dns: Prevent out-of-bounds
read in dns_validate_dns_response() as it prevented from taking into account
the last byte of the payload.  this patch aims at fixing it.

this must be backported in 1.8.
2018-12-20 17:13:02 +01:00
Willy Tarreau
645b33d233 BUG/MEDIUM: mux-h2: Don't forget to quit the send list on error reports
Similar to last fix, we need to quit the send list when reporting an
error via the send side.

This should be backported to 1.9.
2018-12-20 15:35:57 +01:00
Olivier Houchard
f29cd5c8a8 BUG/MEDIUM: h2: Don't forget to quit the sending_list if SUB_CALL_UNSUBSCRIBE.
In mux_h2_unsubscribe, don't forget to leave the sending_list if
SUB_CALL_UNSUBSCRIBE was set. SUB_CALL_UNSUBSCRIBE means we were about
to be woken up for writing, unless the mux was too full to get more data.
If there's an unsubscribe call in the meanwhile, we should leave the list,
or we may be put back in the send_list.

This should be backported to 1.9.
2018-12-20 12:24:43 +01:00
Frédéric Lécaille
15685c7912 REGTEST: Exclude freebsd target for some reg tests.
This patch excludes freebsd target for health-checks reg tests
which rely on TCP_DEFER_ACCEPT usage. This is specific to Linux.
2018-12-20 11:00:17 +01:00
Frédéric Lécaille
a702947bef REGTEST: Missing enclosing quotes for ${tmpdir} macro. 2018-12-20 11:00:17 +01:00
Frédéric Lécaille
9ffb88d3cf REGTEST: Be less Linux specific with a syslog regex.
This patch makes at least this test pass on FreeBSD systems.
The regex to be matched with syslog message was too much Linux specific.
2018-12-20 11:00:17 +01:00
Frédéric Lécaille
3b3a883171 REGTEST: Fix a typo about barrier type.
Fix a typo, furthermore there is no reason to use a barrier with "sock" as type
which are more varnish cache specific: used to synchronize the VCL.
2018-12-20 11:00:17 +01:00
Frédéric Lécaille
a3b4cbff7b REGTEST: Make reg-tests target support argument.
With this patch we can provide a list of argument to reg-tests target.
Useful to run reg tests for a list of VTC files like that:

    $ VARNISHTEST_PROGRAM=<...> make reg-tests reg-tests/checks/*.vtc
2018-12-20 10:37:32 +01:00