Commit Graph

491 Commits

Author SHA1 Message Date
Willy Tarreau
ae94d4df8f [MINOR] http: make the "HTTP 200" status code configurable.
This status code is used in response to requests matching "monitor-uri".
Some users need to adjust it to fit their needs (eg: make some strings
appear there). As it's already defined as a chunked string and used
exactly like other status codes, it makes sense to make it configurable
with the usual "errorfile", "errorloc", ...
2011-05-11 16:31:43 +02:00
Willy Tarreau
027a85bb03 [MINOR] http: don't report the "haproxy" word on the monitoring response
Some people like to make the monitoring URL testable from unsafe locations.
Reporting haproxy's existence there can sometimes be problematic. This patch
should not be backported to 1.4 because it is possible, eventhough unlikely,
that some scripts rely on this word to appear there.
2011-05-11 16:31:43 +02:00
Willy Tarreau
1fc1f45618 [CRITICAL] fix risk of crash when dealing with space in response cookies
When doing fix 24581bae02 to correctly handle
response cookies, an unfortunate typo was inserted in the less likely code
path, resulting in a risk of crash when cookie-based persistence is enabled
and the server emits a cookie with several spaces around the equal sign.

This bug was noticed during a code backport. Its effects were never reported
because this situation is very unlikely to appear, but it can be provoked on
purpose by the server.

This patch must be backported to 1.4 versions which contain the fix above
(anything > 1.4.8), and to similar 1.3 versions > 1.3.25. 1.5-dev versions
after 1.5-dev2 are affected too.
2011-04-08 00:50:36 +02:00
Willy Tarreau
d8ee85a0a3 [BUG] http: fix content-length handling on 32-bit platforms
Despite much care around handling the content-length as a 64-bit integer,
forwarding was broken on 32-bit platforms due to the 32-bit nature of
the ->to_forward member of the "buffer" struct. The issue is that this
member is declared as a long, so while it works OK on 64-bit platforms,
32-bit truncate the content-length to the lower 32-bits.

One solution could consist in turning to_forward to a long long, but it
is used a lot in the critical path, so it's not acceptable to perform
all buffer size computations on 64-bit there.

The fix consists in changing the to_forward member to a strict 32-bit
integer and ensure in buffer_forward() that only the amount of bytes
that can fit into it is considered. Callers of buffer_forward() are
responsible for checking that their data were taken into account. We
arbitrarily ensure we never consider more than 2G at once.

That's the way it was intended to work on 32-bit platforms except that
it did not.

This issue was tracked down hard at Exosec with Bertrand Jacquin,
Thierry Fournier and Julien Thomas. It remained undetected for a long
time because files larger than 4G are almost always transferred in
chunked-encoded format, and most platforms dealing with huge contents
these days run on 64-bit.

The bug affects all 1.5 and 1.4 versions, and must be backported.
2011-03-28 16:25:16 +02:00
Willy Tarreau
26f0f17200 [BUG] http: fix possible incorrect forwarded wrapping chunk size (take 2)
Fix acd20f80 was incomplete, the computed "bytes" value was not used.

This fix must be backported to 1.4.
2011-03-27 20:00:03 +02:00
Willy Tarreau
7b7a8e9d83 [BUG] log: retrieve the target from the session, not the SI
Since we now have the copy of the target in the session, use it instead
of relying on the SI for it. The SI drops the target upon unregister()
so applets such as stats were logged as "NOSRV".
2011-03-27 19:53:06 +02:00
Willy Tarreau
0b3a411543 [BUG] session: conn_retries was not always initialized
Johannes Smith reported some wrong retries count in logs associated with bad
requests. The cause was that the conn_retries field in the stream interface
was only initialized when attempting to connect, but is used when logging,
possibly with an uninitialized value holding last connection's conn_retries.
This could have been avoided by making use of a stream interface initializer.

This bug is 1.5-specific.
2011-03-27 19:16:56 +02:00
Willy Tarreau
6da0f6d6dd [BUG] http: stats were not incremented on http-request deny
A counter increase was missing here.
This should be backported to 1.4 with care, as the code has changed a bit.
2011-03-13 22:00:24 +01:00
Willy Tarreau
ff011f26e9 [REORG] http: move the http-request rules to proto_http
And also rename "req_acl_rule" "http_req_rule". At the beginning that
was a bit confusing to me, especially the "req_acl" list which in fact
holds what we call rules. After some digging, it appeared that some
part of the code is 100% HTTP and not just related to authentication
anymore, so let's move that part to HTTP and keep the auth-only code
in auth.c.
2011-03-13 22:00:24 +01:00
Willy Tarreau
f68a15a951 [MEDIUM] http: always evaluate http-request rules before stats http-request
Right now, http-request rules are not evaluated if the URL matches the
stats request. This is quite unexpected. For instance, in the config
below, an abuser present in the abusers list will not be prevented access
to the stats.

   listen pub
        bind :8181
	acl abuser src -f abusers.lst
        http-request deny if abuser
        stats uri /stats

