Commit Graph

551 Commits

Author SHA1 Message Date
Christopher Faulet
82004145d4 BUG/MINOR: ssl: always check for ssl connection before getting its XPRT context
In several SSL functions, the XPRT context is retrieved before any check on the
connection. In the function ssl_sock_is_ssl(), a test suggests the connection
may be null. So, it is safer to test the ssl connection before retrieving its
XPRT context. It removes any ambiguities and prevents possible null pointer
dereferences.

This patch fixes the issue #265. It must be backported to 2.0.
2019-09-10 10:29:54 +02:00
Emeric Brun
5762a0db0a BUG/MAJOR: ssl: ssl_sock was not fully initialized.
'ssl_sock' wasn't fully initialized so a new session can inherit some
flags from an old one.

This causes some fetches, related to client's certificate presence or
its verify status and errors, returning erroneous values.

This issue could generate other unexpected behaviors because a new
session could also inherit other flags such as SSL_SOCK_ST_FL_16K_WBFSIZE,
SSL_SOCK_SEND_UNLIMITED, or SSL_SOCK_RECV_HEARTBEAT from an old session.

This must be backported to 2.0 but it's useless for previous.
2019-09-06 17:33:33 +02:00
Willy Tarreau
9d00869323 CLEANUP: cli: replace all occurrences of manual handling of return messages
There were 221 places where a status message or an error message were built
to be returned on the CLI. All of them were replaced to use cli_err(),
cli_msg(), cli_dynerr() or cli_dynmsg() depending on what was expected.
This removed a lot of duplicated code because most of the times, 4 lines
are replaced by a single, safer one.
2019-08-09 11:26:10 +02:00
Emmanuel Hocdet
c9858010c2 MINOR: ssl: ssl_fc_has_early should work for BoringSSL
CO_FL_EARLY_SSL_HS/CO_FL_EARLY_DATA are removed for BoringSSL. Early
data can be checked via BoringSSL API and ssl_fc_has_early can used it.

This should be backported to all versions till 1.8.
2019-08-07 18:44:49 +02:00
Emmanuel Hocdet
f967c31e75 BUG/MINOR: ssl: fix 0-RTT for BoringSSL
Since BoringSSL commit 777a2391 "Hold off flushing NewSessionTicket until write.",
0-RTT doesn't work. It appears that half-RTT data (response from 0-RTT) never
worked before the BoringSSL fix. For HAProxy the regression come from 010941f8
"BUG/MEDIUM: ssl: Use the early_data API the right way.": the problem is link to
the logic of CO_FL_EARLY_SSL_HS used for OpenSSL. With BoringSSL, handshake is
done before reading early data, 0-RTT data and half-RTT data are processed as
normal data: CO_FL_EARLY_SSL_HS/CO_FL_EARLY_DATA is not needed, simply remove
it.

This should be backported to all versions till 1.8.
2019-08-07 18:44:48 +02:00
William Lallemand
6e5f2ceead BUG/MEDIUM: ssl: open the right path for multi-cert bundle
Multi-cert bundle was not working anymore because we tried to open the
wrong path.
2019-08-01 14:47:57 +02:00
Emmanuel Hocdet
1503e05362 BUG/MINOR: ssl: fix ressource leaks on error
Commit 36b84637 "MEDIUM: ssl: split the loading of the certificates"
introduce leaks on fd/memory in case of error.
2019-08-01 11:27:24 +02:00
William Lallemand
6dee29d63d BUG/MEDIUM: ssl: don't free the ckch in multi-cert bundle
When using a ckch we should never try to free its content, because it
won't be usable  after and can result in a NULL derefence during
parsing.

