]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
5 years agoMINOR: stick-table: allow sc-set-gpt0 to set value from an expression
Cédric Dufour [Wed, 6 Nov 2019 17:38:53 +0000 (18:38 +0100)] 
MINOR: stick-table: allow sc-set-gpt0 to set value from an expression

Allow the sc-set-gpt0 action to set GPT0 to a value dynamically evaluated from
its <expr> argument (in addition to the existing static <int> alternative).

5 years agoBUG/MINOR: log: make "show startup-log" use a ring buffer instead
Willy Tarreau [Fri, 15 Nov 2019 14:16:57 +0000 (15:16 +0100)] 
BUG/MINOR: log: make "show startup-log" use a ring buffer instead

The copy of the startup logs used to rely on a re-allocated memory area
on the fly, that would attempt to be delivered at once over the CLI. But
if it's too large (too many warnings) it will take time to start up, and
may not even show up on the CLI as it doesn't fit in a buffer.

The ring buffer infrastructure solves all this with no more code, let's
switch to this instead. It simply requires a parsing function to attach
the ring via ring_attach_cli() and all the rest is automatically handled.

Initially this was imagined as a code cleanup, until a test with a config
involving 100k backends and just one occurrence of
"load-server-state-from-file global" in the defaults section took approx
20 minutes to parse due to the O(N^2) cost of concatenating the warnings
resulting in ~1 TB of data to be copied, while it took only 0.57s with
the ring.

Ideally this patch should be backported to 2.0 and 1.9, though it relies
on the ring infrastructure which will then also need to be backported.
Configs able to trigger the bug are uncommon, so another workaround for
older versions without backporting the rings would consist in simply
limiting the size of the error message in print_message() to something
always printable, which will only return the first errors.

5 years agoMINOR: ring: make the parse function automatically set the handler/release
Willy Tarreau [Fri, 15 Nov 2019 14:07:21 +0000 (15:07 +0100)] 
MINOR: ring: make the parse function automatically set the handler/release

ring_attach_cli() is called by the keyword parsing function to dump a
ring to the CLI. It can only work with a specific handler and release
function. Let's make it set them appropriately instead of having the
caller know these functions. This way adding a command to dump a ring
is as simple as declaring a parsing function calling ring_attach_cli().

5 years agoMINOR: sink: Set the default max length for a message to BUFSIZE
Christopher Faulet [Fri, 15 Nov 2019 14:10:12 +0000 (15:10 +0100)] 
MINOR: sink: Set the default max length for a message to BUFSIZE

It was set to MAX_SYSLOG_LEN (1K). It is a bit short to print debug
traces. Especially when part of a buffers is dump. Now, the maximum length is
set to BUFSIZE (16K).

5 years agoMINOR: mux-h1: Set EOI on the conn-stream when EOS is reported in TUNNEL state
Christopher Faulet [Fri, 15 Nov 2019 08:50:22 +0000 (09:50 +0100)] 
MINOR: mux-h1: Set EOI on the conn-stream when EOS is reported in TUNNEL state

It could help to distinguish client/server aborts from legitimate shudowns for
reads.

5 years agoBUG/MINOR: mux-h1: Don't set CS_FL_EOS on a read0 when receiving data to pipe
Christopher Faulet [Fri, 15 Nov 2019 08:41:32 +0000 (09:41 +0100)] 
BUG/MINOR: mux-h1: Don't set CS_FL_EOS on a read0 when receiving data to pipe

This is mandatory to process input one more time to add the EOM in the HTX
message and to set CS_FL_EOI on the conn-stream. Otherwise, in the stream, a
SHUTR will be reported on the corresponding channel without the EOI. It may be
erroneously interpreted as an abort.

This patch must be backported to 2.0 and 1.9.

5 years agoBUG/MINOR: mux-h1: Properly catch parsing errors on payload and trailers
Christopher Faulet [Fri, 15 Nov 2019 08:36:28 +0000 (09:36 +0100)] 
BUG/MINOR: mux-h1: Properly catch parsing errors on payload and trailers

Errors during the payload or the trailers parsing are reported with the
HTX_FL_PARSING_ERROR flag on the HTX message and not a negative return
value. This change was introduced when the fonctions to convert an H1 message to
HTX one were moved to a dedicated file. But the h1 mux was not fully updated
accordingly.

No backport needed except if the commits about file h1_htx.c are backported.

5 years agoDOC: Add missing stats fields in the management manual
Christopher Faulet [Fri, 8 Nov 2019 14:27:27 +0000 (15:27 +0100)] 
DOC: Add missing stats fields in the management manual

Following fields was missing : srv_icur, src_ilim, qtime_max, ctime_max,
rtime_max and ttime_max.

5 years agoMINOR: contrib/prometheus-exporter: report the number of idle conns per server
Christopher Faulet [Fri, 8 Nov 2019 14:24:32 +0000 (15:24 +0100)] 
MINOR: contrib/prometheus-exporter: report the number of idle conns per server

This adds two extra metrics per server, one for the current number of idle
connections and one for the configured limit :

 * haproxy_server_idle_connections_current
 * haproxy_server_idle_connections_limit

5 years agoBUG/MINOR: contrib/prometheus-exporter: Rename some metrics
Christopher Faulet [Fri, 8 Nov 2019 14:12:29 +0000 (15:12 +0100)] 
BUG/MINOR: contrib/prometheus-exporter: Rename some metrics

The following metrics have been renamed without the "_http" part :

 * http_queue_time_average_seconds     => queue_time_average_seconds
 * http_connect_time_average_seconds   => connect_time_average_seconds
 * http_response_time_average_seconds  => response_time_average_seconds
 * http_total_time_average_seconds     => total_time_average_seconds

These metrics are reported per backend and per server and are not specific to
HTTP sessions.

5 years agoMINOR: contrib/prometheus-exporter: Report metrics about max times for sessions
Christopher Faulet [Fri, 8 Nov 2019 14:05:31 +0000 (15:05 +0100)] 
MINOR: contrib/prometheus-exporter: Report metrics about max times for sessions

Now, for the sessions, the maximum times (queue, connect, response, total) are
reported in addition of the averages over the last 1024 connections. These
metrics are reported per backend and per server. Here are the metrics name :

  * haproxy_backend_max_queue_time_seconds
  * haproxy_backend_max_connect_time_seconds
  * haproxy_backend_max_response_time_seconds
  * haproxy_backend_max_total_time_seconds

and

  * haproxy_server_max_queue_time_seconds
  * haproxy_server_max_connect_time_seconds
  * haproxy_server_max_response_time_seconds
  * haproxy_server_max_total_time_seconds

This patch is related to #272.

5 years agoMINOR: stats: Report max times in addition of the averages for sessions
Christopher Faulet [Fri, 8 Nov 2019 13:59:51 +0000 (14:59 +0100)] 
MINOR: stats: Report max times in addition of the averages for sessions

Now, for the sessions, the maximum times (queue, connect, response, total) are
reported in addition of the averages over the last 1024 connections. These
values are called qtime_max, ctime_max, rtime_max and ttime_max.

This patch is related to #272.

5 years agoMINOR: counters: Add fields to store the max observed for {q,c,d,t}_time
Christopher Faulet [Fri, 8 Nov 2019 13:53:15 +0000 (14:53 +0100)] 
MINOR: counters: Add fields to store the max observed for {q,c,d,t}_time

For backends and servers, some average times for last 1024 connections are
already calculated. For the moment, the averages for the time passed in the
queue, the connect time, the response time (for HTTP session only) and the total
time are calculated. Now, in addition, the maximum time observed for these
values are also stored.

In addition, These new counters are cleared as all other max values with the CLI
command "clear counters".

This patch is related to #272.

5 years agoMINOR: stream: Remove the lock on the proxy to update time stats
Christopher Faulet [Fri, 8 Nov 2019 13:45:41 +0000 (14:45 +0100)] 
MINOR: stream: Remove the lock on the proxy to update time stats

swrate_add() is now thread-safe. So the lock on the proxy is no longer needed to
update q_time, c_time, d_time and t_time.

5 years agoMINOR: freq_ctr: Make the sliding window sums thread-safe
Christopher Faulet [Fri, 8 Nov 2019 13:40:18 +0000 (14:40 +0100)] 
MINOR: freq_ctr: Make the sliding window sums thread-safe

swrate_add() and swrate_add_scaled() now rely on the CAS atomic operation. So
the sliding window sums are atomically updated.

5 years agoMEDIUM: filters: Adapt filters API to allow again TCP filtering on HTX streams
Christopher Faulet [Tue, 12 Nov 2019 10:13:01 +0000 (11:13 +0100)] 
MEDIUM: filters: Adapt filters API to allow again TCP filtering on HTX streams

This change make the payload filtering uniform between TCP and HTTP
filters. Now, in TCP, like in HTTP, there is only one callback responsible to
forward data. Thus, old callbacks, tcp_data() and tcp_forward_data(), are
replaced by a single callback function, tcp_payload(). This new callback gets
the offset in the payload to (re)start the filtering and the maximum amount of
data it can forward. It is the filter's responsibility to be compatible with HTX
streams. If not, it must not set the flag FLT_CFG_FL_HTX.

Because of this change, nxt and fwd offsets are no longer needed. Thus they are
removed from the filter structure with their update functions,
flt_change_next_size() and flt_change_forward_size(). Moreover, the trace filter
has been updated accordingly.

This patch breaks the compatibility with the old API. Thus it should probably
not be backported. But, AFAIK, there is no TCP filter, thus the breakage is very
limited.