It is not a big deal but it's not documented as such either. For 1.5, let's
have both lists be evaluated in turn, until one blocks. For 1.4 we'll simply
update the doc to indicate that.

Also instead of duplicating the code, the patch factors out the list walking
code. The HTTP auth has been moved slightly earlier, because it was set after
the header addition code, but we don't need to add headers to a request we're
dropping.
2011-03-13 22:00:24 +01:00
Willy Tarreau
7d0aaf39d1 [MEDIUM] stats: split frontend and backend stats
It's very annoying that frontend and backend stats are merged because we
don't know what we're observing. For instance, if a "listen" instance
makes use of a distinct backend, it's impossible to know what the bytes_out
means.

Some points take care of not updating counters twice if the backend points
to the frontend, indicating a "listen" instance. The thing becomes more
complex when we try to add support for server side keep-alive, because we
have to maintain a pointer to the backend used for last request, and to
update its stats. But we can't perform such comparisons anymore because
the counters will not match anymore.

So in order to get rid of this situation, let's have both frontend AND
backend stats in the "struct proxy". We simply update the relevant ones
during activity. Some of them are only accounted for in the backend,
while others are just for frontend. Maybe we can improve a bit on that
later, but the essential part is that those counters now reflect what
they really mean.
2011-03-13 22:00:23 +01:00
David du Colombier
6f5ccb1589 [MEDIUM] add internal support for IPv6 server addresses
This patch turns internal server addresses to sockaddr_storage to
store IPv6 addresses, and makes the connect() function use it. This
code already works but some caveats with getaddrinfo/gethostbyname
still need to be sorted out while the changes had to be merged at
this stage of internal architecture changes. So for now the config
parser will not emit an IPv6 address yet so that user experience
remains unchanged.

This change should have absolutely zero user-visible effect, otherwise
it's a bug introduced during the merge, that should be reported ASAP.
2011-03-13 22:00:12 +01:00
Willy Tarreau
827aee913f [MAJOR] session: remove the ->srv pointer from struct session
This one has been removed and is now totally superseded by ->target.
To get the server, one must use target_srv(&s->target) instead of
s->srv now.

The function ensures that non-server targets still return NULL.
2011-03-10 23:32:17 +01:00
Willy Tarreau
9e000c6ec8 [CLEANUP] stream_interface: use inline functions to manipulate targets
The connection target involves a type and a union of pointers, let's
make the code cleaner using simple wrappers.
2011-03-10 23:32:17 +01:00
Willy Tarreau
3d80d911aa [MEDIUM] session: remove s->prev_srv which is not needed anymore
s->prev_srv is used by assign_server() only, but all code paths leading
to it now take s->prev_srv from the existing s->srv. So assign_server()
can do that copy into its own stack.

If at one point a different srv is needed, we still have a copy of the
last server on which we failed a connection attempt in s->target.
2011-03-10 23:32:16 +01:00
Willy Tarreau
664beb8610 [MINOR] session: add a pointer to the new target into the session
When dealing with HTTP keep-alive, we'll have to know if we can reuse
an existing connection. For that, we'll have to check if the current
connection was made on the exact same target (referenced in the stream
interface).

Thus, we need to first assign the next target to the session, then
copy it to the stream interface upon connect(). Later we'll check for
equivalence between those two operations.
2011-03-10 23:32:16 +01:00
Willy Tarreau
295a837726 [REORG] session: move the data_ctx struct to the stream interface's applet
This is in fact where those parts belong to. The old data_state was replaced
by applet.state and is now initialized when the applet is registered. It's
worth noting that the applet does not need to know the session nor the
buffer anymore since everything is brought by the stream interface.

It is possible that having a separate applet struct would simplify the
code but that's not a big deal.
2011-03-10 23:32:16 +01:00
Willy Tarreau
75581aebb0 [CLEANUP] session: remove data_source from struct session
This one was only used for logging purposes, it's not needed
anymore.
2011-03-10 23:32:15 +01:00
Willy Tarreau
71904a4ee8 [MEDIUM] log: take the logged server name from the stream interface
With HTTP keep-alive, logging the right server name will be quite
complex because the assigned server will possibly change before we log.
Also, when we want to log accesses to an applet, it's not easy because
the applet becomes NULL again before logging.