The content was previously freed because the ckch wasn't stored in a
tree to be used later, now that we use it multiple time, we need to keep
the data.
2019-08-01 11:27:24 +02:00
William Lallemand
a8c73748f8 BUG/MEDIUM: ssl: does not try to free a DH in a ckch
ssl_sock_load_dh_params() should not free the DH * of a ckch, or the
ckch won't be usable during the next call.
2019-07-31 19:35:31 +02:00
William Lallemand
c4ecddf418 BUG/BUILD: ssl: fix build with openssl < 1.0.2
Recent changes use struct cert_key_and_chain to load all certificates in
frontends, this structure was previously used only to load multi-cert
bundle, which is supported only on >= 1.0.2.
2019-07-31 17:05:09 +02:00
Emmanuel Hocdet
a7a0f991c9 MINOR: ssl: clean ret variable in ssl_sock_load_ckchn
In ssl_sock_load_ckchn, ret variable is now in a half dead usage.
Remove it to clean compilation warnings.
2019-07-30 17:54:35 +02:00
Emmanuel Hocdet
efa4b95b78 CLEANUP: ssl: ssl_sock_load_crt_file_into_ckch
Fix comments for this function and remove free before alloc call: ckch
call is correctly balanced  (alloc/free).
2019-07-30 17:54:34 +02:00
Emmanuel Hocdet
54227d8add MINOR: ssl: do not look at DHparam with OPENSSL_NO_DH
OPENSSL_NO_DH can be defined to avoid obsolete and heavy DH processing.
With OPENSSL_NO_DH, parse the entire PEM file to look at DHparam is wast
of time.
2019-07-30 17:54:34 +02:00
Emmanuel Hocdet
03e09f3818 MINOR: ssl: check private key consistency in loading
Load a PEM certificate and use it in CTX are now decorrelated.
Checking the certificate and private key consistency can be done
earlier: in loading phase instead CTX set phase.
2019-07-30 15:53:54 +02:00
Emmanuel Hocdet
1c65fdd50e MINOR: ssl: add extra chain compatibility
cert_key_and_chain handling is now outside openssl 1.0.2 #if: the
code must be libssl compatible. SSL_CTX_add1_chain_cert and
SSL_CTX_set1_chain requires openssl >= 1.0.2, replace it by legacy
SSL_CTX_add_extra_chain_cert when SSL_CTX_set1_chain is not provided.
2019-07-30 15:53:54 +02:00
Emmanuel Hocdet
9246f8bc83 MINOR: ssl: use STACK_OF for chain certs
Used native cert chain manipulation with STACK_OF from ssl lib.
2019-07-30 15:53:54 +02:00
William Lallemand
fa8922285d MEDIUM: ssl: load DH param in struct cert_key_and_chain
Load the DH param at the same time as the certificate, we don't need to
open the file once more and read it again. We store it in the ckch_node.

There is a minor change comparing to the previous way of loading the DH
param in a bundle. With a bundle, the DH param in a certificate file was
never loaded, it only used the global DH or the default DH, now it's
able to use the DH param from a certificate file.
2019-07-29 15:28:46 +02:00
William Lallemand
6af03991da MEDIUM: ssl: lookup and store in a ckch_node tree
Don't read a certificate file again if it was already stored in the
ckchn tree. It allows HAProxy to start more quickly if the same
certificate is used at different places in the configuration.

HAProxy lookup in the ssl_sock_load_cert() function, doing it at this
level allows to skip the reading of the certificate in the filesystem.

If the certificate is not found in the tree, we insert the ckch_node in
the tree once the certificate is read on the filesystem, the filename or
the bundle name is used as the key.
2019-07-29 15:28:46 +02:00
William Lallemand
36b8463777 MEDIUM: ssl: split the loading of the certificates
Split the functions which open the certificates.

Instead of opening directly the certificates and inserting them directly
into a SSL_CTX, we use a struct cert_key_and_chain to store them in
memory and then we associate a SSL_CTX to the certificate stored in that
structure.

Introduce the struct ckch_node for the multi-cert bundles so we can
store multiple cert_key_and_chain in the same structure.

The functions ssl_sock_load_multi_cert() and ssl_sock_load_cert_file()
were modified so they don't open the certicates anymore on the
filesystem. (they still open the sctl and ocsp though).  These functions
were renamed ssl_sock_load_ckchn() and ssl_sock_load_multi_ckchn().

The new function ckchn_load_cert_file() is in charge of loading the
files in the cert_key_and_chain. (TODO: load ocsp and sctl from there
too).

The ultimate goal is to be able to load a certificate from a certificate
tree without doing any filesystem access, so we don't try to open it
again if it was already loaded, and we share its configuration.
2019-07-29 15:28:46 +02:00
William Lallemand
a59191b894 MEDIUM: ssl: use cert_key_and_chain struct in ssl_sock_load_cert_file()
This structure was only used in the case of the multi-cert bundle.

