A while ago rpki-client was changed to validate the sha256 hashes of
authorclaudio <claudio@openbsd.org>
Fri, 29 Jan 2021 10:13:16 +0000 (10:13 +0000)
committerclaudio <claudio@openbsd.org>
Fri, 29 Jan 2021 10:13:16 +0000 (10:13 +0000)
files referenced in MFT files during the validation of the MFT file.
An MFT is only valid if all files are present and their hashes are valid.
Because of this there is no longer the need to check the hash when
these files are parsed later on. Remove these checks for CRT, ROA and CRL
files. Use the presence of the pkey when parsing cert files to decide
if it is a root cert or not.
OK tb@

usr.sbin/rpki-client/cert.c
usr.sbin/rpki-client/cms.c
usr.sbin/rpki-client/crl.c
usr.sbin/rpki-client/extern.h
usr.sbin/rpki-client/gbr.c
usr.sbin/rpki-client/main.c
usr.sbin/rpki-client/mft.c
usr.sbin/rpki-client/roa.c

index 1d9c2b7..c0fad5e 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: cert.c,v 1.22 2021/01/08 08:09:07 claudio Exp $ */
+/*     $OpenBSD: cert.c,v 1.23 2021/01/29 10:13:16 claudio Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -973,18 +973,16 @@ out:
  * is also dereferenced.
  */
 static struct cert *
-cert_parse_inner(X509 **xp, const char *fn, const unsigned char *dgst, int ta)
+cert_parse_inner(X509 **xp, const char *fn, int ta)
 {
-       int              rc = 0, extsz, c, sz;
+       int              rc = 0, extsz, c;
        size_t           i;
        X509            *x = NULL;
        X509_EXTENSION  *ext = NULL;
        ASN1_OBJECT     *obj;
        struct parse     p;
-       BIO             *bio = NULL, *shamd;
+       BIO             *bio = NULL;
        FILE            *f;
-       EVP_MD          *md;
-       char             mdbuf[EVP_MAX_MD_SIZE];
 
        *xp = NULL;
 
@@ -1004,49 +1002,11 @@ cert_parse_inner(X509 **xp, const char *fn, const unsigned char *dgst, int ta)
        if ((p.res = calloc(1, sizeof(struct cert))) == NULL)
                err(1, NULL);
 
-       /*
-        * If we have a digest specified, create an MD chain that will
-        * automatically compute a digest during the X509 creation.
-        */
-
-       if (dgst != NULL) {
-               if ((shamd = BIO_new(BIO_f_md())) == NULL)
-                       cryptoerrx("BIO_new");
-               if (!BIO_set_md(shamd, EVP_sha256()))
-                       cryptoerrx("BIO_set_md");
-               if ((bio = BIO_push(shamd, bio)) == NULL)
-                       cryptoerrx("BIO_push");
-       }
-
        if ((x = *xp = d2i_X509_bio(bio, NULL)) == NULL) {
                cryptowarnx("%s: d2i_X509_bio", p.fn);
                goto out;
        }
 
-       /*
-        * If we have a digest, find it in the chain (we'll already have
-        * made it, so assert otherwise) and verify it.
-        */
-
-       if (dgst != NULL) {
-               shamd = BIO_find_type(bio, BIO_TYPE_MD);
-               assert(shamd != NULL);
-
-               if (!BIO_get_md(shamd, &md))
-                       cryptoerrx("BIO_get_md");
-               assert(EVP_MD_type(md) == NID_sha256);
-
-               if ((sz = BIO_gets(shamd, mdbuf, EVP_MAX_MD_SIZE)) < 0)
-                       cryptoerrx("BIO_gets");
-               assert(sz == SHA256_DIGEST_LENGTH);
-
-               if (memcmp(mdbuf, dgst, SHA256_DIGEST_LENGTH)) {
-                       if (verbose > 0)
-                               warnx("%s: bad message digest", p.fn);
-                       goto out;
-               }
-       }
-
        /* Look for X509v3 extensions. */
 
        if ((extsz = X509_get_ext_count(x)) < 0)
@@ -1156,10 +1116,10 @@ out:
 }
 
 struct cert *