The logged server's name is now taken from the target stored in the
stream interface. That way we can log an applet, a server name, or we
could even log a proxy or anything else if we wanted to. Ideally the
session should contain a desired target which is the one which should
be logged.
2011-03-10 23:32:15 +01:00
Willy Tarreau
957c0a5845 [REORG] session: move client and server address to the stream interface
This will be needed very soon for the keep-alive.
2011-03-10 23:32:14 +01:00
Willy Tarreau
bc4af0573c [REORG] stream_interface: move the st0, st1 and private members to the applet
Those fields are only used by the applets, so let's move them to the
struct.
2011-03-10 23:32:14 +01:00
Willy Tarreau
b24281b0ff [MINOR] stream_interface: make use of an applet descriptor for IO handlers
I/O handlers are still delicate to manipulate. They have no type, they're
just raw functions which have no knowledge of themselves. Let's have them
declared as applets once for all. That way we can have multiple applets
share the same handler functions and we can store their names there. When
we later need to add more parameters (eg: usage stats), we'll be able to
do so in the applets themselves.

The CLI functions has been prefixed with "cli" instead of "stats" as it's
clearly what is going on there.

The applet descriptor in the stream interface should get all the applet
specific data (st0, ...) but this will be done in the next patch so that
we don't pollute this one too much.
2011-03-10 23:32:14 +01:00
Cyril Bonté
1e2a170cf8 [BUG] stats: admin web interface must check the proxy state
Similar to the stats socket bug, we must check that the proxy is not disabled
before trying to enable/disable a server.

Even if a disabled proxy is not displayed, someone can inject a faulty proxy
name in the POST parameters. So, we must ensure that no disabled proxy can be
used.
2011-03-04 10:01:40 +01:00
Willy Tarreau
61a21a34da [BUG] http: balance url_param did not work with first parameters on POST
Bryan Talbot reported that POST requests with a query string were not
correctly processed if the hash parameter was the first one, because
the delimiter that was looked for to trigger the parsing was '&' instead
of '?'.

Also, while checking the code, it became apparent that it was enough for
a query string to be present in the request for POST parameters to be
ignored, even if the url_param was in the body and not in the URL.

The code has then been fixed like this :
   1) look for URL param. If found, return it.
   2) if no URL param was found and method is POST, then look it up into
      the body

The code now seems to pass all request combinations.

This patch must be backported to 1.4 since 1.4 is equally broken right now.
2011-03-01 20:42:20 +01:00
Willy Tarreau
124d99181c [BUG] http: fix computation of message body length after forwarding has started
Till now, the forwarding code was making use of the hdr_content_len member
to hold the size of the last chunk parsed. As such, it was reset after being
scheduled for forwarding. The issue is that this entry was reset before the
data could be viewed by backend.c in order to parse a POST body, so the
"balance url_param check_post" did not work anymore.

In order to fix this, we need two things :
  - the chunk size (reset upon every forward)
  - the total body size (not reset)

hdr_content_len was thus replaced by the former (hence the size of the patch)
as it makes more sense to have it stored that way than the way around.

This patch should be backported to 1.4 with care, considering that it affects
the forwarding code.
2011-03-01 20:30:48 +01:00
Willy Tarreau
acd20f80c1 [BUG] http: fix possible incorrect forwarded wrapping chunk size
It seems like if a response message is chunked and the chunk size wraps
at the end of the buffer and the crlf sequence is incomplete, then we
can forward a wrong chunk size due to incorrect handling of the wrapped
size. It seems extremely unlikely to occur on real traffic (no reason to
have half of the CRLF after a chunk) but nothing prevents it from being
possible.

This fix must be backported to 1.4.
2011-03-01 20:04:36 +01:00
Willy Tarreau
910ef306bc [BUG] http: use correct ACL pointer when evaluating authentication
req_acl was used instead of req_acl_final. As a matter of luck, both
happen to be the same at this point, but this is not granted in the
future.

This fix should be backported to 1.4.
2011-02-13 12:18:22 +01:00
Cyril Bonté
23b39d9859 [MINOR] stats: add support for several packets in stats admin
Some browsers send POST requests in several packets, which was not supported
by the "stats admin" function.

This patch allows to wait for more data when they are not fully received
(we are still limited to a certain size defined by the buffer size minus its
reserved space).
It also adds support for the "Expect: 100-Continue" header.
2011-02-12 13:10:18 +01:00
Willy Tarreau
5c4784f4b8 [BUG] http: update the header list's tail when removing the last header
Stefan Behte reported a strange case where depending on the position of
the Connection header in the header list, some headers added after it
were or were not usable in "balance hdr()". The reason is that when the
last header is removed, the list's tail was not updated, so any header
added after that one was not visible from the list.

