Commit Graph

682 Commits

Author SHA1 Message Date
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
5f1fa7db86 and
8b453912ce

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: bc6ca7ccaa ("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 8b453912ce.
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 8b453912ce ("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: 8b453912ce ("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
William Lallemand
0dfae6c315 MINOR: ssl: load sctl from buf OR from a file
The ssl_sock_load_sctl_from_file() function was modified to
fill directly a struct cert_key_and_chain.

The function prototype was normalized in order to be used with the CLI
payload parser.

This function either read  text from a buffer or read a file on the
filesystem.

It fills the ocsp_response buffer of the struct cert_key_and_chain.
2019-10-23 11:54:51 +02:00
William Lallemand
3b5f360744 MINOR: ssl: OCSP functions can load from file or buffer
The ssl_sock_load_ocsp_response_from_file() function was modified to
fill directly a struct cert_key_and_chain.

The function prototype was normalized in order to be used with the CLI
payload parser.

This function either read a base64 from a buffer or read a binary file
on the filesystem.

It fills the ocsp_response buffer of the struct cert_key_and_chain.
2019-10-23 11:54:51 +02:00
William Lallemand
02010478e9 CLEANUP: ssl: fix SNI/CKCH lock labels
The CKCH and the SNI locks originally used the same label, we split them
but we forgot to change some of them.
2019-10-23 11:54:51 +02:00
William Lallemand
34779c34fc CLEANUP: ssl: remove old TODO commentary
Remove an old commentary above ckch_inst_new_load_multi_store().
This function doe not do filesystem syscalls anymore.
2019-10-23 11:54:51 +02:00
Emeric Brun
eb46965bbb BUG/MINOR: ssl: fix memcpy overlap without consequences.
A trick is used to set SESSION_ID, and SESSION_ID_CONTEXT lengths
to 0 and avoid ASN1 encoding of these values.

There is no specific function to set the length of those parameters
to 0 so we fake this calling these function to a different value
with the same buffer but a length to zero.

But those functions don't seem to check the length of zero before
performing a memcpy of length zero but with src and dst buf on the
same pointer, causing valgrind to bark.

So the code was re-work to pass them different pointers even
if buffer content is un-used.

In a second time, reseting value, a memcpy overlap
happened on the SESSION_ID_CONTEXT. It was re-worked and this is
now reset using the constant global value SHCTX_APPNAME which is a
different pointer with the same content.

This patch should be backported in every version since ssl
support was added to haproxy if we want valgrind to shut up.
This is tracked in github issue #56.
2019-10-22 18:57:45 +02:00
Christopher Faulet
e566f3db11 BUG/MINOR: ssl: Fix fd leak on error path when a TLS ticket keys file is parsed
When an error occurred in the function bind_parse_tls_ticket_keys(), during the
configuration parsing, the opened file is not always closed. To fix the bug, all
errors are catched at the same place, where all ressources are released.

This patch fixes the bug #325. It must be backported as far as 1.7.
2019-10-21 10:04:51 +02:00
Emeric Brun
a9363eb6a5 BUG/MEDIUM: ssl: 'tune.ssl.default-dh-param' value ignored with openssl > 1.1.1
If openssl 1.1.1 is used, c2aae74f0 commit mistakenly enables DH automatic
feature from openssl instead of ECDH automatic feature. There is no impact for
the ECDH one because the feature is always enabled for that version. But doing
this, the 'tune.ssl.default-dh-param' was completely ignored for DH parameters.

This patch fix the bug calling 'SSL_CTX_set_ecdh_auto' instead of
'SSL_CTX_set_dh_auto'.

Currently some users may use a 2048 DH bits parameter, thinking they're using a
1024 bits one. Doing this, they may experience performance issue on light hardware.

This patch warns the user if haproxy fails to configure the given DH parameter.
In this case and if openssl version is > 1.1.0, haproxy will let openssl to
automatically choose a default DH parameter. For other openssl versions, the DH
ciphers won't be usable.

A commonly case of failure is due to the security level of openssl.cnf
which could refuse a 1024 bits DH parameter for a 2048 bits key:

 $ cat /etc/ssl/openssl.cnf
 ...

 [system_default_sect]
 MinProtocol = TLSv1
 CipherString = DEFAULT@SECLEVEL=2

This should be backport into any branch containing the commit c2aae74f0.
It requires all or part of the previous CLEANUP series.

This addresses github issue #324.
2019-10-18 15:18:52 +02:00
Emeric Brun
7a88336cf8 CLEANUP: ssl: make ssl_sock_load_dh_params handle errcode/warn
ssl_sock_load_dh_params used to return >0 or -1 to indicate success
or failure. Make it return a set of ERR_* instead so that its callers
can transparently report its status. Given that its callers only used
to know about ERR_ALERT | ERR_FATAL, this is the only code returned
for now. An error message was added in the case of failure and the
comment was updated.
2019-10-18 15:18:52 +02:00
Emeric Brun
a96b582d0e CLEANUP: ssl: make ssl_sock_put_ckch_into_ctx handle errcode/warn
ssl_sock_put_ckch_into_ctx used to return 0 or >0 to indicate success
or failure. Make it return a set of ERR_* instead so that its callers
can transparently report its status. Given that its callers only used
to know about ERR_ALERT | ERR_FATAL, this is the only code returned
for now. And a comment was updated.
2019-10-18 15:18:52 +02:00
Emeric Brun
054563de13 CLEANUP: ssl: make ckch_inst_new_load_(multi_)store handle errcode/warn
ckch_inst_new_load_store() and ckch_inst_new_load_multi_store used to
return 0 or >0 to indicate success or failure. Make it return a set of
ERR_* instead so that its callers can transparently report its status.
Given that its callers only used to know about ERR_ALERT | ERR_FATAL,
his is the only code returned for now. And the comment was updated.
2019-10-18 15:18:52 +02:00
Emeric Brun
f69ed1d21c CLEANUP: ssl: make cli_parse_set_cert handle errcode and warnings.
cli_parse_set_cert was re-work to show errors and warnings depending
of ERR_* bitfield value.
2019-10-18 15:18:52 +02:00
Willy Tarreau
8c5414a546 CLEANUP: ssl: make ssl_sock_load_ckchs() return a set of ERR_*
ssl_sock_load_ckchs() used to return 0 or >0 to indicate success or
failure even though this was not documented. Make it return a set of
ERR_* instead so that its callers can transparently report its status.
Given that its callers only used to know about ERR_ALERT | ERR_FATAL,
this is the only code returned for now. And a comment was added.
2019-10-18 15:18:52 +02:00
Willy Tarreau
bbc91965bf CLEANUP: ssl: make ssl_sock_load_cert*() return real error codes
These functions were returning only 0 or 1 to mention success or error,
and made it impossible to return a warning. Let's make them return error
codes from ERR_* and map all errors to ERR_ALERT|ERR_FATAL for now since
this is the only code that was set on non-zero return value.

In addition some missing comments were added or adjusted around the
functions' return values.
2019-10-18 15:18:52 +02:00
William Lallemand
e0f48ae976 BUG/MINOR: ssl: can't load ocsp files
246c024 ("MINOR: ssl: load the ocsp in/from the ckch") broke the loading
of OCSP files. The function ssl_sock_load_ocsp_response_from_file() was
not returning 0 upon success which lead to an error after the .ocsp was
read.
2019-10-15 13:50:20 +02:00
William Lallemand
786188f6bf BUG/MINOR: ssl: fix error messages for OCSP loading
The error messages for OCSP in ssl_sock_load_crt_file_into_ckch() add a
double extension to the filename, that can be confusing. The messages
reference a .issuer.issuer file.
2019-10-15 13:50:20 +02:00
William Lallemand
4a66013069 BUG/MINOR: ssl: fix OCSP build with BoringSSL
246c024 broke the build of the OCSP code with BoringSSL.

Rework it a little so it could load the OCSP buffer of the ckch.

Issue #322.
2019-10-14 15:07:44 +02:00
William Lallemand
104a7a6c14 BUILD: ssl: wrong #ifdef for SSL engines code
The SSL engines code was written below the OCSP #ifdef, which means you
can't build the engines code if the OCSP is deactived in the SSL lib.

Could be backported in every version since 1.8.
2019-10-14 15:07:44 +02:00
William Lallemand
963b2e70ba BUG/MINOR: ssl: fix build without multi-cert bundles
Commit 150bfa8 broke the build with ssl libs that does not support
multi certificate bundles.

Issue #322.
2019-10-14 11:41:18 +02:00
William Lallemand
e15029bea9 BUG/MEDIUM: ssl: NULL dereference in ssl_sock_load_cert_sni()
A NULL dereference can occur when inserting SNIs. In the case of
checking for duplicates, if there is already several sni_ctx with the
same key.

Fix issue #321.
2019-10-14 10:57:16 +02:00
William Lallemand
246c0246d3 MINOR: ssl: load the ocsp in/from the ckch
Don't try to load the files containing the issuer and the OCSP response
each time we generate a SSL_CTX.

The .ocsp and the .issuer are now loaded in the struct
cert_key_and_chain only once and then loaded from this structure when
creating a SSL_CTX.
2019-10-11 17:32:03 +02:00
William Lallemand
a17f4116d5 MINOR: ssl: load the sctl in/from the ckch
Don't try to load the file containing the sctl each time we generate a
SSL_CTX.

The .sctl is now loaded in the struct cert_key_and_chain only once and
then loaded from this structure when creating a SSL_CTX.

Note that this now make possible the use of sctl with multi-cert
bundles.
2019-10-11 17:32:03 +02:00
William Lallemand
150bfa84e3 MEDIUM: ssl/cli: 'set ssl cert' updates a certificate from the CLI
$ echo -e "set ssl cert certificate.pem <<\n$(cat certificate2.pem)\n" | \
    socat stdio /var/run/haproxy.stat
    Certificate updated!

The operation is locked at the ckch level with a HA_SPINLOCK_T which
prevents the ckch architecture (ckch_store, ckch_inst..) to be modified
at the same time. So you can't do a certificate update at the same time
from multiple CLI connections.

SNI trees are also locked with a HA_RWLOCK_T so reading operations are
locked only during a certificate update.

Bundles are supported but you need to update each file (.rsa|ecdsa|.dsa)
independently. If a file is used in the configuration as a bundle AND
as a unique certificate, both will be updated.

Bundles, directories and crt-list are supported, however filters in
crt-list are currently unsupported.

The code tries to allocate every SNIs and certificate instances first,
so it can rollback the operation if that was unsuccessful.

If you have too much instances of the certificate (at least 20000 in my
tests on my laptop), the function can take too much time and be killed
by the watchdog. This will be fixed later. Also with too much
certificates it's possible that socat exits before the end of the
generation without displaying a message, consider changing the socat
timeout in this case (-t2 for example).

The size of the certificate is currently limited by the maximum size of
a payload, that must fit in a buffer.
2019-10-11 17:32:03 +02:00
William Lallemand
f11365b26a MINOR: ssl: ssl_sock_load_crt_file_into_ckch() is filling from a BIO
The function ssl_sock_load_crt_file_into_ckch() is now able to fill a
ckch using a BIO in input.
2019-10-11 17:32:03 +02:00
William Lallemand
614ca0d370 MEDIUM: ssl: ssl_sock_load_ckchs() alloc a ckch_inst
The ssl_sock_load_{multi}_ckchs() function were renamed and modified:

- allocate a ckch_inst and loads the sni in it
- return a ckch_inst or NULL
- the sni_ctx are not added anymore in the sni trees from there
- renamed in ckch_inst_new_load_{multi}_store()
- new ssl_sock_load_ckchs() function calls
ckch_inst_new_load_{multi}_store() and add the sni_ctx to the sni trees.
2019-10-11 17:32:03 +02:00
William Lallemand
0c6d12fb66 MINOR: ssl: ssl_sock_load_multi_ckchs() can properly fail
ssl_sock_load_multi_ckchs() is now able to fail without polluting the
bind_conf trees and leaking memory.

It is a prerequisite to load certificate on-the-fly with the CLI.

The insertion of the sni_ctxs in the trees are done once everything has
been allocated correctly.
2019-10-11 17:32:03 +02:00
William Lallemand
d919937991 MINOR: ssl: ssl_sock_load_ckchn() can properly fail
ssl_sock_load_ckchn() is now able to fail without polluting the
bind_conf trees and leaking memory.

It is a prerequisite to load certificate on-the-fly with the CLI.

The insertion of the sni_ctxs in the trees are done once everything has
been allocated correctly.
2019-10-11 17:32:03 +02:00
William Lallemand
1d29c7438e MEDIUM: ssl: split ssl_sock_add_cert_sni()
In order to allow the creation of sni_ctx in runtime, we need to split
the function to allow rollback.

We need to be able to allocate all sni_ctxs required before inserting
them in case we need to rollback if we didn't succeed the allocation.

The function was splitted in 2 parts.

The first one ckch_inst_add_cert_sni() allocates a struct sni_ctx, fill
it with the right data and insert it in the ckch_inst's list of sni_ctx.

The second will take every sni_ctx in the ckch_inst and insert them in
the bind_conf's sni tree.
2019-10-11 17:32:03 +02:00
William Lallemand
9117de9e37 MEDIUM: ssl: introduce the ckch instance structure
struct ckch_inst represents an instance of a certificate (ckch_node)
used in a bind_conf. Every sni_ctx created for 1 ckch_node in a
bind_conf are linked in this structure.

This patch allocate the ckch_inst for each bind_conf and inserts the
sni_ctx in its linked list.
2019-10-11 17:32:03 +02:00
William Lallemand
28a8fce485 BUG/MINOR: ssl: abort on sni_keytypes allocation failure
The ssl_sock_populate_sni_keytypes_hplr() function does not return an
error upon an allocation failure.

The process would probably crash during the configuration parsing if the
allocation fail since it tries to copy some data in the allocated
memory.

This patch could be backported as far as 1.5.
2019-10-11 17:32:02 +02:00
William Lallemand
8ed5b96587 BUG/MINOR: ssl: free the sni_keytype nodes
This patch frees the sni_keytype nodes once the sni_ctxs have been
allocated in ssl_sock_load_multi_ckchn();

Could be backported in every version using the multi-cert SSL bundles.
2019-10-11 17:32:02 +02:00
William Lallemand
fe49bb3d0c BUG/MINOR: ssl: abort on sni allocation failure
The ssl_sock_add_cert_sni() function never return an error when a
sni_ctx allocation fail. It silently ignores the problem and continues
to try to allocate other snis.

It is unlikely that a sni allocation will succeed after one failure and
start a configuration without all the snis. But to avoid any problem we
return a -1 upon an sni allocation error and stop the configuration
parsing.

This patch must be backported in every version supporting the crt-list
sni filters. (as far as 1.5)
2019-10-11 17:32:02 +02:00
William Lallemand
4b989f2fac MINOR: ssl: initialize the sni_keytypes_map as EB_ROOT
The sni_keytypes_map was initialized to {0}, it's better to initialize
it explicitly to EB_ROOT
2019-10-11 17:32:02 +02:00
William Lallemand
f6adbe9f28 REORG: ssl: move structures to ssl_sock.h 2019-10-11 17:32:02 +02:00
William Lallemand
e3af8fbad3 REORG: ssl: rename ckch_node to ckch_store
A ckch_store is a storage which contains a pointer to one or several
cert_key_and_chain structures.

This patch renames ckch_node to ckch_store, and ckch_n, ckchn to ckchs.
2019-10-11 17:32:02 +02:00
William Lallemand
eed4bf234e MINOR: ssl: crt-list do ckchn_lookup 2019-10-11 17:32:02 +02:00
William Lallemand
1633e39d91 BUILD: ssl: fix a warning when built with openssl < 1.0.2
src/ssl_sock.c:2928:12: warning: ‘ssl_sock_is_ckch_valid’ defined but not used [-Wunused-function]
 static int ssl_sock_is_ckch_valid(struct cert_key_and_chain *ckch)

This function is only used with openssl >= 1.0.2, this patch adds a
condition to build the function.
2019-09-30 13:40:53 +02:00
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