Using these primitives everywhere when we load the file are a first step
in the deduplication of the code.
2019-07-29 15:28:46 +02:00
William Lallemand
c940207d39 MINOR: ssl: merge ssl_sock_load_cert_file() and ssl_sock_load_cert_chain_file()
This commit merges the function ssl_sock_load_cert_file() and
ssl_sock_load_cert_chain_file().

The goal is to refactor the SSL code and use the cert_key_and_chain
struct to load everything.
2019-07-29 15:28:46 +02:00
Willy Tarreau
085a1513ad MINOR: ssl-sock: use conn->dst instead of &conn->addr.to
This part can be definitive as the check was already in place.
2019-07-19 13:50:09 +02:00
Willy Tarreau
f5bdb64d35 MINOR: ssl: switch to conn_get_dst() to retrieve the destination address
This replaces conn_get_to_addr() and the subsequent check.
2019-07-19 13:50:09 +02:00
Christopher Faulet
fc9cfe4006 REORG: proto_htx: Move HTX analyzers & co to http_ana.{c,h} files
The old module proto_http does not exist anymore. All code dedicated to the HTTP
analysis is now grouped in the file proto_htx.c. So, to finish the polishing
after removing the legacy HTTP code, proto_htx.{c,h} files have been moved in
http_ana.{c,h} files.

In addition, all HTX analyzers and related functions prefixed with "htx_" have
been renamed to start with "http_" instead.
2019-07-19 09:24:12 +02:00
Lukas Tribus
4979916134 BUG/MINOR: ssl: revert empty handshake detection in OpenSSL <= 1.0.2
Commit 54832b97 ("BUILD: enable several LibreSSL hacks, including")
changed empty handshake detection in OpenSSL <= 1.0.2 and LibreSSL,
from accessing packet_length directly (not available in LibreSSL) to
calling SSL_state() instead.

However, SSL_state() appears to be fully broken in both OpenSSL and
LibreSSL.

Since there is no possibility in LibreSSL to detect an empty handshake,
let's not try (like BoringSSL) and restore this functionality for
OpenSSL 1.0.2 and older, by reverting to the previous behavior.

Should be backported to 2.0.
2019-07-09 04:47:18 +02:00
Olivier Houchard
e488ea865a BUG/MEDIUM: ssl: Don't attempt to set alpn if we're not using SSL.
Checks use ssl_sock_set_alpn() to set the ALPN if check-alpn is used, however
check-alpn failed to check if the connection was indeed using SSL, and thus,
would crash if check-alpn was used on a non-SSL connection. Fix this by
making sure the connection uses SSL before attempting to set the ALPN.

This should be backported to 2.0 and 1.9.
2019-06-28 14:12:28 +02:00
Olivier Houchard
0ff28651c1 BUG/MEDIUM: ssl: Don't do anything in ssl_subscribe if we have no ctx.
In ssl_subscribe(), make sure we have a ssl_sock_ctx before doing anything.
When ssl_sock_close() is called, it wakes any subscriber up, and that
subscriber may decide to subscribe again, for some reason. If we no longer
have a context, there's not much we can do.

This should be backported to 2.0.
2019-06-24 19:00:16 +02:00
Olivier Houchard
965e84e2df BUG/MEDIUM: ssl: Make sure we initiate the handshake after using early data.
When we're done sending/receiving early data, and we add the handshake
flags on the connection, make sure we wake the associated tasklet up, so that
the handshake will be initiated.
2019-06-15 21:00:39 +02:00
Willy Tarreau
3c39a7d889 CLEANUP: connection: rename the wait_event.task field to .tasklet
It's really confusing to call it a task because it's a tasklet and used
in places where tasks and tasklets are used together. Let's rename it
to tasklet to remove this confusion.
2019-06-14 14:42:29 +02:00
Willy Tarreau
9faebe34cd MEDIUM: tools: improve time format error detection
As reported in GH issue #109 and in discourse issue
https://discourse.haproxy.org/t/haproxy-returns-408-or-504-error-when-timeout-client-value-is-every-25d
the time parser doesn't error on overflows nor underflows. This is a
recurring problem which additionally has the bad taste of taking a long
time before hitting the user.

This patch makes parse_time_err() return special error codes for overflows
and underflows, and adds the control in the call places to report suitable
errors depending on the requested unit. In practice, underflows are almost
never returned as the parsing function takes care of rounding values up,
so this might possibly happen on 64-bit overflows returning exactly zero
after rounding though. It is not really possible to cut the patch into
pieces as it changes the function's API, hence all callers.

