From: claudio Date: Fri, 29 Jan 2021 10:13:16 +0000 (+0000) Subject: A while ago rpki-client was changed to validate the sha256 hashes of X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=fc5c0efe7652b178b6c85fda052d8ce5336f2ce7;p=openbsd A while ago rpki-client was changed to validate the sha256 hashes of 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@ --- diff --git a/usr.sbin/rpki-client/cert.c b/usr.sbin/rpki-client/cert.c index 1d9c2b7b956..c0fad5ef901 100644 --- a/usr.sbin/rpki-client/cert.c +++ b/usr.sbin/rpki-client/cert.c @@ -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 * @@ -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) { diff --git a/usr.sbin/rpki-client/cms.c b/usr.sbin/rpki-client/cms.c index 25788af5d99..593efa80684 100644 --- a/usr.sbin/rpki-client/cms.c +++ b/usr.sbin/rpki-client/cms.c @@ -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 * @@ -36,17 +36,16 @@ */ 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. diff --git a/usr.sbin/rpki-client/crl.c b/usr.sbin/rpki-client/crl.c index bf0e05496b1..b7f5688dfff 100644 --- a/usr.sbin/rpki-client/crl.c +++ b/usr.sbin/rpki-client/crl.c @@ -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 * @@ -29,14 +29,12 @@ #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); diff --git a/usr.sbin/rpki-client/extern.h b/usr.sbin/rpki-client/extern.h index d0cfdc2a65b..9cde6505f2d 100644 --- a/usr.sbin/rpki-client/extern.h +++ b/usr.sbin/rpki-client/extern.h @@ -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 * @@ -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. */ diff --git a/usr.sbin/rpki-client/gbr.c b/usr.sbin/rpki-client/gbr.c index 601cc7d1f3c..074ef9dc824 100644 --- a/usr.sbin/rpki-client/gbr.c +++ b/usr.sbin/rpki-client/gbr.c @@ -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 * @@ -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; diff --git a/usr.sbin/rpki-client/main.c b/usr.sbin/rpki-client/main.c index e46316cd9d2..57ab1faa9de 100644 --- a/usr.sbin/rpki-client/main.c +++ b/usr.sbin/rpki-client/main.c @@ -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 * @@ -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)); diff --git a/usr.sbin/rpki-client/mft.c b/usr.sbin/rpki-client/mft.c index 767a84da490..1f41bfecd03 100644 --- a/usr.sbin/rpki-client/mft.c +++ b/usr.sbin/rpki-client/mft.c @@ -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 * @@ -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); diff --git a/usr.sbin/rpki-client/roa.c b/usr.sbin/rpki-client/roa.c index 98985b15a42..24a615ce40d 100644 --- a/usr.sbin/rpki-client/roa.c +++ b/usr.sbin/rpki-client/roa.c @@ -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 * @@ -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;