229 Commits

Author SHA1 Message Date
Remi Tricot-Le Breton
1fda0a5202 BUG/MINOR: ssl: Duplicate ocsp update mode when dup'ing ckch
When calling ckchs_dup (during a "set ssl cert" CLI command), if the
modified store had OCSP auto update enabled then the new certificate
would not keep the previous update mode and would not appear in the auto
update list.

This patch can be backported to 2.8.
2024-02-07 17:09:34 +01:00
William Lallemand
0bf9d122a9 MEDIUM: ssl: generate '*' SNI filters for default certificates
This patch follows the previous one about default certificate selection
("MEDIUM: ssl: allow multiple fallback certificate to allow ECDSA/RSA
selection").

This patch generates '*" SNI filters for the first certificate of a
bind line, it will be used to match default certificates. Instead of
setting the default_ctx pointer in the bind line.

Since the filters are in the SNI tree, it allows to have multiple
default certificate and restore the ecdsa/rsa selection with a
multi-cert bundle.

This configuration:
   # foobar.pem.ecdsa and foobar.pem.rsa
   bind *:8443 ssl crt foobar.pem crt next.pem

will use "foobar.pem.ecdsa" and "foobar.pem.rsa" as default
certificates.

Note: there is still cleanup needed around default_ctx.

This was discussed in github issue #2392.
2024-01-12 17:40:42 +01:00
Frédéric Lécaille
75f5977ff4 BUG/MINOR: ssl: Wrong OCSP CID after modifying an SSL certficate
This bug could be reproduced with the "set ssl cert" CLI command to update
a certificate. The OCSP CID is duplicated by ckchs_dup() which calls
ssl_sock_copy_cert_key_and_chain(). It should be computed again by
ssl_sock_load_ocsp(). This may be accomplished resetting the new ckch OCSP CID
returned by ckchs_dup().

This bug may be in relation with GH #2319.

Must be backported to 2.8.
2023-12-06 16:12:08 +01:00
Frédéric Lécaille
456ba6e95f MINOR: ssl/cli: Add ha_(warning|alert) msgs to CLI ckch callback
This patch allows cli_io_handler_commit_cert() callback called upon
a "commit ssl cert ..." command to prefix the messages returned by the CLI
to the by the ones built by ha_warining(), ha_alert().

Should be interesting to backport this commit to 2.8.
2023-12-06 16:12:08 +01:00
Ilya Shipitsin
80813cdd2a CLEANUP: assorted typo fixes in the code and comments
This is 37th iteration of typo fixes
2023-11-23 16:23:14 +01:00
Miroslav Zagorac
3cfc30416c MINOR: properly mark the end of the CLI command in error messages
In several places in the file src/ssl_ckch.c, in the message about the
incorrect use of the CLI command, the end of that CLI command is not
correctly marked with the sign ' .
2023-09-04 18:13:43 +02:00
William Lallemand
e7d9082315 BUG/MINOR: ssl/cli: can't find ".crt" files when replacing a certificate
Bug was introduced by commit 26654 ("MINOR: ssl: add "crt" in the
cert_exts array").

When looking for a .crt directly in the cert_exts array, the
ssl_sock_load_pem_into_ckch() function will be called with a argument
which does not have its ".crt" extensions anymore.

If "ssl-load-extra-del-ext" is used this is not a problem since we try
to add the ".crt" when doing the lookup in the tree.

However when using directly a ".crt" without this option it will failed
looking for the file in the tree.

The fix removes the "crt" entry from the array since it does not seem to
be really useful without a rework of all the lookups.

Should fix issue #2265

Must be backported as far as 2.6.
2023-08-28 18:20:39 +02:00
Christopher Faulet
208c712b40 MINOR: stconn: Rename SC_FL_SHUTW in SC_FL_SHUT_DONE
Here again, it is just a flag renaming. In SC flags, there is no longer
shutdown for writes but shutdowns.
2023-04-14 15:01:21 +02:00
Christopher Faulet
7faac7cf34 MINOR: tree-wide: Simplifiy some tests on SHUT flags by accessing SCs directly
At many places, we simplify the tests on SHUT flags to remove calls to
chn_prod() or chn_cons() function because the corresponding SC is available.
2023-04-05 08:57:06 +02:00
Christopher Faulet
87633c3a11 MEDIUM: tree-wide: Move flags about shut from the channel to the SC
The purpose of this patch is only a one-to-one replacement, as far as
possible.

CF_SHUTR(_NOW) and CF_SHUTW(_NOW) flags are now carried by the
stream-connecter. CF_ prefix is replaced by SC_FL_ one. Of course, it is not
so simple because at many places, we were testing if a channel was shut for
reads and writes in same time. To do the same, shut for reads must be tested
on one side on the SC and shut for writes on the other side on the opposite
SC. A special care was taken with process_stream(). flags of SCs must be
saved to be able to detect changes, just like for the channels.
2023-04-05 08:57:06 +02:00
Christopher Faulet
11fddb9655 MINOR: ssl-ckch: Stop to test CF_WRITE_ERROR to commit CA/CRL file
This change was performed on all applet I/O handlers but one was missed. In
the CLI I/O handler used to commit a CA/CRL file, we can remove the test on
CF_WRITE_ERROR because there is already a test on CF_SHUTW.
2023-02-13 09:43:38 +01:00
Christopher Faulet
da89e9b95b MINOR: channel/applets: Stop to test CF_WRITE_ERROR flag if CF_SHUTW is enough
In applets, we stop processing when a write error (CF_WRITE_ERROR) or a shutdown
for writes (CF_SHUTW) is detected. However, any write error leads to an
immediate shutdown for writes. Thus, it is enough to only test if CF_SHUTW is
set.
2023-01-09 18:41:08 +01:00
Remi Tricot-Le Breton
cc346678dc MEDIUM: ssl: Add ocsp_certid in ckch structure and discard ocsp buffer early
The ocsp_response member of the cert_key_and_chain structure is only
used temporarily. During a standard init process where an ocsp response
is provided, this ocsp file is first copied into the ocsp_response
buffer without any ocsp-related parsing (see
ssl_sock_load_ocsp_response_from_file), and then the contents are
actually interpreted and inserted into the actual ocsp tree
(cert_ocsp_tree) later in the process (see ssl_sock_load_ocsp). If the
response was deemed valid, it is then copied into the actual
ocsp_response structure's 'response' field (see
ssl_sock_load_ocsp_response). From this point, the ocsp_response field
of the cert_key_and_chain object could be discarded since actual ocsp
operations will be based of the certificate_ocsp object.

The only remaining runtime use of the ckch's ocsp_response field was in
the CLI, and more precisely in the 'show ssl cert' mechanism.
This constraint could be removed by adding an OCSP_CERTID directly in
the ckch because the buffer was only used to get this id.

This patch then adds the OCSP_CERTID pointer in the ckch, it clears the
ocsp_response buffer early and simplifies the ckch_store_build_certid
function.
2022-12-21 11:21:07 +01:00
William Lallemand
52ddd99940 MEDIUM: ssl: rename the struct "cert_key_and_chain" to "ckch_data"
Rename the structure "cert_key_and_chain" to "ckch_data" in order to
avoid confusion with the store whcih often called "ckchs".

The "cert_key_and_chain *ckch" were renamed "ckch_data *data", so we now
have store->data instead of ckchs->ckch.

Marked medium because it changes the API.
2022-12-02 11:48:30 +01:00
William Lallemand
0a2d63236c BUG/MINOR: ssl: shut the ca-file errors emitted during httpclient init
With an OpenSSL library which use the wrong OPENSSLDIR, HAProxy tries to
load the OPENSSLDIR/certs/ into @system-ca, but emits a warning when it
can't.

This patch fixes the issue by allowing to shut the error when the SSL
configuration for the httpclient is not explicit.

Must be backported in 2.6.
2022-11-24 19:14:19 +01:00
William Lallemand
3992f55ff3 MINOR: ssl: forgotten newline in error messages on ca-file
Add forgotten newlines in ssl_store_load_ca_from_buf() error messages.
2022-11-24 18:45:28 +01:00
Uriah Pollock
3cbf09ed64 MEDIUM: ssl: add minimal WolfSSL support with OpenSSL compatibility mode
This adds a USE_OPENSSL_WOLFSSL option, wolfSSL must be used with the
OpenSSL compatibility layer. This must be used with USE_OPENSSL=1.

WolfSSL build options:

   ./configure --prefix=/opt/wolfssl --enable-haproxy

HAProxy build options:

  USE_OPENSSL=1 USE_OPENSSL_WOLFSSL=1 WOLFSSL_INC=/opt/wolfssl/include/ WOLFSSL_LIB=/opt/wolfssl/lib/ ADDLIB='-Wl,-rpath=/opt/wolfssl/lib'

Using at least the commit 54466b6 ("Merge pull request #5810 from
Uriah-wolfSSL/haproxy-integration") from WolfSSL. (2022-11-23).

This is still to be improved, reg-tests are not supported yet, and more
tests are to be done.

Signed-off-by: William Lallemand <wlallemand@haproxy.org>
2022-11-24 11:29:03 +01:00
William Lallemand
78c7a06e4f MINOR: ssl: reintroduce ERR_GET_LIB(ret) == ERR_LIB_PEM in ssl_sock_load_pem_into_ckch()
Commit 432cd1a ("MEDIUM: ssl: be stricter about chain error") removed
the ERR_GET_LIB(ret) != ERR_LIB_PEM to be stricter about errors.

However, PEM_R_NO_START_LINE is better be checked with ERR_LIB_PEM.
So this patch complete the previous one.

The original problem was that the condition was wrongly inversed. This
original code from openssl:

  if (ERR_GET_LIB(err) == ERR_LIB_PEM &&
      ERR_GET_REASON(err) == PEM_R_NO_START_LINE)

became:

  if (ret && (ERR_GET_LIB(ret) != ERR_LIB_PEM &&
              ERR_GET_REASON(ret) != PEM_R_NO_START_LINE))

instead of:

  if (ret && !(ERR_GET_LIB(ret) == ERR_LIB_PEM &&
               ERR_GET_REASON(ret) == PEM_R_NO_START_LINE))

This must not be backported as it will break a lot of setup. That's too
bad because a lot of errors are lost. Not marked as a bug because of the
breakage it could cause on working setups.
2022-11-15 18:24:17 +01:00
Willy Tarreau
6fcc86b312 BUILD: ssl: use __fallthrough in cli_io_handler_commit_{cert,cafile_crlfile}()
This avoids 8 build warnings when preprocessing happens before compiling
with gcc >= 7.
2022-11-14 11:14:02 +01:00
William Lallemand
1ef1b859d0 CLEANUP: ssl: remove dead code in ssl_sock_load_pem_into_ckch()
Commit 432cd1a ("MEDIUM: ssl: be stricter about chain error")
introduced some dead code, let's remove it.

Should fix issue #1909.
2022-10-30 19:00:06 +01:00
William Lallemand
432cd1a7f8 MEDIUM: ssl: be stricter about chain error
The error check on certificate chain was ignoring all decoding error,
silently ignoring some errors.

This patch fixes the issue by being stricter on errors when reading the
chain, this is a change of behavior, it could break existing setup that
has a wrong chain.
2022-10-25 15:55:13 +02:00
William Lallemand
a538452fa4 MINOR: ssl: add the SSL error string before the chain
Add the SSL error string when failing to load a certificate in
ssl_sock_load_pem_into_ckch(). It's difficult to know what happen when no
 descriptive errror are emitted. This one is for the certificate before
trying to load the complete chain.
2022-10-25 15:53:01 +02:00
William Lallemand
f784b90eae MINOR: ssl: add the SSL error string when failing to load a certificate
Add the SSL error string when failing to load a certificate in
ssl_sock_load_pem_into_ckch(). It's difficult to know what happen when no
descriptive errror are emitted.

Example:
[ALERT]    (1264006) : config : parsing [ssl_default_server.cfg:51] : 'bind /tmp/ssl.sock' in section 'listen' : unable to load certificate chain from file 'reg-tests/ssl//common.pem': ASN no PEM Header Error
2022-10-25 12:36:48 +02:00
William Lallemand
e0fa91ffe1 BUG/MINOR: ssl: leak of ckch_inst_link in ckch_inst_free() v2
ckch_inst_free() unlink the ckch_inst_link structure but never free it.

It can't be fixed simply because cli_io_handler_commit_cafile_crlfile()
is using a cafile_entry list to iterate a list of ckch_inst entries
to free. So both cli_io_handler_commit_cafile_crlfile() and
ckch_inst_free() would modify the list at the same time.

In order to let the caller manipulate the ckch_inst_link,
ckch_inst_free() now checks if the element is still attached before
trying to detach and free it.

For this trick to work, the caller need to do a LIST_DEL_INIT() during
the iteration over the ckch_inst_link.

list_for_each_entry was also replace by a while (!LIST_ISEMPTY()) on the
head list in cli_io_handler_commit_cafile_crlfile() so the iteration
works correctly, because it could have been stuck on the first detached
element. list_for_each_entry_safe() is not enough to fix the issue since
multiple element could have been removed.

Must be backported as far as 2.5.
2022-08-31 15:24:01 +02:00
William Lallemand
0bfa3e7ff2 BUG/MINOR: ssl: revert two wrong fixes with ckhi_link
This reverts commit 056ad01d55675ab2d65c7b41a2e1096db27b3d14.
This reverts commit ddd480cbdc0d54b3426ce9b6dd68cd849747cb07.

The architecture is ambiguous here: ckch_inst_free() is detaching and
freeing the "ckch_inst_link" linked list which must be free'd only from
the cafile_entry side.

The problem was also hidden by the fix ddd480c ("BUG/MEDIUM: ssl: Fix a
UAF when old ckch instances are released") which change the ckchi_link
inner loop by a safe one. However this can't fix entirely the problem
since both __ckch_inst_free_locked() could remove several nodes in the
ckchi_link linked list.

This revert is voluntary reintroducing a memory leak before really fixing
the problem.

Must be backported in 2.5 + 2.6.
2022-08-30 18:12:28 +02:00
Christopher Faulet
ddd480cbdc BUG/MEDIUM: ssl: Fix a UAF when old ckch instances are released
When old chck instances is released at the end of "commit ssl ca-file" or
"commit ssl crl-file" commands, the link is released. But we walk through
the list using the unsafe macro. list_for_each_entry_safe() must be used.

This bug was introduced by commit 056ad01d5 ("BUG/MINOR: ssl: leak of
ckch_inst_link in ckch_inst_free()"). Thus this patch must be backported as
far as 2.5.
2022-08-30 16:27:51 +02:00
William Lallemand
056ad01d55 BUG/MINOR: ssl: leak of ckch_inst_link in ckch_inst_free()
ckch_inst_free() unlink the ckch_inst_link structure but never free it.
It can cause a memory leak upon a ckch_inst_free() done with CLI
operation.

Bug introduced by commit 4458b97 ("MEDIUM: ssl: Chain ckch instances in
ca-file entries").

Must be backported as far as 2.5.
2022-08-29 18:53:34 +02:00
William Lallemand
946580e17a BUG/MINOR: ssl: fix deinit of the ca-file tree
Commit b0c4827 ("BUG/MINOR: ssl: free the cafile entries on deinit")
introduced a double free.

The node was never removed from the tree before its free.

Fix issue #1836.

Must be backported where b0c4827 was backported. (2.6 for now).
2022-08-29 18:51:39 +02:00
William Lallemand
62c0b99e3b MINOR: ssl/cli: implement "add ssl ca-file"
In ticket #1805 an user is impacted by the limitation of size of the CLI
buffer when updating a ca-file.

This patch allows a user to append new certificates to a ca-file instead
of trying to put them all with "set ssl ca-file"

The implementation use a new function ssl_store_dup_cafile_entry() which
duplicates a cafile_entry and its X509_STORE.

ssl_store_load_ca_from_buf() was modified to take an apped parameter so
we could share the function for "set" and "add".
2022-08-19 19:58:53 +02:00
William Lallemand
d4774d3cfa MINOR: ssl: handle ca-file appending in cafile_entry
In order to be able to append new CA in a cafile_entry,
ssl_store_load_ca_from_buf() was reworked and a "append" parameter was
added.

The function is able to keep the previous X509_STORE which was already
present in the cafile_entry.
2022-08-19 19:58:53 +02:00
William Lallemand
ec7eb59d20 BUG/MINOR: ssl/cli: error when the ca-file is empty
"set ssl ca-file" does not return any error when a ca-file is empty or
only contains comments. This could be a problem is the file was
malformated and did not contain any PEM header.

It must be backported as far as 2.5.
2022-08-19 19:56:53 +02:00
William Lallemand
0f17ab2fdd MINOR: ssl: enhance ca-file error emitting
Enhance the errors and warnings when trying to load a ca-file with
ssl_store_load_locations_file().

Add errors from ERR_get_error() and strerror to give more information to
the user.
2022-07-19 19:13:08 +02:00
William Lallemand
4348232231 BUG/MINOR: ssl: allow duplicate certificates in ca-file directories
It looks like OpenSSL 1.0.2 returns an error when trying to insert a
certificate whis is already present in a X509_STORE.

This patch simply ignores the X509_R_CERT_ALREADY_IN_HASH_TABLE error if
emitted.

Should fix part of issue #1780.

Must be backported in 2.6.
2022-07-18 18:49:27 +02:00
Remi Tricot-Le Breton
1bad7db4a1 BUG/MINOR: ssl: Do not look for key in extra files if already in pem
A bug was introduced by commit 9bf3a1f67eb3bc6f02abcabf8ab141840c7a1db2
"BUG/MINOR: ssl: Fix crash when no private key is found in pem".
If a private key is already contained in a pem file, we will still look
for a .key file and load its private key if it exists when we should
not.

This patch should be backported to all branches where the original fix
was backported (all the way to 2.2).
2022-06-22 10:45:47 +02:00
Willy Tarreau
d543ae0e68 BUILD: ssl_ckch: fix "maybe-uninitialized" build error on gcc-9.4 + ARM
As reported in issue #1755, gcc-9.3 and 9.4 emit a "maybe-uninitialized"
warning in cli_io_handler_commit_cafile_crlfile() because it sees that
when the "path" variable is not set, we're jumping to the error label
inside the loop but cannot see that the new state will avoid the places
where the value is used. Thus it's a false positive but a difficult one.
Let's just preset the value to NULL to make it happy.

This was introduced in 2.7-dev by commit ddc8e1cf8 ("MINOR: ssl_ckch:
Simplify I/O handler to commit changes on CA/CRL entry"), thus no
backport is needed for now.
2022-06-22 05:45:34 +02:00
Christopher Faulet
1f90f33b69 BUG/MINOR: ssl_ckch: Fix another possible uninitialized value
Commit d6c66f06a ("MINOR: ssl_ckch: Remove service context for "set ssl
crl-file" command") introduced a regression leading to a build error because
of a possible uninitialized value. It is now fixed.

This patch must be backported as far as 2.5.
2022-06-03 16:49:53 +02:00
Christopher Faulet
ea2c8c6ba7 BUILD: ssl_ckch: Fix build error about a possible uninitialized value
A build error is reported about the path variable in the switch statement on
the commit type, in cli_io_handler_commit_cafile_crlfile() function. The
enum contains only 2 values, but a default clause has been added to return an
error to make GCC happy.

This patch must be backported as far as 2.5.
2022-06-03 16:41:11 +02:00
Christopher Faulet
88041b35c3 BUG/MINOR: ssl_ckch: Fix possible uninitialized value in show_crlfile I/O handler
Commit 9a99e5478 ("BUG/MINOR: ssl_ckch: Dump CRL transaction only once if
show command yield") introduced a regression leading to a build error
because of a possible uninitialized value. It is now fixed.

This patch must be backported as far as 2.5.
2022-06-03 16:28:11 +02:00
Christopher Faulet
677cb4fa91 BUG/MINOR: ssl_ckch: Fix possible uninitialized value in show_cafile I/O handler
Commit 5a2154bf7 ("BUG/MINOR: ssl_ckch: Dump CA transaction only once if
show command yield") introduced a regression leading to a build error
because of a possible uninitialized value. It is now fixed.

This patch must be backported as far as 2.5.
2022-06-03 16:27:49 +02:00
Christopher Faulet
d1d2e4dfe5 BUG/MINOR: ssl_ckch: Fix possible uninitialized value in show_cert I/O handler
Commit 3e94f5d4b ("BUG/MINOR: ssl_ckch: Dump cert transaction only once if
show command yield") introduced a regression leading to a build error
because of a possible uninitialized value. It is now fixed.

This patch must be backported as far as 2.2.
2022-06-03 16:27:41 +02:00
Christopher Faulet
d6c66f06ac MINOR: ssl_ckch: Remove service context for "set ssl crl-file" command
This command does not have I/O handle function. All is done in the command
parsing function. So there is no reason to have dedicated context.
2022-06-03 12:12:04 +02:00
Christopher Faulet
132c595673 MINOR: ssl_ckch: Remove service context for "set ssl ca-file" command
This command does not have I/O handle function. All is done in the command
parsing function. So there is no reason to have dedicated context.
2022-06-03 12:12:04 +02:00
Christopher Faulet
24a20b9808 MINOR: ssl_ckch: Remove service context for "set ssl cert" command
This command does not have I/O handle function. All is done in the command
parsing function. So there is no reason to have dedicated context.
2022-06-03 12:12:04 +02:00
Christopher Faulet
6af2fc6a3f MINOR: ssl_ckch: Simplify structure used to commit changes on CA/CRL entries
The same type is used for CA and CRL entries. So, in commit_cert_ctx
structure, there is no reason to have different fields for the CA and CRL
entries.
2022-06-03 12:12:04 +02:00
Christopher Faulet
dd0c4834ef CLEANUP: ssl_ckch: Remove unused field in commit_cacrlfile_ctx structure
.next_ckchi field is not used by functions responsible to commit changes on
CA/CRL entries. It can be removed.
2022-06-03 12:12:04 +02:00
Christopher Faulet
f814c4aa98 BUG/MINOR: ssl_ckch: Init right field when parsing "commit ssl crl-file" cmd
.next_ckchi_link field must be initialized to NULL instead of .next_ckchi in
cli_parse_commit_crlfile() function. Only '.nex_ckchi_link' is used in the
I/O handler.

This patch must be backported as far as 2.5 with some adaptations for the 2.5.
2022-06-03 12:12:04 +02:00
Christopher Faulet
3e94f5d4b6 BUG/MINOR: ssl_ckch: Dump cert transaction only once if show command yield
When loaded SSL certificates are displayed via "show ssl cert" command, the
in-progess transaction, if any, is also displayed. However, if the command
yield, the transaction is re-displayed again and again.

To fix the issue, old_ckchs field is used to remember the transaction was
already displayed.

This patch must be backported as far as 2.2.
2022-06-03 11:20:41 +02:00
Christopher Faulet
5a2154bf7c BUG/MINOR: ssl_ckch: Dump CA transaction only once if show command yield
When loaded CA files are displayed via "show ssl ca-file" command, the
in-progress transaction, if any, is also displayed. However, if the command
yield, the transaction is re-displayed again and again.

To fix the issue, old_cafile_entry field is used to remember the transaction
was already displayed.

This patch must be backported as far as 2.5.
2022-06-03 11:20:38 +02:00
Christopher Faulet
9a99e54787 BUG/MINOR: ssl_ckch: Dump CRL transaction only once if show command yield
When loaded CRL files are displayed via "show ssl crl-file" command, the
in-progess transaction, if any, is also displayed. However, if the command
yield, the transaction is re-displayed again and again.

To fix the issue, old_crlfile_entry field is used to remember the transaction
was already displayed.

This patch must be backported as far as 2.5.
2022-06-03 11:20:34 +02:00
Christopher Faulet
51095ee236 BUG/MINOR: ssl_ckch: Use right type for old entry in show_crlfile_ctx
Because of a typo (I guess), an unknown type is used for the old entry in
show_crlfile_ctx structure. Because this field is unused, there is no
compilation error. But it must be a cafile_entry and not a crlfile_entry.

Note this field is not used for now, but it will be used.

This patch must be backported to 2.6.
2022-06-03 11:20:16 +02:00