From 32c8d2fec81c05802745a5172933d5d01d6cf52e Mon Sep 17 00:00:00 2001 From: job Date: Thu, 13 Apr 2023 17:04:02 +0000 Subject: [PATCH] Check whether products listed on a manifest were issued by the same authority as the manifest itself OK tb@ --- usr.sbin/rpki-client/extern.h | 5 ++-- usr.sbin/rpki-client/main.c | 26 +++++++++++++------ usr.sbin/rpki-client/parser.c | 46 +++++++++++++++++++++------------ usr.sbin/rpki-client/validate.c | 17 ++++++++---- 4 files changed, 62 insertions(+), 32 deletions(-) diff --git a/usr.sbin/rpki-client/extern.h b/usr.sbin/rpki-client/extern.h index 563d43f9640..ea83b04dbde 100644 --- a/usr.sbin/rpki-client/extern.h +++ b/usr.sbin/rpki-client/extern.h @@ -1,4 +1,4 @@ -/* $OpenBSD: extern.h,v 1.176 2023/03/30 15:29:15 claudio Exp $ */ +/* $OpenBSD: extern.h,v 1.177 2023/04/13 17:04:02 job Exp $ */ /* * Copyright (c) 2019 Kristaps Dzonsons * @@ -507,6 +507,7 @@ struct entity { TAILQ_ENTRY(entity) entries; char *path; /* path relative to repository */ char *file; /* filename or valid repo path */ + char *mftaki; /* expected AKI (taken from Manifest) */ unsigned char *data; /* optional data blob */ size_t datasz; /* length of optional data blob */ unsigned int repoid; /* repository identifier */ @@ -666,7 +667,7 @@ void crl_tree_free(struct crl_tree *); /* Validation of our objects. */ struct auth *valid_ski_aki(const char *, struct auth_tree *, - const char *, const char *); + const char *, const char *, const char *); int valid_ta(const char *, struct auth_tree *, const struct cert *); int valid_cert(const char *, struct auth *, const struct cert *); diff --git a/usr.sbin/rpki-client/main.c b/usr.sbin/rpki-client/main.c index 84aefa62311..bfa71ff65c0 100644 --- a/usr.sbin/rpki-client/main.c +++ b/usr.sbin/rpki-client/main.c @@ -1,4 +1,4 @@ -/* $OpenBSD: main.c,v 1.232 2023/02/23 09:50:40 claudio Exp $ */ +/* $OpenBSD: main.c,v 1.233 2023/04/13 17:04:02 job Exp $ */ /* * Copyright (c) 2021 Claudio Jeker * Copyright (c) 2019 Kristaps Dzonsons @@ -119,6 +119,7 @@ entity_free(struct entity *ent) free(ent->path); free(ent->file); + free(ent->mftaki); free(ent->data); free(ent); } @@ -137,6 +138,7 @@ entity_read_req(struct ibuf *b, struct entity *ent) io_read_buf(b, &ent->talid, sizeof(ent->talid)); io_read_str(b, &ent->path); io_read_str(b, &ent->file); + io_read_str(b, &ent->mftaki); io_read_buf_alloc(b, (void **)&ent->data, &ent->datasz); } @@ -156,6 +158,7 @@ entity_write_req(const struct entity *ent) io_simple_buffer(b, &ent->talid, sizeof(ent->talid)); io_str_buffer(b, ent->path); io_str_buffer(b, ent->file); + io_str_buffer(b, ent->mftaki); io_buf_buffer(b, ent->data, ent->datasz); io_close_buffer(&procq, b); } @@ -180,7 +183,8 @@ entity_write_repo(struct repo *rp) io_simple_buffer(b, &talid, sizeof(talid)); io_str_buffer(b, path); io_str_buffer(b, altpath); - io_buf_buffer(b, NULL, 0); + io_buf_buffer(b, NULL, 0); /* ent->mftaki */ + io_buf_buffer(b, NULL, 0); /* ent->data */ io_close_buffer(&procq, b); free(path); free(altpath); @@ -209,7 +213,8 @@ entityq_flush(struct entityq *q, struct repo *rp) */ static void entityq_add(char *path, char *file, enum rtype type, enum location loc, - struct repo *rp, unsigned char *data, size_t datasz, int talid) + struct repo *rp, unsigned char *data, size_t datasz, int talid, + char *mftaki) { struct entity *p; @@ -219,6 +224,7 @@ entityq_add(char *path, char *file, enum rtype type, enum location loc, p->type = type; p->location = loc; p->talid = talid; + p->mftaki = mftaki; p->path = path; if (rp != NULL) p->repoid = repo_id(rp); @@ -377,7 +383,7 @@ queue_add_from_mft(const struct mft *mft) size_t i; struct repo *rp; const struct mftfile *f; - char *nfile, *npath = NULL; + char *mftaki, *nfile, *npath = NULL; rp = repo_byid(mft->repoid); for (i = 0; i < mft->filesz; i++) { @@ -391,8 +397,10 @@ queue_add_from_mft(const struct mft *mft) err(1, NULL); if ((nfile = strdup(f->file)) == NULL) err(1, NULL); + if ((mftaki = strdup(mft->aki)) == NULL) + err(1, NULL); entityq_add(npath, nfile, f->type, f->location, rp, NULL, 0, - -1); + -1, mftaki); } } @@ -415,7 +423,8 @@ queue_add_file(const char *file, enum rtype type, int talid) if ((nfile = strdup(file)) == NULL) err(1, NULL); /* Not in a repository, so directly add to queue. */ - entityq_add(NULL, nfile, type, DIR_UNKNOWN, NULL, buf, len, talid); + entityq_add(NULL, nfile, type, DIR_UNKNOWN, NULL, buf, len, talid, + NULL); } /* @@ -450,7 +459,7 @@ queue_add_from_tal(struct tal *tal) data = tal->pkey; tal->pkey = NULL; entityq_add(NULL, nfile, RTYPE_CER, DIR_VALID, repo, data, - tal->pkeysz, tal->id); + tal->pkeysz, tal->id, NULL); } /* @@ -518,7 +527,8 @@ queue_add_from_cert(const struct cert *cert) err(1, NULL); } - entityq_add(npath, nfile, RTYPE_MFT, DIR_UNKNOWN, repo, NULL, 0, -1); + entityq_add(npath, nfile, RTYPE_MFT, DIR_UNKNOWN, repo, NULL, 0, -1, + NULL); } /* diff --git a/usr.sbin/rpki-client/parser.c b/usr.sbin/rpki-client/parser.c index 3dd64b851ba..58e95235b53 100644 --- a/usr.sbin/rpki-client/parser.c +++ b/usr.sbin/rpki-client/parser.c @@ -1,4 +1,4 @@ -/* $OpenBSD: parser.c,v 1.89 2023/03/13 09:24:37 job Exp $ */ +/* $OpenBSD: parser.c,v 1.90 2023/04/13 17:04:02 job Exp $ */ /* * Copyright (c) 2019 Claudio Jeker * Copyright (c) 2019 Kristaps Dzonsons @@ -125,7 +125,8 @@ parse_filepath(unsigned int repoid, const char *path, const char *file, * Returns the roa on success, NULL on failure. */ static struct roa * -proc_parser_roa(char *file, const unsigned char *der, size_t len) +proc_parser_roa(char *file, const unsigned char *der, size_t len, + const char *mftaki) { struct roa *roa; struct auth *a; @@ -136,7 +137,7 @@ proc_parser_roa(char *file, const unsigned char *der, size_t len) if ((roa = roa_parse(&x509, file, der, len)) == NULL) return NULL; - a = valid_ski_aki(file, &auths, roa->ski, roa->aki); + a = valid_ski_aki(file, &auths, roa->ski, roa->aki, mftaki); crl = crl_get(&crlt, a); if (!valid_x509(file, ctx, x509, a, crl, &errstr)) { @@ -228,12 +229,18 @@ parse_load_crl_from_mft(struct entity *entp, struct mft *mft, enum location loc, if (crl == NULL) goto out; + if (strcmp(crl->aki, mft->aki) != 0) { + warnx("%s: AKI doesn't match Manifest AKI", fn); + goto out; + } + *crlfile = fn; free(f); return crl; out: + crl_free(crl); free(f); free(fn); @@ -278,7 +285,7 @@ proc_parser_mft_pre(struct entity *entp, enum location loc, char **file, if (*crl == NULL) *crl = parse_load_crl_from_mft(entp, mft, DIR_VALID, crlfile); - a = valid_ski_aki(*file, &auths, mft->ski, mft->aki); + a = valid_ski_aki(*file, &auths, mft->ski, mft->aki, NULL); if (!valid_x509(*file, ctx, x509, a, *crl, errstr)) { X509_free(x509); mft_free(mft); @@ -400,7 +407,8 @@ proc_parser_mft(struct entity *entp, struct mft **mp, char **crlfile) * parse failure. */ static struct cert * -proc_parser_cert(char *file, const unsigned char *der, size_t len) +proc_parser_cert(char *file, const unsigned char *der, size_t len, + const char *mftaki) { struct cert *cert; struct crl *crl; @@ -414,7 +422,7 @@ proc_parser_cert(char *file, const unsigned char *der, size_t len) if (cert == NULL) return NULL; - a = valid_ski_aki(file, &auths, cert->ski, cert->aki); + a = valid_ski_aki(file, &auths, cert->ski, cert->aki, mftaki); crl = crl_get(&crlt, a); if (!valid_x509(file, ctx, cert->x509, a, crl, &errstr) || @@ -478,7 +486,8 @@ proc_parser_root_cert(char *file, const unsigned char *der, size_t len, * Parse a ghostbuster record */ static void -proc_parser_gbr(char *file, const unsigned char *der, size_t len) +proc_parser_gbr(char *file, const unsigned char *der, size_t len, + const char *mftaki) { struct gbr *gbr; X509 *x509; @@ -489,7 +498,7 @@ proc_parser_gbr(char *file, const unsigned char *der, size_t len) if ((gbr = gbr_parse(&x509, file, der, len)) == NULL) return; - a = valid_ski_aki(file, &auths, gbr->ski, gbr->aki); + a = valid_ski_aki(file, &auths, gbr->ski, gbr->aki, mftaki); crl = crl_get(&crlt, a); /* return value can be ignored since nothing happens here */ @@ -504,7 +513,8 @@ proc_parser_gbr(char *file, const unsigned char *der, size_t len) * Parse an ASPA object */ static struct aspa * -proc_parser_aspa(char *file, const unsigned char *der, size_t len) +proc_parser_aspa(char *file, const unsigned char *der, size_t len, + const char *mftaki) { struct aspa *aspa; struct auth *a; @@ -515,7 +525,7 @@ proc_parser_aspa(char *file, const unsigned char *der, size_t len) if ((aspa = aspa_parse(&x509, file, der, len)) == NULL) return NULL; - a = valid_ski_aki(file, &auths, aspa->ski, aspa->aki); + a = valid_ski_aki(file, &auths, aspa->ski, aspa->aki, mftaki); crl = crl_get(&crlt, a); if (!valid_x509(file, ctx, x509, a, crl, &errstr)) { @@ -537,7 +547,8 @@ proc_parser_aspa(char *file, const unsigned char *der, size_t len) * Parse a TAK object. */ static struct tak * -proc_parser_tak(char *file, const unsigned char *der, size_t len) +proc_parser_tak(char *file, const unsigned char *der, size_t len, + const char *mftaki) { struct tak *tak; X509 *x509; @@ -549,7 +560,7 @@ proc_parser_tak(char *file, const unsigned char *der, size_t len) if ((tak = tak_parse(&x509, file, der, len)) == NULL) return NULL; - a = valid_ski_aki(file, &auths, tak->ski, tak->aki); + a = valid_ski_aki(file, &auths, tak->ski, tak->aki, mftaki); crl = crl_get(&crlt, a); if (!valid_x509(file, ctx, x509, a, crl, &errstr)) { @@ -646,7 +657,8 @@ parse_entity(struct entityq *q, struct msgbuf *msgq) f, flen, entp->data, entp->datasz, entp->talid); else - cert = proc_parser_cert(file, f, flen); + cert = proc_parser_cert(file, f, flen, + entp->mftaki); c = (cert != NULL); io_simple_buffer(b, &c, sizeof(int)); if (cert != NULL) { @@ -687,7 +699,7 @@ parse_entity(struct entityq *q, struct msgbuf *msgq) case RTYPE_ROA: file = parse_load_file(entp, &f, &flen); io_str_buffer(b, file); - roa = proc_parser_roa(file, f, flen); + roa = proc_parser_roa(file, f, flen, entp->mftaki); c = (roa != NULL); io_simple_buffer(b, &c, sizeof(int)); if (roa != NULL) @@ -697,12 +709,12 @@ parse_entity(struct entityq *q, struct msgbuf *msgq) case RTYPE_GBR: file = parse_load_file(entp, &f, &flen); io_str_buffer(b, file); - proc_parser_gbr(file, f, flen); + proc_parser_gbr(file, f, flen, entp->mftaki); break; case RTYPE_ASPA: file = parse_load_file(entp, &f, &flen); io_str_buffer(b, file); - aspa = proc_parser_aspa(file, f, flen); + aspa = proc_parser_aspa(file, f, flen, entp->mftaki); c = (aspa != NULL); io_simple_buffer(b, &c, sizeof(int)); if (aspa != NULL) @@ -712,7 +724,7 @@ parse_entity(struct entityq *q, struct msgbuf *msgq) case RTYPE_TAK: file = parse_load_file(entp, &f, &flen); io_str_buffer(b, file); - proc_parser_tak(file, f, flen); + proc_parser_tak(file, f, flen, entp->mftaki); break; case RTYPE_CRL: default: diff --git a/usr.sbin/rpki-client/validate.c b/usr.sbin/rpki-client/validate.c index d7ac327e8f8..99247fe47a8 100644 --- a/usr.sbin/rpki-client/validate.c +++ b/usr.sbin/rpki-client/validate.c @@ -1,4 +1,4 @@ -/* $OpenBSD: validate.c,v 1.55 2023/03/06 16:04:52 job Exp $ */ +/* $OpenBSD: validate.c,v 1.56 2023/04/13 17:04:02 job Exp $ */ /* * Copyright (c) 2019 Kristaps Dzonsons * @@ -80,16 +80,23 @@ valid_ip(struct auth *a, enum afi afi, } /* - * Make sure that the SKI doesn't already exist and return the parent by - * its AKI. - * Returns the parent auth or NULL on failure. + * Make sure the AKI is the same as the AKI listed on the Manifest, + * and that the SKI doesn't already exist. + * Return the parent by its AKI, or NULL on failure. */ struct auth * valid_ski_aki(const char *fn, struct auth_tree *auths, - const char *ski, const char *aki) + const char *ski, const char *aki, const char *mftaki) { struct auth *a; + if (mftaki != NULL) { + if (strcmp(aki, mftaki) != 0) { + warnx("%s: AKI doesn't match Manifest AKI", fn); + return NULL; + } + } + if (auth_find(auths, ski) != NULL) { warnx("%s: RFC 6487: duplicate SKI", fn); return NULL; -- 2.20.1