haproxy supports generating SSL certificates based on SNI using a provided
CA signing certificate. Because CA certificates may be signed by multiple
CAs, in some scenarios, it is neccesary for the server to attach the trust chain
in addition to the generated certificate.
The following patch adds the ability to serve the entire trust chain with
the generated certificate. The chain is loaded from the provided
`ca-sign-file` PEM file.
As reported in issue #816, when building task.o at -O1 with gcc 4.7 or
4.8, we get the following warning:
CC src/task.o
In file included from include/haproxy/proxy.h:31:0,
from include/haproxy/cfgparse.h:27,
from src/task.c:19:
src/task.c: In function 'next_timer_expiry':
include/haproxy/ticks.h:121:10: warning: 'key' may be used uninitialized in this function [-Wmaybe-uninitialized]
src/task.c:349:2: note: 'key' was declared here
It is wrong since the condition to use 'key' is exactly the same as
the one used to set it. This warning disappears at -O2 and disappeared
from gcc 5 and above. Let's just initialize 'key' there, it only adds
16 bytes of code and remains cheap enough for this function.
This should be backported to 2.2.
As reported in https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=24745,
haproxy fails to build with TARGET=generic and without extra options due
to auxv.h not being included, since the __GLIBC__ macro is not yet defined.
Let's include it after other libc headers so that the __GLIBC__ definition
is known. Thanks to David and Tim for the diag.
This should be backported to 2.2.
Since commit f92afb732 ("MEDIUM: cfgparse: Emit hard error on truncated lines")
we now produce parsing errors on truncated lines, in an effort to clean
up dangerous or broken config files. And it turns out that one of our own
regression tests was suffering from this, as diagnosed by William and Tim.
The cause is the leading spaces in front of "} -start" that vtest makes
part of the output file, so the file finishes with a partial line made
of spaces.
Using a duplicate cache name most likely is the result of a misgenerated
configuration. There is no good reason to allow this, as the duplicate
caches can't be referred to.
This commit resolves GitHub issue #820.
It can be argued whether this is a fix for a bug or not. I'm erring on the
side of caution and marking this as a "new feature". It can be considered for
backporting to 2.2, but for other branches the risk of accidentally breaking
some working (but non-ideal) configuration might be too large.
When the cache name is left out in 'filter cache' the error message refers
to a missing '<id>'. The name of the cache is called 'name' within the docs.
Adjust the error message for consistency.
The error message was introduced in 99a17a2d91.
This commit first appeared in 1.9, thus the patch must be backported to 1.9+.
as per the suggestions from Cyril and Willy on the mailing list:
Message-ID: <a0010c72-70b8-0647-c269-55c6614627c3@free.fr>
and with direct contributions from Tim, this changes large parts
of the bug issue template.
The Feature template is also updated as well as a new template for
Code Reports is introduced.
Co-authored-by: Tim Duesterhus <tim@bastelstu.be>
The negative filters which are supposed to exclude a SNI from a
wildcard, never worked. Indeed the negative filters were skipped in the
code.
To fix the issue, this patch looks for negative filters that are on the
same line as a the wildcard that just matched.
This patch should fix issue #818. It must be backported in 2.2. The
problem also exists in versions > 1.8 but the infrastructure required to
fix this was only introduced in 2.1. In older versions we should
probably change the documentation to state that negative filters are
useless.
Released version 2.3-dev3 with the following main changes :
- SCRIPTS: git-show-backports: make -m most only show the left branch
- SCRIPTS: git-show-backports: emit the shell command to backport a commit
- BUILD: Makefile: require SSL_LIB, SSL_INC to be explicitly set
- CI: travis-ci: specify SLZ_LIB, SLZ_INC for travis builds
- BUG/MEDIUM: mux-h1: Refresh H1 connection timeout after a synchronous send
- CLEANUP: dns: typo in reported error message
- BUG/MAJOR: dns: disabled servers through SRV records never recover
- BUG/MINOR: spoa-server: fix size_t format printing
- DOC: spoa-server: fix false friends `actually`
- BUG/MINOR: ssl: fix memory leak at OCSP loading
- BUG/MEDIUM: ssl: memory leak of ocsp data at SSL_CTX_free()
- BUG/MEDIUM: map/lua: Return an error if a map is loaded during runtime
- MINOR: arg: Add an argument type to keep a reference on opaque data
- BUG/MINOR: converters: Store the sink in an arg pointer for debug() converter
- BUG/MINOR: lua: Duplicate map name to load it when a new Map object is created
- BUG/MINOR: arg: Fix leaks during arguments validation for fetches/converters
- BUG/MINOR: lua: Check argument type to convert it to IPv4/IPv6 arg validation
- BUG/MINOR: lua: Check argument type to convert it to IP mask in arg validation
- MINOR: hlua: Don't needlessly copy lua strings in trash during args validation
- BUG/MINOR: lua: Duplicate lua strings in sample fetches/converters arg array
- MEDIUM: lua: Don't filter exported fetches and converters
- MINOR: lua: Add support for userlist as fetches and converters arguments
- MINOR: lua: Add support for regex as fetches and converters arguments
- MINOR: arg: Use chunk_destroy() to release string arguments
- BUG/MINOR: snapshots: leak of snapshots on deinit()
- CLEANUP: ssl: ssl_sock_crt2der semicolon and spaces
- MINOR: ssl: add ssl_{c,s}_chain_der fetch methods
- CLEANUP: fix all duplicated semicolons
- BUG/MEDIUM: ssl: fix the ssl-skip-self-issued-ca option
- BUG/MINOR: ssl: ssl-skip-self-issued-ca requires >= 1.0.2
- BUG/MINOR: stats: use strncmp() instead of memcmp() on health states
- BUILD: makefile: don't disable -Wstringop-overflow anymore
- BUG/MINOR: ssl: double free w/ smp_fetch_ssl_x_chain_der()
- BUG/MEDIUM: htx: smp_prefetch_htx() must always validate the direction
- BUG/MEDIUM: ssl: never generates the chain from the verify store
- OPTIM: regex: PCRE2 use JIT match when JIT optimisation occured.
- BUG/MEDIUM: ssl: does not look for all SNIs before chosing a certificate
- CLEANUP: ssl: remove poorly readable nested ternary
In bug #810, the SNI are not matched correctly, indeed when trying to
match a certificate type in ssl_sock_switchctx_cbk() all SNIs were not
looked up correctly.
In the case you have in a crt-list:
wildcard.subdomain.domain.tld.pem.rsa *.subdomain.domain.tld record.subdomain.domain.tld
record.subdomain.domain.tld.pem.ecdsa record.subdomain.domain.tld another-record.subdomain.domain.tld
If the client only supports RSA and requests
"another-record.subdomain.domain.tld", HAProxy will find the single
ECDSA certificate and won't try to look up for a wildcard RSA
certificate.
This patch fixes the code so we look for all single and
wildcard before chosing the certificate type.
This bug was introduced by commit 3777e3a ("BUG/MINOR: ssl: certificate
choice can be unexpected with openssl >= 1.1.1").
It must be backported as far as 1.8 once it is heavily tested.
When a regex had been succesfully compiled by the JIT pass, it is better
to use the related match, thanksfully having same signature, for better
performance.
Signed-off-by: David Carlier <devnexen@gmail.com>
In bug #781 it was reported that HAProxy completes the certificate chain
using the verify store in the case there is no chain.
Indeed, according to OpenSSL documentation, when generating the chain,
OpenSSL use the chain store OR the verify store in the case there is no
chain store.
As a workaround, this patch always put a NULL chain in the SSL_CTX so
OpenSSL does not tries to complete it.
This must be backported in all branches, the code could be different,
the important part is to ALWAYS set a chain, and uses sk_X509_new_null()
if the chain is NULL.
It is possible to process a channel based on desynchronized info if a
request fetch is called from a response and conversely. However, the
code in smp_prefetch_htx() already makes sure the analysis has already
started before trying to fetch from a buffer, so the problem effectively
lies in response rules making use of request expressions only.
Usually it's not a problem as extracted data are checked against the
current HTTP state, except when it comes to the start line, which is
usually accessed directly from sample fetch functions such as status,
path, url, url32, query and so on. In this case, trying to access the
request buffer from the response path will lead to unpredictable
results. When building with DEBUG_STRICT, a process violating these
rules will simply die after emitting:
FATAL: bug condition "htx->first == -1" matched at src/http_htx.c:67
But when this is not enabled, it may or may not crash depending on what
the pending request buffer data look like when trying to spot a start
line there. This is typically what happens in issue #806.
This patch adds a test in smp_prefetch_htx() so that it does not try
to parse an HTX buffer in a channel belonging to the wrong direction.
There's one special case on the "method" sample fetch since it can
retrieve info even without a buffer, from the other direction, as
long as the method is one of the well known ones. Three, we call
smp_prefetch_htx() only if needed.
This was reported in 2.0 and must be backported there (oldest stable
version with HTX).
smp_fetch_ssl_x_chain_der() uses the SSL_get_peer_cert_chain() which
does not increment the refcount of the chain, so it should not be free'd.
The bug was introduced by a598b50 ("MINOR: ssl: add ssl_{c,s}_chain_der
fetch methods"). No backport needed.
The reports for health states are checked using memcmp() in order to
only focus on the first word and possibly ignore trailing %d/%d etc.
This makes gcc unhappy about a potential use of "" as the string, which
never happens since the string is always set. This resulted in commit
c4e6460f6 ("MINOR: build: Disable -Wstringop-overflow.") to silence
these messages. However some lengths are incorrect (though cannot cause
trouble), and in the end strncmp() is just safer and cleaner.
This can be backported to all stable branches as it will shut a warning
with gcc 8 and above.
In commit f187ce6, the ssl-skip-self-issued-ca option was accidentally
made useless by reverting the SSL_CTX reworking.
The previous attempt of making this feature was putting each certificate
of the chain in the SSL_CTX with SSL_CTX_add_extra_chain_cert() and was
skipping the Root CA.
The problem here is that doing it this way instead of doing a
SSL_CTX_set1_chain() break the support of the multi-certificate bundles.
The SSL_CTX_build_cert_chain() function allows one to remove the Root CA
with the SSL_BUILD_CHAIN_FLAG_NO_ROOT flag. Use it instead of doing
tricks with the CA.
Should fix issue #804.
Must be backported in 2.2.
Following work from Arjen and Mathilde, it adds ssl_{c,s}_chain_der
methods; it returns DER encoded certs from SSL_get_peer_cert_chain
Also update existing vtc tests to add random intermediate certificates
When getting the result through this header:
http-response add-header x-ssl-chain-der %[ssl_c_chain_der,hex]
One can parse it with any lib accepting ASN.1 DER data, such as in go:
bin, err := encoding/hex.DecodeString(cert)
certs_parsed, err := x509.ParseCertificates(bin)
Cc: Arjen Nienhuis <arjen@zorgdoc.nl>
Signed-off-by: Mathilde Gilles <m.gilles@criteo.com>
Signed-off-by: William Dauchy <w.dauchy@criteo.com>
Free the snapshots on deinit() when they were initialized in a proxy
upon an error.
This was introduced by c55015e ("MEDIUM: snapshots: dynamically allocate
the snapshots").
Should be backported as far as 1.9.
This way, all fields of the buffer structure are reset when a string argument
(ARGT_STR) is released. It is also a good way to explicitly specify this kind
of argument is a chunk. So .data and .size fields must be set.
This patch may be backported to ease backports.
It means now regsub() converter is now exported to the lua. Map converters based
on regex are not available because the map arguments are not supported.
Thanks to previous commits, it is now safe to use from lua the sample fetches
and sample converters that convert arguments, especially the strings
(ARGT_STR). So now, there are all exported to the lua. They was filtered on the
validation functions. Only fetches without validation functions or with val_hdr
or val_payload_lv functions were exported, and converters without validation
functions.
This patch depends on following commits :
* aec27ef44 "BUG/MINOR: lua: Duplicate lua strings in sample fetches/converters arg array"
* fdea1b631 "MINOR: hlua: Don't needlessly copy lua strings in trash during args validation"
It must be backported as far as 2.1 because the date() and http_date()
converters are no longer exported because of the filter on the validation
function, since the commit ae6f125c7 ("MINOR: sample: add us/ms support to
date/http_date)".
Strings in the argument array used by sample fetches and converters must be
duplicated. This is mandatory because, during the arguments validations, these
strings may be converted and released. It works this way during the
configuration parsing and there is no reason to adapt this behavior during the
runtime when a sample fetch or a sample converter is called from the lua. In
fact, there is a reason to not change the behavior. It must reamain simple for
everyone to add new fetches or converters.
Thus, lua strings are duplicated. It is only performed at the end of the
hlua_lua2arg_check() function, if the argument is still a ARGT_STR. Of course,
it requires a cleanup loop after the call or when an error is triggered.
This patch depends on following commits:
* 959171376 "BUG/MINOR: arg: Fix leaks during arguments validation for fetches/converters"
* fdea1b631 "MINOR: hlua: Don't needlessly copy lua strings in trash during args validation"
It may be backported to all supported versions, most probably as far as 2.1
only.
Lua strings are NULL terminated. So in the hlua_lua2arg_check() function, used
to check arguments against the sample fetches specification, there is no reason
to copy these strings in a trash to add a terminating null byte.
In addition, when the array of arguments is built from lua values, we must take
care to count this terminating null bytes in the size of the buffer where a
string is stored. The same must be done when a sample is built from a lua value.
This patch may be backported to easy backports.
In hlua_lua2arg_check() function, before converting an argument to an IPv4 or
IPv6 mask, we must be sure to have an integer or a string argument (ARGT_SINT or
ARGT_STR).
This patch must be backported to all supported versions.
In hlua_lua2arg_check() function, before converting a string to an IP address,
we must be to sure to have a string argument (ARGT_STR).
This patch must be backported to all supported versions.
Some sample fetches or sample converters uses a validation functions for their
arguments. In these function, string arguments (ARGT_STR) may be converted to
another type (for instance a regex, a variable or a integer). Because these
strings are allocated when the argument list is built, they must be freed after
a conversion. Most of time, it is done. But not always. This patch fixes these
minor memory leaks (only on few strings, during the configuration parsing).
This patch may be backported to all supported versions, most probably as far as
2.1 only. If this commit is backported, the previous one 73292e9e6 ("BUG/MINOR:
lua: Duplicate map name to load it when a new Map object is created") must also
be backported. Note that some validation functions does not exists on old
version. It should be easy to resolve conflicts.
When a new map is created, the sample_load_map() function is called. To do so,
an argument array is created with the name as first argument. Because it is a
lua string, owned by the lua, it must be duplicated. The sample_load_map()
function will convert this argument to a map. In theory, after the conversion,
it must release the original string. It is not performed for now and it is a bug
that will be fixed in the next commit.
This patch may be backported to all supported versions, most probably as far as
2.1 only. But it must be backported with the next commit "BUG/MINOR: arg: Fix
leaks during arguments validation for fetches/converters".
The debug() converter uses a string to reference the sink where to send debug
events. During the configuration parsing, this string is converted to a sink
object but it is still store as a string argument. It is a problem on deinit
because string arguments are released. So the sink pointer will be released
twice.
To fix the bug, we keep a reference on the sink using an ARGT_PTR argument. This
way, it will not be freed on the deinit.
This patch depends on the commit e02fc4d0d ("MINOR: arg: Add an argument type to
keep a reference on opaque data"). Both must be backported as far as 2.1.
The ARGT_PTR argument type may now be used to keep a reference to opaque data in
the argument array used by sample fetches and converters. It is a generic way to
point on data. I guess it could be used for some other arguments, like proxy,
server, map or stick-table.
In sample_load_map() function, the global mode is now tested to be sure to be in
the starting mode. If not, an error is returned.
At first glance, this patch may seem useless because maps are loaded during the
configuration parsing. But in fact, it is possible to load a map from the lua,
using Map:new() method. And, there is nothing to forbid to call this method at
runtime, during a script execution. It must never be done because it may perform
an filesystem access for unknown maps or allocation for known ones. So at
runtime, it means a blocking call or a memroy leak. Note it is still possible to
load a map from the lua, but in the global part of a script only. This part is
executed during the configuration parsing.
This patch must be backported in all stable versions.
This bug affects all version of HAProxy since the OCSP data is not free
in the deinit(), but leaking on exit() is not really an issue. However,
when doing dynamic update of certificates over the CLI, those data are
not free'd upon the free of the SSL_CTX.
3 leaks are happening, the first leak is the one of the ocsp_arg
structure which serves the purpose of containing the pointers in the
case of a multi-certificate bundle. The second leak is the one ocsp
struct. And the third leak is the one of the struct buffer in the
ocsp_struct.
The problem lies with SSL_CTX_set_tlsext_status_arg() which does not
provide a way to free the argument upon an SSL_CTX_free().
This fix uses ex index functions instead of registering a
tlsext_status_arg(). This is really convenient because it allows to
register a free callback which will free the ex index content upon a
SSL_CTX_free().
A refcount was also added to the ocsp_response structure since it is
stored in a tree and can be reused in another SSL_CTX.
Should fix part of the issue #746.
This must be backported in 2.2 and 2.1.
Fix a memory leak when loading an OCSP file when the file was already
loaded elsewhere in the configuration.
Indeed, if the OCSP file already exists, a useless chunk_dup() will be
done during the load.
To fix it we reverts "ocsp" to "iocsp" like it was done previously.
This was introduced by commit 246c024 ("MINOR: ssl: load the ocsp
in/from the ckch").
Should fix part of the issue #746.
It must be backported in 2.1 and 2.2.
From https://www.python.org/dev/peps/pep-0353/
"A new type Py_ssize_t is introduced, which has the same size as the
compiler's size_t type, but is signed. It will be a typedef for ssize_t
where available."
For integer types, causes printf to expect a size_t-sized integer
argument.
This should fix github issue #702
This should be backported to >= v2.2
Signed-off-by: William Dauchy <w.dauchy@criteo.com>
A regression was introduced by 13a9232ebc
when I added support for Additional section of the SRV responses..
Basically, when a server is managed through SRV records additional
section and it's disabled (because its associated Additional record has
disappeared), it never leaves its MAINT state and so never comes back to
production.
This patch updates the "snr_update_srv_status()" function to clear the
MAINT status when the server now has an IP address and also ensure this
function is called when parsing Additional records (and associating them
to new servers).
This can cause severe outage for people using HAProxy + consul (or any
other service registry) through DNS service discovery).
This should fix issue #793.
This should be backported to 2.2.
The H1 multiplexer is able to perform synchronous send. When a large body is
transfer, if nothing is received and if no error or shutdown occurs, it is
possible to not go down at the H1 connection level to do I/O for a long
time. When this happens, we must still take care to refresh the H1 connection
timeout. Otherwise it is possible to hit the connection timeout during the
transfer while it should not expire.
This bug exists because only h1_process() refresh the H1 connection timeout. To
fix the bug, h1_snd_buf() must also refresh this timeout. To make things more
readable, a dedicated function has been introduced and called to refresh the
timeout.
This bug exists on all HTX versions. But it is harder to hit it on 2.1 and below
because when a H1 mux is initialized, we actively try to read data instead of
subscribing for receiving. So there is at least one call to h1_process().
This patch should fix the issue #790. It must be backported as far as 2.0.
The SSL_INC and SSL_LIB variables were not initialized in the Makefile,
so they could be accidently inherited from the environment. We require
that any makefile variable is explicitly set on the command line so they
must be initialized.
Note that the Travis scripts used to rely only on these variables to be
exported, so it was adjusted as well.
It's cumbersome to copy-paste a commit ID into another window after having
typed "git cherry-pick -sx", so let's have the suggested output format of
git-show prepare this line just before the subject line, it remains at a
stable position on the terminal when searching for "/^commit". One just
has to copy-paste the line into another terminal will result in the commit
being properly picked.
We've never used the output of the rightmost branch with this tool,
and it systematically causes two identical outputs making the job
harder during backport sessions. Let's simply remove the right part
when it's identical to the left one. This also adds a few line feeds
to make the output more readable.
