]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
5 years agoMINOR: log: Don't depends on a stream to process samples in log-format string
Christopher Faulet [Mon, 6 Apr 2020 08:40:02 +0000 (10:40 +0200)] 
MINOR: log: Don't depends on a stream to process samples in log-format string

When a log-format string is evaluated, there is no reason to process sample
fetches only when a stream is defined. Several sample fetches are available
outside the stream scope. All others should handle calls without stream. This
patch is mandatory to support log-format string in tcp-check rules.

5 years agoMEDIUM: checks: Support log-format strings for tcp-check send rules
Christopher Faulet [Mon, 30 Mar 2020 17:52:29 +0000 (19:52 +0200)] 
MEDIUM: checks: Support log-format strings for tcp-check send rules

An extra parameter for tcp-check send rules can be specified to handle the
string or the hexa string as a log-format one. Using "log-format" option,
instead of considering the data to send as raw data, it is parsed as a
log-format string. Thus it is possible to call sample fetches to customize data
sent to a server. Of course, because we have no stream attached to healthchecks,
not all sample fetches are available. So be careful.

    tcp-check set-var(check.port) int(8000)
    tcp-check set-var(check.uri) str(/status)
    tcp-check connect port var(check.port)
    tcp-check send "GET %[check.uri] HTTP/1.0\r\n" log-format
    tcp-check send "Host: %[srv_name]\r\n" log-format
    tcp-check send "\r\n"

5 years agoMEDIUM: checks: Support expression to set the port
Christopher Faulet [Mon, 30 Mar 2020 13:19:03 +0000 (15:19 +0200)] 
MEDIUM: checks: Support expression to set the port

Since we have a session attached to tcp-check healthchecks, It is possible use
sample expression and variables. In addition, it is possible to add tcp-check
set-var rules to define custom variables. So, now, a sample expression can be
used to define the port to use to establish a connection for a tcp-check connect
rule. For instance:

    tcp-check set-var(check.port) int(8888)
    tcp-check connect port var(check.port)

5 years agoMINOR: checks: Add the addr option for tcp-check connect rule
Christopher Faulet [Tue, 31 Mar 2020 06:15:58 +0000 (08:15 +0200)] 
MINOR: checks: Add the addr option for tcp-check connect rule

With this option, it is now possible to use a specific address to open the
connection for a tcp-check connect rule. If the port option is also specified,
it is used in priority.

5 years agoMINOR: checks: Add the default option for tcp-check connect rules
Christopher Faulet [Mon, 30 Mar 2020 11:54:42 +0000 (13:54 +0200)] 
MINOR: checks: Add the default option for tcp-check connect rules

With this option, it is possible to open a connection from a tcp-check connect
rule using all parameter of the server line, like any other healthcheck. For
now, this parameter is exclusive with all other option for a tcp-check connect
rule.

5 years agoMINOR: ssl: Export a generic function to parse an alpn string
Christopher Faulet [Mon, 20 Apr 2020 16:32:29 +0000 (18:32 +0200)] 
MINOR: ssl: Export a generic function to parse an alpn string

Parsing of an alpn string has been moved in a dedicated function and exposed to
be used from outside the ssl_sock module.

5 years agoMINOR: checks: Add the alpn option for tcp-check connect rules
Christopher Faulet [Mon, 30 Mar 2020 11:16:44 +0000 (13:16 +0200)] 
MINOR: checks: Add the alpn option for tcp-check connect rules

This option defines which protocols to advertise with ALPN on the SSL conection
opened by a tcp-check connect rule.

5 years agoMINOR: checks: Add the via-socks4 option for tcp-check connect rules
Christopher Faulet [Mon, 30 Mar 2020 11:07:02 +0000 (13:07 +0200)] 
MINOR: checks: Add the via-socks4 option for tcp-check connect rules

With this option, it is possible to establish the connection opened by a
tcp-check connect rule using upstream socks4 proxy. Info from the socks4
parameter on the server are used.

5 years agoMINOR: checks: Add the sni option for tcp-check connect rules
Christopher Faulet [Mon, 30 Mar 2020 11:00:05 +0000 (13:00 +0200)] 
MINOR: checks: Add the sni option for tcp-check connect rules

With this option, it is possible to specify the SNI to be used for SSL
conncection opened by a tcp-check connect rule.

5 years agoMINOR: checks: Add support to set-var and unset-var rules in tcp-checks
Gaetan Rivet [Mon, 24 Feb 2020 16:34:11 +0000 (17:34 +0100)] 
MINOR: checks: Add support to set-var and unset-var rules in tcp-checks

Evaluate the registered action_ptr associated with each CHK_ACTION_KW rules from
a ruleset. Currently only the 'set-var' and 'unset-var' are parsed by the
tcp-check parser. Thus it is now possible to set or unset variables. It is
possible to use such rules before the first connect of the ruleset.

5 years agoMEDIUM: checks: Parse custom action rules in tcp-checks
Gaetan Rivet [Fri, 21 Feb 2020 17:14:59 +0000 (18:14 +0100)] 
MEDIUM: checks: Parse custom action rules in tcp-checks

Register the custom action rules "set-var" and "unset-var", that will
call the parse_store() command upon parsing.

These rules are thus built and integrated to the tcp-check ruleset, but
have no further effect for the moment.

5 years agoMINOR: checks/vars: Add a check scope for variables
Gaetan Rivet [Fri, 21 Feb 2020 17:13:44 +0000 (18:13 +0100)] 
MINOR: checks/vars: Add a check scope for variables

Add a dedicated vars scope for checks. This scope is considered as part of the
session scope for accounting purposes.

The scope can be addressed by a valid session, even embryonic. The stream is not
necessary.

The scope is initialized after the check session is created. All variables are
then pruned before the session is destroyed.

5 years agoMEDIUM: checks: Associate a session to each tcp-check healthcheck
Gaetan Rivet [Fri, 14 Feb 2020 16:42:54 +0000 (17:42 +0100)] 
MEDIUM: checks: Associate a session to each tcp-check healthcheck

Create a session for each healthcheck relying on a tcp-check ruleset. When such
check is started, a session is allocated, which will be freed when the check
finishes. A dummy static frontend is used to create these sessions. This will be
useful to support variables and sample expression. This will also be used,
later, by HTTP healthchecks to rely on HTTP muxes.

5 years agoMAJOR: checks: Refactor and simplify the tcp-check loop
Christopher Faulet [Mon, 30 Mar 2020 09:05:10 +0000 (11:05 +0200)] 
MAJOR: checks: Refactor and simplify the tcp-check loop

The loop in tcpcheck_main() function is quite hard to understand. Depending
where we are in the loop, The current_step is the currentely executed rule or
the one to execute on the next call to tcpcheck_main(). When the check result is
reported, we rely on the rule pointed by last_started_step or the one pointed by
current_step. In addition, the loop does not use the common list_for_each_entry
macro and it is thus quite confusing.

So the loop has been totally rewritten and splitted to several functions to
simplify its reading and its understanding. Tcp-check rules are evaluated in
dedicated functions. And a common for_each loop is used and only one rule is
referenced, the current one.

5 years agoMEDIUM: checks: Add implicit tcp-check connect rule
Christopher Faulet [Thu, 26 Mar 2020 16:38:49 +0000 (17:38 +0100)] 
MEDIUM: checks: Add implicit tcp-check connect rule

