Commit Graph

620 Commits

Author SHA1 Message Date
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