Commit Graph

754 Commits

Author SHA1 Message Date
Christopher Faulet
d75f57e94c MINOR: ssl: Export a generic function to parse an alpn string
Parsing of an alpn string has been moved in a dedicated function and exposed to
be used from outside the ssl_sock module.
2020-04-27 09:39:37 +02:00
Christopher Faulet
8892e5d30b BUG/MEDIUM: server/checks: Init server check during config validity check
The options and directives related to the configuration of checks in a backend
may be defined after the servers declarations. So, initialization of the check
of each server must not be performed during configuration parsing, because some
info may be missing. Instead, it must be done during the configuration validity
check.

Thus, callback functions are registered to be called for each server after the
config validity check, one for the server check and another one for the server
agent-check. In addition deinit callback functions are also registered to
release these checks.

This patch should be backported as far as 1.7. But per-server post_check
callback functions are only supported since the 2.1. And the initcall mechanism
does not exist before the 1.9. Finally, in 1.7, the code is totally
different. So the backport will be harder on older versions.
2020-04-27 09:39:37 +02:00
Christopher Faulet
f61f33a1b2 BUG/MINOR: checks: Respect the no-check-ssl option
This options is used to force a non-SSL connection to check a SSL server or to
invert a check-ssl option inherited from the default section. The use_ssl field
in the check structure is used to know if a SSL connection must be used
(use_ssl=1) or not (use_ssl=0). The server configuration is used by default.

The problem is that we cannot distinguish the default case (no specific SSL
check option) and the case of an explicit non-SSL check. In both, use_ssl is set
to 0. So the server configuration is always used. For a SSL server, when
no-check-ssl option is set, the check is still performed using a SSL
configuration.

To fix the bug, instead of a boolean value (0=TCP, 1=SSL), we use a ternary value :

  * 0  = use server config
  * 1  = force SSL
  * -1 = force non-SSL

The same is done for the server parameter. It is not really necessary for
now. But it is a good way to know is the server no-ssl option is set.

In addition, the PR_O_TCPCHK_SSL proxy option is no longer used to set use_ssl
to 1 for a check. Instead the flag is directly tested to prepare or destroy the
server SSL context.

This patch should be backported as far as 1.8.
2020-04-27 09:39:37 +02:00
Jerome Magnin
b203ff6e20 MINOR: config: add a global directive to set default SSL curves
This commit adds a new keyword to the global section to set default
curves for ssl binds:
  - ssl-default-bind-curves
2020-04-22 17:26:08 +02:00
Jerome Magnin
2e8d52f869 BUG/MINOR: ssl: default settings for ssl server options are not used
Documentation states that default settings for ssl server options can be set
using either ssl-default-server-options or default-server directives. In practice,
not all ssl server options can have default values, such as ssl-min-ver, ssl-max-ver,
etc..

This patch adds the missing ssl options in srv_ssl_settings_cpy() and srv_parse_ssl(),
making it possible to write configurations like the following examples, and have them
behave as expected.

   global
     ssl-default-server-options ssl-max-ver TLSv1.2

   defaults
     mode http

   listen l1
     bind 1.2.3.4:80
     default-server ssl verify none
     server s1 1.2.3.5:443

   listen l2
     bind 2.2.3.4:80
     default-server ssl verify none ssl-max-ver TLSv1.3 ssl-min-ver TLSv1.2
     server s1 1.2.3.6:443

This should be backported as far as 1.8.
This fixes issue #595.
2020-04-22 15:43:03 +02:00
Emmanuel Hocdet
c3b7e74455 MINOR: ssl: add ssl-skip-self-issued-ca global option
This option activate the feature introduce in commit 16739778:
"MINOR: ssl: skip self issued CA in cert chain for ssl_ctx".
The patch disable the feature per default.
2020-04-22 15:35:56 +02:00
William Lallemand
916d0b523d MINOR: ssl/cli: restrain certificate path when inserting into a directory
When trying to insert a new certificate into a directory with "add ssl
crt-list", no check were done on the path of the new certificate.

