71 Commits

Author SHA1 Message Date
Remi Tricot-Le Breton
9e94df3e55 MINOR: ssl: Add ocsp update success/failure counters
Those counters will be used for debugging purposes and will be dumped
via a cli command.
2023-03-02 15:37:11 +01:00
Remi Tricot-Le Breton
6de7b78c9f MINOR: ssl: Reinsert ocsp update entries later in case of unknown error
In case of allocation error during the construction of an OCSP request
for instance, we would have ended reinserting the ocsp entry at the same
place in the ocsp update tree which could potentially lead to an
"endless" loop of errors in ssl_ocsp_update_responses. In such a case,
entries are now reinserted further in the tree (1 minute later) in order
to avoid such a chain of alloc failure.
2023-03-02 15:37:10 +01:00
Remi Tricot-Le Breton
926f34bc36 MINOR: ssl: Destroy ocsp update http_client during cleanup
If a deinit is started while an OCSP update is in progress we might end
up with a dangling http_client instance that should be destroyed
properly.
2023-03-02 15:37:07 +01:00
William Lallemand
a14686d096 MINOR: ssl/ocsp: add a function to check the OCSP update configuration
Deduplicate the code which checks the OCSP update in the ckch_store and
in the crtlist_entry.

Also, jump immediatly to error handling when the ERR_FATAL is catched.
2023-02-08 11:40:31 +01:00
Remi Tricot-Le Breton
083b230699 MINOR: ssl: Remove debug fprintf in 'update ssl ocsp-response' cli command
A debug fprintf was left behind in the new cli function.
2023-01-25 11:51:39 +01:00
Remi Tricot-Le Breton
305a4f32a5 BUG/MINOR: ssl: Fix leaks in 'update ssl ocsp-response' CLI command
This patch fixes two leaks in the 'update ssl ocsp-response' cli
command. One rather significant one since a whole trash buffer was
allocated for every call of the command, and another more marginal one
in an error path.

This patch does not need to be backported.
2023-01-25 11:51:39 +01:00
Remi Tricot-Le Breton
10f113ec55 MINOR: ssl: Reinsert updated ocsp response later in tree in case of http error
When updating an OCSP response, in case of HTTP error (host unreachable
for instance) we do not want to reinsert the entry at the same place in
the update tree otherwise we might retry immediately the update of the
same response. This patch adds an arbitrary 1min time to the next_update
of a response in such a case.
After an HTTP error, instead of waking the update task up after an
arbitrary 10s time, we look for the first entry of the update tree and
sleep for the apropriate time.
2023-01-12 13:13:45 +01:00
Remi Tricot-Le Breton
1c647adf46 MINOR: ssl: Do not wake ocsp update task if update tree empty
In the unlikely event that the ocsp update task is started but the
update tree is empty, put the update task to sleep indefinitely.
The only way this can happen is if the same certificate is loaded under
two different names while the second one has the 'ocsp-update on'
option. Since the certificate names are distinct we will have two
ckch_stores but a single certificate_ocsp because they are identified by
the OCSP_CERTID which is built out of the issuer certificate and the
certificate id (which are the same regardless of the .pem file name).
2023-01-12 13:13:45 +01:00
Remi Tricot-Le Breton
bdd84c5ffb BUG/MINOR: ssl: OCSP minimum update threshold not properly set
An arbitrary 5 minutes minimum interval between two updates of the same
OCSP response is defined but it was not properly used when inserting
entries in the update tree.

This patch does not need to be backported.
2023-01-12 13:13:45 +01:00
Remi Tricot-Le Breton
71237a1457 BUG/MINOR: ssl: Remove unneeded pointer check in ocsp cli release function
The ctx pointer cannot be NULL so we can remove the check.

