From: claudio Date: Tue, 18 Jan 2022 16:29:06 +0000 (+0000) Subject: More fixing. Previous revert was incomplete. X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=4bccd3c188aa092d42a1c9615a976641585644d1;p=openbsd More fixing. Previous revert was incomplete. --- diff --git a/usr.sbin/rpki-client/parser.c b/usr.sbin/rpki-client/parser.c index 1ac396a4a22..9f5d6a32979 100644 --- a/usr.sbin/rpki-client/parser.c +++ b/usr.sbin/rpki-client/parser.c @@ -1,4 +1,4 @@ -/* $OpenBSD: parser.c,v 1.41 2022/01/18 16:24:55 claudio Exp $ */ +/* $OpenBSD: parser.c,v 1.42 2022/01/18 16:29:06 claudio Exp $ */ /* * Copyright (c) 2019 Claudio Jeker * Copyright (c) 2019 Kristaps Dzonsons @@ -50,6 +50,7 @@ static struct crl_tree crlt = RB_INITIALIZER(&crlt); struct parse_repo { RB_ENTRY(parse_repo) entry; char *path; + char *validpath; unsigned int id; }; @@ -72,20 +73,75 @@ repo_get(unsigned int id) } static void -repo_add(unsigned int id, char *path) +repo_add(unsigned int id, char *path, char *validpath) { struct parse_repo *rp; - if ((rp = malloc(sizeof(*rp))) == NULL) + if ((rp = calloc(1, sizeof(*rp))) == NULL) err(1, NULL); rp->id = id; - if ((rp->path = strdup(path)) == NULL) - err(1, NULL); + if (path != NULL) + if ((rp->path = strdup(path)) == NULL) + err(1, NULL); + if (validpath != NULL) + if ((rp->validpath = strdup(validpath)) == NULL) + err(1, NULL); if (RB_INSERT(repo_tree, &repos, rp) != NULL) errx(1, "repository already added: id %d, %s", id, path); } +/* + * Build access path to file based on repoid, path and file values. + * If wantalt == 1 the function can return NULL, if wantalt == 0 it + * can not fail. + */ +static char * +parse_filepath(unsigned int repoid, const char *path, const char *file, + int wantalt) +{ + struct parse_repo *rp; + char *fn, *repopath; + + /* build file path based on repoid, entity path and filename */ + rp = repo_get(repoid); + if (rp == NULL) { + /* no repo so no alternative path. */ + if (wantalt) + return NULL; + + if (path == NULL) { + if ((fn = strdup(file)) == NULL) + err(1, NULL); + } else { + if (asprintf(&fn, "%s/%s", path, file) == -1) + err(1, NULL); + } + } else { + if (wantalt || rp->path == NULL) + repopath = rp->validpath; + else + repopath = rp->path; + + if (repopath == NULL) + return NULL; + + if (path == NULL) { + if (asprintf(&fn, "%s/%s", repopath, file) == -1) + err(1, NULL); + } else { + if (asprintf(&fn, "%s/%s/%s", repopath, path, + file) == -1) + err(1, NULL); + } + } + return fn; +} + +/* + * Callback for X509_verify_cert() to handle critical extensions in old + * LibreSSL libraries or OpenSSL libs without RFC3779 support. + */ static int verify_cb(int ok, X509_STORE_CTX *store_ctx) { @@ -144,52 +200,74 @@ verify_cb(int ok, X509_STORE_CTX *store_ctx) } /* - * Parse and validate a ROA. - * This is standard stuff. - * Returns the roa on success, NULL on failure. + * 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 */ -static struct roa * -proc_parser_roa(char *file, const unsigned char *der, size_t len) +static int +valid_x509(char *file, X509 *x509, struct auth *a, struct crl *crl) { - struct roa *roa; - X509 *x509; - int c; - struct auth *a; STACK_OF(X509) *chain; - STACK_OF(X509_CRL) *crls; - struct crl *crl; - - if ((roa = roa_parse(&x509, file, der, len)) == NULL) - return NULL; + STACK_OF(X509_CRL) *crls = NULL; + int c; - a = valid_ski_aki(file, &auths, roa->ski, roa->aki); build_chain(a, &chain); - crl = get_crl(a); - build_crls(crl, &crls); + if (crl != NULL) + build_crls(crl, &crls); assert(x509 != NULL); if (!X509_STORE_CTX_init(ctx, NULL, x509, NULL)) cryptoerrx("X509_STORE_CTX_init"); + 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"); - X509_STORE_CTX_set_flags(ctx, X509_V_FLAG_CRL_CHECK); + if (crl != NULL) + 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); - X509_STORE_CTX_set0_crls(ctx, crls); + if (crl != NULL) + X509_STORE_CTX_set0_crls(ctx, crls); if (X509_verify_cert(ctx) <= 0) { c = X509_STORE_CTX_get_error(ctx); + warnx("%s: %s", file, X509_verify_cert_error_string(c)); X509_STORE_CTX_cleanup(ctx); - if (verbose > 0 || c != X509_V_ERR_UNABLE_TO_GET_CRL) - warnx("%s: %s", file, X509_verify_cert_error_string(c)); - X509_free(x509); - roa_free(roa); sk_X509_free(chain); sk_X509_CRL_free(crls); - return NULL; + return 0; } + X509_STORE_CTX_cleanup(ctx); + sk_X509_free(chain); + sk_X509_CRL_free(crls); + return 1; +} + +/* + * Parse and validate a ROA. + * This is standard stuff. + * Returns the roa on success, NULL on failure. + */ +static struct roa * +proc_parser_roa(char *file, const unsigned char *der, size_t len) +{ + struct roa *roa; + struct crl *crl; + struct auth *a; + X509 *x509; + + if ((roa = roa_parse(&x509, file, der, len)) == NULL) + return NULL; + + a = valid_ski_aki(file, &auths, roa->ski, roa->aki); + crl = get_crl(a); + + if (!valid_x509(file, x509, a, crl)) { + X509_free(x509); + roa_free(roa); + return NULL; + } + X509_free(x509); /* * Check CRL to figure out the soonest transitive expiry moment @@ -214,10 +292,6 @@ proc_parser_roa(char *file, const unsigned char *der, size_t len) if (valid_roa(file, &auths, roa)) roa->valid = 1; - sk_X509_free(chain); - sk_X509_CRL_free(crls); - X509_free(x509); - return roa; } @@ -229,13 +303,8 @@ int mft_check(const char *fn, struct mft *p) { size_t i; - int fd, rc = 1; - char *cp, *h, *path = NULL; - - /* Check hash of file now, but first build path for it */ - cp = strrchr(fn, '/'); - assert(cp != NULL); - assert(cp - fn < INT_MAX); + int fd, try, rc = 1; + char *h, *path; for (i = 0; i < p->filesz; i++) { const struct mftfile *m = &p->files[i]; @@ -246,15 +315,24 @@ mft_check(const char *fn, struct mft *p) free(h); continue; } - if (asprintf(&path, "%.*s/%s", (int)(cp - fn), fn, - m->file) == -1) - err(1, NULL); - fd = open(path, O_RDONLY); + + fd = -1; + try = 0; + path = NULL; + do { + free(path); + if ((path = parse_filepath(p->repoid, p->path, m->file, + try++)) == NULL) + break; + fd = open(path, O_RDONLY); + } while (fd == -1 && try < 2); + + free(path); + if (!valid_filehash(fd, m->hash, sizeof(m->hash))) { warnx("%s: bad message digest for %s", fn, m->file); rc = 0; } - free(path); } return rc; @@ -276,49 +354,31 @@ proc_parser_mft(char *file, const unsigned char *der, size_t len, { struct mft *mft; X509 *x509; - int c; struct auth *a; - STACK_OF(X509) *chain; if ((mft = mft_parse(&x509, file, der, len)) == NULL) return NULL; a = valid_ski_aki(file, &auths, mft->ski, mft->aki); - build_chain(a, &chain); - - if (!X509_STORE_CTX_init(ctx, NULL, x509, NULL)) - cryptoerrx("X509_STORE_CTX_init"); - - /* CRL checks disabled here because CRL is referenced from mft */ - 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"); - X509_STORE_CTX_set_depth(ctx, MAX_CERT_DEPTH); - X509_STORE_CTX_set0_trusted_stack(ctx, chain); - if (X509_verify_cert(ctx) <= 0) { - c = X509_STORE_CTX_get_error(ctx); - X509_STORE_CTX_cleanup(ctx); - warnx("%s: %s", file, X509_verify_cert_error_string(c)); + if (!valid_x509(file, x509, a, NULL)) { mft_free(mft); X509_free(x509); - sk_X509_free(chain); return NULL; } - - X509_STORE_CTX_cleanup(ctx); - sk_X509_free(chain); X509_free(x509); - if (!mft_check(file, mft)) { - mft_free(mft); - return NULL; - } - + mft->repoid = repoid; if (path != NULL) if ((mft->path = strdup(path)) == NULL) err(1, NULL); - mft->repoid = repoid; + + if (!mft->stale) + if (!mft_check(file, mft)) { + mft_free(mft); + return NULL; + } + return mft; } @@ -334,10 +394,8 @@ proc_parser_cert(char *file, const unsigned char *der, size_t len) { struct cert *cert; X509 *x509; - int c; - struct auth *a = NULL; - STACK_OF(X509) *chain; - STACK_OF(X509_CRL) *crls; + struct auth *a; + struct crl *crl; /* Extract certificate data and X509. */ @@ -346,35 +404,13 @@ proc_parser_cert(char *file, const unsigned char *der, size_t len) return NULL; a = valid_ski_aki(file, &auths, cert->ski, cert->aki); - build_chain(a, &chain); - build_crls(get_crl(a), &crls); - - assert(x509 != NULL); - if (!X509_STORE_CTX_init(ctx, NULL, x509, NULL)) - cryptoerrx("X509_STORE_CTX_init"); - - 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"); - 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); - X509_STORE_CTX_set0_crls(ctx, crls); + crl = get_crl(a); - if (X509_verify_cert(ctx) <= 0) { - c = X509_STORE_CTX_get_error(ctx); - warnx("%s: %s", file, X509_verify_cert_error_string(c)); - X509_STORE_CTX_cleanup(ctx); + if (!valid_x509(file, x509, a, crl)) { cert_free(cert); - sk_X509_free(chain); - sk_X509_CRL_free(crls); X509_free(x509); return NULL; } - - X509_STORE_CTX_cleanup(ctx); - sk_X509_free(chain); - sk_X509_CRL_free(crls); X509_free(x509); cert->talid = a->cert->talid; @@ -535,39 +571,18 @@ proc_parser_gbr(char *file, const unsigned char *der, size_t len) { struct gbr *gbr; X509 *x509; - int c; struct auth *a; - STACK_OF(X509) *chain; - STACK_OF(X509_CRL) *crls; + struct crl *crl; if ((gbr = gbr_parse(&x509, file, der, len)) == NULL) return; a = valid_ski_aki(file, &auths, gbr->ski, gbr->aki); + crl = get_crl(a); - build_chain(a, &chain); - build_crls(get_crl(a), &crls); - - assert(x509 != NULL); - if (!X509_STORE_CTX_init(ctx, NULL, x509, NULL)) - cryptoerrx("X509_STORE_CTX_init"); - 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"); - 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); - X509_STORE_CTX_set0_crls(ctx, crls); - - if (X509_verify_cert(ctx) <= 0) { - c = X509_STORE_CTX_get_error(ctx); - if (verbose > 0 || c != X509_V_ERR_UNABLE_TO_GET_CRL) - warnx("%s: %s", file, X509_verify_cert_error_string(c)); - } + /* return value can be ignored since nothing happens here */ + valid_x509(file, x509, a, crl); - X509_STORE_CTX_cleanup(ctx); - sk_X509_free(chain); - sk_X509_CRL_free(crls); X509_free(x509); gbr_free(gbr); } @@ -629,33 +644,40 @@ build_crls(const struct crl *crl, STACK_OF(X509_CRL) **crls) } static char * -parse_filepath(struct entity *entp) +parse_load_file(struct entity *entp, unsigned char **f, size_t *flen) { - struct parse_repo *rp; - char *file; + char *file, *nfile; - /* build file path based on repoid, entity path and filename */ - rp = repo_get(entp->repoid); - if (rp == NULL) { - if (entp->path == NULL) { - if ((file = strdup(entp->file)) == NULL) - err(1, NULL); - } else { - if (asprintf(&file, "%s/%s", entp->path, - entp->file) == -1) - err(1, NULL); - } - } else { - if (entp->path == NULL) { - if (asprintf(&file, "%s/%s", rp->path, - entp->file) == -1) - err(1, NULL); - } else { - if (asprintf(&file, "%s/%s/%s", rp->path, - entp->path, entp->file) == -1) - err(1, NULL); - } + file = parse_filepath(entp->repoid, entp->path, entp->file, 0); + + /* TAL files include the data already */ + if (entp->type == RTYPE_TAL) { + *f = NULL; + *flen = 0; + return file; } + + *f = load_file(file, flen); + if (*f != NULL) + return file; + + if (errno != ENOENT) + goto fail; + + /* try alternate file location */ + nfile = parse_filepath(entp->repoid, entp->path, entp->file, 1); + if (nfile == NULL) + goto fail; + + free(file); + file = nfile; + + *f = load_file(file, flen); + if (*f != NULL) + return file; + +fail: + warn("parse file %s", file); return file; } @@ -678,20 +700,14 @@ parse_entity(struct entityq *q, struct msgbuf *msgq) /* handle RTYPE_REPO first */ if (entp->type == RTYPE_REPO) { - repo_add(entp->repoid, entp->path); + repo_add(entp->repoid, entp->path, entp->file); entity_free(entp); continue; } - f = NULL; - file = parse_filepath(entp); - if (entp->type != RTYPE_TAL) { - f = load_file(file, &flen); - if (f == NULL) - warn("%s", file); - } + file = parse_load_file(entp, &f, &flen); - /* pass back at least type and filename */ + /* pass back at least type, repoid and filename */ b = io_new_buffer(); io_simple_buffer(b, &entp->type, sizeof(entp->type)); io_str_buffer(b, file); @@ -776,6 +792,7 @@ proc_parser(int fd) ERR_load_crypto_strings(); OpenSSL_add_all_ciphers(); OpenSSL_add_all_digests(); + x509_init_oid(); if ((ctx = X509_STORE_CTX_new()) == NULL) cryptoerrx("X509_STORE_CTX_new"); diff --git a/usr.sbin/rpki-client/roa.c b/usr.sbin/rpki-client/roa.c index 21cebd0d310..a67e6ff322d 100644 --- a/usr.sbin/rpki-client/roa.c +++ b/usr.sbin/rpki-client/roa.c @@ -1,4 +1,4 @@ -/* $OpenBSD: roa.c,v 1.36 2022/01/18 16:18:22 claudio Exp $ */ +/* $OpenBSD: roa.c,v 1.37 2022/01/18 16:29:06 claudio Exp $ */ /* * Copyright (c) 2019 Kristaps Dzonsons * @@ -346,7 +346,7 @@ roa_parse(X509 **x509, const char *fn, const unsigned char *der, size_t len) memset(&p, 0, sizeof(struct parse)); p.fn = fn; - cms = cms_parse_validate(x509, fn, der, len, roa_oid, &cmsz, 0); + cms = cms_parse_validate(x509, fn, der, len, roa_oid, &cmsz); if (cms == NULL) return NULL;