After the configuration parsing, when its validity check, an implicit tcp-check
connect rule is added in front of the tcp-check ruleset if the first non-comment
rule is not a connect one. This implicit rule is flagged to use the default
check parameter.

This means now, all tcp-check rulesets begin with a connect and are never
empty. When tcp-check healthchecks are used, all connections are thus handled by
tcpcheck_main() function.

5 years agoMINOR: checks: define a tcp-check connect type
Gaetan Rivet [Fri, 21 Feb 2020 17:49:05 +0000 (18:49 +0100)] 
MINOR: checks: define a tcp-check connect type

The check rule itself is not changed, only its representation.

5 years agoMINOR: checks: define tcp-check send type
Gaetan Rivet [Fri, 21 Feb 2020 17:41:28 +0000 (18:41 +0100)] 
MINOR: checks: define tcp-check send type

The check rule itself is not changed, only its representation.

5 years agoMINOR: checks: Set the tcp-check rule index during parsing
Gaetan Rivet [Tue, 25 Feb 2020 16:19:17 +0000 (17:19 +0100)] 
MINOR: checks: Set the tcp-check rule index during parsing

Now the position of a tcp-check rule in a chain is set during the parsing. This
simplify significantly the function retrieving the current step id.

5 years agoMEDIUM: proxy/checks: Register a keyword to parse tcp-check rules
Christopher Faulet [Wed, 25 Mar 2020 17:20:15 +0000 (18:20 +0100)] 
MEDIUM: proxy/checks: Register a keyword to parse tcp-check rules

The keyword 'tcp-check' is now parsed in a dedicated callback function. Thus the
code to parse these rules is now located in checks.c. In addition, a deinit
function have been added to release proxy tcp-check rules, on error or when
HAProxy is stopped.

This patch is based on Gaetan Rivet work. It uses a callback function registerd
on the 'tcp-check' keyword instead, but the spirit is the same.

5 years agoMEDIUM: checks: Use a non-comment rule iterator to get next rule
Gaetan Rivet [Fri, 14 Feb 2020 16:47:08 +0000 (17:47 +0100)] 
MEDIUM: checks: Use a non-comment rule iterator to get next rule

This kind of iteration is used several times with various degrees of
clarity. Make a proper function for this use.

5 years agoMINOR: checks: Don't use a static tcp rule list head
Gaetan Rivet [Fri, 7 Feb 2020 14:37:17 +0000 (15:37 +0100)] 
MINOR: checks: Don't use a static tcp rule list head

To allow reusing these blocks without consuming more memory, their list
should be static and share-able accross uses. The head of the list will
be shared as well.

It is thus necessary to extract the head of the rule list from the proxy
itself. Transform it into a pointer instead, that can be easily set to
an external dynamically allocated head.

5 years agoMEDIUM: checks: capture groups in expect regexes
Gaetan Rivet [Fri, 7 Feb 2020 14:37:17 +0000 (15:37 +0100)] 
MEDIUM: checks: capture groups in expect regexes

Parse back-references in comments of tcp-check expect rules.  If references are
made, capture groups in the match and replace references to it within the
comment when logging the error. Both text and binary regex can caputre groups
and reference them in the expect rule comment.

[Cf: I slightly updated the patch. exp_replace() function is used instead of a
custom one. And if the trash buffer is too small to contain the comment during
the substitution, the comment is ignored.]

5 years agoMINOR: checks: Simplify functions to get step id and comment
Christopher Faulet [Tue, 24 Mar 2020 12:31:19 +0000 (13:31 +0100)] 
MINOR: checks: Simplify functions to get step id and comment

The loop to find the id corresponding to the current rule in
tcpcheck_get_step_id() function has been simplified. And
tcpcheck_get_step_comment() function now only relies on the current rule to find
the rigth comment string. The step id is no longer used. To do so, we iterate
backward from the current step to find the first COMMENT rule immediately
preceedding the expect rule chain.

5 years agoMINOR: checks: add rbinary expect match type
Gaetan Rivet [Fri, 7 Feb 2020 14:37:17 +0000 (15:37 +0100)] 
MINOR: checks: add rbinary expect match type

The rbinary match works similarly to the rstring match type, however the
received data is rewritten as hex-string before the match operation is
done.

This allows using regexes on binary content even with the POSIX regex
engine.

[Cf: I slightly updated the patch. mem2hex function was removed and dump_binary
is used instead.]

5 years agoMINOR: checks: Stop xform buffers to null-terminated string for tcp-check rules
Christopher Faulet [Thu, 19 Mar 2020 15:59:45 +0000 (16:59 +0100)] 
MINOR: checks: Stop xform buffers to null-terminated string for tcp-check rules

On the input buffer, it was mainly done to call regex_exec() function. But
regex_exec2() can be used instead. This way, it is no more required to add the
terminating null byte. For the output buffer, it was only done for debugging
purpose.

5 years agoMEDIUM: checks: rewrite tcp-check expect block
Gaetan Rivet [Wed, 26 Feb 2020 14:59:22 +0000 (15:59 +0100)] 
MEDIUM: checks: rewrite tcp-check expect block

Simplify and shorten the tcp-check expect rule processing, to clarify
steps and avoid code duplication.

5 years agoMINOR: checks: define a tcp expect type
Gaetan Rivet [Fri, 7 Feb 2020 14:37:17 +0000 (15:37 +0100)] 
MINOR: checks: define a tcp expect type

Extract the expect definition from its tcpcheck ; create a standalone type.

5 years agoMINOR: checks: add linger option to tcp connect
Gaetan Rivet [Fri, 7 Feb 2020 14:37:17 +0000 (15:37 +0100)] 
MINOR: checks: add linger option to tcp connect

Allow declaring tcpcheck connect commands with a new parameter,
"linger". This option will configure the connection to avoid using an
RST segment to close, instead following the four-way termination
handshake. Some servers would otherwise log each healthcheck as
an error.

5 years agoMINOR: checks: add min-recv tcp-check expect option
Gaetan Rivet [Fri, 7 Feb 2020 14:37:17 +0000 (15:37 +0100)] 
MINOR: checks: add min-recv tcp-check expect option

Some expect rules cannot be satisfied due to inherent ambiguity towards
the received data: in the absence of match, the current behavior is to
be forced to wait either the end of the connection or a buffer full,
whichever comes first. Only then does the matching diagnostic is
considered  conclusive. For instance :

    tcp-check connect
    tcp-check expect !rstring "^error"
    tcp-check expect string "valid"

This check will only succeed if the connection is closed by the server before
the check timeout. Otherwise the first expect rule will wait for more data until
"^error" regex matches or the check expires.

Allow the user to explicitly define an amount of data that will be
considered enough to determine the value of the check.

This allows succeeding on negative rstring rules, as previously
in valid condition no match happened, and the matching was repeated
until the end of the connection. This could timeout the check
while no error was happening.

[Cf: I slighly updated the patch. The parameter was renamed and the value is a
signed integer to support -1 as default value to ignore the parameter.]

5 years agoMINOR: checks: simplify tcp expect config parser
Gaetan Rivet [Fri, 7 Feb 2020 14:37:17 +0000 (15:37 +0100)] 
MINOR: checks: simplify tcp expect config parser

Reduce copy of parsing portions that is common to all three types of
expect actions.

This reduces the amount of code, helping maintainability and reducing
future change spread.

