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 d303bdd..d946f8b 100644 (file)
--- 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 <kristaps@bsd.lv>
  *
@@ -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 2b30421..f546f09 100644 (file)
--- 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 <claudio@openbsd.org>
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
@@ -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 6f84e18..5f4ac6a 100644 (file)
--- 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 <claudio@openbsd.org>
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
@@ -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 d762380..e69503d 100644 (file)
--- 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 <kristaps@bsd.lv>
  *
@@ -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);