This fix must be backported to 1.4 and possibly 1.3.
2011-02-12 13:07:35 +01:00
Willy Tarreau
0013433b09 [MINOR] http: improve url_param pattern extraction to ignore empty values
It's better to avoid sticking on empty parameter values, as this almost
always indicates a missing parameter. Otherwise it's easy to enter a
situation where all new visitors stick to the same server.
2011-01-04 14:57:34 +01:00
David Cournapeau
16023eef0b [MINOR] http: add pattern extraction method to stick on query string parameter
This is an updated version of my patch for url parameter extraction on
stick table. It adds "url_param(name)" as a possible stick method.
2011-01-03 13:26:02 +01:00
Cyril Bonté
9ea2b9ac75 [BUG] http: fix http-pretend-keepalive and httpclose/tunnel mode
Since haproxy 1.4.9, combining option httpclose and option
http-pretend-keepalive can leave the connections opened until the backend
keep-alive timeout is reached, providing bad performances.
The same can occur when the proxy is in tunnel mode.

This patch ensures that the server side connection is closed after the
response and ignore http-pretend-keepalive in tunnel mode.
2010-12-29 15:24:48 +01:00
Willy Tarreau
ed2fd2daea [BUG] http: fix incorrect error reporting during data transfers
We've had several issues related to data transfers. First, if a
client aborted an upload before the server started to respond, it
would get a 502 followed by a 400. The same was true (in the other
way around) if the server suddenly aborted while the client was
uploading the data.

The flags reported in the logs were misleading. Request errors could
be reported while the transfer was stopped during the data phase. The
status codes could also be overwritten by a 400 eventhough the start
of the response was transferred to the client.

The stats were also wrong in case of data aborts. The server or the
client could sometimes be miscredited for being the author of the
abort depending on where the abort was detected. Some client aborts
could also be accounted as request errors and some server aborts as
response errors.

Now it seems like all such issues are fixed. Since we don't have a
specific state for data flowing from the client to the server
before the server responds, we're still counting the client aborted
transfers as "CH", and they become "CD" when the server starts to
respond. Ideally a "P" state would be desired.

This patch should be backported to 1.4.
2010-12-29 13:55:32 +01:00
Willy Tarreau
0499e3575c [BUG] http: analyser optimizations broke pipelining
HTTP pipelining currently needs to monitor the response buffer to wait
for some free space to be able to send a response. It was not possible
for the HTTP analyser to be called based on response buffer activity.
Now we introduce a new buffer flag BF_WAKE_ONCE which is set when the
HTTP request analyser is set on the response buffer and some activity
is detected. This is not clean at all but once of the only ways to fix
the issue before we make it possible to register events for analysers.

Also it appeared that one realign condition did not cover all cases.
2010-12-17 07:15:57 +01:00
Willy Tarreau
10479e4bac [MINOR] stats: add global event ID and count
This counter will help quickly spot whether there are new errors or not.
It is also assigned to each capture so that a script can keep trace of
which capture was taken when.
2010-12-12 14:00:34 +01:00
Willy Tarreau
e1582eb7f6 [MINOR] http: capture incorrectly chunked message bodies
It is possible to block on incorrectly chunked requests or responses,
but this becomes very hard to debug when it happens once in a while.
This patch adds the ability to also capture incorrectly chunked requests
and responses. The chunk will appear in the error buffer and will be
verifiable with the usual "show errors". The incorrect byte will match
the error location.
2010-12-12 13:10:11 +01:00
Willy Tarreau
81f2fb97fe [MINOR] http: support wrapping messages in error captures
Error captures did only support contiguous messages. This is annoying
for capturing chunking errors, so let's ensure the function is able to
copy wrapped messages.
2010-12-12 13:09:08 +01:00
Willy Tarreau
3fe693b4d6 [BUG] http chunking: don't report a parsing error on connection errors
When haproxy parses chunk-encoded data that are scheduled to be sent, it is
possible that the other end is closed (mainly due to a client abort returning
as an error). The message state thus changes to HTTP_MSG_ERROR and the error
is reported as a chunk parsing error ("PD--") while it is not. Detect this
case before setting the flags and set the appropriate flag in this case.
2010-12-12 12:50:05 +01:00
Willy Tarreau
078272e115 [MINOR] stats: report HTTP message state and buffer flags in error dumps
Debugging parsing errors can be greatly improved if we know what the parser
state was and what the buffer flags were (especially for closed inputs/outputs
and full buffers). Let's add that to the error snapshots.
2010-12-12 12:46:33 +01:00
Willy Tarreau
57f5c12c04 [OPTIM] http: don't send each chunk in a separate packet
When forwarding chunk-encoded data, each chunk gets a TCP PUSH flag when
going onto the wire simply because the send() function does not know that
some data remain after it (next chunk). Now we set the BF_EXPECT_MORE flag
on the buffer if the chunk size is not null. That way we can reduce the
number of packets sent, which is particularly noticeable when forwarding
compressed data, especially as it requires less ACKs from the client.
2010-12-02 00:39:33 +01:00
Willy Tarreau
342b11c4d4 [BUG] http: do not re-enable the PROXY analyser on keep-alive
The PROXY analyser is connection-oriented and must only be set once. When
an HTTP transaction is done, we must not re-enable it.
2010-11-29 07:32:02 +01:00
Willy Tarreau
26db59ea6b [BUG] http: correctly update the header list when removing two consecutive headers
When a header is removed, the previous header's next pointer is updated
to reflect the next of the current header. However, when cycling through
the loop, we update the prev pointer to point to the deleted header, which
means that if we delete another header, it's the deleted header's next
pointer that will be updated, leaving the deleted header in the list with
a null length, which is forbidden.