To be more consistent with the HAProxy reload, when adding a file to
a crt-list, if this crt-list is a directory, the certificate will need
to have the directory in its path.
2020-04-21 18:42:42 +02:00
William Lallemand
b74d564043 MINOR: ssl/cli: disallow SSL options for directory in 'add ssl crt-list'
Allowing the use of SSL options and filters when adding a file in a
directory is not really consistent with the reload of HAProxy. Disable
the ability to use these options if one try to use them with a directory.
2020-04-21 17:23:54 +02:00
William Lallemand
1b2988bc42 MINOR: ssl: don't alloc ssl_conf if no option found
When no SSL options were found between brackets, the structure ssl_conf
was still allocated for nothing.
2020-04-10 17:43:58 +02:00
William Lallemand
87a0db9993 BUG/MINOR: ssl: ssl_conf always set to NULL on crt-list parsing
When reading a crt-list file, the SSL options betweeen square brackets
are parsed, however the calling function sets the ssl_conf ptr to NULL
leading to all options being ignored, and a memory leak.

This is a remaining of the previous code which was forgotten.

This bug was introduced by 97b0810 ("MINOR: ssl: split the line parsing
of the crt-list").
2020-04-10 17:43:58 +02:00
William Lallemand
e718dfb4c2 MINOR: ssl: crtlist_entry_{new, free}
New functions that create and delete a crtlist_entry in order to remove
duplicated code.
2020-04-10 11:14:01 +02:00
William Lallemand
82b21bbe86 REORG: ssl: move some free/new functions
Move crtlist_free_filters(), crtlist_dup_filters(),
crtlist_free(), crtlist_new(), ssl_sock_free_ssl_conf() upper in the
file.
2020-04-10 11:14:01 +02:00
William Lallemand
ec2d493621 MINOR: ssl: crtlist_new() alloc and initialize a struct crtlist
Allocate and initialize a struct crtlist with crtlist_new() to remove
duplicated code.
2020-04-10 11:14:01 +02:00
William Lallemand
8a874e4c6a MINOR: ssl: ckch_store_new() alloc and init a ckch_store
Create a ckch_store_new() function which alloc and initialize a
ckch_store, allowing us to remove duplicated code and avoiding wrong
initialization in the future.
2020-04-10 11:14:01 +02:00
William Lallemand
d5e9377312 BUG/MEDIUM: ssl/cli: trying to access to free'd memory
Bug introduced by d9d5d1b ("MINOR: ssl: free instances and SNIs with
ckch_inst_free()").

Upon an 'commit ssl cert' the HA_RWLOCK_WRUNLOCK of the SNI lock is done
with using the bind_conf pointer of the ckch_inst which was freed.

Fix the problem by using an intermediate variable to store the
bind_conf pointer.
2020-04-09 17:12:16 +02:00
William Lallemand
ba1c33f826 MINOR: ssl: replace ckchs_free() by ckch_store_free()
Replace ckchs_free() by ckch_store_free() which frees the ckch_store but
now also all its ckch_inst with ckch_inst_free().

Also remove the "ckchs" naming since its confusing.
2020-04-09 17:00:18 +02:00
William Lallemand
d9d5d1b1df MINOR: ssl: free instances and SNIs with ckch_inst_free()
Remove duplicated code by creating a new function ckch_inst_free() which
deals with the SNIs linked in a ckch_inst and free the ckch_inst.
2020-04-09 16:51:29 +02:00
William Lallemand
9cef2e2c06 MINOR: ssl: initialize all list in ckch_inst_new()
The ckch_inst_new() function is not up to date with the latest
list added into the structure. Update the list of structure to
initialize.
2020-04-09 16:46:50 +02:00
William Lallemand
8621ac5570 BUG/MINOR: ssl: memleak of the struct cert_key_and_chain
Free the struct cert_key_and_chain when calling ckchs_free(),
a memory leak can occur when using 'commit ssl cert'.

Must be backported to 2.1.
2020-04-09 15:40:26 +02:00
William Lallemand
caa161982f CLEANUP: ssl/cli: use the list of filters in the crtlist_entry
In 'commit ssl cert', instead of trying to regenerate a list of filters
from the SNIs, use the list provided by the crtlist_entry used to
generate the ckch_inst.

This list of filters doesn't need to be free'd anymore since they are
always reused from the crtlist_entry.
2020-04-08 16:52:51 +02:00
William Lallemand
02e19a5c7b CLEANUP: ssl: use the refcount for the SSL_CTX'
Use the refcount of the SSL_CTX' to free them instead of freeing them on
certains conditions. That way we can free the SSL_CTX everywhere its
pointer is used.
2020-04-08 16:52:51 +02:00
William Lallemand
24be710609 BUG/MINOR: ssl/cli: memory leak in 'set ssl cert'
When deleting the previous SNI entries with 'set ssl cert', the old
SSL_CTX' were not free'd, which probably prevent the completion of the
free of the X509 in the old ckch_store, because of the refcounts in the
SSL library.

This bug was introduced by 150bfa8 ("MEDIUM: cli/ssl: handle the
creation of SSL_CTX in an IO handler").

Must be backported to 2.1.
2020-04-08 15:29:10 +02:00
William Lallemand
41ca930e58 BUG/MINOR: ssl: trailing slashes in directory names wrongly cached
The crtlist_load_cert_dir() caches the directory name without trailing
slashes when ssl_sock_load_cert_list_file() tries to lookup without
cleaning the trailing slashes.

This bug leads to creating the crtlist twice and prevents to remove
correctly a crtlist_entry since it exists in the serveral crtlists
created by accident.

Move the trailing slashes cleanup in ssl_sock_load_cert_list_file() to
fix the problem.

This bug was introduced by 6be66ec ("MINOR: ssl: directories are loaded
like crt-list")
2020-04-08 13:28:07 +02:00
William Lallemand
419e6349f6 MINOR: ssl/cli: 'del ssl cert' deletes a certificate
Delete a certificate store from HAProxy and free its memory. The
certificate must be unused and removed from any crt-list or directory.
The deletion doesn't work with a certificate referenced directly with
the "crt" directive in the configuration.
2020-04-08 12:08:03 +02:00
William Lallemand
36ccc3922d MINOR: ssl/cli: improve error for bundle in add/del ssl crt-list
Bundles are deprecated and can't be used with the crt-list command of
the CLI, improve the error output when trying to use them so the users
can disable them.
2020-04-08 11:01:44 +02:00
William Lallemand
463b524298 BUG/MINOR: ssl/cli: lock the ckch structures during crt-list delete
The cli_parse_del_crtlist() does unlock the ckch big lock, but it does
not lock it at the beginning of the function which is dangerous.
As a side effect it let the structures locked once it called the unlock.

This bug was introduced by 0a9b941 ("MINOR: ssl/cli: 'del ssl crt-list'
delete an entry")
2020-04-08 10:39:38 +02:00
William Lallemand
7fd01b3625 MINOR: ssl: improve the errors when a crt can't be open
Issue #574 reported an unclear error when trying to open a file with not
enough permission.

  [ALERT] 096/032117 (835) : parsing [/etc/haproxy/haproxy.cfg:54] : 'bind :443' : error encountered while processing 'crt'.
  [ALERT] 096/032117 (835) : Error(s) found in configuration file : /etc/haproxy/haproxy.cfg
  [ALERT] 096/032117 (835) : Fatal errors found in configuration.

Improve the error to give us more information:

  [ALERT] 097/142030 (240089) : parsing [test.cfg:22] : 'bind :443' : cannot open the file 'kikyo.pem.rsa'.
  [ALERT] 097/142030 (240089) : Error(s) found in configuration file : test.cfg
  [ALERT] 097/142030 (240089) : Fatal errors found in configuration.

This patch could be backported in 2.1.
2020-04-07 14:26:54 +02:00
William Lallemand
c69f02d0f0 MINOR: ssl/cli: replace dump/show ssl crt-list by '-n' option
The dump and show ssl crt-list commands does the same thing, they dump
the content of a crt-list, but the 'show' displays an ID in the first
column. Delete the 'dump' command so it is replaced by the 'show' one.
The old 'show' command is replaced by an '-n' option to dump the ID.
And the ID which was a pointer is replaced by a line number and placed
after colons in the filename.

Example:
  $ echo "show ssl crt-list -n kikyo.crt-list" | socat /tmp/sock1 -
  # kikyo.crt-list
  kikyo.pem.rsa:1 secure.domain.tld
  kikyo.pem.ecdsa:2 secure.domain.tld
2020-04-06 19:33:33 +02:00
William Lallemand
0a9b9414f0 MINOR: ssl/cli: 'del ssl crt-list' delete an entry
Delete an entry in a crt-list, this is done by iterating over the
ckch_inst in the crtlist_entry. For each ckch_inst the bind_conf lock is
held during the deletion of the sni_ctx in the SNI trees. Everything
is free'd.

If there is several entries with the same certificate, a line number
must be provided to chose with entry delete.
2020-04-06 19:33:28 +02:00
William Lallemand
58a522227b BUG/MINOR: ssl/cli: fix spaces in 'show ssl crt-list'
Fix a inconsistency in the spaces which were not printed everywhere if
there was no SSL options but some filters.
2020-04-02 18:15:30 +02:00
William Lallemand
a690fed5be BUG/MINOR: ssl/cli: initialize fcount int crtlist_entry
Initialize fcount to 0 when 'add ssl crt-list' does not contain any
filters. This bug can lead to trying to read some filters even if they
doesn't exist.
2020-04-02 15:40:19 +02:00
William Lallemand
557823f847 MINOR: ssl: add a comment above the ssl_bind_conf keywords
Add a warning above the ssl_bind_conf keywords list so developers check
if their keywords are relevant for the list.
2020-04-01 20:10:53 +02:00
William Lallemand
c7c7a6b39f MINOR: ssl/cli: support filters and options in add ssl crt-list
Add the support for filters and SSL options in the CLI command
"add ssl crt-list".

The feature was implemented by applying the same parser as the crt-list
file to the payload.

The new options are passed to the command as a payload with the same
format that is suppported by the crt-list file itself, so you can easily
copy a line from a file and push it via the CLI.

Example:
  printf "add ssl crt-list localhost.crt-list <<\necdsa.pem [verify none allow-0rtt] localhost !www.test1.com\n\n" | socat /tmp/sock1 -
2020-04-01 20:10:53 +02:00
William Lallemand
97b0810f4c MINOR: ssl: split the line parsing of the crt-list
In order to reuse the crt-list line parsing in "add ssl crt-list",
the line parsing was extracted from crtlist_parse_file() to a new
function crtlist_parse_line().

This function fills a crtlist_entry structure with a bind_ssl_conf and
the list of filters.
2020-04-01 20:10:53 +02:00
William Lallemand
c2e3b72adf BUG/MINOR: ssl: entry->ckch_inst not initialized
The head of the list entry->ckch_inst was not initialized when opening a
directory or reading a crt-list.
2020-03-31 14:40:51 +02:00
William Lallemand
e67c80be7f MEDIUM: ssl/cli: 'add ssl crt-list' command
The new 'add ssl crt-list' command allows to add a new crt-list entry in
a crt-list (or a directory since they are handled the same way).

The principle is basicaly the same as the "commit ssl cert" command with
the exception that it iterates on every bind_conf that uses the crt-list
and generates a ckch instance (ckch_inst) for each of them.

The IO handler yield every 10 bind_confs so HAProxy does not get stuck in
a too much time consuming generation if it needs to generate too much
SSL_CTX'.

This command does not handle the SNI filters and the SSL configuration
yet.

Example:

  $ echo "new ssl cert foo.net.pem" | socat /tmp/sock1 -
  New empty certificate store 'foo.net.pem'!

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

  $ echo "commit ssl cert foo.net.pem" | socat /tmp/sock1 -
  Committing foo.net.pem
  Success!

  $ echo "add ssl crt-list one.crt-list foo.net.pem" | socat /tmp/sock1 -
  Inserting certificate 'foo.net.pem' in crt-list 'one.crt-list'......
  Success!

  $ echo "show ssl crt-list one.crt-list" | socat /tmp/sock1 -
  # one.crt-list
  0x55d17d7be360 kikyo.pem.rsa [ssl-min-ver TLSv1.0 ssl-max-ver TLSv1.3]
  0x55d17d82cb10 foo.net.pem
2020-03-31 12:32:18 +02:00
William Lallemand
90afe90681 MINOR: ssl/cli: update pointer to store in 'commit ssl cert'
The crtlist_entry structure use a pointer to the store as key.
That's a problem with the dynamic update of a certificate over the CLI,
because it allocates a new ckch_store. So updating the pointers is
needed. To achieve that, a linked list of the crtlist_entry was added in
the ckch_store, so it's easy to iterate on this list to update the
pointers. Another solution would have been to rework the system so we
don't allocate a new ckch_store, but it requires a rework of the ckch
code.
2020-03-31 12:32:17 +02:00
William Lallemand
fa8cf0c476 MINOR: ssl: store a ptr to crtlist in crtlist_entry
Store a pointer to crtlist in crtlist_entry so we can re-insert a
crtlist_entry in its crtlist ebpt after updating its key.
2020-03-31 12:32:17 +02:00
William Lallemand
23d61c00b9 MINOR: ssl: add a list of crtlist_entry in ckch_store
When updating a ckch_store we may want to update its pointer in the
crtlist_entry which use it. To do this, we need the list of the entries
using the store.
2020-03-31 12:32:17 +02:00
William Lallemand
09bd5a0787 MINOR: ssl: use crtlist_free() upon error in directory loading
Replace the manual cleaninp which is done in crtlist_load_cert_dir() by
a call to the crtlist_free() function.
2020-03-31 12:32:17 +02:00
William Lallemand
4c68bba5c1 REORG: ssl: move some functions above crtlist_load_cert_dir()
Move some function above crtlist_load_cert_dir() so
crtlist_load_cert_dir() is at the right place, and crtlist_free() can be
used inside.
2020-03-31 12:32:17 +02:00
William Lallemand
493983128b BUG/MINOR: ssl: ckch_inst wrongly inserted in crtlist_entry
The instances were wrongly inserted in the crtlist entries, all
instances of a crt-list were inserted in the last crt-list entry.
Which was kind of handy to free all instances upon error.

Now that it's done correctly, the error path was changed, it must
iterate on the entries and find the ckch_insts which were generated for
this bind_conf. To avoid wasting time, it stops the iteration once it
found the first unsuccessful generation.
2020-03-31 12:32:17 +02:00
William Lallemand
ad3c37b760 REORG: ssl: move SETCERT enum to ssl_sock.h
Move the SETCERT enum at the right place to cleanup ssl_sock.c.
2020-03-31 12:32:17 +02:00
William Lallemand
79d31ec0d4 MINOR: ssl: add a list of bind_conf in struct crtlist
In order to be able to add new certificate in a crt-list, we need the
list of bind_conf that uses this crt-list so we can create a ckch_inst
for each of them.
2020-03-31 12:32:17 +02:00
Emmanuel Hocdet
1673977892 MINOR: ssl: skip self issued CA in cert chain for ssl_ctx
First: self issued CA, aka root CA, is the enchor for chain validation,
no need to send it, client must have it. HAProxy can skip it in ssl_ctx.
Second: the main motivation to skip root CA in ssl_ctx is to be able to
provide it in the chain without drawback. Use case is to provide issuer
for ocsp without the need for .issuer and be able to share it in
issuers-chain-path. This concerns all certificates without intermediate
certificates. It's useless for BoringSSL, .issuer is ignored because ocsp
bits doesn't need it.
2020-03-26 12:53:53 +01:00
Emmanuel Hocdet
4fed93eb72 MINOR: ssl: rework add cert chain to CTX to be libssl independent
SSL_CTX_set1_chain is used for openssl >= 1.0.2 and a loop with
SSL_CTX_add_extra_chain_cert for openssl < 1.0.2.
SSL_CTX_add_extra_chain_cert exist with openssl >= 1.0.2 and can be
used for all openssl version (is new name is SSL_CTX_add0_chain_cert).
This patch use SSL_CTX_add_extra_chain_cert to remove any #ifdef for
compatibilty. In addition sk_X509_num()/sk_X509_value() replace
sk_X509_shift() to extract CA from chain, as it is used in others part
of the code.
2020-03-24 14:46:01 +01:00
Emmanuel Hocdet
ef87e0a3da CLEANUP: ssl: rename ssl_get_issuer_chain to ssl_get0_issuer_chain
Rename ssl_get_issuer_chain to ssl_get0_issuer_chain to be consistent
with openssl >= 1.0.2 API.
2020-03-23 15:35:39 +01:00
Emmanuel Hocdet
f4f14eacd3 BUG/MINOR: ssl: memory leak when find_chain is NULL
This bug was introduced by 85888573 "BUG/MEDIUM: ssl: chain must be
initialized with sk_X509_new_null()". No need to set find_chain with
sk_X509_new_null(), use find_chain conditionally to fix issue #516.

This bug was referenced by issue #559.

[wla: fix some alignment/indentation issue]
2020-03-23 13:10:10 +01:00
William Lallemand
18eeb8e815 BUG/MINOR: ssl/cli: fix a potential NULL dereference
Fix a potential NULL dereference in "show ssl cert" when we can't
allocate the <out> trash buffer.

This patch creates a new label so we could jump without trying to do the
ci_putchk in this case.

This bug was introduced by ea987ed ("MINOR: ssl/cli: 'new ssl cert'
command"). 2.2 only.

This bug was referenced by issue #556.
2020-03-20 14:49:25 +01:00
William Lallemand
67b991d370 BUG/MINOR: ssl/cli: free BIO upon error in 'show ssl cert'
Fix a memory leak that could happen upon a "show ssl cert" if notBefore:
or notAfter: failed to extract its ASN1 string.

Introduced by d4f946c ("MINOR: ssl/cli: 'show ssl cert' give information
on the certificates"). 2.2 only.
2020-03-20 14:22:35 +01:00