5 years agoBUG/MEDIUM: filters: Don't call TCP callbacks for HTX streams
Christopher Faulet [Fri, 8 Nov 2019 14:31:49 +0000 (15:31 +0100)] 
BUG/MEDIUM: filters: Don't call TCP callbacks for HTX streams

For now, TCP callbacks are incompatible with the HTX streams because they are
designed to manipulate raw buffers. A new callback will probably be added to be
used in both modes, raw and HTX. So, for HTX streams, these callbacks are
ignored. This should not be a real problem because there is no known filters,
expect the trace filter, implementing these callbacks.

This patch must be backported to 2.0 and 1.9.

5 years agoBUILD: contrib/da: remove an "unused" warning
Willy Tarreau [Fri, 15 Nov 2019 12:39:16 +0000 (13:39 +0100)] 
BUILD: contrib/da: remove an "unused" warning

The rcsid variable is static an unused, causing a build warning. Let's
just add __attribute__((unused)) to shut the warning.

This may be backported to 2.0.

5 years agoBUG/MEDIUM: listeners: always pause a listener on out-of-resource condition
Willy Tarreau [Fri, 15 Nov 2019 09:20:07 +0000 (10:20 +0100)] 
BUG/MEDIUM: listeners: always pause a listener on out-of-resource condition

A corner case was opened in the listener_accept() code by commit 3f0d02bbc2
("MAJOR: listener: do not hold the listener lock in listener_accept()"). The
issue is when one listener (or a group of) managed to eat all the proxy's or
all the process's maxconn, and another listener tries to accept a new socket.
This results in the atomic increment to detect the excess connection count
and immediately abort, without pausing the listener, thus the call is
immediately performed again. This doesn't happen when the test is run on a
single listener because this listener got limited when crossing the limit.
But with 2 or more listeners, we don't have this luxury.

The solution consists in limiting the listener as soon as we have to
decline accepting an incoming connection. This means that the listener
will not be marked full yet if it gets the exact connection count but
this is not a problem in practice since all other listeners will only be
marked full after their first attempt. Thus from now on, a listener is
only full once it has already failed taking an incoming connection.

This bug was definitely responsible for the unreproduceable occasional
reports of high CPU usage showing epoll_wait() returning immediately
without accepting an incoming connection, like in bug #129.

This fix must be backported to 1.9 and 1.8.

5 years agoCLEANUP: stats: use srv_shutdown_streams() instead of open-coding it
Willy Tarreau [Thu, 14 Nov 2019 15:42:04 +0000 (16:42 +0100)] 
CLEANUP: stats: use srv_shutdown_streams() instead of open-coding it

The "shutdown sessions" admin-mode command used to open-code the list
traversal while there's already a function for this: srv_shutdown_streams().
Better use it.

5 years agoCLEANUP: cli: use srv_shutdown_streams() instead of open-coding it
Willy Tarreau [Thu, 14 Nov 2019 15:37:16 +0000 (16:37 +0100)] 
CLEANUP: cli: use srv_shutdown_streams() instead of open-coding it

The "shutdown session server" command used to open-code the list traversal
while there's already a function for this: srv_shutdown_streams(). Better
use it.

5 years agoMINOR: memory: also poison the area on freeing
Willy Tarreau [Fri, 15 Nov 2019 05:59:54 +0000 (06:59 +0100)] 
MINOR: memory: also poison the area on freeing

Doing so sometimes helps detect some UAF situations without the overhead
associated to the DEBUG_UAF define.

5 years agoCLEANUP: session: slightly simplify idle connection cleanup logic
Willy Tarreau [Fri, 15 Nov 2019 06:04:24 +0000 (07:04 +0100)] 
CLEANUP: session: slightly simplify idle connection cleanup logic

