On Thu, May 11, 2023 at 11:47:44AM +0200, Claudio Jeker wrote:
> 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.
you'd end up with 3 warnings for 3 problems:
- temp file is corrupted
- DIR_VALID file is corrupted
- manifest is incomplete because of the above 2 issues
> 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.
I added a conditional for noent == 2.
> We really want to avoid excessive warnings especially for things that
> are not an error.
Agreed.
Index: extern.h
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
retrieving revision 1.181
diff -u -p -r1.181 extern.h
--- extern.h 9 May 2023 10:34:32 -0000 1.181
+++ extern.h 11 May 2023 09:54:25 -0000
@@ -681,7 +681,8 @@ int valid_ta(const char *, struct auth
const struct cert *);
int valid_cert(const char *, struct auth *, const struct cert *);
int valid_roa(const char *, struct cert *, struct roa *);
-int valid_filehash(int, const char *, size_t);
+int valid_filehash(const char *, const char *, int,
+ const unsigned char *, size_t);
int valid_hash(unsigned char *, size_t, const char *, size_t);
int valid_filename(const char *, size_t);
int valid_uri(const char *, size_t, const char *);
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:54:25 -0000
@@ -177,20 +177,24 @@ 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);
+
+ if (noent == 2)
+ 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:54:25 -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:54:25 -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,19 @@ valid_filehash(int fd, const char *hash,
close(fd);
SHA256_Final(filehash, &ctx);
- if (memcmp(hash, filehash, sizeof(filehash)) != 0)
+ if (memcmp(hash, filehash, sizeof(filehash)) != 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;
}