-cert_parse(X509 **xp, const char *fn, const unsigned char *dgst)
+cert_parse(X509 **xp, const char *fn)
 {
 
-       return cert_parse_inner(xp, fn, dgst, 0);
+       return cert_parse_inner(xp, fn, 0);
 }
 
 struct cert *
@@ -1169,7 +1129,7 @@ ta_parse(X509 **xp, const char *fn, const unsigned char *pkey, size_t pkeysz)
        struct cert     *p;
        int              rc = 0;
 
-       if ((p = cert_parse_inner(xp, fn, NULL, 1)) == NULL)
+       if ((p = cert_parse_inner(xp, fn, 1)) == NULL)
                return NULL;
 
        if (pkey != NULL) {
index 25788af..593efa8 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: cms.c,v 1.7 2020/04/02 09:16:43 claudio Exp $ */
+/*     $OpenBSD: cms.c,v 1.8 2021/01/29 10:13:16 claudio Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
  */
 unsigned char *
 cms_parse_validate(X509 **xp, const char *fn,
-    const char *oid, const unsigned char *dgst, size_t *rsz)
+    const char *oid, size_t *rsz)
 {
        const ASN1_OBJECT       *obj;
        ASN1_OCTET_STRING       **os = NULL;
-       BIO                     *bio = NULL, *shamd;
+       BIO                     *bio = NULL;
        CMS_ContentInfo         *cms;
        FILE                    *f;
-       char                     buf[128], mdbuf[EVP_MAX_MD_SIZE];
+       char                     buf[128];
        int                      rc = 0, sz;
        STACK_OF(X509)          *certs = NULL;
-       EVP_MD                  *md;
        unsigned char           *res = NULL;
 
        *rsz = 0;
@@ -66,48 +65,11 @@ cms_parse_validate(X509 **xp, const char *fn,
                return NULL;
        }
 
-       /*
-        * If we have a digest specified, create an MD chain that will
-        * automatically compute a digest during the CMS creation.
-        */
-
-       if (dgst != NULL) {
-               if ((shamd = BIO_new(BIO_f_md())) == NULL)
-                       cryptoerrx("BIO_new");
-               if (!BIO_set_md(shamd, EVP_sha256()))
-                       cryptoerrx("BIO_set_md");
-               if ((bio = BIO_push(shamd, bio)) == NULL)
-                       cryptoerrx("BIO_push");
-       }
-
        if ((cms = d2i_CMS_bio(bio, NULL)) == NULL) {
                cryptowarnx("%s: RFC 6488: failed CMS parse", fn);
                goto out;
        }
 
-       /*
-        * If we have a digest, find it in the chain (we'll already have
-        * made it, so assert otherwise) and verify it.
-        */
-
-       if (dgst != NULL) {
-               shamd = BIO_find_type(bio, BIO_TYPE_MD);
-               assert(shamd != NULL);
-
-               if (!BIO_get_md(shamd, &md))
-                       cryptoerrx("BIO_get_md");
-               assert(EVP_MD_type(md) == NID_sha256);
-
-               if ((sz = BIO_gets(shamd, mdbuf, EVP_MAX_MD_SIZE)) < 0)
-                       cryptoerrx("BIO_gets");
-               assert(sz == SHA256_DIGEST_LENGTH);
-
-               if (memcmp(mdbuf, dgst, SHA256_DIGEST_LENGTH)) {
-                       warnx("%s: RFC 6488: bad message digest", fn);
-                       goto out;
-               }
-       }
-
        /*
         * The CMS is self-signed with a signing certifiate.
         * Verify that the self-signage is correct.
index bf0e054..b7f5688 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: crl.c,v 1.9 2020/09/12 15:46:48 claudio Exp $ */
+/*     $OpenBSD: crl.c,v 1.10 2021/01/29 10:13:16 claudio Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
 #include "extern.h"
 
 X509_CRL *
-crl_parse(const char *fn, const unsigned char *dgst)
+crl_parse(const char *fn)
 {
-       int              rc = 0, sz;
+       int              rc = 0;
        X509_CRL        *x = NULL;
-       BIO             *bio = NULL, *shamd;
+       BIO             *bio = NULL;
        FILE            *f;
-       EVP_MD          *md;
-       char             mdbuf[EVP_MAX_MD_SIZE];
 
        if ((f = fopen(fn, "rb")) == NULL) {
                warn("%s", fn);
@@ -49,49 +47,11 @@ crl_parse(const char *fn, const unsigned char *dgst)
                return NULL;
        }
 
-       /*
-        * If we have a digest specified, create an MD chain that will
-        * automatically compute a digest during the X509 creation.
-        */
-
-       if (dgst != NULL) {
-               if ((shamd = BIO_new(BIO_f_md())) == NULL)
-                       cryptoerrx("BIO_new");
-               if (!BIO_set_md(shamd, EVP_sha256()))
-                       cryptoerrx("BIO_set_md");
-               if ((bio = BIO_push(shamd, bio)) == NULL)
-                       cryptoerrx("BIO_push");
-       }
-
        if ((x = d2i_X509_CRL_bio(bio, NULL)) == NULL) {
                cryptowarnx("%s: d2i_X509_CRL_bio", fn);
                goto out;
        }
 
-       /*
-        * If we have a digest, find it in the chain (we'll already have
-        * made it, so assert otherwise) and verify it.
-        */
-
-       if (dgst != NULL) {
-               shamd = BIO_find_type(bio, BIO_TYPE_MD);
-               assert(shamd != NULL);
-
-               if (!BIO_get_md(shamd, &md))
-                       cryptoerrx("BIO_get_md");
-               assert(EVP_MD_type(md) == NID_sha256);
-
-               if ((sz = BIO_gets(shamd, mdbuf, EVP_MAX_MD_SIZE)) < 0)
-                       cryptoerrx("BIO_gets");
-               assert(sz == SHA256_DIGEST_LENGTH);
-
-               if (memcmp(mdbuf, dgst, SHA256_DIGEST_LENGTH)) {
-                       if (verbose > 0)
-                               warnx("%s: bad message digest", fn);
-                       goto out;
-               }
-       }
-
        rc = 1;
 out:
        BIO_free_all(bio);
index d0cfdc2..9cde650 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: extern.h,v 1.37 2021/01/08 08:09:07 claudio Exp $ */
+/*     $OpenBSD: extern.h,v 1.38 2021/01/29 10:13:16 claudio Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -299,7 +299,7 @@ struct tal  *tal_read(int);
 
 void            cert_buffer(struct ibuf *, const struct cert *);
 void            cert_free(struct cert *);
-struct cert    *cert_parse(X509 **, const char *, const unsigned char *);
+struct cert    *cert_parse(X509 **, const char *);
 struct cert    *ta_parse(X509 **, const char *, const unsigned char *, size_t);
 struct cert    *cert_read(int);
 
@@ -311,7 +311,7 @@ struct mft  *mft_read(int);
 
 void            roa_buffer(struct ibuf *, const struct roa *);
 void            roa_free(struct roa *);
-struct roa     *roa_parse(X509 **, const char *, const unsigned char *);
+struct roa     *roa_parse(X509 **, const char *);
 struct roa     *roa_read(int);
 void            roa_insert_vrps(struct vrp_tree *, struct roa *, size_t *,
                    size_t *);
@@ -320,7 +320,7 @@ void                 gbr_free(struct gbr *);
 struct gbr     *gbr_parse(X509 **, const char *);
 
 /* crl.c */
-X509_CRL       *crl_parse(const char *, const unsigned char *);
+X509_CRL       *crl_parse(const char *);
 void            free_crl(struct crl *);
 
 /* Validation of our objects. */
@@ -336,7 +336,7 @@ int          valid_roa(const char *, struct auth_tree *, struct roa *);
 /* Working with CMS files. */
 
 unsigned char  *cms_parse_validate(X509 **, const char *,
-                       const char *, const unsigned char *, size_t *);
+                       const char *, size_t *);
 
 /* Work with RFC 3779 IP addresses, prefixes, ranges. */
 