Tests were run on about every relevant part (cookie maxlife/maxidle,
server inter, stats timeout, timeout*, cli's set timeout command,
tcp-request/response inspect-delay).
2019-06-07 19:32:02 +02:00
Olivier Houchard
81284e6908 BUG/MEDIUM: ssl: Don't forget to initialize ctx->send_recv and ctx->recv_wait.
When creating a new ssl_sock_ctx, don't forget to initialize its send_recv
and recv_wait to NULL, or we may end up dereferencing random values, and
crash.
2019-06-06 13:21:23 +02:00
Olivier Houchard
03abf2d31e MEDIUM: connections: Remove CONN_FL_SOCK*
Now that the various handshakes come with their own XPRT, there's no
need for the CONN_FL_SOCK* flags, and the conn_sock_want|stop functions,
so garbage-collect them.
2019-06-05 18:03:38 +02:00
Olivier Houchard
2e055483ff MINOR: connections: Add a new xprt method, add_xprt().
Add a new method to xprt_ops, add_xprt(), that changes the underlying
xprt to the one provided, and optionally provide the old one.
2019-06-05 18:03:38 +02:00
Olivier Houchard
5149b59851 MINOR: connections: Add a new xprt method, remove_xprt.
Add a new method to xprt_ops, remove_xprt. When called, if the provided
xprt_ctx is the same as the xprt's underlying xprt_ctx, it then uses the
new xprt provided, otherwise it calls the remove_xprt method of the next
xprt.
The goal is to be able to add a temporary xprt, that removes itself from
the chain when it did what it had to do. This will be used to implement
a pseudo-xprt for anything that just requires a handshake (such as the
proxy protocol).
2019-06-05 18:03:38 +02:00
Olivier Houchard
000694cf96 MINOR: ssl: Make ssl_sock_handshake() static.
ssl_sock_handshake is now only used by the ssl code itself, there's no need
to export it anymore, so make it static.
2019-06-05 18:03:38 +02:00
Olivier Houchard
ea8dd949e4 MEDIUM: ssl: Handle subscribe by itself.
As the SSL code may have different needs than the upper layer, ie it may want
to receive when the upper layer wants to right, instead of directly forwarding
the subscribe to the underlying xprt, handle it ourself. The SSL code will
know remember any subscribe call, and wake the tasklet when it is ready
for more I/O.
2019-06-05 18:03:38 +02:00
Patrick Hemmer
65674662b4 MINOR: SSL: add client/server random sample fetches
This adds 4 sample fetches:
- ssl_fc_client_random
- ssl_fc_server_random
- ssl_bc_client_random
- ssl_bc_server_random

These fetches retrieve the client or server random value sent during the
handshake.

