From: claudio Date: Tue, 29 Nov 2022 10:33:09 +0000 (+0000) Subject: Return an error string instead of surpressing the warning in valid_x509. X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=fd7a2857fa4728c58e12c9288700a816af23f87b;p=openbsd Return an error string instead of surpressing the warning in valid_x509. This way manifests can should a better error message when something fails. With and OK tb@ --- diff --git a/usr.sbin/rpki-client/extern.h b/usr.sbin/rpki-client/extern.h index d303bdd9092..d946f8b8b20 100644 --- a/usr.sbin/rpki-client/extern.h +++ b/usr.sbin/rpki-client/extern.h @@ -1,4 +1,4 @@ -/* $OpenBSD: extern.h,v 1.161 2022/11/26 12:02:36 job Exp $ */ +/* $OpenBSD: extern.h,v 1.162 2022/11/29 10:33:09 claudio Exp $ */ /* * Copyright (c) 2019 Kristaps Dzonsons * @@ -629,7 +629,7 @@ int valid_filename(const char *, size_t); int valid_uri(const char *, size_t, const char *); int valid_origin(const char *, const char *); int valid_x509(char *, X509_STORE_CTX *, X509 *, struct auth *, - struct crl *, int); + struct crl *, const char **); int valid_rsc(const char *, struct cert *, struct rsc *); int valid_econtent_version(const char *, const ASN1_INTEGER *); int valid_aspa(const char *, struct cert *, struct aspa *); diff --git a/usr.sbin/rpki-client/filemode.c b/usr.sbin/rpki-client/filemode.c index 2b3042163ff..f546f099aa9 100644 --- a/usr.sbin/rpki-client/filemode.c +++ b/usr.sbin/rpki-client/filemode.c @@ -1,4 +1,4 @@ -/* $OpenBSD: filemode.c,v 1.17 2022/11/26 12:02:37 job Exp $ */ +/* $OpenBSD: filemode.c,v 1.18 2022/11/29 10:33:09 claudio Exp $ */ /* * Copyright (c) 2019 Claudio Jeker * Copyright (c) 2019 Kristaps Dzonsons @@ -141,6 +141,7 @@ parse_load_certchain(char *uri) struct cert *cert; struct crl *crl; struct auth *a; + const char *errstr; int i; for (i = 0; i < MAX_CERT_DEPTH; i++) { @@ -171,9 +172,12 @@ parse_load_certchain(char *uri) uri = filestack[i]; crl = crl_get(&crlt, a); - if (!valid_x509(uri, ctx, cert->x509, a, crl, 0) || - !valid_cert(uri, a, cert)) + if (!valid_x509(uri, ctx, cert->x509, a, crl, &errstr) || + !valid_cert(uri, a, cert)) { + if (errstr != NULL) + warnx("%s: %s", uri, errstr); goto fail; + } cert->talid = a->cert->talid; a = auth_insert(&auths, cert, a); stack[i] = NULL; @@ -407,6 +411,7 @@ proc_parser_file(char *file, unsigned char *buf, size_t len) if (aia != NULL) { struct auth *a; struct crl *c; + const char *errstr; char *crl_uri; int status; @@ -418,7 +423,7 @@ proc_parser_file(char *file, unsigned char *buf, size_t len) a = auth_find(&auths, aki); c = crl_get(&crlt, a); - if ((status = valid_x509(file, ctx, x509, a, c, 0))) { + if ((status = valid_x509(file, ctx, x509, a, c, &errstr))) { switch (type) { case RTYPE_ROA: status = roa->valid; @@ -438,8 +443,11 @@ proc_parser_file(char *file, unsigned char *buf, size_t len) } if (status) printf("OK"); - else + else { printf("Failed"); + if (errstr != NULL) + printf(", %s", errstr); + } } else if (is_ta) { if ((tal = find_tal(cert)) != NULL) { cert = ta_parse(file, cert, tal->pkey, tal->pkeysz); diff --git a/usr.sbin/rpki-client/parser.c b/usr.sbin/rpki-client/parser.c index 6f84e18ce51..5f4ac6a4300 100644 --- a/usr.sbin/rpki-client/parser.c +++ b/usr.sbin/rpki-client/parser.c @@ -1,4 +1,4 @@ -/* $OpenBSD: parser.c,v 1.78 2022/11/02 12:43:02 job Exp $ */ +/* $OpenBSD: parser.c,v 1.79 2022/11/29 10:33:09 claudio Exp $ */ /* * Copyright (c) 2019 Claudio Jeker * Copyright (c) 2019 Kristaps Dzonsons @@ -132,6 +132,7 @@ proc_parser_roa(char *file, const unsigned char *der, size_t len) struct auth *a; struct crl *crl; X509 *x509; + const char *errstr; if ((roa = roa_parse(&x509, file, der, len)) == NULL) return NULL; @@ -139,7 +140,8 @@ proc_parser_roa(char *file, const unsigned char *der, size_t len) a = valid_ski_aki(file, &auths, roa->ski, roa->aki); crl = crl_get(&crlt, a); - if (!valid_x509(file, ctx, x509, a, crl, 0)) { + if (!valid_x509(file, ctx, x509, a, crl, &errstr)) { + warnx("%s: %s", file, errstr); X509_free(x509); roa_free(roa); return NULL; @@ -232,6 +234,7 @@ parse_load_crl_from_mft(struct entity *entp, struct mft *mft, enum location loc) if (!valid_hash(f, flen, mft->crlhash, sizeof(mft->crlhash))) goto next; crl = crl_parse(fn, f, flen); + next: free(f); free(fn); @@ -255,19 +258,21 @@ next: */ static struct mft * proc_parser_mft_pre(char *file, const unsigned char *der, size_t len, - struct entity *entp, enum location loc, struct crl **crl) + struct entity *entp, enum location loc, struct crl **crl, + const char **errstr) { struct mft *mft; X509 *x509; struct auth *a; *crl = NULL; + *errstr = NULL; if ((mft = mft_parse(&x509, file, der, len)) == NULL) return NULL; *crl = parse_load_crl_from_mft(entp, mft, loc); a = valid_ski_aki(file, &auths, mft->ski, mft->aki); - if (!valid_x509(file, ctx, x509, a, *crl, 1)) { + if (!valid_x509(file, ctx, x509, a, *crl, errstr)) { X509_free(x509); mft_free(mft); crl_free(*crl); @@ -285,13 +290,16 @@ proc_parser_mft_pre(char *file, const unsigned char *der, size_t len, * Return the mft on success or NULL on failure. */ static struct mft * -proc_parser_mft_post(char *file, struct mft *mft, const char *path) +proc_parser_mft_post(char *file, struct mft *mft, const char *path, + const char *errstr) { /* check that now is not before from */ time_t now = time(NULL); if (mft == NULL) { - warnx("%s: no valid mft available", file); + if (errstr == NULL) + errstr = "no valid mft available"; + warnx("%s: %s", file, errstr); return NULL; } @@ -330,6 +338,7 @@ proc_parser_mft(struct entity *entp, struct mft **mp) struct mft *mft1 = NULL, *mft2 = NULL; struct crl *crl, *crl1 = NULL, *crl2 = NULL; char *f, *file, *file1, *file2; + const char *err1, *err2; size_t flen; *mp = NULL; @@ -341,7 +350,7 @@ proc_parser_mft(struct entity *entp, struct mft **mp) if (f == NULL && errno != ENOENT) warn("parse file %s", file1); mft1 = proc_parser_mft_pre(file1, f, flen, entp, DIR_VALID, - &crl1); + &crl1, &err1); free(f); } if (file2 != NULL) { @@ -349,22 +358,27 @@ proc_parser_mft(struct entity *entp, struct mft **mp) if (f == NULL && errno != ENOENT) warn("parse file %s", file2); mft2 = proc_parser_mft_pre(file2, f, flen, entp, DIR_TEMP, - &crl2); + &crl2, &err2); free(f); } + /* overload error from temp file if it is set */ + if (mft1 == NULL && mft2 == NULL) + if (err2 != NULL) + err1 = err2; + if (mft_compare(mft1, mft2) == 1) { mft_free(mft2); crl_free(crl2); free(file2); - *mp = proc_parser_mft_post(file1, mft1, entp->path); + *mp = proc_parser_mft_post(file1, mft1, entp->path, err1); crl = crl1; file = file1; } else { mft_free(mft1); crl_free(crl1); free(file1); - *mp = proc_parser_mft_post(file2, mft2, entp->path); + *mp = proc_parser_mft_post(file2, mft2, entp->path, err2); crl = crl2; file = file2; } @@ -393,6 +407,7 @@ proc_parser_cert(char *file, const unsigned char *der, size_t len) struct cert *cert; struct crl *crl; struct auth *a; + const char *errstr = NULL; /* Extract certificate data. */ @@ -404,8 +419,10 @@ proc_parser_cert(char *file, const unsigned char *der, size_t len) a = valid_ski_aki(file, &auths, cert->ski, cert->aki); crl = crl_get(&crlt, a); - if (!valid_x509(file, ctx, cert->x509, a, crl, 0) || + if (!valid_x509(file, ctx, cert->x509, a, crl, &errstr) || !valid_cert(file, a, cert)) { + if (errstr != NULL) + warnx("%s: %s", file, errstr); cert_free(cert); return NULL; } @@ -465,10 +482,11 @@ proc_parser_root_cert(char *file, const unsigned char *der, size_t len, static void proc_parser_gbr(char *file, const unsigned char *der, size_t len) { - struct gbr *gbr; - X509 *x509; - struct crl *crl; - struct auth *a; + struct gbr *gbr; + X509 *x509; + struct crl *crl; + struct auth *a; + const char *errstr; if ((gbr = gbr_parse(&x509, file, der, len)) == NULL) return; @@ -477,7 +495,8 @@ proc_parser_gbr(char *file, const unsigned char *der, size_t len) crl = crl_get(&crlt, a); /* return value can be ignored since nothing happens here */ - valid_x509(file, ctx, x509, a, crl, 0); + if (!valid_x509(file, ctx, x509, a, crl, &errstr)) + warnx("%s: %s", file, errstr); X509_free(x509); gbr_free(gbr); @@ -489,10 +508,11 @@ proc_parser_gbr(char *file, const unsigned char *der, size_t len) static struct aspa * proc_parser_aspa(char *file, const unsigned char *der, size_t len) { - struct aspa *aspa; - struct auth *a; - struct crl *crl; - X509 *x509; + struct aspa *aspa; + struct auth *a; + struct crl *crl; + X509 *x509; + const char *errstr; if ((aspa = aspa_parse(&x509, file, der, len)) == NULL) return NULL; @@ -500,7 +520,8 @@ proc_parser_aspa(char *file, const unsigned char *der, size_t len) a = valid_ski_aki(file, &auths, aspa->ski, aspa->aki); crl = crl_get(&crlt, a); - if (!valid_x509(file, ctx, x509, a, crl, 0)) { + if (!valid_x509(file, ctx, x509, a, crl, &errstr)) { + warnx("%s: %s", file, errstr); X509_free(x509); aspa_free(aspa); return NULL; @@ -526,11 +547,12 @@ proc_parser_aspa(char *file, const unsigned char *der, size_t len) static struct tak * proc_parser_tak(char *file, const unsigned char *der, size_t len) { - struct tak *tak; - X509 *x509; - struct crl *crl; - struct auth *a; - int rc = 0; + struct tak *tak; + X509 *x509; + struct crl *crl; + struct auth *a; + const char *errstr; + int rc = 0; if ((tak = tak_parse(&x509, file, der, len)) == NULL) return NULL; @@ -538,8 +560,10 @@ proc_parser_tak(char *file, const unsigned char *der, size_t len) a = valid_ski_aki(file, &auths, tak->ski, tak->aki); crl = crl_get(&crlt, a); - if (!valid_x509(file, ctx, x509, a, crl, 0)) + if (!valid_x509(file, ctx, x509, a, crl, &errstr)) { + warnx("%s: %s", file, errstr); goto out; + } /* TAK EE must be signed by self-signed CA */ if (a->parent != NULL) diff --git a/usr.sbin/rpki-client/validate.c b/usr.sbin/rpki-client/validate.c index d7623808704..e69503d15ae 100644 --- a/usr.sbin/rpki-client/validate.c +++ b/usr.sbin/rpki-client/validate.c @@ -1,4 +1,4 @@ -/* $OpenBSD: validate.c,v 1.47 2022/11/26 12:02:37 job Exp $ */ +/* $OpenBSD: validate.c,v 1.48 2022/11/29 10:33:09 claudio Exp $ */ /* * Copyright (c) 2019 Kristaps Dzonsons * @@ -369,18 +369,20 @@ build_crls(const struct crl *crl, STACK_OF(X509_CRL) **crls) /* * Validate the X509 certificate. If crl is NULL don't check CRL. * Returns 1 for valid certificates, returns 0 if there is a verify error + * and sets *errstr to the error returned by X509_verify_cert_error_string(). */ int valid_x509(char *file, X509_STORE_CTX *store_ctx, X509 *x509, struct auth *a, - struct crl *crl, int nowarn) + struct crl *crl, const char **errstr) { X509_VERIFY_PARAM *params; ASN1_OBJECT *cp_oid; STACK_OF(X509) *chain; STACK_OF(X509_CRL) *crls = NULL; unsigned long flags; - int c; + int error; + *errstr = NULL; build_chain(a, &chain); build_crls(crl, &crls); @@ -405,9 +407,8 @@ valid_x509(char *file, X509_STORE_CTX *store_ctx, X509 *x509, struct auth *a, X509_STORE_CTX_set0_crls(store_ctx, crls); if (X509_verify_cert(store_ctx) <= 0) { - c = X509_STORE_CTX_get_error(store_ctx); - if (!nowarn || verbose > 1) - warnx("%s: %s", file, X509_verify_cert_error_string(c)); + error = X509_STORE_CTX_get_error(store_ctx); + *errstr = X509_verify_cert_error_string(error); X509_STORE_CTX_cleanup(store_ctx); sk_X509_free(chain); sk_X509_CRL_free(crls);