Since previous commit a132e5efa9 ("BUG/MEDIUM: Make sure we leave the
session list in session_free().") it's pointless to delete the conn
element inside "if" blocks given that the second test is always true
as well. Let's simplify this with a single LIST_DEL_INIT() before the
test.

5 years agoBUG/MEDIUM: Make sure we leave the session list in session_free().
Olivier Houchard [Thu, 14 Nov 2019 18:26:14 +0000 (19:26 +0100)] 
BUG/MEDIUM: Make sure we leave the session list in session_free().

In session_free(), if we're about to destroy a connection that had no mux,
make sure we leave the session_list before calling conn_free(). Otherwise,
conn_free() would call session_unown_conn(), which would potentially free
the associated srv_list, but session_free() also frees it, so that would
lead to a double free, and random memory corruption.

This should be backported to 1.9 and 2.0.

5 years agoBUG/MINOR: queue/threads: make the queue unlinking atomic
Willy Tarreau [Thu, 14 Nov 2019 13:58:39 +0000 (14:58 +0100)] 
BUG/MINOR: queue/threads: make the queue unlinking atomic

There is a very short race in the queues which happens in the following
situation:
  - stream A on thread 1 is being processed by a server
  - stream B on thread 2 waits in the backend queue for a server
  - stream B on thread 2 is fed up with waiting and expires, calls
    stream_free() which calls pendconn_free(), which sees the
    stream attached
  - at the exact same instant, stream A finishes on thread 1, sees
    one stream is waiting (B), detaches it and wakes it up
  - stream B continues pendconn_free() and calls pendconn_unlink()
  - pendconn_unlink() now detaches the node again and performs a
    second deletion (harmless since idempotent), and decrements
    srv/px->nbpend again

=> the number of connections on the proxy or server may reach -1 if/when
   this race occurs.

It is extremely tight as it can only occur during the test on p->leaf_p
though it has been witnessed at least once. The solution consists in
testing leaf_p again once the lock is held to make sure the element was
not removed in the mean time.

This should be backported to 2.0 and 1.9, probably even 1.8.

5 years agoBUG/MEDIUM: tasks: Make tasklet_remove_from_tasklet_list() no matter the tasklet.
Olivier Houchard [Fri, 8 Nov 2019 14:41:55 +0000 (15:41 +0100)] 
BUG/MEDIUM: tasks: Make tasklet_remove_from_tasklet_list() no matter the tasklet.

In tasklet_remove_from_tasket_list(), we can be called for a tasklet that is
either in the private task list, or in the shared tasklet list. Take that into
account and always use MT_LIST_DEL() to remove it, otherwise if we're in the
shared list and another thread attempts to add a tasklet in it, bad things
will happen.

__tasklet_remove_from_tasklet_list() is left unchanged, it's only supposed
to be used by process_runnable_task() to remove task/tasklets from the private
tast list.

This should not be backported.
This should fix github issue #357.

5 years agoBUG/MINOR: stream: init variables when the list is empty
Jerome Magnin [Sat, 9 Nov 2019 17:00:47 +0000 (18:00 +0100)] 
BUG/MINOR: stream: init variables when the list is empty

We need to call vars_init() when the list is empty otherwise we
can't use variables in the response scope. This regression was
introduced by cda7f3f5 (MINOR: stream: don't prune variables if
the list is empty).

The following config reproduces the issue:

 defaults
   mode http

 frontend in
   bind *:11223
   http-request set-var(req.foo) str("foo")  if { path /bar }
   http-request set-header bar %[var(req.foo)]  if { var(req.foo) -m found }
   http-response set-var(res.bar) str("bar")
   http-response set-header foo %[var(res.bar)] if { var(res.bar) -m found }
   use_backend out

 backend out
   server s1 127.0.0.1:11224

 listen back
   bind *:11224
   http-request deny deny_status 200

 > GET /ba HTTP/1.1
 > Host: localhost:11223
 > User-Agent: curl/7.66.0
 > Accept: */*
 >
 < HTTP/1.0 200 OK
 < Cache-Control: no-cache
 < Content-Type: text/html

 > GET /bar HTTP/1.1
 > Host: localhost:11223
 > User-Agent: curl/7.66.0
 > Accept: */*
 >
 < HTTP/1.0 200 OK
 < Cache-Control: no-cache
 < Content-Type: text/html
 < foo: bar

This must be backported as far as 1.9.

5 years agoDOC: management: fix typo on "cache_lookups" stats output
Willy Tarreau [Fri, 8 Nov 2019 06:29:34 +0000 (07:29 +0100)] 
DOC: management: fix typo on "cache_lookups" stats output

The trailing "s" was missing.

5 years agoBUG: dns: timeout resolve not applied for valid resolutions
Baptiste Assmann [Thu, 7 Nov 2019 10:02:18 +0000 (11:02 +0100)] 
BUG: dns: timeout resolve not applied for valid resolutions

Documentation states that the interval between 2 DNS resolution is
driven by "timeout resolve <time>" directive.
From a code point of view, this was applied unless the latest status of
the resolution was VALID. In such case, "hold valid" was enforce.
This is a bug, because "hold" timers are not here to drive how often we
want to trigger a DNS resolution, but more how long we want to keep an
information if the status of the resolution itself as changed.
This avoid flapping and prevent shutting down an entire backend when a
DNS server is not answering.

This issue was reported by hamshiva in github issue #345.

Backport status: 1.8

5 years agoBUG/MINOR: action: do-resolve now use cached response
Baptiste Assmann [Wed, 30 Oct 2019 15:06:53 +0000 (16:06 +0100)] 
BUG/MINOR: action: do-resolve now use cached response

As reported by David Birdsong on the ML, the HTTP action do-resolve does
not use the DNS cache.
Actually, the action is "registred" to the resolution for said name to
be resolved and wait until an other requester triggers the it. Once the
resolution is finished, then the action is updated with the result.
To trigger this, you must have a server with runtime DNS resolution
enabled and run a do-resolve action with the same fqdn AND they use the
same resolvers section.

This patch fixes this behavior by ensuring the resolution associated to
the action has a valid answer which is not considered as expired. If
those conditions are valid, then we can use it (it's the "cache").

Backport status: 2.0

5 years agoMINOR: http-ana: Remove the unused function http_reset_txn()
Christopher Faulet [Thu, 7 Nov 2019 14:26:45 +0000 (15:26 +0100)] 
MINOR: http-ana: Remove the unused function http_reset_txn()

Since the legacy HTTP mode was removed, the stream is always released at the end
of each HTTP transaction and a new is created to handle the next request for
keep-alive connections. So the HTTP transaction is no longer reset and the
function http_reset_txn() can be removed.

5 years agoBUG/MEDIUM: stream: Be sure to release allocated captures for TCP streams
Christopher Faulet [Thu, 7 Nov 2019 13:27:52 +0000 (14:27 +0100)] 
BUG/MEDIUM: stream: Be sure to release allocated captures for TCP streams

All TCP and HTTP captures are stored in 2 arrays, one for the request and
another for the response. In HAPRoxy 1.5, these arrays are part of the HTTP
transaction and thus are released during its cleanup. Because in this version,
the transaction is part of the stream (in 1.5, streams are still called
sessions), the cleanup is always performed, for HTTP and TCP streams.

In HAProxy 1.6, the HTTP transaction was moved out from the stream and is now
dynamically allocated only when required (becaues of an HTTP proxy or an HTTP
sample fetch). In addition, still in 1.6, the captures arrays were moved from
the HTTP transaction to the stream. This way, it is still possible to capture
elements from TCP rules for a full TCP stream. Unfortunately, the release is
still exclusively performed during the HTTP transaction cleanup. Thus, for a TCP
stream where the HTTP transaction is not required, the TCP captures, if any, are
never released.

Now, all captures are released when the stream is freed. This fixes the memory
leak for TCP streams. For streams with an HTTP transaction, the captures are now
released when the transaction is reset and not systematically during its
cleanup.

This patch must be backported as fas as 1.6.

5 years agoMINOR: doc: http-reuse connection pool fix
Lukas Tribus [Wed, 6 Nov 2019 10:50:25 +0000 (11:50 +0100)] 
MINOR: doc: http-reuse connection pool fix

Since 1.9 we actually do use a connection pool, configurable with
pool-max-conn.

Update the documentation in this regard.

Must be backported to 1.9.

5 years agoMEDIUM: stream/trace: Register a new trace source with its events
Christopher Faulet [Tue, 5 Nov 2019 15:18:10 +0000 (16:18 +0100)] 
MEDIUM: stream/trace: Register a new trace source with its events

Runtime traces are now supported for the streams, only if compiled with
debug. process_stream() is covered as well as TCP/HTTP analyzers and filters.

In traces, the first argument is always a stream. So it is easy to get the info
about the channels and the stream-interfaces. The second argument, when defined,
is always a HTTP transaction. And the third one is an HTTP message. The trace
message is adapted to report HTTP info when possible.

5 years agoMINOR: trace: Add a set of macros to trace events if HA is compiled with debug
Christopher Faulet [Mon, 4 Nov 2019 10:40:10 +0000 (11:40 +0100)] 
MINOR: trace: Add a set of macros to trace events if HA is compiled with debug

The macros DBG_TRACE_*() can be used instead of existing trace macros to emit
trace messages in debug mode only, ie, when HAProxy is compiled with DEBUG_FULL
or DEBUG_DEV. Otherwise, these macros do nothing. So it is possible to add
traces for development purpose without impacting performance of production
instances.

5 years agoMINOR: flt_trace: Rename macros to print trace messages
Christopher Faulet [Mon, 4 Nov 2019 10:35:42 +0000 (11:35 +0100)] 
MINOR: flt_trace: Rename macros to print trace messages

Names of these macros may enter in conflict with the macros of the runtime
tracing mechanism. So the prefix "FLT_" has been added to avoid any ambiguities.

5 years agoBUG/MEDIUM: stream: Be sure to support splicing at the mux level to enable it
Christopher Faulet [Tue, 5 Nov 2019 15:49:23 +0000 (16:49 +0100)] 
BUG/MEDIUM: stream: Be sure to support splicing at the mux level to enable it

Despite the addition of the mux layer, no change have been made on how to enable
the TCP splicing on process_stream(). We still check if transport layer on both
sides support the splicing, but we don't check the muxes support. So it is
possible to start to splice data with an unencrypted H2 connection on a side and
an H1 connection on the other. This leads to a freeze of the stream until a
client or server timeout is reached.

This patch fixed a part of the issue #356. It must be backported as far as 1.8.

5 years agoBUG/MEDIUM: mux-h1: Disable splicing for chunked messages
Christopher Faulet [Tue, 5 Nov 2019 15:24:27 +0000 (16:24 +0100)] 
BUG/MEDIUM: mux-h1: Disable splicing for chunked messages

The mux H1 announces the support of the TCP splicing. It only works for payload
data. It works for messages with an explicit content-length or for tunnelled
data. For chunked messages, the mux H1 should normally not try to xfer more than
the current chunk through the pipe. Unfortunately, this works on the read side
but the send is completely bogus. During the output formatting, the announced
size of chunks does not handle the size that will be spliced. Because there is
no formatting when spliced data are sent, the produced message is malformed and
rejected by the peer.

For now, because it is quick and simple, the TCP splicing is disabled for
chunked messages. I will try to enable it again in a proper way. I don't know
for now if it will be backportable in previous versions. This will depend on the
amount of changes required to handle it.

This patch fixes a part of the issue #356. It must be backported to 2.0 and 1.9.

5 years agoMINOR: peers: Add "log" directive to "peers" section.
Frédéric Lécaille [Tue, 5 Nov 2019 08:57:45 +0000 (09:57 +0100)] 
MINOR: peers: Add "log" directive to "peers" section.

This patch is easy to review: let's call parse_logsrv() function to parse
"log" directive as this is already for other sections for proxies.
This enable us to log incoming TCP connections for the listeners for "peers"
sections.

Update the documentation for "peers" section.

5 years agoDOC: fix date and http_date keywords syntax
Cyril Bonté [Tue, 5 Nov 2019 22:13:59 +0000 (23:13 +0100)] 
DOC: fix date and http_date keywords syntax

These keywords received a second argument with commit ae6f125 ("MINOR:
sample: add us/ms support to date/http_date"). Each argument is optional,
it's not either both or none.

5 years agoMINOR: ssl/cli: replace the default_ctx during 'commit ssl cert'
William Lallemand [Mon, 4 Nov 2019 16:56:13 +0000 (17:56 +0100)] 
MINOR: ssl/cli: replace the default_ctx during 'commit ssl cert'

If the SSL_CTX of a previous instance (ckch_inst) was used as a
default_ctx, replace the default_ctx of the bind_conf by the first
SSL_CTX inserted in the SNI tree.

Use the RWLOCK of the sni tree to handle the change of the default_ctx.

5 years agoBUG/MINOR: ssl/cli: fix an error when a file is not found
William Lallemand [Mon, 4 Nov 2019 13:02:11 +0000 (14:02 +0100)] 
BUG/MINOR: ssl/cli: fix an error when a file is not found

When trying to update a certificate <file>.{rsa,ecdsa,dsa}, but this one
does not exist and if <file> was used as a regular file in the
configuration, the error was ambiguous. Correct it so we can return a
certificate not found error.

5 years agoBUG/MINOR: ssl/cli: unable to update a certificate without bundle extension
William Lallemand [Mon, 4 Nov 2019 12:38:53 +0000 (13:38 +0100)] 
BUG/MINOR: ssl/cli: unable to update a certificate without bundle extension

Commit bc6ca7c ("MINOR: ssl/cli: rework 'set ssl cert' as 'set/commit'")
broke the ability to commit a unique certificate which does not use a
bundle extension .{rsa,ecdsa,dsa}.

5 years agoBUG/MEDIUM: ssl/cli: don't alloc path when cert not found
William Lallemand [Mon, 4 Nov 2019 09:59:32 +0000 (10:59 +0100)] 
BUG/MEDIUM: ssl/cli: don't alloc path when cert not found

When doing an 'ssl set cert' with a certificate which does not exist in
configuration, the appctx->ctx.ssl.old_ckchs->path was duplicated while
app->ctx.ssl.old_ckchs was NULL, resulting in a NULL dereference.

Move the code so the 'not referenced' error is done before this.

5 years ago[RELEASE] Released version 2.1-dev4 v2.1-dev4
Willy Tarreau [Sun, 3 Nov 2019 14:43:10 +0000 (15:43 +0100)] 
[RELEASE] Released version 2.1-dev4

Released version 2.1-dev4 with the following main changes :
    - BUG/MINOR: cli: don't call the kw->io_release if kw->parse failed
    - BUG/MINOR: mux-h2: Don't pretend mux buffers aren't full anymore if nothing sent
    - BUG/MAJOR: stream-int: Don't receive data from mux until SI_ST_EST is reached
    - DOC: remove obsolete section about header manipulation
    - BUG/MINOR: ssl/cli: cleanup on cli_parse_set_cert error
    - MINOR: ssl/cli: rework the 'set ssl cert' IO handler
    - BUILD: CI: comment out cygwin build, upgrade various ssl libraries
    - DOC: Improve documentation of http-re(quest|sponse) replace-(header|value|uri)
    - BUILD/MINOR: tools: shut up the format truncation warning in get_gmt_offset()
    - BUG/MINOR: spoe: fix off-by-one length in UUID format string
    - BUILD/MINOR: ssl: shut up a build warning about format truncation
    - BUILD: do not disable -Wformat-truncation anymore
    - MINOR: chunk: add chunk_istcat() to concatenate an ist after a chunk
    - Revert "MINOR: istbuf: add b_fromist() to make a buffer from an ist"
    - MINOR: mux: Add a new method to get informations about a mux.
    - BUG/MEDIUM: stream_interface: Only use SI_ST_RDY when the mux is ready.
    - BUG/MEDIUM: servers: Only set SF_SRV_REUSED if the connection if fully ready.
    - MINOR: doc: fix busy-polling performance reference
    - MINOR: config: allow no set-dumpable config option
    - MINOR: init: always fail when setrlimit fails
    - MINOR: ssl/cli: rework 'set ssl cert' as 'set/commit'
    - CLEANUP: ssl/cli: remove leftovers of bundle/certs (it < 2)
    - REGTEST: vtest can now enable mcli with its own flag
    - BUG/MINOR: config: Update cookie domain warn to RFC6265
    - MINOR: sample: add us/ms support to date/http_date
    - BUG/MINOR: ssl/cli: check trash allocation in cli_io_handler_commit_cert()
    - BUG/MEDIUM: mux-h2: report no available stream on a connection having errors
    - BUG/MEDIUM: mux-h2: immediately remove a failed connection from the idle list
    - BUG/MEDIUM: mux-h2: immediately report connection errors on streams
    - BUG/MINOR: stats: properly check the path and not the whole URI
    - BUG/MINOR: ssl: segfault in cli_parse_set_cert with old openssl/boringssl
    - BUG/MINOR: ssl: ckch->chain must be initialized
    - BUG/MINOR: ssl: double free on error for ckch->{key,cert}
    - MINOR: ssl: BoringSSL ocsp_response does not need issuer
    - BUG/MEDIUM: ssl/cli: fix dot research in cli_parse_set_cert
    - MINOR: backend: Add srv_name sample fetche
    - DOC: Add GitHub issue config.yml

5 years agoDOC: Add GitHub issue config.yml
Tim Duesterhus [Fri, 1 Nov 2019 16:51:27 +0000 (17:51 +0100)] 
DOC: Add GitHub issue config.yml

This allows us to link to the mailing list and forum within the
issue tracker, hopefully discouraging users to ask questions even more.

see https://help.github.com/en/github/building-a-strong-community/configuring-issue-templates-for-your-repository#configuring-the-template-chooser

5 years agoMINOR: backend: Add srv_name sample fetche
vkill [Wed, 30 Oct 2019 08:58:14 +0000 (16:58 +0800)] 
MINOR: backend: Add srv_name sample fetche

The sample fetche can get srv_name without foreach
`core.backends["bk"].servers`.

Then we can get Server class quickly via
`core.backends[txn.f:be_name()].servers[txn.f:srv_name()]`.

Issue#342

5 years agoBUG/MEDIUM: ssl/cli: fix dot research in cli_parse_set_cert
Emmanuel Hocdet [Wed, 30 Oct 2019 16:31:28 +0000 (17:31 +0100)] 
BUG/MEDIUM: ssl/cli: fix dot research in cli_parse_set_cert

During a 'set ssl cert', the result of the strrchr was wrongly tested
and can lead to a segfault when the certificate path did not contained a
dot.

5 years agoMINOR: ssl: BoringSSL ocsp_response does not need issuer
Emmanuel Hocdet [Fri, 25 Oct 2019 10:19:00 +0000 (12:19 +0200)] 
MINOR: ssl: BoringSSL ocsp_response does not need issuer

HAproxy can fail when issuer is not found, it must not with BoringSSL.

5 years agoBUG/MINOR: ssl: double free on error for ckch->{key,cert}
Emmanuel Hocdet [Fri, 25 Oct 2019 09:55:03 +0000 (11:55 +0200)] 
BUG/MINOR: ssl: double free on error for ckch->{key,cert}

On last error in ssl_sock_load_pem_into_ckch, key/cert are released
and ckch->{key,cert} are released in ssl_sock_free_cert_key_and_chain_contents.

5 years agoBUG/MINOR: ssl: ckch->chain must be initialized
Emmanuel Hocdet [Thu, 24 Oct 2019 16:28:33 +0000 (18:28 +0200)] 
BUG/MINOR: ssl: ckch->chain must be initialized

It's a regression from 96a9c973 "MINOR: ssl: split
ssl_sock_load_crt_file_into_ckch()".

5 years agoBUG/MINOR: ssl: segfault in cli_parse_set_cert with old openssl/boringssl
Emmanuel Hocdet [Wed, 30 Oct 2019 16:41:27 +0000 (17:41 +0100)] 
BUG/MINOR: ssl: segfault in cli_parse_set_cert with old openssl/boringssl

Fix 541a534 ("BUG/MINOR: ssl/cli: fix build of SCTL and OCSP") was not
enough.

[wla: It will probably be better later to put the #ifdef in the
functions so they can return an error if they are not implemented]

5 years agoBUG/MINOR: stats: properly check the path and not the whole URI
Willy Tarreau [Thu, 31 Oct 2019 14:50:28 +0000 (15:50 +0100)] 
BUG/MINOR: stats: properly check the path and not the whole URI

Since we now have full URIs with h2, stats may fail to work over H2
so we must carefully only check the path there if the stats URI was
passed with a path only. This way it remains possible to intercept
proxy requests to report stats on explicit domains but it continues
to work as expected on origin requests.

No backport needed.

5 years agoBUG/MEDIUM: mux-h2: immediately report connection errors on streams
Willy Tarreau [Thu, 31 Oct 2019 14:48:18 +0000 (15:48 +0100)] 
BUG/MEDIUM: mux-h2: immediately report connection errors on streams

In case a stream tries to send on a connection error, we must report the
error so that the stream interface keeps the data available and may safely
retry on another connection. Till now this would happen only before the
connection was established, not in case of a failed handshake or an early
GOAWAY for example.

This should be backported to 2.0 and 1.9.

5 years agoBUG/MEDIUM: mux-h2: immediately remove a failed connection from the idle list
Willy Tarreau [Thu, 31 Oct 2019 14:36:30 +0000 (15:36 +0100)] 
BUG/MEDIUM: mux-h2: immediately remove a failed connection from the idle list

If a connection faces an error or a timeout, it must be removed from its
idle list ASAP. We certainly don't want to risk sending new streams on
it.

This should be backported to 2.0 (replacing MT_LIST_DEL with LIST_DEL_LOCKED)
and 1.9 (there's no lock there, the idle lists are per-thread and per-server
however a LIST_DEL_INIT will be needed).

5 years agoBUG/MEDIUM: mux-h2: report no available stream on a connection having errors
Willy Tarreau [Thu, 31 Oct 2019 14:10:03 +0000 (15:10 +0100)] 
BUG/MEDIUM: mux-h2: report no available stream on a connection having errors

If an H2 mux has met an error, we must not report available streams
anymore, or it risks to accumulate new streams while not being able
to process them.

This should be backported to 2.0 and 1.9.

5 years agoBUG/MINOR: ssl/cli: check trash allocation in cli_io_handler_commit_cert()
William Lallemand [Thu, 31 Oct 2019 10:43:45 +0000 (11:43 +0100)] 
BUG/MINOR: ssl/cli: check trash allocation in cli_io_handler_commit_cert()

Possible NULL pointer dereference found by coverity.

Fix #350 #340.

5 years agoMINOR: sample: add us/ms support to date/http_date
Damien Claisse [Wed, 30 Oct 2019 15:57:28 +0000 (15:57 +0000)] 
MINOR: sample: add us/ms support to date/http_date

It can be sometimes interesting to have a timestamp with a
resolution of less than a second.
It is currently painful to obtain this, because concatenation
of date and date_us lead to a shorter timestamp during first
100ms of a second, which is not parseable and needs ugly ACLs
in configuration to prepend 0s when needed.
To improve this, add an optional <unit> parameter to date sample
to report an integer with desired unit.
Also support this unit in http_date converter to report
a date string with sub-second precision.

5 years agoBUG/MINOR: config: Update cookie domain warn to RFC6265
Joao Morais [Thu, 31 Oct 2019 00:04:00 +0000 (21:04 -0300)] 
BUG/MINOR: config: Update cookie domain warn to RFC6265

The domain option of the cookie keyword allows to define which domain or
domains should use the the cookie value of a cookie-based server
affinity. If the domain does not start with a dot, the user agent should
only use the cookie on hosts that matches the provided domains. If the
configured domain starts with a dot, the user agent can use the cookie
with any host ending with the configured domain.

haproxy config parser helps the admin warning about a potentially buggy
config: defining a domain without an embedded dot which does not start
with a dot, which is forbidden by the RFC.

The current condition to issue the warning implements RFC2109. This
change updates the implementation to RFC6265 which allows domain without
a leading dot.

Should be backported to all supported versions. The feature exists at least
since 1.5.

5 years agoREGTEST: vtest can now enable mcli with its own flag
Jerome Magnin [Wed, 30 Oct 2019 17:27:45 +0000 (18:27 +0100)] 
REGTEST: vtest can now enable mcli with its own flag

VTest can now enable mworker and mcli with separate flags so lets
update vtc files that need it. This also allows to revert the change
made with 1545a59c ("REGTESTS: make seamless-reload depend on 1.9
and above").

5 years agoCLEANUP: ssl/cli: remove leftovers of bundle/certs (it < 2)
William Lallemand [Wed, 30 Oct 2019 16:45:33 +0000 (17:45 +0100)] 
CLEANUP: ssl/cli: remove leftovers of bundle/certs (it < 2)

Remove the leftovers of the certificate + bundle updating in 'ssl set
cert' and 'commit ssl cert'.

* Remove the it variable in appctx.ctx.ssl.
* Stop doing everything twice.
* Indent

5 years agoMINOR: ssl/cli: rework 'set ssl cert' as 'set/commit'
William Lallemand [Tue, 29 Oct 2019 22:48:19 +0000 (23:48 +0100)] 
MINOR: ssl/cli: rework 'set ssl cert' as 'set/commit'

This patch splits the 'set ssl cert' CLI command into 2 commands.

The previous way of updating the certificate on the CLI was limited with
the bundles. It was only able to apply one of the tree part of the
certificate during an update, which mean that we needed 3 updates to
update a full 3 certs bundle.

It was also not possible to apply atomically several part of a
certificate with the ability to rollback on error. (For example applying
a .pem, then a .ocsp, then a .sctl)

The command 'set ssl cert' will now duplicate the certificate (or
bundle) and update it in a temporary transaction..

The second command 'commit ssl cert' will commit all the changes made
during the transaction for the certificate.

This commit breaks the ability to update a certificate which was used as
a unique file and as a bundle in the HAProxy configuration. This way of
using the certificates wasn't making any sense.

Example:

// For a bundle:

$ echo -e "set ssl cert localhost.pem.rsa <<\n$(cat kikyo.pem.rsa)\n" | socat /tmp/sock1 -
Transaction created for certificate localhost.pem!

$ echo -e "set ssl cert localhost.pem.dsa <<\n$(cat kikyo.pem.dsa)\n" | socat /tmp/sock1 -
Transaction updated for certificate localhost.pem!

$ echo -e "set ssl cert localhost.pem.ecdsa <<\n$(cat kikyo.pem.ecdsa)\n" | socat /tmp/sock1 -
Transaction updated for certificate localhost.pem!

$ echo "commit ssl cert localhost.pem" | socat /tmp/sock1 -
Committing localhost.pem.
Success!

5 years agoMINOR: init: always fail when setrlimit fails
William Dauchy [Sun, 27 Oct 2019 19:08:11 +0000 (20:08 +0100)] 
MINOR: init: always fail when setrlimit fails

this patch introduces a strict-limits parameter which enforces the
setrlimit setting instead of a warning. This option can be forcingly
disable with the "no" keyword.
The general aim of this patch is to avoid bad surprises on a production
environment where you change the maxconn for example, a new fd limit is
calculated, but cannot be set because of sysfs setting. In that case you
might want to have an explicit failure to be aware of it before seeing
your traffic going down. During a global rollout it is also useful to
explictly fail as most progressive rollout would simply check the
general health check of the process.

As discussed, plan to use the strict by default mode starting from v2.3.

Signed-off-by: William Dauchy <w.dauchy@criteo.com>
5 years agoMINOR: config: allow no set-dumpable config option
William Dauchy [Sun, 27 Oct 2019 19:08:10 +0000 (20:08 +0100)] 
MINOR: config: allow no set-dumpable config option

in global config parsing, we currently expect to have a possible no
keyword (KWN_NO), but we never allow it in config parsing.
another patch could have been to simply remove the code handling a
possible KWN_NO.
take this opportunity to update documentation of set-dumpable.

Signed-off-by: William Dauchy <w.dauchy@criteo.com>
5 years agoMINOR: doc: fix busy-polling performance reference
William Dauchy [Sun, 27 Oct 2019 19:08:09 +0000 (20:08 +0100)] 
MINOR: doc: fix busy-polling performance reference

busy-polling parameter was forgotten in the list of performance tuning

Signed-off-by: William Dauchy <w.dauchy@criteo.com>
5 years agoBUG/MEDIUM: servers: Only set SF_SRV_REUSED if the connection if fully ready.
Olivier Houchard [Fri, 25 Oct 2019 15:00:54 +0000 (17:00 +0200)] 
BUG/MEDIUM: servers: Only set SF_SRV_REUSED if the connection if fully ready.

In connect_server(), if we're reusing a connection, only use SF_SRV_REUSED
if the connection is fully ready. We may be using a multiplexed connection
created by another stream that is not yet ready, and may fail.
If we set SF_SRV_REUSED, process_stream() will then not wait for the timeout
to expire, and retry to connect immediately.

This should be backported to 1.9 and 2.0.
This commit depends on 55234e33708c5a584fb9efea81d71ac47235d518.

5 years agoBUG/MEDIUM: stream_interface: Only use SI_ST_RDY when the mux is ready.
Olivier Houchard [Fri, 25 Oct 2019 14:25:20 +0000 (16:25 +0200)] 
BUG/MEDIUM: stream_interface: Only use SI_ST_RDY when the mux is ready.

In si_connect(), only switch the strema_interface status to SI_ST_RDY if
we're reusing a connection and if the connection's mux is ready. Otherwise,
maybe we're reusing a connection that is not fully established yet, and may
fail, and setting SI_ST_RDY would mean we would not be able to retry to
connect.

This should be backported to 1.9 and 2.0.
This commit depends on 55234e33708c5a584fb9efea81d71ac47235d518.

5 years agoMINOR: mux: Add a new method to get informations about a mux.
Olivier Houchard [Fri, 25 Oct 2019 14:19:26 +0000 (16:19 +0200)] 
MINOR: mux: Add a new method to get informations about a mux.

Add a new method, ctl(), to muxes. It uses a "enum mux_ctl_type" to
let it know which information we're asking for, and can output it either
directly by returning the expected value, or by using an optional argument.
"output" argument.
Right now, the only known mux_ctl_type is MUX_STATUS, that will return 0 if
the mux is not ready, or MUX_STATUS_READY if the mux is ready.

We probably want to backport this to 1.9 and 2.0.

5 years agoRevert "MINOR: istbuf: add b_fromist() to make a buffer from an ist"
Willy Tarreau [Tue, 29 Oct 2019 12:06:21 +0000 (13:06 +0100)] 
Revert "MINOR: istbuf: add b_fromist() to make a buffer from an ist"

This reverts commit 9e46496d45ff06317ae8f4f785e6117e5b786f6f. It was
wrong and is not reliable, depending on the compiler's version and
optimization, as the struct is assigned inside a statement, thus on
its own stack. It's not needed anymore now so let's remove this.

5 years agoMINOR: chunk: add chunk_istcat() to concatenate an ist after a chunk
Willy Tarreau [Tue, 29 Oct 2019 12:02:15 +0000 (13:02 +0100)] 
MINOR: chunk: add chunk_istcat() to concatenate an ist after a chunk

We previously relied on chunk_cat(dst, b_fromist(src)) for this but it
is not reliable as the allocated buffer is inside the expression and
may be on a temporary stack. While it's possible to allocate stack space
for a struct and return a pointer to it, it's not possible to initialize
it form a temporary variable to prevent arguments from being evaluated
multiple times. Since this is only used to append an ist after a chunk,
let's instead have a chunk_istcat() function to perform exactly this
from a native ist.

The only call place (URI computation in the cache) was updated.

5 years agoBUILD: do not disable -Wformat-truncation anymore
Willy Tarreau [Tue, 29 Oct 2019 09:54:24 +0000 (10:54 +0100)] 
BUILD: do not disable -Wformat-truncation anymore

There were very few entries to fix and this warning, while often
wrong, can actually spot future issues. If it can help developers
adjust their code in the future to make it more robust, it's not
necessarily that bad. Let's re-enable it and see how it goes.

5 years agoBUILD/MINOR: ssl: shut up a build warning about format truncation
Willy Tarreau [Tue, 29 Oct 2019 09:48:50 +0000 (10:48 +0100)] 
BUILD/MINOR: ssl: shut up a build warning about format truncation

Actually gcc believes it has detected a possible truncation but it
cannot since the output string is necessarily at least one char
shorter than what it expects. However addressing it is easy and
removes the need for an intermediate copy so let's do it.

5 years agoBUG/MINOR: spoe: fix off-by-one length in UUID format string
Willy Tarreau [Tue, 29 Oct 2019 09:25:49 +0000 (10:25 +0100)] 
BUG/MINOR: spoe: fix off-by-one length in UUID format string

The per-thread UUID string produced by generate_pseudo_uuid() could be
off by one character due to too small of size limit in snprintf(). In
practice the UUID remains large enough to avoid any collision though.

This should be backported to 2.0 and 1.9.

5 years agoBUILD/MINOR: tools: shut up the format truncation warning in get_gmt_offset()
Willy Tarreau [Tue, 29 Oct 2019 09:16:11 +0000 (10:16 +0100)] 
BUILD/MINOR: tools: shut up the format truncation warning in get_gmt_offset()

The gcc warning about format truncation in get_gmt_offset() is annoying
since we always call it with a valid time thus it cannot fail. However
it's true that nothing guarantees that future code reuses this function
incorrectly in the future, so better enforce the modulus on one day and
shut the warning.

5 years agoDOC: Improve documentation of http-re(quest|sponse) replace-(header|value|uri)
Tim Duesterhus [Mon, 28 Oct 2019 23:05:13 +0000 (00:05 +0100)] 
DOC: Improve documentation of http-re(quest|sponse) replace-(header|value|uri)

- Clarify that everything and not only the matched part is replaced (GitHub #328)
- Reduce duplication and inconsistencies by referring to a single canonical directive
  that includes everything one needs to know about.
- Fix indentation

5 years agoBUILD: CI: comment out cygwin build, upgrade various ssl libraries
Ilya Shipitsin [Sun, 27 Oct 2019 15:16:29 +0000 (20:16 +0500)] 
BUILD: CI: comment out cygwin build, upgrade various ssl libraries

cirrus ci builds are now limited to branches master, next
travis-ci images are upgraded to ubuntu bionic
cygwin builds are temporarily disabled on travis-ci
(maybe someone will figure out how to fix them, here's link
https://travis-ci.community/t/cygwin-issue-cygheap-base-mismatch-detected/5359/2 )

openssl upgraded to 1.0.2t, 1.1.0l, 1.1.1d
libressl are upgraded (2.9.2, 2.8.3, 2.7.5)  --> (3.0.2, 2.9.2, 2.8.3)

5 years agoMINOR: ssl/cli: rework the 'set ssl cert' IO handler
William Lallemand [Mon, 28 Oct 2019 13:30:47 +0000 (14:30 +0100)] 
MINOR: ssl/cli: rework the 'set ssl cert' IO handler

Rework the 'set ssl cert' IO handler so it is clearer.
Use its own SETCERT_ST_* states insted of the STAT_ST ones.

Use an inner loop in SETCERT_ST_GEN and SETCERT_ST_INSERT to do the work
for both the certificate and the bundle.

The io_release() is now called only when the CKCH spinlock is taken so
we can unlock during a release without any condition.

5 years agoBUG/MINOR: ssl/cli: cleanup on cli_parse_set_cert error
William Lallemand [Mon, 28 Oct 2019 13:26:56 +0000 (14:26 +0100)] 
BUG/MINOR: ssl/cli: cleanup on cli_parse_set_cert error

Since commit 90b098c ("BUG/MINOR: cli: don't call the kw->io_release if
kw->parse failed"), the io_release() callback is not called anymore when
the parse() failed. Call it directly on the error path of the
cli_parse_set_cert() function.

5 years agoDOC: remove obsolete section about header manipulation
Willy Tarreau [Mon, 28 Oct 2019 06:03:13 +0000 (07:03 +0100)] 
DOC: remove obsolete section about header manipulation

Cyril Bonté reported that the doc contains two chapters number 6,
one of which is a leftover of the section about old header manipulation
directives that were removed by commit a6a56e6 ("MEDIUM: config: Remove
parsing of req* and rsp* directives"). This patch removes this.

5 years agoBUG/MAJOR: stream-int: Don't receive data from mux until SI_ST_EST is reached
Christopher Faulet [Fri, 25 Oct 2019 08:21:01 +0000 (10:21 +0200)] 
BUG/MAJOR: stream-int: Don't receive data from mux until SI_ST_EST is reached

This bug is pretty pernicious and have serious consequences : In 2.1, an
infinite loop in process_stream() because the backend stream-interface remains
in the ready state (SI_ST_RDY). In 2.0, a call in loop to process_stream()
because the stream-interface remains blocked in the connect state
(SI_ST_CON). In both cases, it happens after a connection retry attempt. In 1.9,
it seems to not happen. But it may be just by chance or just because it is
harder to get right conditions to trigger the bug. However, reading the code,
the bug seems to exist too.

Here is how the bug happens in 2.1. When we try to establish a new connection to
a server, the corresponding stream-interface is first set to the connect state
(SI_ST_CON). When the underlying connection is known to be connected (the flag
CO_FL_CONNECTED set), the stream-interface is switched to the ready state
(SI_ST_RDY). It is a transient state between the connect state (SI_ST_CON) and
the established state (SI_ST_EST). It must be handled on the next call to
process_stream(), which is responsible to operate the transition. During all
this time, errors can occur. A connection error or a client abort. The transient
state SI_ST_RDY was introduced to let a chance to process_stream() to catch
these errors before considering the connection as fully established.
Unfortunatly, if a read0 is catched in states SI_ST_CON or SI_ST_RDY, it is
possible to have a shutdown without transition to SI_ST_DIS (in fact, here,
SI_ST_CON is swichted to SI_ST_RDY). This happens if the request was fully
received and analyzed. In this case, the flag SI_FL_NOHALF is set on the backend
stream-interface. If an error is also reported during the connect, the behavior
is undefined because an error is returned to the client and a connection retry
is performed. So on the next connection attempt to the server, if another error
is reported, a client abort is detected. But the shutdown for writes was already
done. So the transition to the state SI_ST_DIS is impossible. We stay in the
state SI_ST_RDY. Because it is a transient state, we loop in process_stream() to
perform the transition.

It is hard to understand how the bug happens reading the code and even harder to
explain. But there is a trivial way to hit the bug by sending h2 requests to a
server only speaking h1. For instance, with the following config :

  listen tst
    bind *:80
    server www 127.0.0.1:8000 proto h2 # in reality, it is a HTTP/1.1 server

It is a configuration error, but it is an easy way to observe the bug. Note it
may happen with a valid configuration.

So, after a careful analyzis, it appears that si_cs_recv() should never be
called for a not fully established stream-interface. This way the connection
retries will be performed before reporting an error to the client. Thus, if a
shutdown is performed because a read0 is handled, the stream-interface is
inconditionnaly set to the transient state SI_ST_DIS.

This patch must be backported to 2.0 and 1.9. However on these versions, this
patch reveals a design flaw about connections and a bad way to perform the
connection retries. We are working on it.

5 years agoBUG/MINOR: mux-h2: Don't pretend mux buffers aren't full anymore if nothing sent
Christopher Faulet [Thu, 24 Oct 2019 08:31:01 +0000 (10:31 +0200)] 
BUG/MINOR: mux-h2: Don't pretend mux buffers aren't full anymore if nothing sent

In h2_send(), when something is sent, we remove the flags
(H2_CF_MUX_MFULL|H2_CF_DEM_MROOM) on the h2 connection. This way, we are able to
wake up all streams waiting to send data. Unfortunatly, these flags are
unconditionally removed, even when nothing was sent. So if the h2c is blocked
because the mux buffers are full and we are unable to send anything, all streams
in the send_list are woken up for nothing. Now, we only remove these flags if at
least a send succeeds.

This patch must be backport to 2.0.

5 years agoBUG/MINOR: cli: don't call the kw->io_release if kw->parse failed
William Lallemand [Fri, 25 Oct 2019 19:10:14 +0000 (21:10 +0200)] 
BUG/MINOR: cli: don't call the kw->io_release if kw->parse failed

The io_release() callback of the cli_kw is supposed to be used to clean
what an io_handler() has made. It is called once the work in the IO
handler is finished, or when the connection was aborted by the client.

This patch fixes a bug where the io_release callback was called even
when the parse() callback failed. Which means that the io_release() could
called even if the io_handler() was not called.

Should be backported in every versions that have a cli_kw->release().
(as far as 1.7)

5 years ago[RELEASE] Released version 2.1-dev3 v2.1-dev3
Willy Tarreau [Fri, 25 Oct 2019 13:48:53 +0000 (15:48 +0200)] 
[RELEASE] Released version 2.1-dev3

Released version 2.1-dev3 with the following main changes :
    - MINOR: mux-h2/trace: missing conn pointer in demux full message
    - MINOR: mux-h2: add a per-connection list of blocked streams
    - BUILD: ebtree: make eb_is_empty() and eb_is_dup() take a const
    - BUG/MEDIUM: mux-h2: do not enforce timeout on long connections
    - BUG/MEDIUM: tasks: Don't forget to decrement tasks_run_queue.
    - BUG/MINOR: peers: crash on reload without local peer.
    - BUG/MINOR: mux-h2/trace: Fix traces on h2c initialization
    - MINOR: h1-htx: Update h1_copy_msg_data() to ease the traces in the mux-h1
    - MINOR: htx: Adapt htx_dump() to be used from traces
    - MINOR: mux-h1/trace: register a new trace source with its events
    - MINOR: proxy: Store http-send-name-header in lower case
    - MINOR: http: Remove headers matching the name of http-send-name-header option
    - BUG/MINOR: mux-h1: Adjust header case when the server name is add to a request
    - BUG/MINOR: mux-h1: Adjust header case when chunked encoding is add to a message
    - MINOR: mux-h1: Try to wakeup the stream on output buffer allocation
    - MINOR: fcgi: Add function to get the string representation of a record type
    - MINOR: mux-fcgi/trace: Register a new trace source with its events
    - BUG/MEDIUM: cache: make sure not to cache requests with absolute-uri
    - DOC: clarify some points around http-send-name-header's behavior
    - MEDIUM: mux-h2: support emitting CONTINUATION frames after HEADERS
    - BUG/MINOR: mux-h1/mux-fcgi/trace: Fix position of the 4th arg in some traces
    - DOC: fix typo in Prometheus exporter doc
    - MINOR: h2: clarify the rules for how to convert an H2 request to HTX
    - MINOR: htx: Add 2 flags on the start-line to have more info about the uri
    - MINOR: http: Add a function to get the authority into a URI
    - MINOR: h1-htx: Set the flag HTX_SL_F_HAS_AUTHORITY during the request parsing
    - MEDIUM: http-htx: Keep the Host header and the request start-line synchronized
    - MINOR: h1-htx: Only use the path of a normalized URI to format a request line
    - MEDIUM: h2: make the request parser rebuild a complete URI
    - MINOR: h2: report in the HTX flags when the request has an authority
    - MEDIUM: mux-h2: do not map Host to :authority on output
    - MEDIUM: h2: use the normalized URI encoding for absolute form requests
    - MINOR: stats: mention in the help message support for "json" and "typed"
    - MINOR: stats: get rid of the ST_CONVDONE flag
    - MINOR: stats: replace the ST_* uri_auth flags with STAT_*
    - MINOR: stats: always merge the uri_auth flags into the appctx flags
    - MINOR: stats: set the appctx flags when initializing the applet only
    - MINOR: stats: get rid of the STAT_SHOWADMIN flag
    - MINOR: stats: make stats_dump_fields_json() directly take flags
    - MINOR: stats: uniformize the calling convention of the dump functions
    - MINOR: stats: support the "desc" output format modifier for info and stat
    - MINOR: stats: prepare to add a description with each stat/info field
    - MINOR: stats: make "show stat" and "show info"
    - MINOR: stats: fill all the descriptions for "show info" and "show stat"
    - BUG/MEDIUM: applet: always check a fast running applet's activity before killing
    - BUILD: stats: fix missing '=' sign in array declaration
    - MINOR: lists: add new macro LIST_SPLICE_END_DETACHED
    - MINOR: list: add new macro MT_LIST_BEHEAD
    - MEDIUM: task: Split the tasklet list into two lists.
    - MINOR: h2: Document traps to be avoided on multithread.
    - MINOR: lists: Try to use local variables instead of macro arguments.
    - MINOR: lists: Fix alignement of \ when relevant.
    - MINOR: mux-h2: also support emitting CONTINUATION on trailers
    - MINOR: ssl: crt-list do ckchn_lookup
    - REORG: ssl: rename ckch_node to ckch_store
    - REORG: ssl: move structures to ssl_sock.h
    - MINOR: ssl: initialize the sni_keytypes_map as EB_ROOT
    - MINOR: ssl: initialize explicitly the sni_ctx trees
    - BUG/MINOR: ssl: abort on sni allocation failure
    - BUG/MINOR: ssl: free the sni_keytype nodes
    - BUG/MINOR: ssl: abort on sni_keytypes allocation failure
    - MEDIUM: ssl: introduce the ckch instance structure
    - MEDIUM: ssl: split ssl_sock_add_cert_sni()
    - MINOR: ssl: ssl_sock_load_ckchn() can properly fail
    - MINOR: ssl: ssl_sock_load_multi_ckchs() can properly fail
    - MEDIUM: ssl: ssl_sock_load_ckchs() alloc a ckch_inst
    - MINOR: ssl: ssl_sock_load_crt_file_into_ckch() is filling from a BIO
    - MEDIUM: ssl/cli: 'set ssl cert' updates a certificate from the CLI
    - MINOR: ssl: load the sctl in/from the ckch
    - MINOR: ssl: load the ocsp in/from the ckch
    - BUG/MEDIUM: ssl: NULL dereference in ssl_sock_load_cert_sni()
    - BUG/MINOR: ssl: fix build without SSL
    - BUG/MINOR: ssl: fix build without multi-cert bundles
    - BUILD: ssl: wrong #ifdef for SSL engines code
    - BUG/MINOR: ssl: fix OCSP build with BoringSSL
    - BUG/MEDIUM: htx: Catch chunk_memcat() failures when HTX data are formatted to h1
    - BUG/MINOR: chunk: Fix tests on the chunk size in functions copying data
    - BUG/MINOR: mux-h1: Mark the output buffer as full when the xfer is interrupted
    - MINOR: mux-h1: Xfer as much payload data as possible during output processing
    - CLEANUP: h1-htx: Move htx-to-h1 formatting functions from htx.c to h1_htx.c
    - BUG/MINOR: mux-h1: Capture ignored parsing errors
    - MINOR: h1: Reject requests with different occurrences of the header host
    - MINOR: h1: Reject requests if the authority does not match the header host
    - REGTESTS: Send valid URIs in peers reg-tests and fix HA config to avoid warnings
    - REGTESTS: Adapt proxy_protocol_random_fail.vtc to match normalized URI too
    - BUG/MINOR: WURFL: fix send_log() function arguments
    - BUG/MINOR: ssl: fix error messages for OCSP loading
    - BUG/MINOR: ssl: can't load ocsp files
    - MINOR: version: make the version strings variables, not constants
    - BUG/MINOR: http-htx: Properly set htx flags on error files to support keep-alive
    - MINOR: htx: Add a flag on HTX to known when a response was generated by HAProxy
    - MINOR: mux-h1: Force close mode for proxy responses with an unfinished request
    - BUILD: travis-ci: limit build to branches "master" and "next"
    - BUILD/MEDIUM: threads: rename thread_info struct to ha_thread_info
    - BUILD/SMALL: threads: enable threads on osx
    - BUILD/MEDIUM: threads: enable cpu_affinity on osx
    - MINOR: istbuf: add b_fromist() to make a buffer from an ist
    - BUG/MINOR: cache: also cache absolute URIs
    - BUG/MINOR: mworker/ssl: close openssl FDs unconditionally
    - BUG/MINOR: tcp: Don't alter counters returned by tcp info fetchers
    - BUG/MEDIUM: lists: Handle 1-element-lists in MT_LIST_BEHEAD().
    - BUG/MEDIUM: mux_pt: Make sure we don't have a conn_stream before freeing.
    - BUG/MEDIUM: tasklet: properly compute the sleeping threads mask in tasklet_wakeup()
    - BUG/MAJOR: idle conns: schedule the cleanup task on the correct threads
    - BUG/MEDIUM: task: make tasklets either local or shared but not both at once
    - Revert e8826ded5fea3593d89da2be5c2d81c522070995.
    - BUG/MEDIUM: mux_pt: Don't destroy the connection if we have a stream attached.
    - BUG/MEDIUM: mux_pt: Only call the wake emthod if nobody subscribed to receive.
    - REGTEST: mcli/mcli_show_info: launch a 'show info' on the master CLI
    - CLEANUP: ssl: make ssl_sock_load_cert*() return real error codes
    - CLEANUP: ssl: make ssl_sock_load_ckchs() return a set of ERR_*
    - CLEANUP: ssl: make cli_parse_set_cert handle errcode and warnings.
    - CLEANUP: ssl: make ckch_inst_new_load_(multi_)store handle errcode/warn
    - CLEANUP: ssl: make ssl_sock_put_ckch_into_ctx handle errcode/warn
    - CLEANUP: ssl: make ssl_sock_load_dh_params handle errcode/warn
    - CLEANUP: bind: handle warning label on bind keywords parsing.
    - BUG/MEDIUM: ssl: 'tune.ssl.default-dh-param' value ignored with openssl > 1.1.1
    - BUG/MINOR: mworker/cli: reload fail with inherited FD
    - BUG/MINOR: ssl: Fix fd leak on error path when a TLS ticket keys file is parsed
    - BUG/MINOR: stick-table: Never exceed (MAX_SESS_STKCTR-1) when fetching a stkctr
    - BUG/MINOR: cache: alloc shctx after check config
    - BUG/MINOR: sample: Make the `field` converter compatible with `-m found`
    - BUG/MINOR: server: check return value of fopen() in apply_server_state()
    - REGTESTS: make seamless-reload depend on 1.9 and above
    - REGTESTS: server/cli_set_fqdn requires version 1.8 minimum
    - BUG/MINOR: dns: allow srv record weight set to 0
    - BUG/MINOR: ssl: fix memcpy overlap without consequences.
    - BUG/MINOR: stick-table: fix an incorrect 32 to 64 bit key conversion
    - BUG/MEDIUM: pattern: make the pattern LRU cache thread-local and lockless
    - BUG/MINOR: mux-h2: do not emit logs on backend connections
    - CLEANUP: ssl: remove old TODO commentary
    - CLEANUP: ssl: fix SNI/CKCH lock labels
    - MINOR: ssl: OCSP functions can load from file or buffer
    - MINOR: ssl: load sctl from buf OR from a file
    - MINOR: ssl: load issuer from file or from buffer
    - MINOR: ssl: split ssl_sock_load_crt_file_into_ckch()
    - BUG/MINOR: ssl/cli: fix looking up for a bundle
    - MINOR: ssl/cli: update ocsp/issuer/sctl file from the CLI
    - MINOR: ssl: update ssl_sock_free_cert_key_and_chain_contents
    - MINOR: ssl: copy a ckch from src to dst
    - MINOR: ssl: new functions duplicate and free a ckch_store
    - MINOR: ssl/cli: assignate a new ckch_store
    - MEDIUM: cli/ssl: handle the creation of SSL_CTX in an IO handler
    - BUG/MINOR: ssl/cli: fix build of SCTL and OCSP
    - BUG/MINOR: ssl/cli: out of bounds when built without ocsp/sctl
    - BUG/MINOR: ssl: fix build with openssl < 1.1.0
    - BUG/MINOR: ssl: fix build of X509_chain_up_ref() w/ libreSSL
    - MINOR: tcp: avoid confusion in time parsing init
    - MINOR: debug: add a new "debug dev stream" command
    - MINOR: cli/debug: validate addresses using may_access() in "debug dev stream"
    - REORG: move CLI access level definitions to cli.h
    - MINOR: cli: add an expert mode to hide dangerous commands
    - MINOR: debug: make most debug CLI commands accessible in expert mode
    - MINOR: stats/debug: maintain a counter of debug commands issued
    - BUG/MEDIUM: debug: address a possible null pointer dereference in "debug dev stream"

5 years agoBUG/MEDIUM: debug: address a possible null pointer dereference in "debug dev stream"
Willy Tarreau [Fri, 25 Oct 2019 08:06:55 +0000 (10:06 +0200)] 
BUG/MEDIUM: debug: address a possible null pointer dereference in "debug dev stream"

As reported in issue #343, there is one case where a NULL stream can
still be dereferenced, when getting &s->txn->flags. Let's protect all
assignments to stay on the safe side for future additions.

No backport is needed.

5 years agoMINOR: stats/debug: maintain a counter of debug commands issued
Willy Tarreau [Thu, 24 Oct 2019 16:18:02 +0000 (18:18 +0200)] 
MINOR: stats/debug: maintain a counter of debug commands issued

Debug commands will usually mark the fate of the process. We'd rather
have them counted and visible in a core or in stats output than trying
to guess how a flag combination could happen. The counter is only
incremented when the command is about to be issued however, so that
failed attempts are ignored.

5 years agoMINOR: debug: make most debug CLI commands accessible in expert mode
Willy Tarreau [Thu, 24 Oct 2019 16:03:39 +0000 (18:03 +0200)] 
MINOR: debug: make most debug CLI commands accessible in expert mode

Instead of relying on DEBUG_DEV for most debugging commands, which is
limiting, let's condition them to expert mode. Only one ("debug dev exec")
remains conditionned to DEBUG_DEV because it can have a security implication
on the system. The commands are not listed unless "expert-mode on" was first
entered on the CLI :

 > expert-mode on
 > help
   debug dev close <fd>        : close this file descriptor
   debug dev delay [ms]        : sleep this long
   debug dev exec  [cmd] ...   : show this command's output
   debug dev exit  [code]      : immediately exit the process
   debug dev hex   <addr> [len]: dump a memory area
   debug dev log   [msg] ...   : send this msg to global logs
   debug dev loop  [ms]        : loop this long
   debug dev panic             : immediately trigger a panic
   debug dev stream ...        : show/manipulate stream flags
   debug dev tkill [thr] [sig] : send signal to thread

 > debug dev stream
 Usage: debug dev stream { <obj> <op> <value> | wake }*
      <obj>   = {strm | strm.f | sif.f | sif.s | sif.x | sib.f | sib.s | sib.x |
                 txn.f | req.f | req.r | req.w | res.f | res.r | res.w}
      <op>    = {'' (show) | '=' (assign) | '^' (xor) | '+' (or) | '-' (andnot)}
      <value> = 'now' | 64-bit dec/hex integer (0x prefix supported)
      'wake' wakes the stream asssigned to 'strm' (default: current)

5 years agoMINOR: cli: add an expert mode to hide dangerous commands
Willy Tarreau [Thu, 24 Oct 2019 15:55:53 +0000 (17:55 +0200)] 
MINOR: cli: add an expert mode to hide dangerous commands

Some commands like the debug ones are not enabled by default but can be
useful on some production environments. In order to avoid the temptation
of using them incorrectly, let's introduce an "expert" mode for a CLI
connection, which allows some commands to appear and be used. It is
enabled by command "expert-mode on" which is not listed by default.

5 years agoREORG: move CLI access level definitions to cli.h
Willy Tarreau [Thu, 24 Oct 2019 11:50:48 +0000 (13:50 +0200)] 
REORG: move CLI access level definitions to cli.h

These ones were still in global.h which is misplaced.

5 years agoMINOR: cli/debug: validate addresses using may_access() in "debug dev stream"
Willy Tarreau [Thu, 24 Oct 2019 16:28:23 +0000 (18:28 +0200)] 
MINOR: cli/debug: validate addresses using may_access() in "debug dev stream"

This function adds some control by verifying that the target address is
really readable. It will not protect against writing to wrong places,
but will at least protect against a large number of mistakes such as
incorrectly copy-pasted addresses.

5 years agoMINOR: debug: add a new "debug dev stream" command
Willy Tarreau [Wed, 23 Oct 2019 15:23:25 +0000 (17:23 +0200)] 
MINOR: debug: add a new "debug dev stream" command

This new "debug dev stream" command allows to manipulate flags, timeouts,
states for streams, channels and stream interfaces, as well as waking a
stream up. These may be used to help reproduce certain bugs during
development. The operations are performed to the stream assigned by
"strm" which defaults to the CLI's stream. This stream pointer can be
chosen from one of those reported in "show sess". Example:

  socat - /tmp/sock1 <<< "debug dev stream strm=0x1555b80 req.f=-1 req.r=now wake"

5 years agoMINOR: tcp: avoid confusion in time parsing init
William Dauchy [Wed, 23 Oct 2019 17:31:36 +0000 (19:31 +0200)] 
MINOR: tcp: avoid confusion in time parsing init

We never enter val_fc_time_value when an associated fetcher such as `fc_rtt` is
called without argument.  meaning `type == ARGT_STOP` will never be true and so
the default `data.sint = TIME_UNIT_MS` will never be set.  remove this part to
avoid thinking default data.sint is set to ms while reading the code.

Signed-off-by: William Dauchy <w.dauchy@criteo.com>
[Cf: This patch may safely backported as far as 1.7. But no matter if not.]

5 years agoBUG/MINOR: ssl: fix build of X509_chain_up_ref() w/ libreSSL
William Lallemand [Wed, 23 Oct 2019 21:15:46 +0000 (23:15 +0200)] 
BUG/MINOR: ssl: fix build of X509_chain_up_ref() w/ libreSSL

LibreSSL brought X509_chain_up_ref() in 2.7.5, so no need to build our
own version starting from this version.

5 years agoBUG/MINOR: ssl: fix build with openssl < 1.1.0
William Lallemand [Wed, 23 Oct 2019 17:40:28 +0000 (19:40 +0200)] 
BUG/MINOR: ssl: fix build with openssl < 1.1.0

8c1cddef ("MINOR: ssl: new functions duplicate and free a ckch_store")
use some OpenSSL refcount functions that were introduced in OpenSSL
1.0.2 and OpenSSL 1.1.0.

Fix the problem by introducing them in openssl-compat.h.

Fix #336.

5 years agoBUG/MINOR: ssl/cli: out of bounds when built without ocsp/sctl
William Lallemand [Wed, 23 Oct 2019 13:00:52 +0000 (15:00 +0200)] 
BUG/MINOR: ssl/cli: out of bounds when built without ocsp/sctl

Commit 541a534 ("BUG/MINOR: ssl/cli: fix build of SCTL and OCSP")
introduced a bug in which we iterate outside the array durint a 'set ssl
cert' if we didn't built with the ocsp or sctl.

5 years agoBUG/MINOR: ssl/cli: fix build of SCTL and OCSP
William Lallemand [Wed, 23 Oct 2019 12:11:54 +0000 (14:11 +0200)] 
BUG/MINOR: ssl/cli: fix build of SCTL and OCSP

Fix the build issue of SCTL and OCSP for boring/libressl introduced by
44b3532 ("MINOR: ssl/cli: update ocsp/issuer/sctl file from the CLI")

5 years agoMEDIUM: cli/ssl: handle the creation of SSL_CTX in an IO handler
William Lallemand [Wed, 23 Oct 2019 08:53:05 +0000 (10:53 +0200)] 
MEDIUM: cli/ssl: handle the creation of SSL_CTX in an IO handler

To avoid affecting too much the traffic during a certificate update,
create the SNIs in a IO handler which yield every 10 ckch instances.

This way haproxy continues to respond even if we tries to update a
certificate which have 50 000 instances.

5 years agoMINOR: ssl/cli: assignate a new ckch_store
William Lallemand [Fri, 18 Oct 2019 09:27:07 +0000 (11:27 +0200)] 
MINOR: ssl/cli: assignate a new ckch_store

When updating a certificate from the CLI, it is not possible to revert
some of the changes if part of the certicate update failed. We now
creates a copy of the ckch_store for the changes so we can revert back
if something goes wrong.

Even if the ckch_store was affected before this change, it wasn't
affecting the SSL_CTXs used for the traffic. It was only a problem if we
try to update a certificate after we failed to do it the first time.

The new ckch_store is also linked to the new sni_ctxs so it's easy to
insert the sni_ctxs before removing the old ones.

5 years agoMINOR: ssl: new functions duplicate and free a ckch_store
William Lallemand [Fri, 18 Oct 2019 08:58:14 +0000 (10:58 +0200)] 
MINOR: ssl: new functions duplicate and free a ckch_store

ckchs_dup() alloc a new ckch_store and copy the content of its source.

ckchs_free() frees a ckch_store and its content.

5 years agoMINOR: ssl: copy a ckch from src to dst
William Lallemand [Thu, 17 Oct 2019 16:03:58 +0000 (18:03 +0200)] 
MINOR: ssl: copy a ckch from src to dst

ssl_sock_copy_cert_key_and_chain() copy the content of a
<src> cert_key_and_chain to a <dst>.

It applies a refcount increasing on every SSL structures (X509, DH,
privte key..) and allocate new buffers for the other fields.

5 years agoMINOR: ssl: update ssl_sock_free_cert_key_and_chain_contents
William Lallemand [Thu, 17 Oct 2019 16:04:45 +0000 (18:04 +0200)] 
MINOR: ssl: update ssl_sock_free_cert_key_and_chain_contents

The struct cert_key_and_chain now contains the DH, the sctl and the
ocsp_response. Free them.