mischief/openbsd-src
1
0
Fork 0
mirror of https://github.com/openbsd/src.git synced 2025-01-10 06:47:55 -08:00
Code

Split the mft file and hash check from the mft parsing. This makes it easier

Browse Source 
to check all files in a mft before failing and also the check is now done
after the embedded cert was checked.
This refactor was triggered because of a bug in mft_parse_econtent().
check_validity() altered rc but later failure code assumed that goto out
is good enough to return an error (rc == -1) but since rc was 1 success
was returned. This bug is now also fixed.
Bug report and OK job@
This commit is contained in:
claudio 2020-04-01 14:15:49 +00:00
parent 81b7cd6931
commit 630e12ade6
3 changed files with 87 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
usr.sbin/rpki-client/extern.h
View File

@ -1,4 +1,4 @@
/* $OpenBSD: extern.h,v 1.26 2020/03/10 14:22:26 jca Exp $ */
/* $OpenBSD: extern.h,v 1.27 2020/04/01 14:15:49 claudio Exp $ */
/*
* Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
*
@ -264,6 +264,7 @@ struct cert *cert_read(int);
void mft_buffer(char **, size_t *, size_t *, const struct mft *);
void mft_free(struct mft *);
struct mft *mft_parse(X509 **, const char *, int);
int mft_check(const char *, struct mft *);
struct mft *mft_read(int);
void roa_buffer(char **, size_t *, size_t *, const struct roa *);

8
usr.sbin/rpki-client/main.c
View File

@ -1,4 +1,4 @@
/* $OpenBSD: main.c,v 1.60 2020/04/01 10:54:19 claudio Exp $ */
/* $OpenBSD: main.c,v 1.61 2020/04/01 14:15:49 claudio Exp $ */
/*
* Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
*
@ -884,6 +884,12 @@ proc_parser_mft(struct entity *entp, int force, X509_STORE *store,
X509_STORE_CTX_cleanup(ctx);
sk_X509_free(chain);
X509_free(x509);
if (!mft_check(entp->uri, mft)) {
mft_free(mft);
return NULL;
}
return mft;
}

133
usr.sbin/rpki-client/mft.c
View File

@ -1,4 +1,4 @@
/* $OpenBSD: mft.c,v 1.12 2020/03/30 12:12:51 claudio Exp $ */
/* $OpenBSD: mft.c,v 1.13 2020/04/01 14:15:49 claudio Exp $ */
/*
* Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
*
@ -115,27 +115,25 @@ hex_pton(u_char const *src, size_t srclen, char *target, size_t targlen)
* Return zero on failure, non-zero on success.
*/
static int
mft_parse_filehash(struct parse *p, const ASN1_OCTET_STRING *os, int just_check)
mft_parse_filehash(struct parse *p, const ASN1_OCTET_STRING *os)
{
char file_hash[SHA256_DIGEST_STRING_LENGTH];
char cert_hash[SHA256_DIGEST_STRING_LENGTH];
ASN1_SEQUENCE_ANY *seq;
const ASN1_TYPE *file, *hash;
char *fn = NULL;
const unsigned char *d = os->data;
size_t dsz = os->length, sz;
int rc = 0;
struct mftfile *fent;
char *cp, *path = NULL;
if ((seq = d2i_ASN1_SEQUENCE_ANY(NULL, &d, dsz)) == NULL) {
cryptowarnx("%s: RFC 6486 section 4.2.1: FileAndHash: "
"failed ASN.1 sequence parse", p->fn);
goto fail;
goto out;
} else if (sk_ASN1_TYPE_num(seq) != 2) {
warnx("%s: RFC 6486 section 4.2.1: FileAndHash: "
"want 2 elements, have %d", p->fn,
sk_ASN1_TYPE_num(seq));
goto fail;
goto out;
}
/* First is the filename itself. */
@ -145,7 +143,7 @@ mft_parse_filehash(struct parse *p, const ASN1_OCTET_STRING *os, int just_check)
warnx("%s: RFC 6486 section 4.2.1: FileAndHash: "
"want ASN.1 IA5 string, have %s (NID %d)",
p->fn, ASN1_tag2str(file->type), file->type);
goto fail;
goto out;
}
fn = strndup((const char *)file->value.ia5string->data,
file->value.ia5string->length);
@ -162,11 +160,11 @@ mft_parse_filehash(struct parse *p, const ASN1_OCTET_STRING *os, int just_check)
if (strchr(fn, '/') != NULL) {
warnx("%s: path components disallowed in filename: %s",
p->fn, fn);
goto fail;
goto out;
} else if ((sz = strlen(fn)) <= 4) {
warnx("%s: filename must be large enough for suffix part: %s",
p->fn, fn);
goto fail;
goto out;
}
if (strcasecmp(fn + sz - 4, ".roa") &&
@ -174,51 +172,28 @@ mft_parse_filehash(struct parse *p, const ASN1_OCTET_STRING *os, int just_check)
strcasecmp(fn + sz - 4, ".cer")) {
/* ignore unknown files */
free(fn);
sk_ASN1_TYPE_pop_free(seq, ASN1_TYPE_free);
return 1;
fn = NULL;
rc = 1;
goto out;
}
/* Now hash value. */
hash = sk_ASN1_TYPE_value(seq, 1);
if (hash->type != V_ASN1_BIT_STRING) {
warnx("%s: RFC 6486 section 4.2.1: FileAndHash: "
"want ASN.1 bit string, have %s (NID %d)",
p->fn, ASN1_tag2str(hash->type), hash->type);
goto fail;
goto out;
}
if (hash->value.bit_string->length != SHA256_DIGEST_LENGTH) {
warnx("%s: RFC 6486 section 4.2.1: hash: "
"invalid SHA256 length, have %d",
p->fn, hash->value.bit_string->length);
goto fail;
goto out;
}
if (hex_pton(hash->value.bit_string->data, SHA256_DIGEST_LENGTH,
cert_hash, sizeof(cert_hash)) == -1)
errx(1, "hexnum conversion failed");
/* Check hash of file now, but first build path for it */
cp = strrchr(p->fn, '/');
assert(cp != NULL);
if (asprintf(&path, "%.*s/%s", (int)(cp - p->fn), p->fn, fn) == -1)
err(1, "asprintf");
if (SHA256File(path, file_hash) == NULL) {
warn("%s: referenced file %s", p->fn, fn);
goto fail;
}
free(path);
path = NULL;
if (strcmp(file_hash, cert_hash) != 0) {
warnx("%s: bad message digest for %s", p->fn, fn);
goto fail;
}
if (just_check)
goto fail;
/* Insert the filename and hash value. */
p->res->files = reallocarray(p->res->files, p->res->filesz + 1,
@ -230,16 +205,14 @@ mft_parse_filehash(struct parse *p, const ASN1_OCTET_STRING *os, int just_check)
memset(fent, 0, sizeof(struct mftfile));
fent->file = fn;
fn = NULL;
memcpy(fent->hash, hash->value.bit_string->data, SHA256_DIGEST_LENGTH);
sk_ASN1_TYPE_pop_free(seq, ASN1_TYPE_free);
return 1;
fail:
free(path);
rc = 1;
out:
free(fn);
sk_ASN1_TYPE_pop_free(seq, ASN1_TYPE_free);
return 0;
return rc;
}
/*
@ -253,7 +226,7 @@ mft_parse_flist(struct parse *p, const ASN1_OCTET_STRING *os)
const ASN1_TYPE *t;
const unsigned char *d = os->data;
size_t dsz = os->length;
int i, rc = 0, failed = 0;
int i, rc = 0;
if ((seq = d2i_ASN1_SEQUENCE_ANY(NULL, &d, dsz)) == NULL) {
cryptowarnx("%s: RFC 6486 section 4.2: fileList: "
@ -268,12 +241,9 @@ mft_parse_flist(struct parse *p, const ASN1_OCTET_STRING *os)
"want ASN.1 sequence, have %s (NID %d)",
p->fn, ASN1_tag2str(t->type), t->type);
goto out;
} else if (!mft_parse_filehash(p, t->value.octet_string,
failed))
failed = 1;
} else if (!mft_parse_filehash(p, t->value.octet_string))
goto out;
}
if (failed)
goto out;
rc = 1;
out:
@ -291,7 +261,7 @@ mft_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p, int forc
ASN1_SEQUENCE_ANY *seq;
const ASN1_TYPE *t;
const ASN1_GENERALIZEDTIME *from, *until;
int i, rc = -1;
int i, rc = -1, validity;
if ((seq = d2i_ASN1_SEQUENCE_ANY(NULL, &d, dsz)) == NULL) {
cryptowarnx("%s: RFC 6486 section 4.2: Manifest: "
@ -360,8 +330,8 @@ mft_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p, int forc
}
until = t->value.generalizedtime;
rc = check_validity(from, until, p->fn, force);
if (rc != 1)
validity = check_validity(from, until, p->fn, force);
if (validity != 1)
goto out;
/* File list algorithm. */
@ -391,7 +361,7 @@ mft_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p, int forc
} else if (!mft_parse_flist(p, t->value.octet_string))
goto out;
rc = 1;
rc = validity;
out:
sk_ASN1_TYPE_pop_free(seq, ASN1_TYPE_free);
return rc;
@ -463,6 +433,59 @@ out:
return p.res;
}
/*
* Check the hash value of a file.
* Return zero on failure, non-zero on success.
*/
static int
mft_validfilehash(const char *fn, const struct mftfile *m)
{
char file_hash[SHA256_DIGEST_STRING_LENGTH];
char mft_hash[SHA256_DIGEST_STRING_LENGTH];
char *cp, *path = NULL;
if (hex_pton(m->hash, SHA256_DIGEST_LENGTH,
mft_hash, sizeof(mft_hash)) == -1)
errx(1, "hexnum conversion failed");
/* Check hash of file now, but first build path for it */
cp = strrchr(fn, '/');
assert(cp != NULL);
if (asprintf(&path, "%.*s/%s", (int)(cp - fn), fn, m->file) == -1)
err(1, "asprintf");
if (SHA256File(path, file_hash) == NULL) {
warn("%s: referenced file %s", fn, m->file);
free(path);
return 0;
}
free(path);
if (strcmp(file_hash, mft_hash) != 0) {
warnx("%s: bad message digest for %s", fn, m->file);
return 0;
}
return 1;
}
/*
* Check all files and their hashes in a MFT structure.
* Return zero on failure, non-zero on success.
*/
int
mft_check(const char *fn, struct mft *p)
{
size_t i;
int rc = 1;
for (i = 0; i < p->filesz; i++)
if (!mft_validfilehash(fn, &p->files[i]))
rc = 0;
return rc;
}
/*
* Free an MFT pointer.
* Safe to call with NULL.