From: claudio Date: Fri, 12 Jul 2024 09:27:32 +0000 (+0000) Subject: Improve duplicate detection and repo_move_valid X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=9236071340bceacb05297bf3b408dd1109f9df97;p=openbsd Improve duplicate detection and repo_move_valid Only trigger a duplicate error if a valid filepath is revisted. It is possible that a bad CA references somebody else's files and if that happens first it would block the valid access. To make this work, pass the ok flag to filepath_add() and only set the talmask bit if the file was ok. Since we need to do the duplicate check before processing the entity introduce filepath_valid() which checks if the path is in the tree and has its talmask bit set. In repo_move_valid() handle conflicts more gracefully. When both a valid and temporary file are present assume that one of the files was never ok (talmask == 0) and silently remove that file from the filepath tree. OK tb@ --- diff --git a/usr.sbin/rpki-client/extern.h b/usr.sbin/rpki-client/extern.h index b811b485e3e..3f74142e432 100644 --- a/usr.sbin/rpki-client/extern.h +++ b/usr.sbin/rpki-client/extern.h @@ -1,4 +1,4 @@ -/* $OpenBSD: extern.h,v 1.224 2024/06/08 13:30:35 tb Exp $ */ +/* $OpenBSD: extern.h,v 1.225 2024/07/12 09:27:32 claudio Exp $ */ /* * Copyright (c) 2019 Kristaps Dzonsons * @@ -831,7 +831,8 @@ void proc_http(char *, int) __attribute__((noreturn)); void proc_rrdp(int) __attribute__((noreturn)); /* Repository handling */ -int filepath_add(struct filepath_tree *, char *, int, time_t); +int filepath_add(struct filepath_tree *, char *, int, time_t, int); +int filepath_valid(struct filepath_tree *, char *, int); void rrdp_clear(unsigned int); void rrdp_session_save(unsigned int, struct rrdp_session *); void rrdp_session_free(struct rrdp_session *); diff --git a/usr.sbin/rpki-client/main.c b/usr.sbin/rpki-client/main.c index 780f40e7c35..f722f9e95bc 100644 --- a/usr.sbin/rpki-client/main.c +++ b/usr.sbin/rpki-client/main.c @@ -1,4 +1,4 @@ -/* $OpenBSD: main.c,v 1.261 2024/07/12 08:54:48 claudio Exp $ */ +/* $OpenBSD: main.c,v 1.262 2024/07/12 09:27:32 claudio Exp $ */ /* * Copyright (c) 2021 Claudio Jeker * Copyright (c) 2019 Kristaps Dzonsons @@ -595,7 +595,7 @@ entity_process(struct ibuf *b, struct stats *st, struct vrp_tree *tree, if (filemode) goto done; - if (filepath_add(&fpt, file, talid, mtime) == 0) { + if (filepath_valid(&fpt, file, talid)) { warnx("%s: File already visited", file); goto done; } @@ -697,6 +697,9 @@ entity_process(struct ibuf *b, struct stats *st, struct vrp_tree *tree, break; } + if (filepath_add(&fpt, file, talid, mtime, ok) == 0) + errx(1, "%s: File already in tree", file); + done: free(file); entity_queue--; diff --git a/usr.sbin/rpki-client/repo.c b/usr.sbin/rpki-client/repo.c index 0dd47cd0bb0..ce97015aedf 100644 --- a/usr.sbin/rpki-client/repo.c +++ b/usr.sbin/rpki-client/repo.c @@ -1,4 +1,4 @@ -/* $OpenBSD: repo.c,v 1.60 2024/06/07 08:22:53 claudio Exp $ */ +/* $OpenBSD: repo.c,v 1.61 2024/07/12 09:27:32 claudio Exp $ */ /* * Copyright (c) 2021 Claudio Jeker * Copyright (c) 2019 Kristaps Dzonsons @@ -133,7 +133,8 @@ RB_PROTOTYPE(filepath_tree, filepath, entry, filepathcmp); * Functions to lookup which files have been accessed during computation. */ int -filepath_add(struct filepath_tree *tree, char *file, int id, time_t mtime) +filepath_add(struct filepath_tree *tree, char *file, int id, time_t mtime, + int ok) { struct filepath *fp, *rfp; @@ -154,7 +155,8 @@ filepath_add(struct filepath_tree *tree, char *file, int id, time_t mtime) return 0; fp = rfp; } - fp->talmask |= (1 << id); + if (ok) + fp->talmask |= (1 << id); return 1; } @@ -179,6 +181,19 @@ filepath_exists(struct filepath_tree *tree, char *file) return filepath_find(tree, file) != NULL; } +/* + * Returns true if file exists and the id bit is set and ok flag is true. + */ +int +filepath_valid(struct filepath_tree *tree, char *file, int id) +{ + struct filepath *fp; + + if ((fp = filepath_find(tree, file)) == NULL) + return 0; + return (fp->talmask & (1 << id)) != 0; +} + /* * Remove entry from tree and free it. */ @@ -922,7 +937,7 @@ rrdp_handle_file(unsigned int id, enum publish_type pt, char *uri, /* write new content or mark uri as deleted. */ if (pt == PUB_DEL) { - filepath_add(&rr->deleted, uri, 0, 0); + filepath_add(&rr->deleted, uri, 0, 0, 1); } else { fp = filepath_find(&rr->deleted, uri); if (fp != NULL) { @@ -1630,7 +1645,7 @@ repo_cleanup_rrdp(struct filepath_tree *tree) static void repo_move_valid(struct filepath_tree *tree) { - struct filepath *fp, *nfp; + struct filepath *fp, *nfp, *ofp; size_t rsyncsz = strlen(".rsync/"); size_t rrdpsz = strlen(".rrdp/"); size_t tasz = strlen(".ta/"); @@ -1677,20 +1692,33 @@ repo_move_valid(struct filepath_tree *tree) if (repo_mkpath(AT_FDCWD, fn) == -1) continue; - if (rename(fp->file, fn) == -1) { - warn("rename %s", fp->file); - continue; - } - /* switch filepath node to new path */ RB_REMOVE(filepath_tree, tree, fp); base = fp->file; if ((fp->file = strdup(fn)) == NULL) err(1, NULL); + + again: + if ((ofp = RB_INSERT(filepath_tree, tree, fp)) != NULL) { + if (ofp->talmask == 0) { + /* conflicting path is not valid, drop it */ + filepath_put(tree, ofp); + goto again; + } + if (fp->talmask != 0) { + warnx("%s: file already present in " + "validated cache", fp->file); + } + free(fp->file); + free(fp); + free(base); + continue; + } + + if (rename(base, fp->file) == -1) + warn("rename to %s", fp->file); + free(base); - if (RB_INSERT(filepath_tree, tree, fp) != NULL) - errx(1, "%s: both possibilities of file present", - fp->file); } }