From 102443c78d43b607a4b5a168e8a80cdcdebf3e58 Mon Sep 17 00:00:00 2001 From: job Date: Sat, 9 Dec 2023 00:44:18 +0000 Subject: [PATCH] Following a failed fetch, use a previously cached and valid Manifest RPKI Manifests enable Relying Parties (RPs) to detect replay attacks, unauthorized in-flight modification, or deletion of signed objects. RPs can accomplish these security functions by comparing (what is expected to be) a monotonically increasing counter (the 'manifestNumber') - to determine what the latest Manifest is; a list of filenames - in order to establish whether the complete set of files was fetched; and a list of SHA256 message digests to ascertain whether the content's of said files are exactly the same as the CA intended them to be. Over time, two schools of thought arose. One philosophy is that the highest numbered cryptographically valid Manifest represents the express intent of the CA, so if manifest-listed files are missing, someone upstream messed up and gets to enjoy the broken pieces. After all, RFC 9286 section 5.2 puts the onus firmly on the repository operator to publish in a consistent manner. Here, "consistent" means that newly issued manifests - in the same RRDP delta - are bundled together with all new or changed ROAs, and that remote RSYNC repositories are atomically updated (for example, using symlink pivots). To overcome various types of inconsistent, transient, or intermediate states of the remote publication point - previous versions of rpki-client did construct the full CARepository state using a mix of objects from both its local validated cache and the RRDP/RSYNC staging directories (which contain purported new versions of the objects). However, another take on RFC 9286 section 6.6's "use cached versions of the objects" is that 'the objects' not only refers to the listed subordinate products (such as ROAs/Certificates/ASPAs), but also to Manifests themselves. The philosophy being that lower numbered cryptographically valid Manifests with a complete & untampered set of files are to be preferred over a higher numbered cryptographically valid Manifests accompanied by incomplete sets of files. Consequently - potentially - producing more stable VRP outputs, at the expense of being magnanimous towards sloppy CAs and repository operators. Going forward, rpki-client logs errors when inconsistent publications are encountered, but also proceeds to use older cryptographically valid Manifests (from previous successful fetches) in order to construct the tree. With and OK tb@, and also thanks to Ties de Kock from RIPE NCC. --- usr.sbin/rpki-client/parser.c | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/usr.sbin/rpki-client/parser.c b/usr.sbin/rpki-client/parser.c index 8e4abdc1da75..61a66db01c4d 100644 --- a/usr.sbin/rpki-client/parser.c +++ b/usr.sbin/rpki-client/parser.c @@ -1,4 +1,4 @@ -/* $OpenBSD: parser.c,v 1.100 2023/10/13 12:06:49 job Exp $ */ +/* $OpenBSD: parser.c,v 1.101 2023/12/09 00:44:18 job Exp $ */ /* * Copyright (c) 2019 Claudio Jeker * Copyright (c) 2019 Kristaps Dzonsons @@ -310,7 +310,7 @@ proc_parser_mft_pre(struct entity *entp, enum location loc, char **file, */ static struct mft * proc_parser_mft_post(char *file, struct mft *mft, const char *path, - const char *errstr) + const char *errstr, int *warned) { /* check that now is not before from */ time_t now = get_current_time(); @@ -318,6 +318,8 @@ proc_parser_mft_post(char *file, struct mft *mft, const char *path, if (mft == NULL) { if (errstr == NULL) errstr = "no valid mft available"; + if ((*warned)++ > 0) + return NULL; warnx("%s: %s", file, errstr); return NULL; } @@ -359,13 +361,14 @@ proc_parser_mft(struct entity *entp, struct mft **mp, char **crlfile, struct crl *crl, *crl1, *crl2; char *file, *file1, *file2, *crl1file, *crl2file; const char *err1, *err2; + int warned = 0; *mp = NULL; *crlmtime = 0; - mft1 = proc_parser_mft_pre(entp, DIR_VALID, &file1, &crl1, &crl1file, + mft1 = proc_parser_mft_pre(entp, DIR_TEMP, &file1, &crl1, &crl1file, &err1); - mft2 = proc_parser_mft_pre(entp, DIR_TEMP, &file2, &crl2, &crl2file, + mft2 = proc_parser_mft_pre(entp, DIR_VALID, &file2, &crl2, &crl2file, &err2); /* overload error from temp file if it is set */ @@ -374,20 +377,36 @@ proc_parser_mft(struct entity *entp, struct mft **mp, char **crlfile, err1 = err2; if (mft_compare(mft1, mft2) == 1) { + *mp = proc_parser_mft_post(file1, mft1, entp->path, err1, + &warned); + if (*mp == NULL) { + mft1 = NULL; + if (mft2 != NULL) + warnx("%s: failed fetch, continuing with #%s", + file2, mft2->seqnum); + } + } + + if (*mp != NULL) { mft_free(mft2); crl_free(crl2); free(crl2file); free(file2); - *mp = proc_parser_mft_post(file1, mft1, entp->path, err1); + crl = crl1; file = file1; *crlfile = crl1file; } else { + if (err2 == NULL) + err2 = err1; + *mp = proc_parser_mft_post(file2, mft2, entp->path, err2, + &warned); + mft_free(mft1); crl_free(crl1); free(crl1file); free(file1); - *mp = proc_parser_mft_post(file2, mft2, entp->path, err2); + crl = crl2; file = file2; *crlfile = crl2file;