Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion src/libsecutils/include/secutils/certstatus/crls.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,18 @@
*******************************************************************************/
void UTIL_print_crl(OPTIONAL BIO* bio, OPTIONAL const X509_CRL* crl);

/*!*****************************************************************************
* @brief check if CRL is within validity period
*
* @param src the source of the CRL, e.g., a URL or file name
* @param crl the CRL to be be checked, or null for no checks
* @param vpm verification parameters, or null, governing if and how to check CRL times,
* depending on X509_V_FLAG_USE_CHECK_TIME and X509_V_FLAG_NO_CHECK_TIME
* @return true if no CRL given or CRL validity period check passed
* @note check failures are logged as a warning if vpm is null, otherwise as an error.
*******************************************************************************/
bool CRL_check(const char *src, OPTIONAL X509_CRL *crl, OPTIONAL const X509_VERIFY_PARAM *vpm);

/*!
* @brief retrieve CRL in DER format (ASN.1) from given CRL distribution point
*
Expand All @@ -44,9 +56,10 @@ static const int CRL_DOWNLOAD_DEFAULT_TIMEOUT = 10; /* in seconds */
*
* @param ctx pointer to verification context structure including the cert to check
* @param crls a list of CRLs that may be useful in addition to the local ones in ctx, or null
* @param src URL or other description of the CRL source
* @return 1 on success, 0 on rejection (i.e., cert revoked), -1 on error
*/
int check_cert_crls(X509_STORE_CTX* ctx, OPTIONAL STACK_OF(X509_CRL) * crls);
int check_cert_crls(X509_STORE_CTX* ctx, OPTIONAL STACK_OF(X509_CRL) * crls, const char* src);

#ifndef SEC_NO_CRL_DOWNLOAD

Expand Down
23 changes: 19 additions & 4 deletions src/libsecutils/include/secutils/credentials/cert.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,21 @@ void CERTS_free(OPTIONAL STACK_OF(X509) *certs);
/* this function is used by the genCMPClient API implementation */
X509_NAME* UTIL_parse_name(const char* dn, int chtype, bool multirdn);

/*!*****************************************************************************
* @brief check if time frame (e.g., certificate validity period) is currently acceptable
*
* @param vpm verification parameters, or null, governing if and how to check cert times,
* depending on X509_V_FLAG_USE_CHECK_TIME and X509_V_FLAG_NO_CHECK_TIME
* @param start begin of the period, or null
* @param end conclusion of the period, or null
* @return 0 if times should not be checked according to vpm or the reference time
* (which is the current time unless vpm gives a different one) is in range,
* otherwise 1 if it is past the end, or -1 if it is before the start.
* @note With OpenSSL before 4.0, invalid start and end times lead to not checking them.
*******************************************************************************/
/* this function is used by the genCMPClient API implementation */
int UTIL_cmp_timeframe(OPTIONAL const X509_VERIFY_PARAM *vpm,
OPTIONAL const ASN1_TIME *start, OPTIONAL const ASN1_TIME *end);

