This is a follow up to the valid_x509() commit form earlier today.
tb@ suggested that the crl check should be grouped together.
After some thought I decided to do this all different.
First of all introduce a checkcrl flag which turns on
X509_V_FLAG_CRL_CHECK. This prevents code that expects a CRL to accept a
cert where the CRL is NULL. Apart from this build_crls(),
X509_STORE_CTX_set0_crls() and sk_X509_CRL_free() handle NULL inputs just
fine so drop the if (crl != NULL) check for them.
I think this is better and more secure
--
:wq Claudio
Index: parser.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v
retrieving revision 1.43
diff -u -p -r1.43 parser.c
--- parser.c 18 Jan 2022 16:36:49 -0000 1.43
+++ parser.c 18 Jan 2022 17:01:55 -0000
@@ -204,15 +204,15 @@ verify_cb(int ok, X509_STORE_CTX *store_
* Returns 1 for valid certificates, returns 0 if there is a verify error
*/
static int
-valid_x509(char *file, X509 *x509, struct auth *a, struct crl *crl)
+valid_x509(char *file, X509 *x509, struct auth *a, struct crl *crl,
+ int checkcrl)
{
STACK_OF(X509) *chain;
STACK_OF(X509_CRL) *crls = NULL;
int c;
build_chain(a, &chain);
- if (crl != NULL)
- build_crls(crl, &crls);
+ build_crls(crl, &crls);
assert(x509 != NULL);
if (!X509_STORE_CTX_init(ctx, NULL, x509, NULL))
@@ -221,12 +221,11 @@ valid_x509(char *file, X509 *x509, struc
X509_STORE_CTX_set_verify_cb(ctx, verify_cb);
if (!X509_STORE_CTX_set_app_data(ctx, file))
cryptoerrx("X509_STORE_CTX_set_app_data");
- if (crl != NULL)
+ if (checkcrl)
X509_STORE_CTX_set_flags(ctx, X509_V_FLAG_CRL_CHECK);
X509_STORE_CTX_set_depth(ctx, MAX_CERT_DEPTH);
X509_STORE_CTX_set0_trusted_stack(ctx, chain);
- if (crl != NULL)
- X509_STORE_CTX_set0_crls(ctx, crls);
+ X509_STORE_CTX_set0_crls(ctx, crls);
if (X509_verify_cert(ctx) <= 0) {
c = X509_STORE_CTX_get_error(ctx);
@@ -262,7 +261,7 @@ proc_parser_roa(char *file, const unsign
a = valid_ski_aki(file, &auths, roa->ski, roa->aki);
crl = get_crl(a);
- if (!valid_x509(file, x509, a, crl)) {
+ if (!valid_x509(file, x509, a, crl, 1)) {
X509_free(x509);
roa_free(roa);
return NULL;
@@ -361,7 +360,7 @@ proc_parser_mft(char *file, const unsign
a = valid_ski_aki(file, &auths, mft->ski, mft->aki);
- if (!valid_x509(file, x509, a, NULL)) {
+ if (!valid_x509(file, x509, a, NULL, 0)) {
mft_free(mft);
X509_free(x509);
return NULL;
@@ -405,7 +404,7 @@ proc_parser_cert(char *file, const unsig
a = valid_ski_aki(file, &auths, cert->ski, cert->aki);
crl = get_crl(a);
- if (!valid_x509(file, cert->x509, a, crl)) {
+ if (!valid_x509(file, cert->x509, a, crl, 1)) {
cert_free(cert);
return NULL;
}
@@ -569,7 +568,7 @@ proc_parser_gbr(char *file, const unsign
crl = get_crl(a);
/* return value can be ignored since nothing happens here */
- valid_x509(file, x509, a, crl);
+ valid_x509(file, x509, a, crl, 1);
X509_free(x509);
gbr_free(gbr);