index 601cc7d..074ef9d 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: gbr.c,v 1.2 2020/12/29 14:51:11 job Exp $ */
+/*     $OpenBSD: gbr.c,v 1.3 2021/01/29 10:13:16 claudio Exp $ */
 /*
  * Copyright (c) 2020 Claudio Jeker <claudio@openbsd.org>
  *
@@ -52,7 +52,7 @@ gbr_parse(X509 **x509, const char *fn)
        /* OID from section 9.1, RFC 6493. */
 
        cms = cms_parse_validate(x509, fn,
-           "1.2.840.113549.1.9.16.1.35", NULL, &cmsz);
+           "1.2.840.113549.1.9.16.1.35", &cmsz);
        if (cms == NULL)
                return NULL;
 
index e46316c..57ab1fa 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: main.c,v 1.90 2021/01/08 08:45:55 claudio Exp $ */
+/*     $OpenBSD: main.c,v 1.91 2021/01/29 10:13:16 claudio Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -109,8 +109,6 @@ static struct       repotab {
 struct entity {
        enum rtype       type; /* type of entity (not RTYPE_EOF) */
        char            *uri; /* file or rsync:// URI */
-       int              has_dgst; /* whether dgst is specified */
-       unsigned char    dgst[SHA256_DIGEST_LENGTH]; /* optional */
        ssize_t          repo; /* repo index or <0 if w/o repo */
        int              has_pkey; /* whether pkey/sz is specified */
        unsigned char   *pkey; /* public key (optional) */