Functionality is identical.

5 years agoMEDIUM: checks: rewind to the first inverse expect rule of a chain on new data
Gaetan Rivet [Wed, 26 Feb 2020 15:19:40 +0000 (16:19 +0100)] 
MEDIUM: checks: rewind to the first inverse expect rule of a chain on new data

When receiving additional data while chaining multiple tcp-check expects,
previous inverse expects might have a different result with the new data. They
need to be evaluated again against the new data.

Add a pointer to the first inverse expect rule of the current expect chain
(possibly of length one) to each expect rule. When receiving new data, the
currently evaluated tcp-check rule is set back to this pointed rule.

Fonctionnaly speaking, it is a bug and it exists since the introduction of the
feature. But there is no way for now to hit it because when an expect rule does
not match, we wait for more data, independently on the inverse flag. The only
way to move to the following rule is to be sure no more data will be received.

This patch depends on the commit "MINOR: mini-clist: Add functions to iterate
backward on a list".

[Cf: I slightly updated the patch. First, it only concerns inverse expect
rule. Normal expect rules are not concerned. Then, I removed the BUG tag
because, for now, it is not possible to move to the following rule when the
current one does not match while more data can be received.]

5 years agoMINOR: checks: Simplify connection flag parsing in tcp-check connect
Gaetan Rivet [Fri, 28 Feb 2020 10:04:21 +0000 (11:04 +0100)] 
MINOR: checks: Simplify connection flag parsing in tcp-check connect

The ternary operator is useless here, this can be simpler and clearer to
read.

5 years agoMINOR: checks: Use an enum to describe the tcp-check rule type
Gaetan Rivet [Fri, 14 Feb 2020 10:25:09 +0000 (11:25 +0100)] 
MINOR: checks: Use an enum to describe the tcp-check rule type

Replace the generic integer with an enumerated list. This allows light
type check and helps debugging (seeing action = 2 in the struct is not
helpful).

5 years agoBUG/MINOR: checks: Forbid tcp-check lines in default section as documented
Christopher Faulet [Mon, 6 Apr 2020 05:49:19 +0000 (07:49 +0200)] 
BUG/MINOR: checks: Forbid tcp-check lines in default section as documented

5 years agoBUG/MINOR: checks: chained expect will not properly wait for enough data
Gaetan Rivet [Thu, 13 Feb 2020 09:30:01 +0000 (10:30 +0100)] 
BUG/MINOR: checks: chained expect will not properly wait for enough data

TCP check expect matching strings or binary patterns are able to know
prior to applying their match function whether the available data is
already sufficient to attempt the match or not.

As such, on insufficient data the expect is postponed. This behavior
avoids unnecessary matches when the data could not possibly match.

When chaining expect, upon passing the previous and going onto the next
however, this length check is not done again. Then the match is done and
will necessarily fail, triggering a new wait for more data. The end
result is the same for a slightly higher cost.

Check received data length for all expects in a chain.

This bug exists since the introduction of the feature:
Fixes: 5ecb77f4c76f ("MEDIUM: checks: add send/expect tcp based check")
Version 1.5+ impacted.

5 years agoCLEANUP: checks: Don't export anymore init_check and srv_check_healthcheck_port
Christopher Faulet [Thu, 26 Mar 2020 20:10:03 +0000 (21:10 +0100)] 
CLEANUP: checks: Don't export anymore init_check and srv_check_healthcheck_port

These functions are no longer called outside the checks.

5 years agoBUG/MEDIUM: server/checks: Init server check during config validity check
Christopher Faulet [Thu, 26 Mar 2020 18:48:20 +0000 (19:48 +0100)] 
BUG/MEDIUM: server/checks: Init server check during config validity check

The options and directives related to the configuration of checks in a backend
may be defined after the servers declarations. So, initialization of the check
of each server must not be performed during configuration parsing, because some
info may be missing. Instead, it must be done during the configuration validity
check.

Thus, callback functions are registered to be called for each server after the
config validity check, one for the server check and another one for the server
agent-check. In addition deinit callback functions are also registered to
release these checks.

This patch should be backported as far as 1.7. But per-server post_check
callback functions are only supported since the 2.1. And the initcall mechanism
does not exist before the 1.9. Finally, in 1.7, the code is totally
different. So the backport will be harder on older versions.

5 years agoBUG/MINOR: checks: Respect the no-check-ssl option
Christopher Faulet [Fri, 27 Mar 2020 17:55:49 +0000 (18:55 +0100)] 
BUG/MINOR: checks: Respect the no-check-ssl option

This options is used to force a non-SSL connection to check a SSL server or to
invert a check-ssl option inherited from the default section. The use_ssl field
in the check structure is used to know if a SSL connection must be used
(use_ssl=1) or not (use_ssl=0). The server configuration is used by default.

The problem is that we cannot distinguish the default case (no specific SSL
check option) and the case of an explicit non-SSL check. In both, use_ssl is set
to 0. So the server configuration is always used. For a SSL server, when
no-check-ssl option is set, the check is still performed using a SSL
configuration.

To fix the bug, instead of a boolean value (0=TCP, 1=SSL), we use a ternary value :

  * 0  = use server config
  * 1  = force SSL
  * -1 = force non-SSL

The same is done for the server parameter. It is not really necessary for
now. But it is a good way to know is the server no-ssl option is set.

In addition, the PR_O_TCPCHK_SSL proxy option is no longer used to set use_ssl
to 1 for a check. Instead the flag is directly tested to prepare or destroy the
server SSL context.

This patch should be backported as far as 1.8.

5 years agoMINOR: server: respect warning and alert semantic
Gaetan Rivet [Tue, 11 Feb 2020 10:42:38 +0000 (11:42 +0100)] 
MINOR: server: respect warning and alert semantic

Error codes ERR_WARN and ERR_ALERT are used to signal that the error
given is of the corresponding level. All errors are displayed as ALERT
in the display_parser_err() function.

Differentiate the display level based on the error code. If both
ERR_WARN and ERR_ALERT are used, ERR_ALERT is given priority.

5 years agoMINOR: checks: Add a way to send custom headers and payload during http chekcs
Christopher Faulet [Thu, 9 Apr 2020 06:44:06 +0000 (08:44 +0200)] 
MINOR: checks: Add a way to send custom headers and payload during http chekcs

