From f6a8e99484a9a5e649a4b7056ebc97e09759f97b Mon Sep 17 00:00:00 2001 From: job Date: Mon, 14 Jun 2021 12:08:50 +0000 Subject: [PATCH] Fix ROA & MFT version check handling This check was incomplete: to correctly verify the ASN1 INTEGER holding the version component, first the context specific explicit tag would need to be checked. However, the X.690 spec (section 11.5) states that if the one should not encode a component if it is the default value. There are no RFCs specifying new versions of ROA or MFT, so checking the content of the version component should be skipped entirely. Thus if the version component is present, something is wrong. --- usr.sbin/rpki-client/mft.c | 31 +++++++------------------------ usr.sbin/rpki-client/roa.c | 24 ++++-------------------- 2 files changed, 11 insertions(+), 44 deletions(-) diff --git a/usr.sbin/rpki-client/mft.c b/usr.sbin/rpki-client/mft.c index e1c68ac1b11..a866ab2c09a 100644 --- a/usr.sbin/rpki-client/mft.c +++ b/usr.sbin/rpki-client/mft.c @@ -1,4 +1,4 @@ -/* $OpenBSD: mft.c,v 1.34 2021/05/11 11:32:51 claudio Exp $ */ +/* $OpenBSD: mft.c,v 1.35 2021/06/14 12:08:50 job Exp $ */ /* * Copyright (c) 2019 Kristaps Dzonsons * @@ -260,8 +260,7 @@ mft_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p) const ASN1_TYPE *t; const ASN1_GENERALIZEDTIME *from, *until; BIGNUM *mft_seqnum = NULL; - long mft_version; - int i, rc = -1; + int i = 0, rc = -1; if ((seq = d2i_ASN1_SEQUENCE_ANY(NULL, &d, dsz)) == NULL) { cryptowarnx("%s: RFC 6486 section 4.2: Manifest: " @@ -269,7 +268,7 @@ mft_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p) goto out; } - /* The profile version is optional. */ + /* The profile version field is optional. */ if (sk_ASN1_TYPE_num(seq) != 5 && sk_ASN1_TYPE_num(seq) != 6) { @@ -279,27 +278,11 @@ mft_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p) goto out; } - /* Start with optional profile version. */ - - i = 0; if (sk_ASN1_TYPE_num(seq) == 6) { - t = sk_ASN1_TYPE_value(seq, i++); - if (t->type != V_ASN1_INTEGER) { - warnx("%s: RFC 6486 section 4.2.1: version: " - "want ASN.1 integer, have %s (NID %d)", - p->fn, ASN1_tag2str(t->type), t->type); - goto out; - } - - if (t->value.integer == NULL) - goto out; - - mft_version = ASN1_INTEGER_get(t->value.integer); - if (mft_version != 0) { - warnx("%s: RFC 6486 section 4.2.1: version: " - "want 0, have %ld", p->fn, mft_version); - goto out; - } + warnx("%s: RFC 6486 section 4.2.1 and X.690, 11.5: not " + "expecting version field, as only version 0 is supported", + p->fn); + goto out; } /* Now the manifest sequence number. */ diff --git a/usr.sbin/rpki-client/roa.c b/usr.sbin/rpki-client/roa.c index 4bcf60ea7d7..5d78d3156e9 100644 --- a/usr.sbin/rpki-client/roa.c +++ b/usr.sbin/rpki-client/roa.c @@ -1,4 +1,4 @@ -/* $OpenBSD: roa.c,v 1.19 2021/05/11 11:32:51 claudio Exp $ */ +/* $OpenBSD: roa.c,v 1.20 2021/06/14 12:08:50 job Exp $ */ /* * Copyright (c) 2019 Kristaps Dzonsons * @@ -268,25 +268,9 @@ roa_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p) /* RFC 6482, section 3.1. */ if (sz == 3) { - t = sk_ASN1_TYPE_value(seq, i++); - - /* - * This check with ASN1_INTEGER_get() is fine since - * we're looking for a value of zero anyway, so any - * overflowing number will be definition be wrong. - */ - - if (t->type != V_ASN1_INTEGER) { - warnx("%s: RFC 6482 section 3.1: version: " - "want ASN.1 integer, have %s (NID %d)", - p->fn, ASN1_tag2str(t->type), t->type); - goto out; - } else if (ASN1_INTEGER_get(t->value.integer) != 0) { - warnx("%s: RFC 6482 section 3.1: version: " - "want version 0, have %ld", - p->fn, ASN1_INTEGER_get(t->value.integer)); - goto out; - } + warnx("%s: RFC 6482 section 3 and X.690, 11.5: not expecting " + "version field, as only version 0 is supported", p->fn); + goto out; } /* -- 2.20.1