@@ -227,9 +225,6 @@ entity_read_req(int fd, struct entity *ent)
 
        io_simple_read(fd, &ent->type, sizeof(enum rtype));
        io_str_read(fd, &ent->uri);
-       io_simple_read(fd, &ent->has_dgst, sizeof(int));
-       if (ent->has_dgst)
-               io_simple_read(fd, ent->dgst, sizeof(ent->dgst));
        io_simple_read(fd, &ent->has_pkey, sizeof(int));
        if (ent->has_pkey)
                io_buf_read_alloc(fd, (void **)&ent->pkey, &ent->pkeysz);
@@ -246,9 +241,6 @@ entity_buffer_req(struct ibuf *b, const struct entity *ent)
 
        io_simple_buffer(b, &ent->type, sizeof(ent->type));
        io_str_buffer(b, ent->uri);
-       io_simple_buffer(b, &ent->has_dgst, sizeof(int));
-       if (ent->has_dgst)
-               io_simple_buffer(b, ent->dgst, sizeof(ent->dgst));
        io_simple_buffer(b, &ent->has_pkey, sizeof(int));
        if (ent->has_pkey)
                io_buf_buffer(b, ent->pkey, ent->pkeysz);
@@ -370,8 +362,8 @@ repo_filename(const struct repo *repo, const char *uri)
  */
 static void
 entityq_add(struct msgbuf *msgq, struct entityq *q, char *file, enum rtype type,
-    const struct repo *rp, const unsigned char *dgst,
-    const unsigned char *pkey, size_t pkeysz, char *descr)
+    const struct repo *rp, const unsigned char *pkey, size_t pkeysz,
+    char *descr)
 {
        struct entity   *p;
 
@@ -381,10 +373,7 @@ entityq_add(struct msgbuf *msgq, struct entityq *q, char *file, enum rtype type,
        p->type = type;
        p->uri = file;
        p->repo = (rp != NULL) ? (ssize_t)rp->id : -1;
-       p->has_dgst = dgst != NULL;
        p->has_pkey = pkey != NULL;
-       if (p->has_dgst)
-               memcpy(p->dgst, dgst, sizeof(p->dgst));
        if (p->has_pkey) {
                p->pkeysz = pkeysz;
                if ((p->pkey = malloc(pkeysz)) == NULL)
@@ -435,7 +424,7 @@ queue_add_from_mft(struct msgbuf *msgq, struct entityq *q, const char *mft,
         * that the repository has already been loaded.
         */
 
-       entityq_add(msgq, q, nfile, type, NULL, file->hash, NULL, 0, NULL);
+       entityq_add(msgq, q, nfile, type, NULL, NULL, 0, NULL);
 }
 
 /*
@@ -526,7 +515,7 @@ queue_add_tal(struct msgbuf *msgq, struct entityq *q, const char *file)
        }
 
        /* Not in a repository, so directly add to queue. */
-       entityq_add(msgq, q, nfile, RTYPE_TAL, NULL, NULL, NULL, 0, buf);
+       entityq_add(msgq, q, nfile, RTYPE_TAL, NULL, NULL, 0, buf);
        /* entityq_add makes a copy of buf */
        free(buf);
 }
@@ -557,7 +546,7 @@ queue_add_from_tal(struct msgbuf *procq, struct msgbuf *rsyncq,
        repo = repo_lookup(rsyncq, uri);
        nfile = repo_filename(repo, uri);
 
-       entityq_add(procq, q, nfile, RTYPE_CER, repo, NULL, tal->pkey,
+       entityq_add(procq, q, nfile, RTYPE_CER, repo, tal->pkey,
            tal->pkeysz, tal->descr);
 }
 
@@ -578,7 +567,7 @@ queue_add_from_cert(struct msgbuf *procq, struct msgbuf *rsyncq,
        repo = repo_lookup(rsyncq, rsyncuri);
        nfile = repo_filename(repo, rsyncuri);
 
-       entityq_add(procq, q, nfile, RTYPE_MFT, repo, NULL, NULL, 0, NULL);
+       entityq_add(procq, q, nfile, RTYPE_MFT, repo, NULL, 0, NULL);
 }
 
 /*
@@ -598,8 +587,7 @@ proc_parser_roa(struct entity *entp,
        STACK_OF(X509)          *chain;
        STACK_OF(X509_CRL)      *crls;
 
-       assert(entp->has_dgst);
-       if ((roa = roa_parse(&x509, entp->uri, entp->dgst)) == NULL)
+       if ((roa = roa_parse(&x509, entp->uri)) == NULL)
                return NULL;
 
        a = valid_ski_aki(entp->uri, auths, roa->ski, roa->aki);
@@ -662,7 +650,6 @@ proc_parser_mft(struct entity *entp, X509_STORE *store, X509_STORE_CTX *ctx,
        struct auth             *a;
        STACK_OF(X509)          *chain;
 
-       assert(!entp->has_dgst);
        if ((mft = mft_parse(&x509, entp->uri)) == NULL)
                return NULL;
 
@@ -717,12 +704,11 @@ proc_parser_cert(const struct entity *entp,
        STACK_OF(X509)          *chain;
        STACK_OF(X509_CRL)      *crls;
 
-       assert(entp->has_dgst);
        assert(!entp->has_pkey);
 
        /* Extract certificate data and X509. */
 
-       cert = cert_parse(&x509, entp->uri, entp->dgst);
+       cert = cert_parse(&x509, entp->uri);
        if (cert == NULL)
                return NULL;
 
@@ -814,7 +800,6 @@ proc_parser_root_cert(const struct entity *entp,
        struct auth             *na;
        char                    *tal;
 
-       assert(!entp->has_dgst);
        assert(entp->has_pkey);
 
        /* Extract certificate data and X509. */
@@ -902,10 +887,8 @@ proc_parser_crl(struct entity *entp, X509_STORE *store,
 {
        X509_CRL                *x509_crl;
        struct crl              *crl;
-       const unsigned char     *dgst;
 
-       dgst = entp->has_dgst ? entp->dgst : NULL;
-       if ((x509_crl = crl_parse(entp->uri, dgst)) != NULL) {
+       if ((x509_crl = crl_parse(entp->uri)) != NULL) {
                if ((crl = malloc(sizeof(*crl))) == NULL)
                        err(1, NULL);
                if ((crl->aki = x509_crl_get_aki(x509_crl)) == NULL)
@@ -1105,18 +1088,17 @@ proc_parser(int fd)
 
                switch (entp->type) {
                case RTYPE_TAL:
-                       assert(!entp->has_dgst);
                        if ((tal = tal_parse(entp->uri, entp->descr)) == NULL)
                                goto out;
                        tal_buffer(b, tal);
                        tal_free(tal);
                        break;
                case RTYPE_CER:
-                       if (entp->has_dgst)
-                               cert = proc_parser_cert(entp, store, ctx,
+                       if (entp->has_pkey)
+                               cert = proc_parser_root_cert(entp, store, ctx,
                                    &auths, &crlt);
                        else
-                               cert = proc_parser_root_cert(entp, store, ctx,
+                               cert = proc_parser_cert(entp, store, ctx,
                                    &auths, &crlt);
                        c = (cert != NULL);
                        io_simple_buffer(b, &c, sizeof(int));
@@ -1140,7 +1122,6 @@ proc_parser(int fd)
                        proc_parser_crl(entp, store, ctx, &crlt);
                        break;
                case RTYPE_ROA:
-                       assert(entp->has_dgst);
                        roa = proc_parser_roa(entp, store, ctx, &auths, &crlt);
                        c = (roa != NULL);
                        io_simple_buffer(b, &c, sizeof(int));
index 767a84d..1f41bfe 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: mft.c,v 1.23 2021/01/08 08:09:07 claudio Exp $ */
+/*     $OpenBSD: mft.c,v 1.24 2021/01/29 10:13:16 claudio Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -384,7 +384,7 @@ mft_parse(X509 **x509, const char *fn)
        p.fn = fn;
 
        cms = cms_parse_validate(x509, fn, "1.2.840.113549.1.9.16.1.26",
-           NULL, &cmsz);
+           &cmsz);
        if (cms == NULL)
                return NULL;
        assert(*x509 != NULL);
index 98985b1..24a615c 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: roa.c,v 1.11 2021/01/08 08:09:07 claudio Exp $ */
+/*     $OpenBSD: roa.c,v 1.12 2021/01/29 10:13:16 claudio Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -323,13 +323,11 @@ out:
 }
 
 /*
- * Parse a full RFC 6482 file with a SHA256 digest "dgst" and signed by
- * the certificate "cacert" (the latter two are optional and may be
- * passed as NULL to disable).
+ * Parse a full RFC 6482 file.
  * Returns the ROA or NULL if the document was malformed.
  */
 struct roa *
-roa_parse(X509 **x509, const char *fn, const unsigned char *dgst)
+roa_parse(X509 **x509, const char *fn)
 {
        struct parse     p;
        size_t           cmsz;
@@ -342,7 +340,7 @@ roa_parse(X509 **x509, const char *fn, const unsigned char *dgst)
        /* OID from section 2, RFC 6482. */
 
        cms = cms_parse_validate(x509, fn,
-           "1.2.840.113549.1.9.16.1.24", dgst, &cmsz);
+           "1.2.840.113549.1.9.16.1.24", &cmsz);
        if (cms == NULL)
                return NULL;