We must just not update the prev pointer after a removal.

This bug was present when either "reqdel" and "rspdel" removed two consecutive
headers. It could also occur when removing cookies in either requests or
responses, but since headers were the last header processing, the issue
remained unnoticed.

Issue reported by Hank A. Paulson.

This fix must be ported to 1.4 and possibly 1.3.
2010-11-28 07:06:23 +01:00
Willy Tarreau
b810554f8f [CRITICAL] cookies: mixing cookies in indirect mode and appsession can crash the process
Cookies in indirect mode are removed from the cookie header. Three pointers
ought to be updated when appsession cookies are processed next, but were not.
The result is that a memcpy() can be called with a negative value causing the
process to crash. It is not sure whether this can be remotely exploited or not.
(cherry picked from commit c5f3749aa3ccfdebc4992854ea79823d26f66213)
2010-11-28 07:06:22 +01:00
Willy Tarreau
77eb9b8a2d [BUG] appsession: fix possible double free in case of out of memory
In out of memory conditions, the ->destroy function would free all
possibly allocated pools from the current appsession, including those
that were not yet allocated nor assigned, which used to point to a
previous allocation, obviously resulting in a segfault.
(cherry picked from commit 75eae485921d3a6ce197915c769673834ecbfa5c)
2010-11-19 13:25:11 +01:00
Willy Tarreau
f70fc75296 [BUG] capture: do not capture a cookie if there is no memory left
In case of out of memory, it was possible to write to a null pointer
when capturing response cookies due to a missing "else" block. The
request handling was fine though.
(cherry picked from commit 62e3604d7dd27741c0b4c9e27d9e7c73495dfc32)
2010-11-19 13:25:11 +01:00
Emeric Brun
485479d8e9 [MEDIUM] Create new protected pattern types CONSTSTRING and CONSTDATA to force memcpy if data from protected areas need to be manipulated.
Enhance pattern convs and fetch argument parsing, now fetchs and convs callbacks used typed args.
Add more details on error messages on parsing pattern expression function.
Update existing pattern convs and fetchs to new proto.
Create stick table key type "binary".
Manage Truncation and padding if pattern's fetch-converted result don't match table key size.
2010-11-11 09:29:07 +01:00
Cyril Bonté
acd7d63ff9 [CLEANUP] Remove unneeded chars allocation
Some arrays used to log addresses add some more bytes for ports but this space
is never used.
2010-11-11 09:26:28 +01:00
Emeric Brun
5bd86a8ff5 [MINOR] Support listener's sockets unix on http logs.
Enhance controls of sockets family on X-Forwarded-For and X-Original-To insert
2010-11-09 15:59:42 +01:00
Willy Tarreau
ba4c5be880 [MINOR] cookie: add support for the "preserve" option
This option makes haproxy preserve any persistence cookie emitted by
the server, which allows the server to change it or to unset it, for
instance, after a logout request.
(cherry picked from commit 52e6d75374c7900c1fe691c5633b4ae029cae8d5)
2010-10-30 19:04:36 +02:00
Willy Tarreau
7f18e52b13 [MINOR] acl: add the http_req_first match
This match returns true when the request calling it is the first one of
a connection.
(cherry picked from commit 922ca979c50653c415852531f36fe409190ad76b)
2010-10-30 19:04:35 +02:00