On Tue, May 30, 2023 at 02:10:31PM +0000, Job Snijders wrote:
> On Tue, May 30, 2023 at 03:12:46PM +0200, Claudio Jeker wrote:
> > On Tue, May 30, 2023 at 02:38:23PM +0200, Claudio Jeker wrote:
> > > On Wed, May 24, 2023 at 04:18:30PM +0000, Job Snijders wrote:
> > > > Dear all,
> > > >
> > > > Claudio made some suggestions to pass the desired modification times
> > > > around in a different way, below is an updated patch proposal.
> > > > I also added some instrumentation to also adjust GBRs and TAKs.
> > > >
> > > > RIPE & APNIC informally indicated some interest in this hack.
> > > >
> > >
> > > This looks good. Some feedback below.
>
> How about this?
Looks good to me. OK
> Index: extern.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
> retrieving revision 1.182
> diff -u -p -r1.182 extern.h
> --- extern.h 30 May 2023 12:14:48 -0000 1.182
> +++ extern.h 30 May 2023 13:49:16 -0000
> @@ -348,6 +348,7 @@ struct gbr {
> time_t notbefore; /* EE cert's Not Before */
> time_t notafter; /* Not After of the GBR EE */
> time_t expires; /* when the signature path expires */
> + int talid; /* TAL the GBR is chained up to */
> };
>
> struct aspa_provider {
> @@ -755,7 +756,7 @@ void proc_http(char *, int) __attribut
> void proc_rrdp(int) __attribute__((noreturn));
>
> /* Repository handling */
> -int filepath_add(struct filepath_tree *, char *);
> +int filepath_add(struct filepath_tree *, char *, time_t);
> void rrdp_clear(unsigned int);
> void rrdp_save_state(unsigned int, struct rrdp_session *);
> int rrdp_handle_file(unsigned int, enum publish_type, char *,
> Index: filemode.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/filemode.c,v
> retrieving revision 1.32
> diff -u -p -r1.32 filemode.c
> --- filemode.c 30 May 2023 12:02:22 -0000 1.32
> +++ filemode.c 30 May 2023 13:49:16 -0000
> @@ -589,6 +589,7 @@ parse_file(struct entityq *q, struct msg
> struct entity *entp;
> struct ibuf *b;
> struct tal *tal;
> + time_t dummy = 0;
>
> while ((entp = TAILQ_FIRST(q)) != NULL) {
> TAILQ_REMOVE(q, entp, entries);
> @@ -615,6 +616,7 @@ parse_file(struct entityq *q, struct msg
> io_simple_buffer(b, &entp->repoid, sizeof(entp->repoid));
> io_simple_buffer(b, &entp->talid, sizeof(entp->talid));
> io_str_buffer(b, entp->file);
> + io_simple_buffer(b, &dummy, sizeof(dummy));
> io_close_buffer(msgq, b);
> entity_free(entp);
> }
> Index: main.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/main.c,v
> retrieving revision 1.240
> diff -u -p -r1.240 main.c
> --- main.c 30 May 2023 12:14:48 -0000 1.240
> +++ main.c 30 May 2023 13:49:16 -0000
> @@ -559,6 +559,7 @@ entity_process(struct ibuf *b, struct st
> struct aspa *aspa;
> struct repo *rp;
> char *file;
> + time_t mtime;
> unsigned int id;
> int talid;
> int c;
> @@ -573,12 +574,13 @@ entity_process(struct ibuf *b, struct st
> io_read_buf(b, &id, sizeof(id));
> io_read_buf(b, &talid, sizeof(talid));
> io_read_str(b, &file);
> + io_read_buf(b, &mtime, sizeof(mtime));
>
> /* in filemode messages can be ignored, only the accounting matters */
> if (filemode)
> goto done;
>
> - if (filepath_add(&fpt, file) == 0) {
> + if (filepath_add(&fpt, file, mtime) == 0) {
> warnx("%s: File already visited", file);
> goto done;
> }
> Index: parser.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v
> retrieving revision 1.95
> diff -u -p -r1.95 parser.c
> --- parser.c 30 May 2023 12:14:48 -0000 1.95
> +++ parser.c 30 May 2023 13:49:16 -0000
> @@ -352,7 +352,8 @@ proc_parser_mft_post(char *file, struct
> * Load the most recent MFT by opening both options and comparing the two.
> */
> static char *
> -proc_parser_mft(struct entity *entp, struct mft **mp, char **crlfile)
> +proc_parser_mft(struct entity *entp, struct mft **mp, char **crlfile,
> + time_t *crlmtime)
> {
> struct mft *mft1 = NULL, *mft2 = NULL;
> struct crl *crl, *crl1, *crl2;
> @@ -360,6 +361,7 @@ proc_parser_mft(struct entity *entp, str
> const char *err1, *err2;
>
> *mp = NULL;
> + *crlmtime = 0;
>
> mft1 = proc_parser_mft_pre(entp, DIR_VALID, &file1, &crl1, &crl1file,
> &err1);
> @@ -392,6 +394,7 @@ proc_parser_mft(struct entity *entp, str
> }
>
> if (*mp != NULL) {
> + *crlmtime = crl->lastupdate;
> if (!crl_insert(&crlt, crl)) {
> warnx("%s: duplicate AKI %s", file, crl->aki);
> crl_free(crl);
> @@ -488,7 +491,7 @@ proc_parser_root_cert(char *file, const
> /*
> * Parse a ghostbuster record
> */
> -static void
> +static struct gbr *
> proc_parser_gbr(char *file, const unsigned char *der, size_t len,
> const char *mftaki)
> {
> @@ -499,17 +502,23 @@ proc_parser_gbr(char *file, const unsign
> const char *errstr;
>
> if ((gbr = gbr_parse(&x509, file, der, len)) == NULL)
> - return;
> + return NULL;
>
> 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 */
> - if (!valid_x509(file, ctx, x509, a, crl, &errstr))
> + if (!valid_x509(file, ctx, x509, a, crl, &errstr)) {
> warnx("%s: %s", file, errstr);
> -
> + X509_free(x509);
> + gbr_free(gbr);
> + return NULL;
> + }
> X509_free(x509);
> - gbr_free(gbr);
> +
> + gbr->talid = a->cert->talid;
> +
> + return gbr;
> }
>
> /*
> @@ -618,8 +627,11 @@ parse_entity(struct entityq *q, struct m
> struct mft *mft;
> struct roa *roa;
> struct aspa *aspa;
> + struct gbr *gbr;
> + struct tak *tak;
> struct ibuf *b;
> unsigned char *f;
> + time_t mtime, crlmtime;
> size_t flen;
> char *file, *crlfile;
> int c;
> @@ -642,9 +654,13 @@ parse_entity(struct entityq *q, struct m
>
> file = NULL;
> f = NULL;
> + mtime = 0;
> + crlmtime = 0;
> +
> switch (entp->type) {
> case RTYPE_TAL:
> io_str_buffer(b, entp->file);
> + io_simple_buffer(b, &mtime, sizeof(mtime));
> if ((tal = tal_parse(entp->file, entp->data,
> entp->datasz)) == NULL)
> errx(1, "%s: could not parse tal file",
> @@ -663,6 +679,9 @@ parse_entity(struct entityq *q, struct m
> else
> cert = proc_parser_cert(file, f, flen,
> entp->mftaki);
> + if (cert != NULL)
> + mtime = cert->notbefore;
> + io_simple_buffer(b, &mtime, sizeof(mtime));
> c = (cert != NULL);
> io_simple_buffer(b, &c, sizeof(int));
> if (cert != NULL) {
> @@ -676,8 +695,11 @@ parse_entity(struct entityq *q, struct m
> */
> break;
> case RTYPE_MFT:
> - file = proc_parser_mft(entp, &mft, &crlfile);
> + file = proc_parser_mft(entp, &mft, &crlfile, &crlmtime);
> io_str_buffer(b, file);
> + if (mft != NULL)
> + mtime = mft->signtime;
> + io_simple_buffer(b, &mtime, sizeof(mtime));
> c = (mft != NULL);
> io_simple_buffer(b, &c, sizeof(int));
> if (mft != NULL)
> @@ -696,6 +718,8 @@ parse_entity(struct entityq *q, struct m
> io_simple_buffer(b2, &entp->talid,
> sizeof(entp->talid));
> io_str_buffer(b2, crlfile);
> + io_simple_buffer(b2, &crlmtime,
> + sizeof(crlmtime));
> free(crlfile);
>
> io_close_buffer(msgq, b2);
> @@ -706,6 +730,9 @@ parse_entity(struct entityq *q, struct m
> file = parse_load_file(entp, &f, &flen);
> io_str_buffer(b, file);
> roa = proc_parser_roa(file, f, flen, entp->mftaki);
> + if (roa != NULL)
> + mtime = roa->signtime;
> + io_simple_buffer(b, &mtime, sizeof(mtime));
> c = (roa != NULL);
> io_simple_buffer(b, &c, sizeof(int));
> if (roa != NULL)
> @@ -715,12 +742,19 @@ parse_entity(struct entityq *q, struct m
> case RTYPE_GBR:
> file = parse_load_file(entp, &f, &flen);
> io_str_buffer(b, file);
> - proc_parser_gbr(file, f, flen, entp->mftaki);
> + gbr = proc_parser_gbr(file, f, flen, entp->mftaki);
> + if (gbr != NULL)
> + mtime = gbr->signtime;
> + io_simple_buffer(b, &mtime, sizeof(mtime));
> + gbr_free(gbr);
> break;
> case RTYPE_ASPA:
> file = parse_load_file(entp, &f, &flen);
> io_str_buffer(b, file);
> aspa = proc_parser_aspa(file, f, flen, entp->mftaki);
> + if (aspa != NULL)
> + mtime = aspa->signtime;
> + io_simple_buffer(b, &mtime, sizeof(mtime));
> c = (aspa != NULL);
> io_simple_buffer(b, &c, sizeof(int));
> if (aspa != NULL)
> @@ -730,13 +764,18 @@ parse_entity(struct entityq *q, struct m
> case RTYPE_TAK:
> file = parse_load_file(entp, &f, &flen);
> io_str_buffer(b, file);
> - proc_parser_tak(file, f, flen, entp->mftaki);
> + tak = proc_parser_tak(file, f, flen, entp->mftaki);
> + if (tak != NULL)
> + mtime = tak->signtime;
> + io_simple_buffer(b, &mtime, sizeof(mtime));
> + tak_free(tak);
> break;
> case RTYPE_CRL:
> default:
> file = parse_filepath(entp->repoid, entp->path,
> entp->file, entp->location);
> io_str_buffer(b, file);
> + io_simple_buffer(b, &mtime, sizeof(mtime));
> warnx("%s: unhandled type %d", file, entp->type);
> break;
> }
> Index: repo.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/repo.c,v
> retrieving revision 1.46
> diff -u -p -r1.46 repo.c
> --- repo.c 25 May 2023 12:49:39 -0000 1.46
> +++ repo.c 30 May 2023 13:49:16 -0000
> @@ -119,6 +119,7 @@ static void remove_contents(char *);
> struct filepath {
> RB_ENTRY(filepath) entry;
> char *file;
> + time_t mtime;
> };
>
> static inline int
> @@ -133,12 +134,13 @@ RB_PROTOTYPE(filepath_tree, filepath, en
> * Functions to lookup which files have been accessed during computation.
> */
> int
> -filepath_add(struct filepath_tree *tree, char *file)
> +filepath_add(struct filepath_tree *tree, char *file, time_t mtime)
> {
> struct filepath *fp;
>
> if ((fp = malloc(sizeof(*fp))) == NULL)
> err(1, NULL);
> + fp->mtime = mtime;
> if ((fp->file = strdup(file)) == NULL)
> err(1, NULL);
>
> @@ -838,7 +840,7 @@ rrdp_handle_file(unsigned int id, enum p
>
> /* write new content or mark uri as deleted. */
> if (pt == PUB_DEL) {
> - filepath_add(&rr->deleted, uri);
> + filepath_add(&rr->deleted, uri, 0);
> } else {
> fp = filepath_find(&rr->deleted, uri);
> if (fp != NULL)
> @@ -1536,6 +1538,27 @@ repo_move_valid(struct filepath_tree *tr
> base = strchr(fp->file + rrdpsz, '/');
> assert(base != NULL);
> fn = base + 1;
> +
> + /*
> + * Adjust file last modification time in order to
> + * minimize RSYNC synchronization load after transport
> + * failover. While serializing RRDP datastructures to
> + * disk, set the last modified timestamp to the CMS
> + * signing-time or the X.509 notBefore timestamp.
> + */
> + if (fp->mtime != 0) {
> + int ret;
> + struct timespec ts[2];
> +
> + ts[0].tv_nsec = UTIME_OMIT;
> + ts[1].tv_sec = fp->mtime;
> + ts[1].tv_nsec = 0;
> + ret = utimensat(AT_FDCWD, fp->file, ts, 0);
> + if (ret == -1) {
> + warn("utimensat %s", fp->file);
> + continue;
> + }
> + }
> }
>
> if (repo_mkpath(AT_FDCWD, fn) == -1)
>
--
:wq Claudio
