687 Commits

Author SHA1 Message Date
Lukas Tribus
a26d1e1324 BUILD: ssl: improve SSL_CTX_set_ecdh_auto compatibility
SSL_CTX_set_ecdh_auto() is not defined when OpenSSL 1.1.1 is compiled
with the no-deprecated option. Remove existing, incomplete guards and
add a compatibility macro in openssl-compat.h, just as OpenSSL does:

bf4006a6f9/include/openssl/ssl.h (L1486)

This should be backported as far as 2.0 and probably even 1.9.
2019-12-21 06:46:55 +01:00
Olivier Houchard
54907bb848 BUG/MEDIUM: ssl: Revamp the way early data are handled.
Instead of attempting to read the early data only when the upper layer asks
for data, allocate a temporary buffer, stored in the ssl_sock_ctx, and put
all the early data in there. Requiring that the upper layer takes care of it
means that if for some reason the upper layer wants to emit data before it
has totally read the early data, we will be stuck forever.

This should be backported to 2.1 and 2.0.
This may fix github issue #411.
2019-12-19 15:22:04 +01:00
William Lallemand
ba22e901b3 BUG/MINOR: ssl/cli: fix build for openssl < 1.0.2
Commit d4f946c ("MINOR: ssl/cli: 'show ssl cert' give information on the
certificates") introduced a build issue with openssl version < 1.0.2
because it uses the certificate bundles.
2019-12-18 20:40:20 +01:00
William Lallemand
d4f946c469 MINOR: ssl/cli: 'show ssl cert' give information on the certificates
Implement the 'show ssl cert' command on the CLI which list the frontend
certificates. With a certificate name in parameter it will show more
details.
2019-12-18 18:16:34 +01:00
Olivier Houchard
545989f37f BUG/MEDIUM: ssl: Don't set the max early data we can receive too early.
When accepting the max early data, don't set it on the SSL_CTX while parsing
the configuration, as at this point global.tune.maxrewrite may still be -1,
either because it was not set, or because it hasn't been set yet. Instead,
set it for each connection, just after we created the new SSL.
Not doing so meant that we could pretend to accept early data bigger than one
of our buffer.

This should be backported to 2.1, 2.0, 1.9 and 1.8.
2019-12-17 15:45:38 +01:00
Emmanuel Hocdet
3777e3ad14 BUG/MINOR: ssl: certificate choice can be unexpected with openssl >= 1.1.1
It's regression from 9f9b0c6 "BUG/MEDIUM: ECC cert should work with
TLS < v1.2 and openssl >= 1.1.1". Wilcard EC certifcate could be selected
at the expense of specific RSA certificate.
In any case, specific certificate should always selected first, next wildcard.
Reflect this rule in a loop to avoid any bug in certificate selection changes.

Fix issue #394.

It should be backported as far as 1.8.
2019-12-05 10:49:24 +01:00
William Lallemand
920b035238 BUG/MINOR: ssl/cli: don't overwrite the filters variable
When a crt-list line using an already used ckch_store does not contain
filters, it will overwrite the ckchs->filters variable with 0.
This problem will generate all sni_ctx of this ckch_store without
filters. Filters generation mustn't be allowed in any case.

Must be backported in 2.1.
2019-12-05 00:00:04 +01:00
William Lallemand
230662a0dd BUG/MINOR: ssl/cli: 'ssl cert' cmd only usable w/ admin rights
The 3 commands 'set ssl cert', 'abort ssl cert' and 'commit ssl cert'
must be only usable with admin rights over the CLI.

Must be backported in 2.1.
2019-12-03 15:10:46 +01:00
Emmanuel Hocdet
140b64fb56 BUG/MINOR: ssl: fix SSL_CTX_set1_chain compatibility for openssl < 1.0.2
Commit 1c65fdd5 "MINOR: ssl: add extra chain compatibility" really implement
SSL_CTX_set0_chain. Since ckch can be used to init more than one ctx with
openssl < 1.0.2 (commit 89f58073 for X509_chain_up_ref compatibility),
SSL_CTX_set1_chain compatibility is required.

This patch must be backported to 2.1.
2019-11-29 17:02:30 +01:00
Emmanuel Hocdet
b270e8166c MINOR: ssl: deduplicate crl-file
Load file for crl or ca-cert is realy done with the same function in OpenSSL,
via X509_STORE_load_locations. Accordingly, deduplicate crl-file and ca-file
can share the same function.
2019-11-28 11:11:20 +01:00
Emmanuel Hocdet
129d3285a5 MINOR: ssl: compute ca-list from deduplicate ca-file
ca-list can be extracted from ca-file already loaded in memory.
This patch set ca-list from deduplicated ca-file when needed
and share it in ca-file tree.

As a corollary, this will prevent file access for ca-list when
updating a certificate via CLI.
2019-11-28 11:11:20 +01:00
Emmanuel Hocdet
d4f9a60ee2 MINOR: ssl: deduplicate ca-file
Typically server line like:
'server-template srv 1-1000 *:443 ssl ca-file ca-certificates.crt'
load ca-certificates.crt 1000 times and stay duplicated in memory.
Same case for bind line: ca-file is loaded for each certificate.
Same 'ca-file' can be load one time only and stay deduplicated in
memory.

As a corollary, this will prevent file access for ca-file when
updating a certificate via CLI.
2019-11-28 11:11:20 +01:00
Tim Duesterhus
9312853530 CLEANUP: ssl: Clean up error handling
This commit removes the explicit checks for `if (err)` before
passing `err` to `memprintf`. `memprintf` already checks itself
whether the `**out*` parameter is `NULL` before doing anything.
This reduces the indentation depth and makes the code more readable,
before there is less boilerplate code.

Instead move the check into the ternary conditional when the error
message should be appended to a previous message. This is consistent
with the rest of ssl_sock.c and with the rest of HAProxy.

Thus this patch is the arguably cleaner fix for issue #374 and builds
upon
5f1fa7db86c53827c97f8a8c3f5fa75bfcb5be9a and
8b453912ce9a4e1a3b1329efb2af04d1e470852e

Additionally it fixes a few places where the check *still* was missing.
2019-11-26 04:16:56 +01:00
William Dauchy
c8bb1539cb CLEANUP: ssl: check if a transaction exists once before setting it
trivial patch to fix issue #351

Fixes: bc6ca7ccaa72 ("MINOR: ssl/cli: rework 'set ssl cert' as 'set/commit'")
Reported-by: Илья Шипицин <chipitsine@gmail.com>
Signed-off-by: William Dauchy <w.dauchy@criteo.com>
2019-11-25 08:58:44 +01:00
Tim Duesterhus
c0e820c352 BUG/MINOR: ssl: Stop passing dynamic strings as format arguments
gcc complains rightfully:

src/ssl_sock.c: In function ‘ssl_sock_prepare_all_ctx’:
src/ssl_sock.c:5507:3: warning: format not a string literal and no format arguments [-Wformat-security]
   ha_warning(errmsg);
   ^
src/ssl_sock.c:5509:3: warning: format not a string literal and no format arguments [-Wformat-security]
   ha_alert(errmsg);
   ^
src/ssl_sock.c: In function ‘cli_io_handler_commit_cert’:
src/ssl_sock.c:10208:3: warning: format not a string literal and no format arguments [-Wformat-security]
   chunk_appendf(trash, err);

Introduced in 8b453912ce9a4e1a3b1329efb2af04d1e470852e.
2019-11-25 08:55:34 +01:00
Lukas Tribus
d14b49c128 BUG/MINOR: ssl: fix curve setup with LibreSSL
Since commit 9a1ab08 ("CLEANUP: ssl-sock: use HA_OPENSSL_VERSION_NUMBER
instead of OPENSSL_VERSION_NUMBER") we restrict LibreSSL to the OpenSSL
1.0.1 API, to avoid breaking LibreSSL every minute. We set
HA_OPENSSL_VERSION_NUMBER to 0x1000107fL if LibreSSL is detected and
only allow curves to be configured if HA_OPENSSL_VERSION_NUMBER is at
least 0x1000200fL.

However all relevant LibreSSL releases actually support settings curves,
which is now broken. Fix this by always allowing curve configuration when
using LibreSSL.

Reported on GitHub in issue #366.

Fixes: 9a1ab08 ("CLEANUP: ssl-sock: use HA_OPENSSL_VERSION_NUMBER instead
of OPENSSL_VERSION_NUMBER").
2019-11-24 18:24:20 +01:00
William Dauchy
5f1fa7db86 MINOR: ssl: fix possible null dereference in error handling
recent commit 8b453912ce9a ("MINOR: ssl: ssl_sock_prepare_ctx() return an error code")
converted all errors handling; in this patch we always test `err`, but
three of them are missing. I did not found a plausible explanation about
it.

this should fix issue #374

Fixes: 8b453912ce9a ("MINOR: ssl: ssl_sock_prepare_ctx() return an error code")
Reported-by: Илья Шипицин <chipitsine@gmail.com>
Signed-off-by: William Dauchy <w.dauchy@criteo.com>
2019-11-23 21:38:15 +01:00
William Lallemand
ed44243de7 MINOR: ssl/cli: display warning during 'commit ssl cert'
Display the warnings on the CLI during a commit of the certificates.
2019-11-21 17:48:11 +01:00
William Lallemand
8ef0c2a569 MEDIUM: ssl/cli: apply SSL configuration on SSL_CTX during commit
Apply the configuration of the ssl_bind_conf on the generated SSL_CTX.
It's a little bit hacky at the moment because the ssl_sock_prepare_ctx()
function was made for the configuration parsing, not for being using at
runtime. Only the 'verify' bind keyword seems to cause a file access so
we prevent it before calling the function.
2019-11-21 17:48:11 +01:00
William Lallemand
8b453912ce MINOR: ssl: ssl_sock_prepare_ctx() return an error code
Rework ssl_sock_prepare_ctx() so it fills a buffer with the error
messages instead of using ha_alert()/ha_warning(). Also returns an error
code (ERR_*) instead of the number of errors.
2019-11-21 17:48:11 +01:00
Eric Salama
3c8bde88ca BUILD/MINOR: ssl: fix compiler warning about useless statement
There is a compiler warning after commit a9363eb6 ("BUG/MEDIUM: ssl:
'tune.ssl.default-dh-param' value ignored with openssl > 1.1.1"):

src/ssl_sock.c: In function 'ssl_sock_prepare_ctx':
src/ssl_sock.c:4481:4: error: statement with no effect [-Werror=unused-value]

Fix it by adding a (void)
2019-11-20 13:49:21 +01:00
William Lallemand
0bc9c8a243 MINOR: ssl/cli: 'abort ssl cert' deletes an on-going transaction
This patch introduces the new CLI command 'abort ssl cert' which abort
an on-going transaction and free its content.

This command takes the name of the filename of the transaction as an
argument.
2019-11-19 16:21:24 +01:00
Emmanuel Hocdet
c5fdf0f3dc BUG/MINOR: ssl: fix crt-list neg filter for openssl < 1.1.1
Certificate selection in client_hello_cb (openssl >= 1.1.1) correctly
handles crt-list neg filter. Certificate selection for openssl < 1.1.1
has not been touched for a while: crt-list neg filter is not the same
than his counterpart and is wrong. Fix it to mimic the same behavior
has is counterpart.

It should be backported as far as 1.6.
2019-11-18 14:58:27 +01:00
Emmanuel Hocdet
c3775d28f9 BUG/MINOR: ssl: ssl_pkey_info_index ex_data can store a dereferenced pointer
With CLI cert update, sni_ctx can be removed at runtime. ssl_pkey_info_index
ex_data is filled with one of sni_ctx.kinfo pointer but SSL_CTX can be shared
between sni_ctx. Remove and free a sni_ctx can lead to a segfault when
ssl_pkey_info_index ex_data is used (in ssl_sock_get_pkey_algo). Removing the
dependency on ssl_pkey_info_index ex_data is the easiest way to fix the issue.
2019-11-18 14:55:32 +01:00
William Lallemand
21724f0807 MINOR: ssl/cli: replace the default_ctx during 'commit ssl cert'
If the SSL_CTX of a previous instance (ckch_inst) was used as a
default_ctx, replace the default_ctx of the bind_conf by the first
SSL_CTX inserted in the SNI tree.

Use the RWLOCK of the sni tree to handle the change of the default_ctx.
2019-11-04 18:16:53 +01:00
William Lallemand
3246d9466a BUG/MINOR: ssl/cli: fix an error when a file is not found
When trying to update a certificate <file>.{rsa,ecdsa,dsa}, but this one
does not exist and if <file> was used as a regular file in the
configuration, the error was ambiguous. Correct it so we can return a
certificate not found error.
2019-11-04 14:11:41 +01:00
William Lallemand
37031b85ca BUG/MINOR: ssl/cli: unable to update a certificate without bundle extension
Commit bc6ca7c ("MINOR: ssl/cli: rework 'set ssl cert' as 'set/commit'")
broke the ability to commit a unique certificate which does not use a
bundle extension .{rsa,ecdsa,dsa}.
2019-11-04 14:11:41 +01:00
William Lallemand
8a7fdf036b BUG/MEDIUM: ssl/cli: don't alloc path when cert not found
When doing an 'ssl set cert' with a certificate which does not exist in
configuration, the appctx->ctx.ssl.old_ckchs->path was duplicated while
app->ctx.ssl.old_ckchs was NULL, resulting in a NULL dereference.

Move the code so the 'not referenced' error is done before this.
2019-11-04 11:22:33 +01:00
Emmanuel Hocdet
40f2f1e341 BUG/MEDIUM: ssl/cli: fix dot research in cli_parse_set_cert
During a 'set ssl cert', the result of the strrchr was wrongly tested
and can lead to a segfault when the certificate path did not contained a
dot.
2019-10-31 17:32:06 +01:00
Emmanuel Hocdet
eaad5cc2d8 MINOR: ssl: BoringSSL ocsp_response does not need issuer
HAproxy can fail when issuer is not found, it must not with BoringSSL.
2019-10-31 17:24:16 +01:00
Emmanuel Hocdet
83cbd3c89f BUG/MINOR: ssl: double free on error for ckch->{key,cert}
On last error in ssl_sock_load_pem_into_ckch, key/cert are released
and ckch->{key,cert} are released in ssl_sock_free_cert_key_and_chain_contents.
2019-10-31 16:56:51 +01:00
Emmanuel Hocdet
ed17f47c71 BUG/MINOR: ssl: ckch->chain must be initialized
It's a regression from 96a9c973 "MINOR: ssl: split
ssl_sock_load_crt_file_into_ckch()".
2019-10-31 16:53:28 +01:00
Emmanuel Hocdet
f6ac4fa745 BUG/MINOR: ssl: segfault in cli_parse_set_cert with old openssl/boringssl
Fix 541a534 ("BUG/MINOR: ssl/cli: fix build of SCTL and OCSP") was not
enough.

[wla: It will probably be better later to put the #ifdef in the
functions so they can return an error if they are not implemented]
2019-10-31 16:21:06 +01:00
William Lallemand
33cc76f918 BUG/MINOR: ssl/cli: check trash allocation in cli_io_handler_commit_cert()
Possible NULL pointer dereference found by coverity.

Fix #350 #340.
2019-10-31 11:48:01 +01:00
William Lallemand
beea2a476e CLEANUP: ssl/cli: remove leftovers of bundle/certs (it < 2)
Remove the leftovers of the certificate + bundle updating in 'ssl set
cert' and 'commit ssl cert'.

* Remove the it variable in appctx.ctx.ssl.
* Stop doing everything twice.
* Indent
2019-10-30 17:52:34 +01:00
William Lallemand
bc6ca7ccaa MINOR: ssl/cli: rework 'set ssl cert' as 'set/commit'
This patch splits the 'set ssl cert' CLI command into 2 commands.

The previous way of updating the certificate on the CLI was limited with
the bundles. It was only able to apply one of the tree part of the
certificate during an update, which mean that we needed 3 updates to
update a full 3 certs bundle.

It was also not possible to apply atomically several part of a
certificate with the ability to rollback on error. (For example applying
a .pem, then a .ocsp, then a .sctl)

The command 'set ssl cert' will now duplicate the certificate (or
bundle) and update it in a temporary transaction..

The second command 'commit ssl cert' will commit all the changes made
during the transaction for the certificate.

This commit breaks the ability to update a certificate which was used as
a unique file and as a bundle in the HAProxy configuration. This way of
using the certificates wasn't making any sense.

Example:

	// For a bundle:

	$ echo -e "set ssl cert localhost.pem.rsa <<\n$(cat kikyo.pem.rsa)\n" | socat /tmp/sock1 -
	Transaction created for certificate localhost.pem!

	$ echo -e "set ssl cert localhost.pem.dsa <<\n$(cat kikyo.pem.dsa)\n" | socat /tmp/sock1 -
	Transaction updated for certificate localhost.pem!

	$ echo -e "set ssl cert localhost.pem.ecdsa <<\n$(cat kikyo.pem.ecdsa)\n" | socat /tmp/sock1 -
	Transaction updated for certificate localhost.pem!

	$ echo "commit ssl cert localhost.pem" | socat /tmp/sock1 -
	Committing localhost.pem.
	Success!
2019-10-30 17:01:07 +01:00
Willy Tarreau
0580052bb6 BUILD/MINOR: ssl: shut up a build warning about format truncation
Actually gcc believes it has detected a possible truncation but it
cannot since the output string is necessarily at least one char
shorter than what it expects. However addressing it is easy and
removes the need for an intermediate copy so let's do it.
2019-10-29 10:50:22 +01:00
William Lallemand
430413e285 MINOR: ssl/cli: rework the 'set ssl cert' IO handler
Rework the 'set ssl cert' IO handler so it is clearer.
Use its own SETCERT_ST_* states insted of the STAT_ST ones.

Use an inner loop in SETCERT_ST_GEN and SETCERT_ST_INSERT to do the work
for both the certificate and the bundle.

The io_release() is now called only when the CKCH spinlock is taken so
we can unlock during a release without any condition.
2019-10-28 14:57:37 +01:00
William Lallemand
1212db417b BUG/MINOR: ssl/cli: cleanup on cli_parse_set_cert error
Since commit 90b098c ("BUG/MINOR: cli: don't call the kw->io_release if
kw->parse failed"), the io_release() callback is not called anymore when
the parse() failed. Call it directly on the error path of the
cli_parse_set_cert() function.
2019-10-28 14:57:37 +01:00
William Lallemand
f29cdefccd BUG/MINOR: ssl/cli: out of bounds when built without ocsp/sctl
Commit 541a534 ("BUG/MINOR: ssl/cli: fix build of SCTL and OCSP")
introduced a bug in which we iterate outside the array durint a 'set ssl
cert' if we didn't built with the ocsp or sctl.
2019-10-23 15:05:00 +02:00
William Lallemand
541a534c9f BUG/MINOR: ssl/cli: fix build of SCTL and OCSP
Fix the build issue of SCTL and OCSP for boring/libressl introduced by
44b3532 ("MINOR: ssl/cli: update ocsp/issuer/sctl file from the CLI")
2019-10-23 14:47:16 +02:00
William Lallemand
8f840d7e55 MEDIUM: cli/ssl: handle the creation of SSL_CTX in an IO handler
To avoid affecting too much the traffic during a certificate update,
create the SNIs in a IO handler which yield every 10 ckch instances.

This way haproxy continues to respond even if we tries to update a
certificate which have 50 000 instances.
2019-10-23 11:54:51 +02:00
William Lallemand
0c3b7d9e1c MINOR: ssl/cli: assignate a new ckch_store
When updating a certificate from the CLI, it is not possible to revert
some of the changes if part of the certicate update failed. We now
creates a copy of the ckch_store for the changes so we can revert back
if something goes wrong.

Even if the ckch_store was affected before this change, it wasn't
affecting the SSL_CTXs used for the traffic. It was only a problem if we
try to update a certificate after we failed to do it the first time.

The new ckch_store is also linked to the new sni_ctxs so it's easy to
insert the sni_ctxs before removing the old ones.
2019-10-23 11:54:51 +02:00
William Lallemand
8c1cddef6d MINOR: ssl: new functions duplicate and free a ckch_store
ckchs_dup() alloc a new ckch_store and copy the content of its source.

ckchs_free() frees a ckch_store and its content.
2019-10-23 11:54:51 +02:00
William Lallemand
8d0f893222 MINOR: ssl: copy a ckch from src to dst
ssl_sock_copy_cert_key_and_chain() copy the content of a
<src> cert_key_and_chain to a <dst>.

It applies a refcount increasing on every SSL structures (X509, DH,
privte key..) and allocate new buffers for the other fields.
2019-10-23 11:54:51 +02:00
William Lallemand
455af50fac MINOR: ssl: update ssl_sock_free_cert_key_and_chain_contents
The struct cert_key_and_chain now contains the DH, the sctl and the
ocsp_response. Free them.
2019-10-23 11:54:51 +02:00
William Lallemand
44b3532250 MINOR: ssl/cli: update ocsp/issuer/sctl file from the CLI
It is now possible to update new parts of a CKCH from the CLI.

Currently you will be able to update a PEM (by default), a OCSP response
in base64, an issuer file, and a SCTL file.

Each update will creates a new CKCH and new sni_ctx structure so we will
need a "commit" command later to apply several changes and create the
sni_ctx only once.
2019-10-23 11:54:51 +02:00
William Lallemand
849eed6b25 BUG/MINOR: ssl/cli: fix looking up for a bundle
If we want a bundle but we didn't find a bundle, we shouldn't try to
apply the changes.
2019-10-23 11:54:51 +02:00
William Lallemand
96a9c97369 MINOR: ssl: split ssl_sock_load_crt_file_into_ckch()
Split the ssl_sock_load_crt_file_into_ckch() in two functions:

- ssl_sock_load_files_into_ckch() which is dedicated to opening every
files related to a filename during the configuration parsing (PEM, sctl,
ocsp, issuer etc)

- ssl_sock_load_pem_into_ckch() which is dedicated to opening a PEM,
either in a file or a buffer
2019-10-23 11:54:51 +02:00
William Lallemand
f9568fcd79 MINOR: ssl: load issuer from file or from buffer
ssl_sock_load_issuer_file_into_ckch() is a new function which is able to
load an issuer from a buffer or from a file to a CKCH.

Use this function directly in ssl_sock_load_crt_file_into_ckch()
2019-10-23 11:54:51 +02:00