Disable temporary the SSL verify by default in the httpclient. The
initialization of the @system-ca during the init of the httpclient is a
problem in some cases.
The verify can be reactivated with "httpclient-ssl-verify required" in
the global section.
The httpclient HTTPS requests now enable the "verify required" option.
To achieve this, the "@system-ca" ca-file is configured in the
httpclient ssl server. Which means all the system CAs will be loaded at
haproxy startup.
Change the init order of the httpclient, a different init sequence is
required to allow a more complicated init.
The init is splitted in two parts:
- the first part is executed before config_check_validity(), which
allows to create proxy and more advanced stuff than STG_INIT, because
we might want to use stuff already initialized in haproxy (trash
buffers for example)
- the second part is executed after the config_check_validity(),
currently it is used for the log configuration.
A conn-stream is never detached from an endpoint or an application alone,
except on a reset. Thus, to avoid any error, these functions are now
private. And cs_destroy() function is added to destroy a conn-stream. This
function is called when a stream is released, on the front and back
conn-streams, and when a health-check is finished.
As anticipated in commit 211ea252d ("BUG/MINOR: logs: fix logsrv leaks
on clean exit"), there were indeed other corner cases that were not
properly covered. Setting the http client's ring_name to NULL make the
sink lookup crash on startup in sink_find () with a config as simple as:
global
log ring@buf0 local0
The fields must be properly initialized (both config file name and
the ring_name). This only needs to be backported if/when the commit
above is backported.
When an HTTP client is started on an HAProxy compiled without the SSL
support, an error is triggered when HTTPS is used. In this case, the freshly
created conn-stream is released. But this code is specific to the non-SSL
part. Thus it is moved the in right #if/#else section.
This patch should fix the issue #1655.
Remaining flags and associated functions are move in the conn-stream
scope. These flags are added on the endpoint and not the conn-stream
itself. This way it will be possible to get them from the mux or the
applet. The functions to get or set these flags are renamed accordingly with
the "cs_" prefix and updated to manipualte a conn-stream instead of a
stream-interface.
si_shutr(), si_shutw(), si_chk_rcv() and si_chk_snd() are moved in the
conn-stream scope and renamed, respectively, cs_shutr(), cs_shutw(),
cs_chk_rcv(), cs_chk_snd() and manipulate a conn-stream instead of a
stream-interface.
Flags to disable lingering and half-close are now handled at the conn-stream
level. Thus SI_FL_NOLINGER and SI_FL_NOHALF stream-int flags are replaced by
CS_FL_NOLINGER and CS_FL_NOHALF conn-stream flags.
The source and destination addresses at the applicative layer are moved from
the stream-interface to the conn-stream. This simplifies a bit the code and
it is a logicial step to remove the stream-interface.
At many places, we now use the new CS functions to get a stream or a channel
from a conn-stream instead of using the stream-interface API. It is the
first step to reduce the scope of the stream-interfaces. The main change
here is about the applet I/O callback functions. Before the refactoring, the
stream-interface was the appctx owner. Thus, it was heavily used. Now, as
far as possible,the conn-stream is used. Of course, it remains many calls to
the stream-interface API.
All old flags CS_FL_* are now moved in the endpoint scope and renamed
CS_EP_* accordingly. It is a systematic replacement. There is no true change
except for the health-check and the endpoint reset. Here it is a bit special
because the same conn-stream is reused. Thus, we must handle endpoint
allocation errors. To do so, cs_reset_endp() has been adapted.
Thanks to this last change, it will now be possible to simplify the
multiplexer and probably the applets too. A review must also be performed to
remove some flags in the channel or the stream-interface. The HTX will
probably be simplified too. Finally, there is now some place in the
conn-stream to move info from the stream-interface.
The conn-stream endpoint is now shared between the conn-stream and the
applet or the multiplexer. If the mux or the applet is created first, it is
responsible to also create the endpoint and share it with the conn-stream.
If the conn-stream is created first, it is the opposite.
When the endpoint is only owned by an applet or a mux, it is called an
orphan endpoint (there is no conn-stream). When it is only owned by a
conn-stream, it is called a detached endpoint (there is no mux/applet).
The last entity that owns an endpoint is responsible to release it. When a
mux or an applet is detached from a conn-stream, the conn-stream
relinquishes the endpoint to recreate a new one. This way, the endpoint
state is never lost for the mux or the applet.
It is a transient commit to prepare next changes. Now, when a conn-stream is
created from an applet or a multiplexer, an endpoint is always provided. In
addition, the API to create a conn-stream was specialized to have one
function per type.
The next step will be to share the endpoint structure.
It is a transient commit to prepare next changes. It is possible to pass a
pre-allocated endpoint to create a new conn-stream. If it is NULL, a new
endpoint is created, otherwise the existing one is used. There no more
change at the conn-stream level.
In the applets, all conn-stream are created with no pre-allocated
endpoint. But for multiplexers, an endpoint is systematically created before
creating the conn-stream.
This change is only significant for the multiplexer part. For the applets,
the context and the endpoint are the same. Thus, there is no much change. For
the multiplexer part, the connection was used to set the conn-stream
endpoint and the mux's stream was the context. But it is a bit strange
because once a mux is installed, it takes over the connection. In a
wonderful world, the connection should be totally hidden behind the mux. The
stream-interface and, in a lesser extent, the stream, still access the
connection because that was inherited from the pre-multiplexer era.
Now, the conn-stream endpoint is the mux's stream (an opaque entity for the
conn-stream) and the connection is the context. Dedicated functions have
been added to attached an applet or a mux to a conn-stream.
The appctx owner is now always a conn-stream. Thus, it can be set during the
appctx allocation. But, to do so, the conn-stream must be created first. It
is not a problem on the server side because the conn-stream is created with
the stream. On the client side, we must take care to create the conn-stream
first.
This change should ease other changes about the applets bootstrapping.
When compiled in debug mode, a BUG_ON triggers an error when the payload is
fully transfered from the http-client buffer to the request channel
buffer. In fact, when channel_add_input() is called, the request buffer is
empty. So an error is reported when those data are directly forwarded,
because we try to add some output data on a buffer with no data.
To fix the bug, we must be sure to call channel_add_input() after the data
transfer.
The bug was introduced by the commit ccc7ee45f ("MINOR: httpclient: enable
request buffering"). So, this patch must be backported if the above commit
is backported.
In case an error provokes the release of the applet, we will never call
the end callback of the httpclient.
In the case of a lua script, it would mean that the lua task will never
be waked up after a yield, letting the lua script stuck forever.
Fix the issue by moving the callback from the end of the iohandler to
the applet release function.
Must be backported in 2.5.
The request buffering is required for doing l7 retry. The IO handler of
the httpclient need to be rework for that.
This patch change the IO handler so it copies partially the data instead
of swapping buffer. This is needed because the b_xfer won't never work
if the destination buffer is not empty, which is the case when
buffering.
CF_SHUTW_NOW shouldn't be a condition alone to exit the io handler, it
must be tested with the emptiness of the response channel.
Must be backported to 2.5.
A server could reply a response with a shut before the end of the htx
transfer, in this case the httpclient would leave before computing the
received response.
This patch fixes the issue by calling the "process_data" label instead of
the "more" label which don't do the si_shut.
Must be bacported in 2.5.
Checking msg >= HTTP_MSG_DATA was useful to check if we received all the
data. However it does not work correctly in case of errors because we
don't reach this state, preventing to catch the error in the httpclient.
The consequence of this problem is that we don't get the status code of
the error response upon an error.
Fix the issue by only checking co_data().
Must be backported to 2.5.
Log servers are a real mess because:
- entries are duplicated using memcpy() without their strings being
reallocated, which results in these ones not being freeable every
time.
- a new field, ring_name, was added in 2.2 by commit 99c453df9
("MEDIUM: ring: new section ring to declare custom ring buffers.")
but it's never initialized during copies, causing the same issue
- no attempt is made at freeing all that.
Of course, running "haproxy -c" under ASAN quickly notices that and
dumps a core.
This patch adds the missing strdup() and initialization where required,
adds a new free_logsrv() function to cleanly free() such a structure,
calls it from the proxy when iterating over logsrvs instead of silently
leaking their file names and ring names, and adds the same logsrv loop
to the proxy_free_defaults() function so that we don't leak defaults
sections on exit.
It looks a bit entangled, but it comes as a whole because all this stuff
is inter-dependent and was missing.
It's probably preferable not to backport this in the foreseable future
as it may reveal other jokes if some obscure parts continue to memcpy()
the logsrv struct.
src/http_client.c: In function ‘httpclient_cfg_postparser’:
src/http_client.c:1065:8: error: unused variable ‘errmsg’ [-Werror=unused-variable]
1065 | char *errmsg = NULL;
| ^~~~~~
src/http_client.c:1064:6: error: unused variable ‘err_code’ [-Werror=unused-variable]
1064 | int err_code = 0;
| ^~~~~~~~
Fix the build of the httpclient without SSL, the problem was introduced
with previous patch 71e3158 ("BUG/MINOR: httpclient: send the SNI using
the host header")
Must be backported in 2.5 as well.
Generate an SNI expression which uses the Host header of the request.
This is mandatory for most of the SSL servers nowadays.
Must be backported in 2.5 with the previous patch which export
server_parse_sni_expr().
The httpclient mistakenly use the htx_get_first{_blk}() functions instead
of the htx_get_head{_blk}() functions. Which could stop the httpclient
because it will be without the start line, waiting for data that won't never
come.
Must be backported in 2.5.
Remove the UNUSED blocks when iterating on headers, we should not stop
when encountering one. We should only stop iterating once we found the
EOH block. It doesn't provoke a problem, since we don't manipulates
the headers before treating them, but it could evolve in the future.
Must be backported to 2.5.
Consume partly the blocks in the httpclient I/O handler when there is
not enough room in the destination buffer for the whole block or when
the block is not contained entirely in the channel's output.
It prevents the I/O handler to be stuck in cases when we need to modify
the buffer with a filter for exemple.
Must be backported in 2.5.
In httpclient_applet_io_handler(), on the response path, we don't check
if the data are in the output part of the channel, and could consume
them before they were analyzed.
To fix this issue, this patch checks for the stline and the headers if
the msg_state is >= HTTP_MSG_DATA which means the stline and headers
were analyzed. For the data part, it checks if each htx blocks is in the
output before copying it.
Must be backported in 2.5.
In HTX, HTX_FL_EOM flag is added on the message to notifiy the end of the
message was received. In addition, the producer must set CS_FL_EOI flag on
the conn-stream. If it is a mux, the stream-interface is responsible to set
CF_EOI flag on the input channel. But, for now, if the producer is an
applet, in addition to the conn-stream flag, it must also set the channel
one.
These flags are used to notify the stream that the message is finished and
no more data are expected. It is especially important when the message
itself it directly forwarded from one side to the other. Because in this
case, the stream has no way to see the HTX_FL_EOM flag on the
message. Otherwise, the stream will detect a client or a server abort,
depending on the side.
For the HTTP client, it is not really easy to diagnose this error because
there is also another bug hiding this one. All HTTP request analyzers are
not set on the input channel. This will be fixed by another patch.
This patch must be backported to 2.5. It is related to the issue #1593.
Thanks to all previous changes, it is now possible to move the
stream-interface into the conn-stream. To do so, some SI functions are
removed and their conn-stream counterparts are added. In addition, the
conn-stream is now responsible to create and release the
stream-interface. While the stream-interfaces were inlined in the stream
structure, there is now a pointer in the conn-stream. stream-interfaces are
now dynamically allocated. Thus a dedicated pool is added. It is a temporary
change because, at the end, the stream-interface structure will most
probably disappear.
To be able to move the stream-interface from the stream to the conn-stream,
all access to the SI is done via the conn-stream. This patch is limited to
the httpclient part.
In the same way the conn-stream has a pointer to the stream endpoint , this
patch adds a pointer to the application entity in the conn-stream
structure. For now, it is a stream or a health-check. It is mandatory to
merge the stream-interface with the conn-stream.
Because appctx is now an endpoint of the conn-stream, there is no reason to
still have the stream-interface as appctx owner. Thus, the conn-stream is
now the appctx owner.
Thanks to previous changes, it is now possible to set an appctx as endpoint
for a conn-stream. This means the appctx is no longer linked to the
stream-interface but to the conn-stream. Thus, a pointer to the conn-stream
is explicitly stored in the stream-interface. The endpoint (connection or
appctx) can be retrieved via the conn-stream.
REGISTER is meant to only assemble static lists, not to initialize
code that may depend on some elements possibly initialized at this
level. For example the init code currently looks up transport protocols
such as XPRT_RAW and XPRT_SSL which ought to be themselves registered
from at REGISTER stage, and which currently work only because they're
still registered directly from a constructor. INIT is perfectly suited
for this level.
Add the ability to set a "server timeout" on the httpclient with either
the httpclient_set_timeout() API or the timeout argument in a request.
Issue #1470.
A bug was uncovered by commit fc5912914 ("MINOR: httpclient: Don't limit
data transfer to 1024 bytes"), it happens that callers of b_xfer() and
b_force_xfer() are expected to check for available room in the target
buffer. Previously it was unlikely to be full but now with full buffer-
sized transfers, it happens more often and in practice it is possible
to crash the process with the debug command "httpclient" on the CLI by
going beyond a the max buffer size. Other call places ought to be
rechecked by now and it might be time to rethink this API if it tends
to generalize.
This must be backported to 2.5.
When starting for the 2nd time a request from the same httpclient *hc
context, the flags are not reinitialized and the httpclient will stop
after the first call to the IO handler, because the END flag is always
present.
This patch also add a test before httpclient_start() to ensure we don't
start a client already started.
Must be backported in 2.5.
ist are not ended by '\0', leading to junk characters being displayed
when using %s for printing the HTTP start line.
Fix the issue by replacing %s by %.*s + istlen.
Must be backported in 2.5.
For debug purpose, no more 1024 bytes were copied at a time. But there is no
reason to keep this limitation. Thus, it is removed.
This patch may be backported to 2.5.
Since the HTTP legacy mode was removed, it is unexpected to create an HTTP
stream without a valid request. Thanks to this change, the wait_for_request
analyzer was significatly simplified. And it is possible because HTTP
multiplexers already take care to have a valid request to create a stream.
But it means that any HTTP applet on the client side must do the same. The
httpclient client is one of them. And it is not a problem because the
request is generated before starting the applet. We must just take care to
set the right state.
For now it works "by chance", because the applet seems to be scheduled
before the stream itself. But if this change, this will lead to crash
because the stream expects to have a request when wait_for_request analyzer.
This patch should be backported to 2.5.
For now, these buffers are allocated when the httpclient is created and
freed when it is released. Usually, we try to avoid to keep buffer allocated
if it is not required. Empty buffers should be released ASAP. Apart for
that, there is no issue with the response side because a copy is always
performed. However, for the request side, a swap with the channel's buffer
is always performed. And there is no guarantee the channel's buffer is
allocated. Thus, after the swap, the httpclient can retrieve a null
buffer. In practice, this never happens. But this may change. And it will be
required for a futur fix.
So, now, we systematically take care to have an allocated buffer when we
want to write in it. And it is released as soon as it becomes empty.
This patch should be backported to 2.5.