/*!*****************************************************************************
* @brief log message about the given certificate, printing its subject
Expand Down Expand Up @@ -150,30 +165,30 @@ void CERTS_print(OPTIONAL const STACK_OF(X509) * certs, OPTIONAL BIO* bio);
/*!*****************************************************************************
* @brief check if certificate is within validity period, optionally check type
*
* @param uri The source of the certificate, e.g., a URL or file name
* @param src the source of the certificate, e.g., a URL or file name
* @param cert certificate to be be checked, or null for no checks
* @param type_CA check for CA cert if 1 or EE if 0; no type check if < 0
* @param vpm verification parameters, or null, governing if and how to check cert times,
* depending on X509_V_FLAG_USE_CHECK_TIME and X509_V_FLAG_NO_CHECK_TIME
* @return true if no cert given or cert validity period check passed
* @note Check failures are logged as a warning if vpm is null, otherwise as an error.
*******************************************************************************/
bool CERT_check(const char *uri, OPTIONAL X509 *cert, int type_CA,
bool CERT_check(const char *src, OPTIONAL X509 *cert, int type_CA,
OPTIONAL const X509_VERIFY_PARAM *vpm);


/*!*****************************************************************************
* @brief check if a cert list member is within validity period, optionally check type
*
* @param uri The source of the certificates, e.g., a URL or file name
* @param src The source of the certificates, e.g., a URL or file name
* @param certs list of certificates to be be checked, or null for no checks
* @param type_CA check for CA cert if 1 or EE if 0; no type check if < 0
* @param vpm verification parameters, or null, governing if and how to check cert times,
* depending on X509_V_FLAG_USE_CHECK_TIME and X509_V_FLAG_NO_CHECK_TIME
* @return true if no certs given or validity period check passed for all certs
* @note Check failures are logged as a warning if vpm is null, otherwise as an error.
*******************************************************************************/
bool CERT_check_all(const char *uri, OPTIONAL STACK_OF(X509) *certs, int type_CA,
bool CERT_check_all(const char *src, OPTIONAL STACK_OF(X509) *certs, int type_CA,
OPTIONAL const X509_VERIFY_PARAM *vpm);


Expand Down
1 change: 1 addition & 0 deletions src/libsecutils/include/secutils/util/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ static const int UTIL_max_name_len = 128; /*!< max length of file name */
# define OPENSSL_V_1_1_0 0x10100000L
# define OPENSSL_V_1_1_1 0x10101000L
# define OPENSSL_V_3_0_0 0x30000000L
# define OPENSSL_V_4_0_0 0x40000000L

# ifndef OpenSSL_version_num
# if OPENSSL_VERSION_NUMBER < 0x10100000L
Expand Down
2 changes: 1 addition & 1 deletion src/libsecutils/src/certstatus/certstatus.c
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ bool check_cert_revocation(X509_STORE_CTX* ctx, OPTIONAL OCSP_RESPONSE* resp)
{
X509_VERIFY_PARAM_clear_flags(param, X509_V_FLAG_NONFINAL_CHECK);
}
ok = check_cert_crls(ctx, 0); /* try locally available CRLs */
ok = check_cert_crls(ctx, 0, "local trust store"); /* try locally available CRLs */
if(ok >= 0 or final)
{
if(ok > 0)
Expand Down
53 changes: 17 additions & 36 deletions src/libsecutils/src/certstatus/crls.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <credentials/store.h>
#include <credentials/verify.h>
#include <connections/conn.h>
#include <credentials/cert.h> /* for UTIL_cmp_timeframe() */

#include <openssl/x509v3.h>
#include <openssl/x509_vfy.h>
Expand Down Expand Up @@ -139,47 +140,26 @@ X509_CRL* CONN_load_crl_http(const char* url, int timeout,
#endif
}

static void warn_crl(const X509_STORE_CTX* ctx, const X509_CRL * crl)
bool CRL_check(const char *src, OPTIONAL X509_CRL *crl, OPTIONAL const X509_VERIFY_PARAM *vpm)
{
if(0 is_eq ctx or 0 is_eq crl)
{
LOG(FL_ERR, "null argument");
return;
}

const X509_VERIFY_PARAM* vpm = X509_STORE_CTX_get0_param((X509_STORE_CTX*)ctx);
unsigned long flags = X509_VERIFY_PARAM_get_flags((X509_VERIFY_PARAM*)vpm);
if((flags bitand X509_V_FLAG_NO_CHECK_TIME) not_eq 0)
{
return;
}
time_t check_time, *ptime = 0;
if((flags bitand X509_V_FLAG_USE_CHECK_TIME) not_eq 0)
{
check_time = X509_VERIFY_PARAM_get_time(vpm);
ptime = &check_time;
}
const ASN1_TIME* crl_end_time = X509_CRL_get0_nextUpdate(crl);
const ASN1_TIME* crl_last_update = X509_CRL_get0_lastUpdate(crl);
char* issuer = X509_NAME_oneline(X509_CRL_get_issuer(crl), 0, 0);
if(issuer not_eq 0)
{
if(crl_end_time not_eq 0 and X509_cmp_time(crl_end_time, ptime) not_eq 1)
{
int res;

LOG(FL_WARN, "CRL issued by %s is no more valid", issuer);
}
if(crl_last_update not_eq 0 and X509_cmp_time(crl_last_update, ptime) not_eq -1)
{
if (crl == NULL)
return true;
res = UTIL_cmp_timeframe(vpm, X509_CRL_get0_lastUpdate(crl), X509_CRL_get0_nextUpdate(crl));
if (res != 0) {
severity level = vpm == NULL ? LOG_WARNING : LOG_ERR;
char *issuer = X509_NAME_oneline(X509_CRL_get_issuer(crl), 0, 0);

LOG(FL_WARN, "CRL issued by %s is not yet valid", issuer);
}
LOG(LOG_FUNC_FILE_LINE, level, "CRL from '%s' issued by '%s' %s",
src, issuer, res > 0 ? "has expired" : "is not yet valid");
OPENSSL_free(issuer);
}
return res == 0;
}

/* like check_cert() of OpenSSL:crypto/x509/x509_vfy.c, may use extra CRLs */
int check_cert_crls(X509_STORE_CTX* ctx, OPTIONAL STACK_OF(X509_CRL) * crls)
int check_cert_crls(X509_STORE_CTX* ctx, OPTIONAL STACK_OF(X509_CRL) * crls, const char* src)
{
int res = -1;
if(0 is_eq ctx)
Expand Down Expand Up @@ -260,7 +240,7 @@ int check_cert_crls(X509_STORE_CTX* ctx, OPTIONAL STACK_OF(X509_CRL) * crls)
while(sk_X509_CRL_num(tmp_crls) > 0)
{
crl = sk_X509_CRL_shift(tmp_crls);
warn_crl(ctx, crl);
(void)CRL_check(src, crl, X509_STORE_CTX_get0_param(ctx));
X509_CRL_free(crl);
}
sk_X509_CRL_free(tmp_crls);
Expand Down Expand Up @@ -358,7 +338,8 @@ static int try_cdp(X509_STORE_CTX* ctx, int timeout, const X509* cert,
{
return res;
}
LOG(FL_TRACE, "successfully retrieved CRL, URL %s", url not_eq 0 ? url : "based on any info in cert");
const char* src = url not_eq 0 ? url : desc;
LOG(FL_TRACE, "successfully retrieved CRL from %s", src);
UTIL_print_crl(bio_trace, crl);

crls = sk_X509_CRL_new_null();
Expand All @@ -372,7 +353,7 @@ static int try_cdp(X509_STORE_CTX* ctx, int timeout, const X509* cert,
if (sk_X509_CRL_push(crls, delta_crl) == 0)
goto err;
}
res = check_cert_crls(ctx, crls);
res = check_cert_crls(ctx, crls, src);
if (delta_crl not_eq 0)
{
sk_X509_CRL_pop(crls); /* delta_crl */
Expand Down
84 changes: 56 additions & 28 deletions src/libsecutils/src/credentials/cert.c
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,48 @@ X509_NAME* UTIL_parse_name(const char* dn, int chtype, bool multirdn)
return n;
}

/*
* Return 0 if time should not be checked or reference time is in range,
* otherwise 1 if it is past the end, or -1 if it is before the start.
* With OpenSSL before 4.0, invalid start and end times leads to not checking them.
*/
int UTIL_cmp_timeframe(OPTIONAL const X509_VERIFY_PARAM *vpm,
OPTIONAL const ASN1_TIME *start, OPTIONAL const ASN1_TIME *end)
{
#if OPENSSL_VERSION_NUMBER < OPENSSL_V_3_0_0
unsigned long flags = vpm == NULL ? 0 : X509_VERIFY_PARAM_get_flags((X509_VERIFY_PARAM *)vpm);
time_t ref_time;
time_t *time = NULL;

if ((flags & X509_V_FLAG_NO_CHECK_TIME) == 0) {
if ((flags & X509_V_FLAG_USE_CHECK_TIME) != 0) {
ref_time = X509_VERIFY_PARAM_get_time(vpm);
time = &ref_time;
} /* else reference time is the current time */

if (end != NULL && X509_cmp_time(end, time) < 0)
return 1;
if (start != NULL && X509_cmp_time(start, time) > 0)
return -1;
}
return 0;
#elif OPENSSL_VERSION_NUMBER < OPENSSL_V_4_0_0
return X509_cmp_timeframe(vpm, start, end);
#else
X509 *dummy_cert = X509_new(); /* needed as a workaround for OpenSSL API restriction */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member Author

@DDvO DDvO Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we wait with merging until this is clarified?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

int res = 1, error = X509_V_OK;

if (dummy_cert != NULL) {
(void)X509_set1_notBefore(dummy_cert, start);
(void)X509_set1_notAfter(dummy_cert, end);
res = X509_check_certificate_times(vpm, dummy_cert, &error);
X509_free(dummy_cert);
}
return res == 1 ? 0 : error == X509_V_ERR_CERT_NOT_YET_VALID ? -1:
error == X509_V_ERR_CERT_HAS_EXPIRED ? 1 : 0;
#endif
}


void CERT_print(OPTIONAL const X509* cert, OPTIONAL BIO* bio, unsigned long neg_cflags)
{
Expand All @@ -239,6 +281,7 @@ void CERT_print(OPTIONAL const X509* cert, OPTIONAL BIO* bio, unsigned long neg_
X509_print_ex(bio, (X509*)cert, flags, compl X509_FLAG_NO_ISSUER);
}
X509_print_ex(bio, (X509*)cert, flags, compl(X509_FLAG_NO_SERIAL bitor X509_FLAG_NO_VALIDITY));
#if OPENSSL_VERSION_NUMBER < OPENSSL_V_4_0_0
if(X509_cmp_current_time(X509_get0_notBefore(cert)) > 0)
{
BIO_printf(bio, " not yet valid\n");
Expand All @@ -247,6 +290,11 @@ void CERT_print(OPTIONAL const X509* cert, OPTIONAL BIO* bio, unsigned long neg_
{
BIO_printf(bio, " no more valid\n");
}
#else
int error;
if (!X509_check_certificate_times(NULL, cert, &error))
BIO_printf(bio, " %s\n", X509_verify_cert_error_string(error));
#endif
X509_print_ex(bio, (X509*)cert, flags, compl(neg_cflags));
}
else
Expand Down Expand Up @@ -299,50 +347,30 @@ void LOG_cert(OPTIONAL const char* func, OPTIONAL const char* file, int lineno,

/* This is similar to LOG_cert() */
static void cert_msg(OPTIONAL const char* func, OPTIONAL const char* file, int lineno, severity level,
const char *uri, X509 *cert, const char *msg)
const char *src, X509 *cert, const char *msg)
{
char *subj = X509_NAME_oneline(X509_get_subject_name(cert), NULL, 0);
LOG(func, file, lineno, level, "Certificate from '%s' with subject '%s' %s", uri, subj, msg);
LOG(func, file, lineno, level, "Certificate from '%s' with subject '%s' %s", src, subj, msg);
OPENSSL_free(subj);
}


bool CERT_check(const char *uri, OPTIONAL X509 *cert, int type_CA,
bool CERT_check(const char *src, OPTIONAL X509 *cert, int type_CA,
OPTIONAL const X509_VERIFY_PARAM *vpm)
{
if (cert == NULL)
return true;
int res = 0;

#if OPENSSL_VERSION_NUMBER >= OPENSSL_V_3_0_0
res = X509_cmp_timeframe(vpm, X509_get0_notBefore(cert),
X509_get0_notAfter(cert));
#else
unsigned long flags = vpm == NULL ? 0 :
X509_VERIFY_PARAM_get_flags((X509_VERIFY_PARAM *)vpm);
if ((flags & X509_V_FLAG_NO_CHECK_TIME) == 0) {
time_t ref_time;
time_t *time = NULL;
if ((flags & X509_V_FLAG_USE_CHECK_TIME) != 0) {
ref_time = X509_VERIFY_PARAM_get_time(vpm);
time = &ref_time;
}
if (X509_cmp_time(X509_get0_notAfter(cert), time) < 0)
res = 1;
else if (X509_cmp_time(X509_get0_notBefore(cert), time) > 0)
res = -1;
}
#endif
int res = UTIL_cmp_timeframe(vpm, X509_get0_notBefore(cert), X509_get0_notAfter(cert));
bool ret = res == 0;
severity level = vpm == NULL ? LOG_WARNING : LOG_ERR;
if (!ret)
cert_msg(LOG_FUNC_FILE_LINE, level,
uri, cert, res > 0 ? "has expired" : "not yet valid");
src, cert, res > 0 ? "has expired" : "not yet valid");
uint32_t ex_flags = X509_get_extension_flags(cert);
if (type_CA >= 0 && (ex_flags & EXFLAG_V1) == 0) {
bool is_CA = (ex_flags & EXFLAG_CA) != 0;
if ((type_CA == 1) != is_CA) {
cert_msg(LOG_FUNC_FILE_LINE, level, uri, cert,
cert_msg(LOG_FUNC_FILE_LINE, level, src, cert,
is_CA ? "is not an EE cert" : "is not a CA cert");
ret = false;
}
Expand All @@ -351,14 +379,14 @@ bool CERT_check(const char *uri, OPTIONAL X509 *cert, int type_CA,
}


bool CERT_check_all(const char *uri, OPTIONAL STACK_OF(X509) *certs, int type_CA,
bool CERT_check_all(const char *src, OPTIONAL STACK_OF(X509) *certs, int type_CA,
OPTIONAL const X509_VERIFY_PARAM *vpm)
{
int i;
bool ret = true;

for (i = 0; i < sk_X509_num(certs /* may be NULL */); i++)
ret = CERT_check(uri, sk_X509_value(certs, i), type_CA, vpm)
ret = CERT_check(src, sk_X509_value(certs, i), type_CA, vpm)
&& ret; /* Having 'ret' after the '&&', all certs are checked. */
return ret;
}
Expand Down
22 changes: 1 addition & 21 deletions src/libsecutils/src/credentials/store.c
Original file line number Diff line number Diff line change
Expand Up @@ -546,26 +546,6 @@ bool STORE_set1_host_ip(X509_STORE* ts, OPTIONAL const char* name, OPTIONAL cons
}


static bool crl_expired(const X509_CRL* crl, const X509_VERIFY_PARAM* vpm)
{
time_t check_time, *ptime = 0;
unsigned long flags = X509_VERIFY_PARAM_get_flags((X509_VERIFY_PARAM*)vpm);

if((flags bitand X509_V_FLAG_NO_CHECK_TIME) not_eq 0)
{
return false;
}
if((flags bitand X509_V_FLAG_USE_CHECK_TIME) not_eq 0)
{
check_time = X509_VERIFY_PARAM_get_time(vpm);
ptime = &check_time;
}
const ASN1_TIME* crl_endtime = X509_CRL_get0_nextUpdate(crl);
/* well, should ignore expiry of base CRL if delta CRL is valid */
return (crl_endtime not_eq 0 and X509_cmp_time(crl_endtime, ptime) < 0);
}


/* extended from add_crls_store() in OpenSSL:apps/s_cb.c */
bool STORE_add_crls(X509_STORE* ts, OPTIONAL const STACK_OF(X509_CRL) * crls)
{
Expand All @@ -580,7 +560,7 @@ bool STORE_add_crls(X509_STORE* ts, OPTIONAL const STACK_OF(X509_CRL) * crls)
for(i = 0; i < sk_X509_CRL_num(crls); i++)
{
crl = sk_X509_CRL_value(crls, i);
if(crl_expired(crl, X509_STORE_get0_param(ts)) not_eq 0)
if (UTIL_cmp_timeframe(X509_STORE_get0_param(ts), NULL, X509_CRL_get0_nextUpdate(crl)) > 0)
{
char* issuer = X509_NAME_oneline(X509_CRL_get_issuer(crl), 0, 0);
if(issuer not_eq 0)
Expand Down
5 changes: 5 additions & 0 deletions src/libsecutils/src/storage/files.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ _Pragma("GCC diagnostic ignored \"-Wdeprecated-declarations\"")

static file_format_t adjust_format(const char* * file, file_format_t format, bool engine_ok)
{
#ifdef OPENSSL_NO_ENGINE
(void)engine_ok; /* unused parameter */
#endif

if(strncasecmp(*file, CONN_http_prefix, strlen(CONN_http_prefix)) is_eq 0 or
strncasecmp(*file, CONN_https_prefix, strlen(CONN_https_prefix)) is_eq 0)
{
Expand Down Expand Up @@ -956,6 +960,7 @@ static EVP_PKEY* load_key_engine(const char* keyid, const char* pass, const char
}
#else
LOG(FL_ERR, "crypto engines not supported in this build, request engine = '%s'", engine);
(void)keyid, (void)pass, (void)desc; /* unused parameters */
#endif
return pkey;
}
Expand Down