This patch fixes GitHub issue #1996.
It does not need to be backported.
2023-01-11 11:20:11 +01:00
Remi Tricot-Le Breton
14419ebf2b MINOR: ssl: Remove mention of ckch_store in error message of cli command
When calling 'update ssl ocsp-response' with an unknown certificate file
name, the error message would mention a "ckch_store" which is an
internal structure unknown by users.
2023-01-09 15:43:41 +01:00
Remi Tricot-Le Breton
648c83ecdd MINOR: ssl: Limit ocsp_uri buffer size to minimum
The ocsp_uri field of the certificate_ocsp structure was a 16k buffer
when it could be hand allocated to just the required size to store the
OCSP uri. This field is now behaving the same way as the sctl and
ocsp_response buffers of the ckch_store structure.
2023-01-09 15:43:41 +01:00
Remi Tricot-Le Breton
14d7f0eb48 MINOR: ssl: Release ssl_ocsp_task_ctx.cur_ocsp when destroying task
In the unlikely event that the OCSP udpate task is killed in the middle
of an update process (request sent but no response received yet) the
cur_ocsp member of the update context would keep an unneeded reference
to a certificate_ocsp object. It must then be freed during the task's
cleanup.
2023-01-09 15:43:41 +01:00
Remi Tricot-Le Breton
8bdd0050e2 MINOR: ssl: Create temp X509_STORE filled with cert chain when checking ocsp response
When calling OCSP_basic_verify to check the validity of the received
OCSP response, we need to provide an untrusted certificate chain as well
as an X509_STORE holding only trusted certificates. Since the
certificate chain and the issuer certificate are all provided by the
user, we assume that they are valid and we add them all to a temporary
store. This enables to focus only on the response's validity.
2023-01-09 15:43:41 +01:00
Remi Tricot-Le Breton
57f60c2316 BUG/MINOR: ssl: Crash during cleanup because of ocsp structure pointer UAF
When ocsp-update is enabled for a given certificate, its
certificate_ocsp objects is inserted in two separate trees (the actual
ocsp response one and the ocsp update one). But since the same instance
is used for the two trees, its ownership is kept by the regular ocsp
response one. The ocsp update task should then never have to free the
ocsp entries. The crash actually occurred because of this. The update
task was freeing entries whose reference counter was not increased while
a reference was still held by the SSL_CTXs.
The only time during which the ocsp update task will need to increase
the reference counter is during an actual update, because at this moment
the entry is taken out of the update tree and a 'flying' reference to
the certificate_ocsp is kept in the ocsp update context.

This bug could be reproduced by calling './haproxy -f conf.cfg -c' with
any of the used certificates having the 'ocsp-update on' option. For
some reason asan caught the bug easily but valgrind did not.

This patch does not need to be backported.
2023-01-09 15:43:41 +01:00
Remi Tricot-Le Breton
15dc0e2a1c BUG/MINOR: ssl: Fix crash in 'update ssl ocsp-response' CLI command
This CLI command crashed when called for a certificate which did not
have an OCSP response during startup because it assumed that the
ocsp_issuer pointer of the ckch_data object would be valid. It was only
true for already known OCSP responses though.
The ocsp issuer certificate is now taken either from the ocsp_issuer
pointer or looked for in the certificate chain. This is the same logic
as the one in ssl_sock_load_ocsp.

This patch does not need to be backported.
2023-01-09 15:43:41 +01:00
Remi Tricot-Le Breton
c389b04bc5 BUG/MINOR: ssl: Missing goto in error path in ocsp update code
When converting an OCSP request's information into base64, the return
value of a2base64 is checked but processing is not interrupted when it
returns a negative value, which was caught by coverity.

This patch fixes GitHub issue #1974.
It does not need to be backported.
2023-01-02 15:21:57 +01:00
William Lallemand
70601c56da BUG/MINOR: ssl/ocsp: httpclient blocked when doing a GET
When the OCSP updater uses the GET method with the payload in the URI,
the body must be set to IST_NULL, or the request won't be sent.
2022-12-22 14:41:31 +01:00
William Lallemand
eb5302023f CLEANUP: ssl/ocsp: add spaces around operators
Add spaces around operators in ssl_ocsp_create_request_details().
2022-12-22 10:20:24 +01:00
William Lallemand
8bc00f8bdc BUG/MINOR: ssl/ocsp: check chunk_strcpy() in ssl_ocsp_get_uri_from_cert()
Check the return value of chunk_strcpy() in
ssl_ocsp_get_uri_from_cert().

Should fix issue #1975.
2022-12-22 10:09:11 +01:00
Remi Tricot-Le Breton
c8d814ed63 MINOR: ssl: Move OCSP code to a dedicated source file
This is a simple cleanup that moves OCSP related code to a dedicated
file instead of interlacing it in some pure ssl connection code.
2022-12-21 11:21:07 +01:00