Their use is to be able to decrypt traffic sent using ephemeral ciphers. Tools
like wireshark expect a TLS log file with lines in a few known formats
(https://code.wireshark.org/review/gitweb?p=wireshark.git;a=blob;f=epan/dissectors/packet-tls-utils.c;h=28a51fb1fb029eae5cea52d37ff5b67d9b11950f;hb=HEAD#l5209).
Previously the only format supported using data retrievable from HAProxy state
was the one utilizing the Session-ID. However an SSL/TLS session ID is
optional, and thus cannot be relied upon for this purpose.

This change introduces the ability to extract the client random instead which
can be used for one of the other formats. The change also adds the ability to
extract the server random, just in case it might have some other use, as the
code change to support this was trivial.
2019-06-05 10:07:44 +02:00
Emmanuel Hocdet
839af57c85 CLEANUP: ssl: remove unneeded defined(OPENSSL_IS_BORINGSSL)
BoringSSL pretend to be compatible with OpenSSL 1.1.0 and
OPENSSL_VERSION_NUMBER is set accordly: cleanup redundante #ifdef.
2019-06-05 10:01:44 +02:00
Olivier Houchard
692c1d07f9 MINOR: ssl: Don't forget to call the close method of the underlying xprt.
In ssl_sock_close(), don't forget to call the underlying xprt's close method
if it exists. For now it's harmless not to do so, because the only available
layer is the raw socket, which doesn't have a close method, but that will
change when we implement QUIC.
2019-05-28 10:08:39 +02:00
Olivier Houchard
19afb274ad MINOR: ssl: Make sure the underlying xprt's init method doesn't fail.
In ssl_sock_init(), when initting the underlying xprt, check the return value,
and give up if it fails.
2019-05-28 10:08:28 +02:00
Ilya Shipitsin
0590f44254 BUILD: ssl: fix latest LibreSSL reg-test error
starting with OpenSSL 1.0.0 recommended way to disable compression is
using SSL_OP_NO_COMPRESSION when creating context.

manipulations with SSL_COMP_get_compression_methods, sk_SSL_COMP_num
are only required for OpenSSL < 1.0.0
2019-05-26 21:26:02 +02:00
Ilya Shipitsin
e242f3dfb8 BUG/MINOR: ssl_sock: Fix memory leak when disabling compression
according to manpage:

       sk_TYPE_zero() sets the number of elements in sk to zero. It
       does not free sk so after this call sk is still valid.

so we need to free all elements

[wt: seems like it has been there forever and should be backported
 to all stable branches]
2019-05-25 07:45:55 +02:00
William Lallemand
7e1770b151 BUG/MAJOR: ssl: segfault upon an heartbeat request
7b5fd1e ("MEDIUM: connections: Move some fields from struct connection
to ssl_sock_ctx.") introduced a bug in the heartbleed mitigation code.

Indeed the code used conn->ctx instead of conn->xprt_ctx for the ssl
context, resulting in a null dereference.
2019-05-13 16:03:44 +02:00
Willy Tarreau
295d614de1 CLEANUP: ssl: move all BIO_* definitions to openssl-compat
The following macros are now defined for openssl < 1.1 so that we
can remove the code performing direct access to the structures :

  BIO_get_data(), BIO_set_data(), BIO_set_init(), BIO_meth_free(),
  BIO_meth_new(), BIO_meth_set_gets(), BIO_meth_set_puts(),
  BIO_meth_set_read(), BIO_meth_set_write(), BIO_meth_set_create(),
  BIO_meth_set_ctrl(), BIO_meth_set_destroy()
2019-05-11 17:39:08 +02:00
Willy Tarreau
11b167167e CLEANUP: ssl: remove ifdef around SSL_CTX_get_extra_chain_certs()
Instead define this one in openssl-compat.h when
SSL_CTRL_GET_EXTRA_CHAIN_CERTS is not defined (which was the current
condition used in the ifdef).
2019-05-11 17:38:21 +02:00
Willy Tarreau
366a6987a7 CLEANUP: ssl: move the SSL_OP_* and SSL_MODE_* definitions to openssl-compat
These ones were defined in the middle of ssl_sock.c, better move them
to the include file to find them.
2019-05-11 17:37:44 +02:00
Willy Tarreau
8d164dc568 CLEANUP: ssl: never include openssl/*.h outside of openssl-compat.h anymore
Since we're providing a compatibility layer for multiple OpenSSL
implementations and their derivatives, it is important that no C file
directly includes openssl headers but only passes via openssl-compat
instead. As a bonus this also gets rid of redundant complex rules for
inclusion of certain files (engines etc).
2019-05-10 09:36:42 +02:00
Willy Tarreau
9356dacd22 REORG: ssl: move some OpenSSL defines from ssl_sock to openssl-compat
Some defines like OPENSSL_VERSION or X509_getm_notBefore() have nothing
to do in ssl_sock and must move to openssl-compat.h so that they are
consistently shared by the whole code. A warning in the code was added
against wild additions of macros there.
2019-05-10 09:31:06 +02:00
Willy Tarreau
5599456ee2 REORG: ssl: move openssl-compat from proto to common
This way we can include it much earlier to cover types/ as well.
2019-05-10 09:19:50 +02:00
Willy Tarreau
df17e0e1a7 BUILD: ssl: fix libressl build again after aes-gcm-enc
Enabling aes-gcm-enc in last commit (MINOR: ssl: enable aes_gcm_dec
on LibreSSL) uncovered a wrong condition on the define of the
EVP_CTRL_AEAD_SET_IVLEN macro which I forgot to add when making the
commit, resulting in breaking libressl build again. In case libressl
later defines this macro, the test will have to change for a version
range instead.
2019-05-10 09:19:07 +02:00