The 'http-check send' directive have been added to add headers and optionnaly a
payload to the request sent during HTTP healthchecks. The request line may be
customized by the "option httpchk" directive but there was not official way to
add extra headers. An old trick consisted to hide these headers at the end of
the version string, on the "option httpchk" line. And it was impossible to add
an extra payload with an "http-check expect" directive because of the
"Connection: close" header appended to the request (See issue #16 for details).

So to make things official and fully support payload additions, the "http-check
send" directive have been added :

    option httpchk POST /status HTTP/1.1

    http-check send hdr Content-Type "application/json;charset=UTF-8" \
        hdr X-test-1 value1 hdr X-test-2 value2 \
        body "{id: 1, field: \"value\"}"

When a payload is defined, the Content-Length header is automatically added. So
chunk-encoded requests are not supported yet. For now, there is no special
validity checks on the extra headers.

This patch is inspired by Kiran Gavali's work. It should fix the issue #16 and
as far as possible, it may be backported, at least as far as 1.8.

5 years agoMINOR: mini-clist: Add functions to iterate backward on a list
Christopher Faulet [Mon, 23 Mar 2020 13:13:26 +0000 (14:13 +0100)] 
MINOR: mini-clist: Add functions to iterate backward on a list

list_for_each_entry_rev() and list_for_each_entry_from_rev() and corresponding
safe versions have been added to iterate on a list in the reverse order. All
these functions work the same way than the forward versions, except they use the
.p field to move for an element to another.

5 years agoBUG/MINOR: check: Update server address and port to execute an external check
Christopher Faulet [Sun, 26 Apr 2020 07:50:31 +0000 (09:50 +0200)] 
BUG/MINOR: check: Update server address and port to execute an external check

Server address and port may change at runtime. So the address and port passed as
arguments and as environment variables when an external check is executed must
be updated. The current number of connections on the server was already updated
before executing the command. So the same mechanism is used for the server
address and port. But in addition, command arguments are also updated.

This patch must be backported to all stable versions. It should fix the
issue #577.

5 years agoBUG/MINOR: http-ana: Throw a 500 error if after-response ruleset fails on errors
Christopher Faulet [Mon, 20 Apr 2020 12:58:38 +0000 (14:58 +0200)] 
BUG/MINOR: http-ana: Throw a 500 error if after-response ruleset fails on errors

It is the intended behaviour. But because of a bug, the 500 error resulting of a
rewrite failure during http-after-response ruleset evaluation is also
rewritten. So if at this step, if there is also a rewrite error, the session is
closed and no error message is returned.

Instead, we must be sure to not evaluate the http-after-response rules on an
error message if it is was thrown because of a rewrite failure on a previous
error message.

It is a 2.2-dev2+ bug. No need to backport. This patch should fix the issue

5 years agoMINOR: contrib: make the peers wireshark dissector a plugin
William Lallemand [Sat, 25 Apr 2020 20:03:29 +0000 (22:03 +0200)] 
MINOR: contrib: make the peers wireshark dissector a plugin

The wireshark dissector could only be build within wireshark, which
means maintaining a wireshark binary just for this dissector. It was not
really convenient to update wireshark because of this.

This patch converts the dissector into a .so plugin which is built with
the .h found in distributions instead of the whole wireshark sources.

5 years agoMEDIUM: memory: make pool_gc() run under thread isolation
Willy Tarreau [Fri, 24 Apr 2020 04:15:24 +0000 (06:15 +0200)] 
MEDIUM: memory: make pool_gc() run under thread isolation

pool_gc() causes quite some stress on the memory allocator because
it calls a lot of free() calls while other threads are using malloc().
In addition, pool_gc() needs to take care of possible locking because
it may be called from pool allocators. One way to avoid all this is to
use thread_isolate() to make sure the gc runs alone. By putting less
pressure on the pools and getting rid of the locks, it may even take
less time to complete.

5 years agoDOC: option logasap does not depend on mode
Jerome Magnin [Thu, 23 Apr 2020 17:01:17 +0000 (19:01 +0200)] 
DOC: option logasap does not depend on mode

The documentation for option logasap misleads into thinking it is
only valid for mode http. It is actually valid for mode tcp too,
so this patch tries to disambiguate the current wording.

5 years agoBUG/MINOR: http: make url_decode() optionally convert '+' to SP
Willy Tarreau [Thu, 23 Apr 2020 15:54:47 +0000 (17:54 +0200)] 
BUG/MINOR: http: make url_decode() optionally convert '+' to SP

The url_decode() function used by the url_dec converter and a few other
call points is ambiguous on its processing of the '+' character which
itself isn't stable in the spec. This one belongs to the reserved
characters for the query string but not for the path nor the scheme,
in which it must be left as-is. It's only in argument strings that
follow the application/x-www-form-urlencoded encoding that it must be
turned into a space, that is, in query strings and POST arguments.

The problem is that the function is used to process full URLs and
paths in various configs, and to process query strings from the stats
page for example.

This patch updates the function to differentiate the situation where
it's parsing a path and a query string. A new argument indicates if a
query string should be assumed, otherwise it's only assumed after seeing
a question mark.

The various locations in the code making use of this function were
updated to take care of this (most call places were using it to decode
POST arguments).

The url_dec converter is usually called on path or url samples, so it
needs to remain compatible with this and will default to parsing a path
and turning the '+' to a space only after a question mark. However in
situations where it would explicitly be extracted from a POST or a
query string, it now becomes possible to enforce the decoding by passing
a non-null value in argument.

It seems to be what was reported in issue #585. This fix may be
backported to older stable releases.

5 years agoBUG/MINOR: mux-fcgi/trace: fix wrong set of trace flags in fcgi_strm_add_eom()
Willy Tarreau [Thu, 23 Apr 2020 15:24:59 +0000 (17:24 +0200)] 
BUG/MINOR: mux-fcgi/trace: fix wrong set of trace flags in fcgi_strm_add_eom()

A typo resulted in '||' being used to concatenate trace flags, which will
only set flag of value '1' there. Noticed by clang 10 and reported in
issue #588.

No backport is needed, this trace was added in 2.2-dev.

5 years agoBUG/MINOR: tools: fix the i386 version of the div64_32 function
Willy Tarreau [Thu, 23 Apr 2020 15:08:02 +0000 (17:08 +0200)] 
BUG/MINOR: tools: fix the i386 version of the div64_32 function

As reported in issue #596, the edx register isn't marked as clobbered
in div64_32(), which could technically allow gcc to try to reuse it
if it needed a copy of the 32 highest bits of the o1 register after
the operation.

Two attempts were tried, one using a dummy 32-bit local variable to
store the intermediary edx and another one switching to "=A" and making
result a long long. It turns out the former makes the resulting object
code significantly dirtier while the latter makes it better and was
kept. This is due to gcc's difficulties at working with register pairs
mixing 32- and 64- bit values on i386. It was verified that no code
change happened at all on x86_64, armv7, aarch64 nor mips32.

In practice it's only used by the frequency counters so this bug
cannot even be triggered but better fix it.

This may be backported to stable branches though it will not fix any
issue.

5 years agoDOC: internals: update the SSL architecture schema
William Lallemand [Thu, 23 Apr 2020 14:28:25 +0000 (16:28 +0200)] 
DOC: internals: update the SSL architecture schema

This commit updates the SSL files architecture schema and adds the
crtlist structures in it.

5 years agoBUG/MEDIUM: http-ana: Handle NTLM messages correctly.
Olivier Houchard [Wed, 22 Apr 2020 19:51:14 +0000 (21:51 +0200)] 
BUG/MEDIUM: http-ana: Handle NTLM messages correctly.

When checking www-authenticate headers, we don't want to just accept
"NTLM" as value, because the server may send "HTLM <base64 value>". Instead,
just check that it starts with NTLM.

This should be backported to 2.1, 2.0, 1.9 and 1.8.

5 years agoMINOR: config: add a global directive to set default SSL curves
Jerome Magnin [Fri, 3 Apr 2020 13:28:22 +0000 (15:28 +0200)] 
MINOR: config: add a global directive to set default SSL curves

This commit adds a new keyword to the global section to set default
curves for ssl binds:
  - ssl-default-bind-curves

5 years agoBUG/MINOR: ssl: default settings for ssl server options are not used
Jerome Magnin [Wed, 22 Apr 2020 09:40:18 +0000 (11:40 +0200)] 
BUG/MINOR: ssl: default settings for ssl server options are not used

Documentation states that default settings for ssl server options can be set
using either ssl-default-server-options or default-server directives. In practice,
not all ssl server options can have default values, such as ssl-min-ver, ssl-max-ver,
etc..

This patch adds the missing ssl options in srv_ssl_settings_cpy() and srv_parse_ssl(),
making it possible to write configurations like the following examples, and have them
behave as expected.

   global
     ssl-default-server-options ssl-max-ver TLSv1.2

   defaults
     mode http

   listen l1
     bind 1.2.3.4:80
     default-server ssl verify none
     server s1 1.2.3.5:443

   listen l2
     bind 2.2.3.4:80
     default-server ssl verify none ssl-max-ver TLSv1.3 ssl-min-ver TLSv1.2
     server s1 1.2.3.6:443

This should be backported as far as 1.8.
This fixes issue #595.

5 years agoMINOR: ssl: add ssl-skip-self-issued-ca global option
Emmanuel Hocdet [Wed, 22 Apr 2020 09:06:19 +0000 (11:06 +0200)] 
MINOR: ssl: add ssl-skip-self-issued-ca global option

This option activate the feature introduce in commit 16739778:
"MINOR: ssl: skip self issued CA in cert chain for ssl_ctx".
The patch disable the feature per default.

5 years agoMINOR: ssl/cli: restrain certificate path when inserting into a directory
William Lallemand [Tue, 21 Apr 2020 16:29:12 +0000 (18:29 +0200)] 
MINOR: ssl/cli: restrain certificate path when inserting into a directory

When trying to insert a new certificate into a directory with "add ssl
crt-list", no check were done on the path of the new certificate.

To be more consistent with the HAProxy reload, when adding a file to
a crt-list, if this crt-list is a directory, the certificate will need
to have the directory in its path.

5 years agoMINOR: ssl/cli: disallow SSL options for directory in 'add ssl crt-list'
William Lallemand [Tue, 21 Apr 2020 14:54:19 +0000 (16:54 +0200)] 
MINOR: ssl/cli: disallow SSL options for directory in 'add ssl crt-list'

Allowing the use of SSL options and filters when adding a file in a
directory is not really consistent with the reload of HAProxy. Disable
the ability to use these options if one try to use them with a directory.

5 years agoDOC: Improve documentation on http-request set-src
Olivier Doucet [Tue, 21 Apr 2020 07:32:56 +0000 (09:32 +0200)] 
DOC: Improve documentation on http-request set-src

This patch adds more explanation on how to use "http-request set-src"
and a link to "option forwardfor".

This patch can be applied to all previous version starting at 1.6

Reviewed-by: Tim Duesterhus <tim@bastelstu.be>
5 years agoCLEANUP: log: fix comment of parse_logformat_string()
Ilya Shipitsin [Sat, 4 Apr 2020 07:59:53 +0000 (12:59 +0500)] 
CLEANUP: log: fix comment of parse_logformat_string()

"fmt" is passed to parse_logformat_string, adjust comment
accordingly

5 years agoCI: run weekly OpenSSL "no-deprecated" builds
Ilya Shipitsin [Thu, 16 Apr 2020 18:10:45 +0000 (23:10 +0500)] 
CI: run weekly OpenSSL "no-deprecated" builds

OpenWRT uses such OpenSSL builds (those builds are smaller)

some details might be found at ML: https://www.mail-archive.com/haproxy@formilux.org/msg35759.html
                               GH: https://github.com/haproxy/haproxy/issues/367

5 years agoMINOR: version: Show uname output in display_version()
Tim Duesterhus [Sat, 18 Apr 2020 14:02:47 +0000 (16:02 +0200)] 
MINOR: version: Show uname output in display_version()

This patch adds the sysname, release, version and machine fields from
the uname results to the version output. It intentionally leaves out the
machine name, because it is usually not useful and users might not want to
expose their machine names for privacy reasons.

May be backported if it is considered useful for debugging.

5 years ago[RELEASE] Released version 2.2-dev6 v2.2-dev6
Willy Tarreau [Fri, 17 Apr 2020 12:19:38 +0000 (14:19 +0200)] 
[RELEASE] Released version 2.2-dev6

Released version 2.2-dev6 with the following main changes :
    - BUG/MINOR: ssl: memory leak when find_chain is NULL
    - CLEANUP: ssl: rename ssl_get_issuer_chain to ssl_get0_issuer_chain
    - MINOR: ssl: rework add cert chain to CTX to be libssl independent
    - BUG/MINOR: peers: init bind_proc to 1 if it wasn't initialized
    - BUG/MINOR: peers: avoid an infinite loop with peers_fe is NULL
    - BUG/MINOR: peers: Use after free of "peers" section.
    - CI: github actions: add weekly h2spec test
    - BUG/MEDIUM: mux_h1: Process a new request if we already received it.
    - MINOR: build: Fix build in mux_h1
    - CLEANUP: remove obsolete comments
    - BUG/MEDIUM: dns: improper parsing of aditional records
    - MINOR: ssl: skip self issued CA in cert chain for ssl_ctx
    - MINOR: listener: add so_name sample fetch
    - MEDIUM: stream: support use-server rules with dynamic names
    - MINOR: servers: Add a counter for the number of currently used connections.
    - MEDIUM: connections: Revamp the way idle connections are killed
    - MINOR: cli: add a general purpose pointer in the CLI struct
    - MINOR: ssl: add a list of bind_conf in struct crtlist
    - REORG: ssl: move SETCERT enum to ssl_sock.h
    - BUG/MINOR: ssl: ckch_inst wrongly inserted in crtlist_entry
    - REORG: ssl: move some functions above crtlist_load_cert_dir()
    - MINOR: ssl: use crtlist_free() upon error in directory loading
    - MINOR: ssl: add a list of crtlist_entry in ckch_store
    - MINOR: ssl: store a ptr to crtlist in crtlist_entry
    - MINOR: ssl/cli: update pointer to store in 'commit ssl cert'
    - MEDIUM: ssl/cli: 'add ssl crt-list' command
    - REGTEST: ssl/cli: test the 'add ssl crt-list' command
    - BUG/MINOR: ssl: entry->ckch_inst not initialized
    - REGTEST: ssl/cli: change test type to devel
    - REGTEST: make the PROXY TLV validation depend on version 2.2
    - CLEANUP: assorted typo fixes in the code and comments
    - BUG/MINOR: stats: Fix color of draining servers on stats page
    - DOC: internals: Fix spelling errors in filters.txt
    - MINOR: connections: Don't mark conn flags 0x00000001 and 0x00000002 as unused.
    - REGTEST: make the unique-id test depend on version 2.0
    - BUG/MEDIUM: dns: Consider the fact that dns answers are case-insensitive
    - MINOR: ssl: split the line parsing of the crt-list
    - MINOR: ssl/cli: support filters and options in add ssl crt-list
    - MINOR: ssl: add a comment above the ssl_bind_conf keywords
    - REGTEST: ssl/cli: tests options and filters w/ add ssl crt-list
    - REGTEST: ssl: pollute the crt-list file
    - BUG/CRITICAL: hpack: never index a header into the headroom after wrapping
    - BUG/MINOR: protocol_buffer: Wrong maximum shifting.
    - CLEANUP: src/fd.c: mask setsockopt with DISGUISE
    - BUG/MINOR: ssl/cli: initialize fcount int crtlist_entry
    - REGTEST: ssl/cli: add other cases of 'add ssl crt-list'
    - CLEANUP: assorted typo fixes in the code and comments
    - DOC: management: add the new crt-list CLI commands
    - BUG/MINOR: ssl/cli: fix spaces in 'show ssl crt-list'
    - MINOR: ssl/cli: 'del ssl crt-list' delete an entry
    - MINOR: ssl/cli: replace dump/show ssl crt-list by '-n' option
    - CI: use better SSL library definition
    - CI: travis-ci: enable DEBUG_STRICT=1 for CI builds
    - CI: travis-ci: upgrade openssl to 1.1.1f
    - MINOR: ssl: improve the errors when a crt can't be open
    - CI: cirrus-ci: rename openssl package after it is renamed in FreeBSD
    - CI: adopt openssl download script to download all versions
    - BUG/MINOR: ssl/cli: lock the ckch structures during crt-list delete
    - MINOR: ssl/cli: improve error for bundle in add/del ssl crt-list
    - MINOR: ssl/cli: 'del ssl cert' deletes a certificate
    - BUG/MINOR: ssl: trailing slashes in directory names wrongly cached
    - BUG/MINOR: ssl/cli: memory leak in 'set ssl cert'
    - CLEANUP: ssl: use the refcount for the SSL_CTX'
    - CLEANUP: ssl/cli: use the list of filters in the crtlist_entry
    - BUG/MINOR: ssl: memleak of the struct cert_key_and_chain
    - CLEANUP: ssl: remove a commentary in struct ckch_inst
    - MINOR: ssl: initialize all list in ckch_inst_new()
    - MINOR: ssl: free instances and SNIs with ckch_inst_free()
    - MINOR: ssl: replace ckchs_free() by ckch_store_free()
    - BUG/MEDIUM: ssl/cli: trying to access to free'd memory
    - MINOR: ssl: ckch_store_new() alloc and init a ckch_store
    - MINOR: ssl: crtlist_new() alloc and initialize a struct crtlist
    - REORG: ssl: move some free/new functions
    - MINOR: ssl: crtlist_entry_{new, free}
    - BUG/MINOR: ssl: ssl_conf always set to NULL on crt-list parsing
    - MINOR: ssl: don't alloc ssl_conf if no option found
    - BUG/MINOR: connection: always send address-less LOCAL PROXY connections
    - BUG/MINOR: peers: Incomplete peers sections should be validated.
    - MINOR: init: report in "haproxy -c" whether there were warnings or not
    - MINOR: init: add -dW and "zero-warning" to reject configs with warnings
    - MINOR: init: report the compiler version in haproxy -vv
    - CLEANUP: assorted typo fixes in the code and comments
    - MINOR: init: report the haproxy version and executable path once on errors
    - DOC: Make how "option redispatch" works more explicit
    - BUILD: Makefile: add linux-musl to TARGET
    - CLEANUP: assorted typo fixes in the code and comments
    - CLEANUP: http: Fixed small typo in parse_http_return
    - DOC: hashing: update link to hashing functions

5 years agoDOC: hashing: update link to hashing functions
Adam Mills [Tue, 14 Apr 2020 23:45:15 +0000 (16:45 -0700)] 
DOC: hashing: update link to hashing functions

Bret Mulvey, the author of the article cited in this pulication
has migrated his work to papa.bretmulvey.com. I was able to
view an archival version of Bret M.'s original post
(http://home.comcast.net/~bretm/hash/3.html) and have validated
that this is the same paper that is originally cited.

5 years agoCLEANUP: http: Fixed small typo in parse_http_return
Dominik Froehlich [Fri, 17 Apr 2020 09:04:35 +0000 (11:04 +0200)] 
CLEANUP: http: Fixed small typo in parse_http_return

It's only two typos in a warning (ober vs over, and rewritting vs rewriting).

5 years agoCLEANUP: assorted typo fixes in the code and comments
Ilya Shipitsin [Thu, 16 Apr 2020 18:51:34 +0000 (23:51 +0500)] 
CLEANUP: assorted typo fixes in the code and comments

This is 8th iteration of typo fixes

5 years agoBUILD: Makefile: add linux-musl to TARGET
Willy Tarreau [Thu, 16 Apr 2020 13:14:17 +0000 (15:14 +0200)] 
BUILD: Makefile: add linux-musl to TARGET

Other users are using musl, namely on Docker. It builds fine with
linux-glibc-legacy but not linux-glibc, which needs to first disable
USE_BACKTRACE. Better add a valid entry for it instead of hacking
around another libc.

5 years agoDOC: Make how "option redispatch" works more explicit
Olivier Carrère [Wed, 15 Apr 2020 09:30:18 +0000 (11:30 +0200)] 
DOC: Make how "option redispatch" works more explicit

People are often misled and think that this option can redirect
connections to backup servers.

This patch makes the documentation more specific about how the option
handles backup servers.

5 years agoMINOR: init: report the haproxy version and executable path once on errors
Willy Tarreau [Thu, 16 Apr 2020 08:52:41 +0000 (10:52 +0200)] 
MINOR: init: report the haproxy version and executable path once on errors

If haproxy fails to start and emits an alert, then it can be useful
to have it also emit the version and the path used to load it. Some
users may be mistakenly launching the wrong binary due to a misconfigured
PATH variable and this will save them some troubleshooting time when it
reports that some keywords are not understood.

What we do here is that we *try* to extract the binary name from the
AUX vector on glibc, and we report this as a NOTICE tag before the
very first alert is emitted.

5 years agoCLEANUP: assorted typo fixes in the code and comments
Ilya Shipitsin [Tue, 7 Apr 2020 20:07:56 +0000 (01:07 +0500)] 
CLEANUP: assorted typo fixes in the code and comments

This is 7th iteration of typo fixes

5 years agoMINOR: init: report the compiler version in haproxy -vv
Willy Tarreau [Wed, 15 Apr 2020 15:00:03 +0000 (17:00 +0200)] 
MINOR: init: report the compiler version in haproxy -vv

Some portability issues were met a few times in the past depending on
compiler versions, but this one was not reported in haproxy -vv output
while it's trivial to add it. This patch tries to be the most accurate
by explicitly reporting the clang version if detected, otherwise the
gcc version.

5 years agoMINOR: init: add -dW and "zero-warning" to reject configs with warnings
Willy Tarreau [Wed, 15 Apr 2020 14:42:39 +0000 (16:42 +0200)] 
MINOR: init: add -dW and "zero-warning" to reject configs with warnings

Since some systems switched to service managers which hide all warnings
by default, some users are not aware of some possibly important warnings
and get caught too late with errors that could have been detected earlier.

This patch adds a new global keyword, "zero-warning" and an equivalent
command-line option "-dW" to refuse to start in case any warning is
detected. It is recommended to use these with configurations that are
managed by humans in order to catch mistakes very early.

5 years agoMINOR: init: report in "haproxy -c" whether there were warnings or not
Willy Tarreau [Wed, 15 Apr 2020 14:06:11 +0000 (16:06 +0200)] 
MINOR: init: report in "haproxy -c" whether there were warnings or not

This helps quickly checking if the config produces any warning. For
this we reuse the "warned" bit field to add a new WARN_ANY bit that is
set by ha_warning(). The rest of the bit field was also cleaned from
unused bits.

5 years agoBUG/MINOR: peers: Incomplete peers sections should be validated.
Frédéric Lécaille [Fri, 3 Apr 2020 07:43:47 +0000 (09:43 +0200)] 
BUG/MINOR: peers: Incomplete peers sections should be validated.

Before supporting "server" line in "peers" section, such sections without
any local peer were removed from the configuration to get it validated.

This patch fixes the issue where a "server" line without address and port which
is a remote peer without address and port makes the configuration parsing fail.
When encoutering such cases we now ignore such lines remove them from the
configuration.

Thank you to Jérôme Magnin for having reported this bug.

Must be backported to 2.1 and 2.0.

5 years agoBUG/MINOR: connection: always send address-less LOCAL PROXY connections
Willy Tarreau [Tue, 14 Apr 2020 10:54:10 +0000 (12:54 +0200)] 
BUG/MINOR: connection: always send address-less LOCAL PROXY connections

Commit 7f26391bc5 ("BUG/MINOR: connection: make sure to correctly tag
local PROXY connections") revealed that some implementations do not
properly ignore addresses in LOCAL connections (at least Dovecot was
spotted). More context information in the thread below:

   https://www.mail-archive.com/haproxy@formilux.org/msg36890.html

The patch above was using LOCAL on top of local addresses in order to
minimize the risk of breakage but revealed worse than a clean fix. So
let's partially revert it and send pure LOCAL connections instead now.

After a bit of observation, this patch should be progressively backported
to stable branches. However if it reveals new breakage, the backport of
the patch above will have to be reverted from stable branches while other
products work on fixing their code based on the master branch.

5 years agoMINOR: ssl: don't alloc ssl_conf if no option found
William Lallemand [Fri, 10 Apr 2020 15:20:45 +0000 (17:20 +0200)] 
MINOR: ssl: don't alloc ssl_conf if no option found

When no SSL options were found between brackets, the structure ssl_conf
was still allocated for nothing.

5 years agoBUG/MINOR: ssl: ssl_conf always set to NULL on crt-list parsing
William Lallemand [Fri, 10 Apr 2020 15:13:09 +0000 (17:13 +0200)] 
BUG/MINOR: ssl: ssl_conf always set to NULL on crt-list parsing

When reading a crt-list file, the SSL options betweeen square brackets
are parsed, however the calling function sets the ssl_conf ptr to NULL
leading to all options being ignored, and a memory leak.

This is a remaining of the previous code which was forgotten.

This bug was introduced by 97b0810 ("MINOR: ssl: split the line parsing
of the crt-list").

5 years agoMINOR: ssl: crtlist_entry_{new, free}
William Lallemand [Fri, 10 Apr 2020 09:09:25 +0000 (11:09 +0200)] 
MINOR: ssl: crtlist_entry_{new, free}

New functions that create and delete a crtlist_entry in order to remove
duplicated code.

5 years agoREORG: ssl: move some free/new functions
William Lallemand [Fri, 10 Apr 2020 08:26:27 +0000 (10:26 +0200)] 
REORG: ssl: move some free/new functions

Move crtlist_free_filters(), crtlist_dup_filters(),
crtlist_free(), crtlist_new(), ssl_sock_free_ssl_conf() upper in the
file.

5 years agoMINOR: ssl: crtlist_new() alloc and initialize a struct crtlist
William Lallemand [Thu, 9 Apr 2020 11:44:21 +0000 (13:44 +0200)] 
MINOR: ssl: crtlist_new() alloc and initialize a struct crtlist

Allocate and initialize a struct crtlist with crtlist_new() to remove
duplicated code.

5 years agoMINOR: ssl: ckch_store_new() alloc and init a ckch_store
William Lallemand [Thu, 9 Apr 2020 08:32:53 +0000 (10:32 +0200)] 
MINOR: ssl: ckch_store_new() alloc and init a ckch_store

Create a ckch_store_new() function which alloc and initialize a
ckch_store, allowing us to remove duplicated code and avoiding wrong
initialization in the future.

5 years agoBUG/MEDIUM: ssl/cli: trying to access to free'd memory
William Lallemand [Thu, 9 Apr 2020 15:12:16 +0000 (17:12 +0200)] 
BUG/MEDIUM: ssl/cli: trying to access to free'd memory

Bug introduced by d9d5d1b ("MINOR: ssl: free instances and SNIs with
ckch_inst_free()").

Upon an 'commit ssl cert' the HA_RWLOCK_WRUNLOCK of the SNI lock is done
with using the bind_conf pointer of the ckch_inst which was freed.

Fix the problem by using an intermediate variable to store the
bind_conf pointer.

5 years agoMINOR: ssl: replace ckchs_free() by ckch_store_free()
William Lallemand [Wed, 8 Apr 2020 15:55:45 +0000 (17:55 +0200)] 
MINOR: ssl: replace ckchs_free() by ckch_store_free()

Replace ckchs_free() by ckch_store_free() which frees the ckch_store but
now also all its ckch_inst with ckch_inst_free().

Also remove the "ckchs" naming since its confusing.

5 years agoMINOR: ssl: free instances and SNIs with ckch_inst_free()
William Lallemand [Thu, 9 Apr 2020 14:31:05 +0000 (16:31 +0200)] 
MINOR: ssl: free instances and SNIs with ckch_inst_free()

Remove duplicated code by creating a new function ckch_inst_free() which
deals with the SNIs linked in a ckch_inst and free the ckch_inst.

5 years agoMINOR: ssl: initialize all list in ckch_inst_new()
William Lallemand [Thu, 9 Apr 2020 14:25:10 +0000 (16:25 +0200)] 
MINOR: ssl: initialize all list in ckch_inst_new()

The ckch_inst_new() function is not up to date with the latest
list added into the structure. Update the list of structure to
initialize.

5 years agoCLEANUP: ssl: remove a commentary in struct ckch_inst
William Lallemand [Wed, 8 Apr 2020 14:56:09 +0000 (16:56 +0200)] 
CLEANUP: ssl: remove a commentary in struct ckch_inst

The struct ckch_inst now handles the ssl_bind_conf so this commentary is
obsolete

5 years agoBUG/MINOR: ssl: memleak of the struct cert_key_and_chain
William Lallemand [Wed, 8 Apr 2020 15:38:27 +0000 (17:38 +0200)] 
BUG/MINOR: ssl: memleak of the struct cert_key_and_chain

Free the struct cert_key_and_chain when calling ckchs_free(),
a memory leak can occur when using 'commit ssl cert'.

Must be backported to 2.1.

5 years agoCLEANUP: ssl/cli: use the list of filters in the crtlist_entry
William Lallemand [Wed, 8 Apr 2020 14:29:15 +0000 (16:29 +0200)] 
CLEANUP: ssl/cli: use the list of filters in the crtlist_entry

In 'commit ssl cert', instead of trying to regenerate a list of filters
from the SNIs, use the list provided by the crtlist_entry used to
generate the ckch_inst.

This list of filters doesn't need to be free'd anymore since they are
always reused from the crtlist_entry.

5 years agoCLEANUP: ssl: use the refcount for the SSL_CTX'
William Lallemand [Wed, 8 Apr 2020 14:11:26 +0000 (16:11 +0200)] 
CLEANUP: ssl: use the refcount for the SSL_CTX'

Use the refcount of the SSL_CTX' to free them instead of freeing them on
certains conditions. That way we can free the SSL_CTX everywhere its
pointer is used.

5 years agoBUG/MINOR: ssl/cli: memory leak in 'set ssl cert'
William Lallemand [Wed, 8 Apr 2020 13:16:51 +0000 (15:16 +0200)] 
BUG/MINOR: ssl/cli: memory leak in 'set ssl cert'

When deleting the previous SNI entries with 'set ssl cert', the old
SSL_CTX' were not free'd, which probably prevent the completion of the
free of the X509 in the old ckch_store, because of the refcounts in the
SSL library.

This bug was introduced by 150bfa8 ("MEDIUM: cli/ssl: handle the
creation of SSL_CTX in an IO handler").

Must be backported to 2.1.

5 years agoBUG/MINOR: ssl: trailing slashes in directory names wrongly cached
William Lallemand [Wed, 8 Apr 2020 11:15:18 +0000 (13:15 +0200)] 
BUG/MINOR: ssl: trailing slashes in directory names wrongly cached

The crtlist_load_cert_dir() caches the directory name without trailing
slashes when ssl_sock_load_cert_list_file() tries to lookup without
cleaning the trailing slashes.

This bug leads to creating the crtlist twice and prevents to remove
correctly a crtlist_entry since it exists in the serveral crtlists
created by accident.

Move the trailing slashes cleanup in ssl_sock_load_cert_list_file() to
fix the problem.

This bug was introduced by 6be66ec ("MINOR: ssl: directories are loaded
like crt-list")

5 years agoMINOR: ssl/cli: 'del ssl cert' deletes a certificate
William Lallemand [Wed, 8 Apr 2020 10:05:39 +0000 (12:05 +0200)] 
MINOR: ssl/cli: 'del ssl cert' deletes a certificate

Delete a certificate store from HAProxy and free its memory. The
certificate must be unused and removed from any crt-list or directory.
The deletion doesn't work with a certificate referenced directly with
the "crt" directive in the configuration.

5 years agoMINOR: ssl/cli: improve error for bundle in add/del ssl crt-list
William Lallemand [Wed, 8 Apr 2020 08:57:24 +0000 (10:57 +0200)] 
MINOR: ssl/cli: improve error for bundle in add/del ssl crt-list

Bundles are deprecated and can't be used with the crt-list command of
the CLI, improve the error output when trying to use them so the users
can disable them.

5 years agoBUG/MINOR: ssl/cli: lock the ckch structures during crt-list delete
William Lallemand [Wed, 8 Apr 2020 08:30:44 +0000 (10:30 +0200)] 
BUG/MINOR: ssl/cli: lock the ckch structures during crt-list delete

The cli_parse_del_crtlist() does unlock the ckch big lock, but it does
not lock it at the beginning of the function which is dangerous.
As a side effect it let the structures locked once it called the unlock.

This bug was introduced by 0a9b941 ("MINOR: ssl/cli: 'del ssl crt-list'
delete an entry")

5 years agoCI: adopt openssl download script to download all versions
Ilya Shipitsin [Tue, 7 Apr 2020 18:35:49 +0000 (23:35 +0500)] 
CI: adopt openssl download script to download all versions

with recent change, OpenSSL download URL was changed in
incompatiable way. i.e. only the most recent openssl version
might be downloaded using previous script.

older versions are available under different URLs. as we need
several openssl versions, let us adopt script accordingly.

bug was caught after travis-ci cache was purged for some reason.

5 years agoCI: cirrus-ci: rename openssl package after it is renamed in FreeBSD
Ilya Shipitsin [Tue, 7 Apr 2020 19:29:26 +0000 (00:29 +0500)] 
CI: cirrus-ci: rename openssl package after it is renamed in FreeBSD

for the reason yet to be determined FreeBSD has renamed openssl111
to openssl. let us rename as well

5 years agoMINOR: ssl: improve the errors when a crt can't be open
William Lallemand [Tue, 7 Apr 2020 12:16:32 +0000 (14:16 +0200)] 
MINOR: ssl: improve the errors when a crt can't be open

Issue #574 reported an unclear error when trying to open a file with not
enough permission.

  [ALERT] 096/032117 (835) : parsing [/etc/haproxy/haproxy.cfg:54] : 'bind :443' : error encountered while processing 'crt'.
  [ALERT] 096/032117 (835) : Error(s) found in configuration file : /etc/haproxy/haproxy.cfg
  [ALERT] 096/032117 (835) : Fatal errors found in configuration.

Improve the error to give us more information:

  [ALERT] 097/142030 (240089) : parsing [test.cfg:22] : 'bind :443' : cannot open the file 'kikyo.pem.rsa'.
  [ALERT] 097/142030 (240089) : Error(s) found in configuration file : test.cfg
  [ALERT] 097/142030 (240089) : Fatal errors found in configuration.

This patch could be backported in 2.1.

5 years agoCI: travis-ci: upgrade openssl to 1.1.1f
Ilya Shipitsin [Thu, 2 Apr 2020 19:20:46 +0000 (00:20 +0500)] 
CI: travis-ci: upgrade openssl to 1.1.1f

openssl has changed download path after 1.1.1f release

5 years agoCI: travis-ci: enable DEBUG_STRICT=1 for CI builds
Ilya Shipitsin [Thu, 2 Apr 2020 19:07:17 +0000 (00:07 +0500)] 
CI: travis-ci: enable DEBUG_STRICT=1 for CI builds

DEBUG_STRICT enables the BUG_ON() macro which validates some developers'
assertions in the code that are not enabled for production build but
may sometimes help catch certain rare bugs.

DEBUG_STRICT is set to all builds except one

5 years agoCI: use better SSL library definition
Ilya Shipitsin [Thu, 2 Apr 2020 18:34:47 +0000 (23:34 +0500)] 
CI: use better SSL library definition

SSL_LIB is already added to LDFLAGS in Makefile, no need to define it
rpath better be defined using ADDLIB variable

5 years agoMINOR: ssl/cli: replace dump/show ssl crt-list by '-n' option
William Lallemand [Mon, 6 Apr 2020 17:07:03 +0000 (19:07 +0200)] 
MINOR: ssl/cli: replace dump/show ssl crt-list by '-n' option

The dump and show ssl crt-list commands does the same thing, they dump
the content of a crt-list, but the 'show' displays an ID in the first
column. Delete the 'dump' command so it is replaced by the 'show' one.
The old 'show' command is replaced by an '-n' option to dump the ID.
And the ID which was a pointer is replaced by a line number and placed
after colons in the filename.

Example:
  $ echo "show ssl crt-list -n kikyo.crt-list" | socat /tmp/sock1 -
  # kikyo.crt-list
  kikyo.pem.rsa:1 secure.domain.tld
  kikyo.pem.ecdsa:2 secure.domain.tld

5 years agoMINOR: ssl/cli: 'del ssl crt-list' delete an entry
William Lallemand [Mon, 6 Apr 2020 15:43:05 +0000 (17:43 +0200)] 
MINOR: ssl/cli: 'del ssl crt-list' delete an entry

Delete an entry in a crt-list, this is done by iterating over the
ckch_inst in the crtlist_entry. For each ckch_inst the bind_conf lock is
held during the deletion of the sni_ctx in the SNI trees. Everything
is free'd.

If there is several entries with the same certificate, a line number
must be provided to chose with entry delete.