Pull mft comparison into proc_parser_mft_pre()

This way we can be sure more easily that both manifests are non-NULL,
thus avoiding some NULL checks and risk of use-after-free. This also
makes it clearer which manifest is the "older" one and will simplify
an upcoming commit doing issuance time comparison.

This adds a bit of a hack to proc_parser_mft_pre() to ensure we don't
look into DIR_TEMP in noop mode.

ok job

diff --git a/usr.sbin/rpki-client/parser.c b/usr.sbin/rpki-client/parser.c
index f8e4f95..3d89f9b 100644 (file)
--- a/usr.sbin/rpki-client/parser.c
+++ b/usr.sbin/rpki-client/parser.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: parser.c,v 1.110 2024/01/31 06:48:27 tb Exp $ */
+/*     $OpenBSD: parser.c,v 1.111 2024/01/31 06:53:21 tb Exp $ */
 /*
  * Copyright (c) 2019 Claudio Jeker <claudio@openbsd.org>
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
@@ -258,22 +258,28 @@ parse_load_crl_from_mft(struct entity *entp, struct mft *mft, enum location loc,
  */
 static struct mft *
 proc_parser_mft_pre(struct entity *entp, enum location loc, char **file,
-    struct crl **crl, char **crlfile, const char **errstr)
+    struct crl **crl, char **crlfile, struct mft *cached_mft,
+    const char **errstr)
 {
        struct mft      *mft;
        X509            *x509;
        struct auth     *a;
        unsigned char   *der;
        size_t           len;
+       int              seqnum_cmp;
 
        *crl = NULL;
        *crlfile = NULL;
        *errstr = NULL;
 
+       /* XXX - pull this into proc_parser_mft. */
        *file = parse_filepath(entp->repoid, entp->path, entp->file, loc);
        if (*file == NULL)
                return NULL;
 
+       if (noop && loc == DIR_TEMP)
+               return NULL;
+
        der = load_file(*file, &len);
        if (der == NULL && errno != ENOENT)
                warn("parse file %s", *file);
@@ -301,6 +307,26 @@ proc_parser_mft_pre(struct entity *entp, enum location loc, char **file,
        mft->repoid = entp->repoid;
        mft->talid = a->cert->talid;
 
+       if (cached_mft == NULL)
+               return mft;
+
+       /*
+        * Check that the cached manifest is older in the sense that it has
+        * a smaller sequence number.
+        */
+
+       if ((seqnum_cmp = mft_compare(mft, cached_mft)) < 0) {
+               warnx("%s: unexpected manifest number (want >= #%s, got #%s)",
+                   *file, cached_mft->seqnum, mft->seqnum);
+               goto err;
+       }
+       if (seqnum_cmp == 0 && memcmp(mft->mfthash,
+           cached_mft->mfthash, SHA256_DIGEST_LENGTH) != 0) {
+               warnx("%s: manifest misissuance, #%s was recycled",
+                   *file, mft->seqnum);
+               goto err;
+       }
+
        return mft;
 
  err:
@@ -370,32 +396,22 @@ 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              r, warned = 0;
+       int              warned = 0;
 
        *mp = NULL;
        *crlmtime = 0;
 
-       mft1 = proc_parser_mft_pre(entp, DIR_TEMP, &file1, &crl1, &crl1file,
-           &err1);
        mft2 = proc_parser_mft_pre(entp, DIR_VALID, &file2, &crl2, &crl2file,
-           &err2);
+           NULL, &err2);
+       mft1 = proc_parser_mft_pre(entp, DIR_TEMP, &file1, &crl1, &crl1file,
+           mft2, &err1);
 
        /* overload error from temp file if it is set */
        if (mft1 == NULL && mft2 == NULL)
                if (err1 != NULL)
                        err2 = err1;
 
-       r = mft_compare(mft1, mft2);
-       if (r == -1 && mft1 != NULL && mft2 != NULL)
-               warnx("%s: unexpected manifest number (want >= #%s, got #%s)",
-                   file1, mft2->seqnum, mft1->seqnum);
-
-       if (r == 0 && memcmp(mft1->mfthash, mft2->mfthash,
-           SHA256_DIGEST_LENGTH) != 0)
-               warnx("%s: manifest misissuance, #%s was recycled",
-                   file1, mft1->seqnum);
-
-       if (!noop && r == 1) {
+       if (!noop && mft1 != NULL) {
                *mp = proc_parser_mft_post(file1, mft1, entp->path, err1,
                    &warned);
                if (*mp == NULL) {