Hi,
I wasn't entirely happy about how parse_load_crl_from_mft() behaved and
refactored the function.
The good: if the MFT at hand was located in DIR_TEMP and no matching CRL
could be found in DIR_TEMP, it would additionally attempt to find a CRL
in DIR_VALID.
The bad: if the MFT at hand was located in DIR_VALID, no attempt would
be made to search for a matching CRL in DIR_TEMP; resulting in less
opportunity to potentially salvage a broken situation at a future point
in time with the help of locally cached artefacts.
If the following 5 commands are run (before and after applying the below
changeset), one can observe that with this diff rpki-client's behaviour
becomes more idempotent.
rm -rf /var/cache/rpki-client/{*,.rrdp,.rsync}
rpki-client -t /etc/rpki/lacnic.tal 2>&1 | fgrep
6QvnUnEXe5JTf7VhQHnUFRwdzeEpbF4rt3b5PLrvdeyy
ls -lahtr
/var/cache/rpki-client/{.rsync,.,.rrdp/*}/rpki-repo.registro.br/repo/6QvnUnEXe5JTf7VhQHnUFRwdzeEpbF4rt3b5PLrvdeyy/0/
rpki-client -t /etc/rpki/lacnic.tal 2>&1 | fgrep
6QvnUnEXe5JTf7VhQHnUFRwdzeEpbF4rt3b5PLrvdeyy
ls -lahtr
/var/cache/rpki-client/{.rsync,.,.rrdp/*}/rpki-repo.registro.br/repo/6QvnUnEXe5JTf7VhQHnUFRwdzeEpbF4rt3b5PLrvdeyy/0/
With the diff applied, the second invocation of rpki-client won't delete
CDD9973303E25E7554D25F5703FB347389D59326.crl & friends from DIR_TEMP;
without this diff, we lose sight of some files. Losing the files hampers
our ability to (re)construct the publication point if a future RRDP
delta publish the correct ROAs (because by then we'd be missing the
CRL).
Since SHA256 hashes are used to confirm the correct object is loaded, it
doesn't matter whether the CRL comes from DIR_VALID, DIR_TEMP, USB
stick, or pigeon carrier.
OK?
Kind regards,
Job
Index: parser.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v
retrieving revision 1.82
diff -u -p -r1.82 parser.c
--- parser.c 6 Jan 2023 16:06:43 -0000 1.82
+++ parser.c 19 Feb 2023 10:17:09 -0000
@@ -210,43 +210,47 @@ proc_parser_mft_check(const char *fn, st
}
/*
- * Load the correct CRL using the info from the MFT.
+ * Load the correct CRL using the SHA256 info from the MFT.
+ * Returns NULL if no valid matching CRL was found in either the staging area
+ * or the validated cache area.
*/
static struct crl *
-parse_load_crl_from_mft(struct entity *entp, struct mft *mft, enum location
loc)
+parse_load_crl_from_mft(struct entity *entp, struct mft *mft)
{
- struct crl *crl = NULL;
- unsigned char *f = NULL;
- char *fn = NULL;
- size_t flen;
+ char *fn = NULL;
+ unsigned char *f = NULL;
+ struct crl *res = NULL;
+ const enum location loc[2] = { DIR_TEMP, DIR_VALID };
+ size_t flen;
+ int i;
- while (1) {
- fn = parse_filepath(entp->repoid, entp->path, mft->crl, loc);
+ for (i = 0; i < 2; i++) {
+ fn = parse_filepath(entp->repoid, entp->path, mft->crl, loc[i]);
if (fn == NULL)
- goto next;
+ continue;
f = load_file(fn, &flen);
if (f == NULL && errno != ENOENT)
warn("parse file %s", fn);
- if (f == NULL)
- goto next;
- if (!valid_hash(f, flen, mft->crlhash, sizeof(mft->crlhash)))
- goto next;
- crl = crl_parse(fn, f, flen);
+ if (f == NULL) {
+ free(fn);
+ continue;
+ }
+
+ if (valid_hash(f, flen, mft->crlhash, sizeof(mft->crlhash))) {
+ res = crl_parse(fn, f, flen);
+ break;
+ }
-next:
free(f);
free(fn);
f = NULL;
fn = NULL;
-
- if (crl != NULL)
- return crl;
- if (loc == DIR_TEMP)
- loc = DIR_VALID;
- else
- return NULL;
}
+
+ free(f);
+ free(fn);
+ return res;
}
/*
@@ -268,7 +272,7 @@ proc_parser_mft_pre(char *file, const un
*errstr = NULL;
if ((mft = mft_parse(&x509, file, der, len)) == NULL)
return NULL;
- *crl = parse_load_crl_from_mft(entp, mft, loc);
+ *crl = parse_load_crl_from_mft(entp, mft);
a = valid_ski_aki(file, &auths, mft->ski, mft->aki);
if (!valid_x509(file, ctx, x509, a, *crl, errstr)) {
