Fix compatibility with upcoming OpenSSL 4.0#70
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request addresses upcoming compatibility issues with OpenSSL 4.0, which plans to deprecate X509_cmp_timeframe() and X509_cmp_time(). The changes introduce new wrapper functions UTIL_cmp_timeframe() and CRL_check() that abstract the version-specific implementations and use the newly exported X509_check_certificate_times() function for OpenSSL 4.0+.
Changes:
- Introduced
UTIL_cmp_timeframe()wrapper function to handle time frame checking across OpenSSL versions - Added
CRL_check()function for consistent CRL validity checking - Renamed parameter
uritosrcacross certificate and CRL checking functions for better clarity
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/libsecutils/src/storage/files.c | Added unused parameter suppressions for OPENSSL_NO_ENGINE builds |
| src/libsecutils/src/credentials/store.c | Replaced local crl_expired() with UTIL_cmp_timeframe() call |
| src/libsecutils/src/credentials/cert.c | Implemented UTIL_cmp_timeframe() with version-specific logic and updated CERT_print() for OpenSSL 4.0 |
| src/libsecutils/src/certstatus/crls.c | Introduced CRL_check() function and added src parameter to check_cert_crls() |
| src/libsecutils/src/certstatus/certstatus.c | Updated check_cert_crls() call with new signature |
| src/libsecutils/include/secutils/credentials/cert.h | Added UTIL_cmp_timeframe() declaration and updated parameter names from uri to src |
| src/libsecutils/include/secutils/certstatus/crls.h | Added CRL_check() declaration and updated check_cert_crls() signature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ac1c07e to
83e3b04
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #elif OPENSSL_VERSION_NUMBER < 0x40000000L | ||
| return X509_cmp_timeframe(vpm, start, end); | ||
| #else | ||
| X509 *dummy_cert = X509_new(); /* needed as a workaround for OpenSSL API restriction */ |
There was a problem hiding this comment.
if i got this correctly, we just want to check if time A is between start time B and end time C.
so:
if B - A < 0 we are after the start time (100 - 101 = -1) -> certificate valid
else we are before B and the certificate is not yet valid
and if C - A < 0 we are after the end time -> certificate expired
else we are before the end time and certificate is valid
this should be realizable farely easy with time.h difftime.
why not use such an approach instead?
There was a problem hiding this comment.
Thank you for having a look at this and your comment.
I agree there should be a fairly easy way to do this,
but unfortunately things are tricky because the ASN1_TIME values may be invalid and need to be converted to time_t, or likely better int64_t, before being able to do the comparisons.
I've just extended this way my yesterday's complaint about that recent OpenSSL move:
openssl/openssl#29638 (comment)
There was a problem hiding this comment.
Should we wait with merging until this is clarified?
There was a problem hiding this comment.
Good question.
So far it is very unclear to me when and how the OpenSSL colleagues will react to that issue.
I suggest that we tentatively merge this PR,
and if/when there is good reason to adapt UTIL_cmp_timeframe(), we can do in a follow-up PR.
The new workaround code anyway will not take effect until someone uses OpenSSL 4.0.
I'm glad that I came across the issue some time before the release, so there is some chance to get it fixed by then.
…frame() and X509_cmp_time() deprecated in OpenSSL 4.0
…mp_timeframe() and X509_cmp_time() deprecated in OpenSSL 4.0
83e3b04 to
98be68c
Compare
Unfortunately, OpenSSL 4.0 plans to deprecate
X509_cmp_timeframe()andX509_cmp_time().To work around this, introduce
UTIL_cmp_timeframe()andCRL_check()making use of the newly exportedX509_check_certificate_times()function where needed.