On Thu, May 11, 2023 at 09:31:30AM +0000, Job Snijders wrote:
> Hi Theo,
>
> On Wed, May 10, 2023 at 09:02:13PM +0200, Theo Buehler wrote:
> > Again, try to keep the code as it was as far as possible.
>
> Indeed, thank you for the feedback! Below is an amended version.
I'm not sure if this is quite right.
valid_filehash() can be called twice first with the temp file and then
with the file from the valid repo. If the temp file has a bad hash then it
will result in a warning but the file from DIR_VALID may still be valid.
If both are invalid you end up with 3 warnings for a single file.
Also 'warnx("%s#%s: missing %s", fn, p->seqnum, m->file);' is incorrect
there may be a file with invalid hashes. The missing message should only
be shown if noent == 2.
We really want to avoid excessive warnings especially for things that are
not an error.
> Index: parser.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v
> retrieving revision 1.93
> diff -u -p -r1.93 parser.c
> --- parser.c 27 Apr 2023 08:37:53 -0000 1.93
> +++ parser.c 11 May 2023 09:26:09 -0000
> @@ -177,20 +177,21 @@ proc_parser_mft_check(const char *fn, st
> fd = open(path, O_RDONLY);
> if (fd == -1 && errno == ENOENT)
> noent++;
> - free(path);
>
> /* remember which path was checked */
> m->location = loc[try];
> - valid = valid_filehash(fd, m->hash, sizeof(m->hash));
> +
> + valid = valid_filehash(path, m->file, fd, m->hash,
> + sizeof(m->hash));
> + free(path);
> }
>
> if (!valid) {
> /* silently skip not-existing unknown files */
> if (m->type == RTYPE_INVALID && noent == 2)
> continue;
> - warnx("%s: bad message digest for %s", fn, m->file);
> + warnx("%s#%s: missing %s", fn, p->seqnum, m->file);
> rc = 0;
> - continue;
> }
> }
>
> Index: repo.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/repo.c,v
> retrieving revision 1.44
> diff -u -p -r1.44 repo.c
> --- repo.c 26 Apr 2023 16:32:41 -0000 1.44
> +++ repo.c 11 May 2023 09:26:09 -0000
> @@ -827,8 +827,7 @@ rrdp_handle_file(unsigned int id, enum p
> fd = open(fn, O_RDONLY);
> } while (fd == -1 && try < 2);
>
> - if (!valid_filehash(fd, hash, hlen)) {
> - warnx("%s: bad file digest for %s", rr->notifyuri, fn);
> + if (!valid_filehash(rr->notifyuri, fn, fd, hash, hlen)) {
> free(fn);
> return 0;
> }
> Index: validate.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rpki-client/validate.c,v
> retrieving revision 1.60
> diff -u -p -r1.60 validate.c
> --- validate.c 9 May 2023 10:34:32 -0000 1.60
> +++ validate.c 11 May 2023 09:26:09 -0000
> @@ -211,10 +211,11 @@ valid_roa(const char *fn, struct cert *c
> * Returns 1 if hash matched, 0 otherwise. Closes fd when done.
> */
> int
> -valid_filehash(int fd, const char *hash, size_t hlen)
> +valid_filehash(const char *loc, const char *fn, int fd,
> + const unsigned char *hash, size_t hlen)
> {
> SHA256_CTX ctx;
> - char filehash[SHA256_DIGEST_LENGTH];
> + unsigned char filehash[SHA256_DIGEST_LENGTH];
> char buffer[8192];
> ssize_t nr;
>
> @@ -230,8 +231,18 @@ valid_filehash(int fd, const char *hash,
> close(fd);
> SHA256_Final(filehash, &ctx);
>
> - if (memcmp(hash, filehash, sizeof(filehash)) != 0)
> + if (memcmp(hash, filehash, SHA256_DIGEST_LENGTH) != 0) {
> + char *expected, *computed;
> + if (base64_encode(hash, hlen, &expected) == -1)
> + errx(1, "base64_encode failed");
> + if (base64_encode(filehash, hlen, &computed) == -1)
> + errx(1, "base64_encode failed");
> + warnx("%s: bad file digest for %s (expected: %s, got %s)",
> + loc, fn, expected, computed);
> + free(expected);
> + free(computed);
> return 0;
> + }
> return 1;
> }
>
>
--
:wq Claudio
