On Wed, Aug 10, 2022 at 03:10:19PM +0000, Job Snijders wrote:
> Hi all,
>
> An errata exists for RFC 6482, which informs us: """The EE certificate
> MUST NOT use "inherit" elements as described in [RFC3779].""" Read the
> full report here: https://www.rfc-editor.org/errata/eid3166
>
> Although it might seem a bit 'wasteful' to d2i the IP Resources
> extension in multiple places, noodling through parameters when to check
> for inheritance and when not to check didn't improve code readability.
> I'm open to suggestions how to perform this check differently.
As I understand it, what really is missing isn't a check for inheritance
per se, but rather a check whether the prefixes in the ROA are covered
by the EE cert's IP address delegation extension (the bullet point in
RFC 6482, section 4). If we had such a check, that would be the natural
place for adding an inheritance check for the EE cert.
Below is my "overclaim" diff from a few weeks back that prepended the EE
cert to the auth chain for ROAs and RSCs so that we check their
resources against the EE cert instead of our currently incorrect checks
that permitted overclaiming. The diff was ok job and claudio told me
that it looked ok - I will need to think it through in detail once more,
however.
I believe that with something like this diff, your desired inheritance
check should be added to valid_roa() above the for() loop.
Does that make sense?
Index: cert.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
retrieving revision 1.84
diff -u -p -r1.84 cert.c
--- cert.c 31 May 2022 18:51:35 -0000 1.84
+++ cert.c 10 Aug 2022 15:40:25 -0000
@@ -467,23 +467,19 @@ sbgp_sia(struct parse *p, X509_EXTENSION
}
}
- if (p->res->mft == NULL || p->res->repo == NULL) {
- warnx("%s: RFC 6487 section 4.8.8: SIA: missing caRepository "
- "or rpkiManifest", p->fn);
- goto out;
- }
-
- if (strstr(p->res->mft, p->res->repo) != p->res->mft) {
- warnx("%s: RFC 6487 section 4.8.8: SIA: "
- "conflicting URIs for caRepository and rpkiManifest",
- p->fn);
- goto out;
- }
+ if (p->res->mft != NULL && p->res->repo != NULL) {
+ if (strstr(p->res->mft, p->res->repo) != p->res->mft) {
+ warnx("%s: RFC 6487 section 4.8.8: SIA: conflicting "
+ "URIs for caRepository and rpkiManifest",
+ p->fn);
+ goto out;
+ }
- if (rtype_from_file_extension(p->res->mft) != RTYPE_MFT) {
- warnx("%s: RFC 6487 section 4.8.8: SIA: "
- "not an MFT file", p->fn);
- goto out;
+ if (rtype_from_file_extension(p->res->mft) != RTYPE_MFT) {
+ warnx("%s: RFC 6487 section 4.8.8: SIA: "
+ "not an MFT file", p->fn);
+ goto out;
+ }
}
rc = 1;
@@ -570,42 +566,20 @@ certificate_policies(struct parse *p, X5
return rc;
}
-/*
- * Parse and partially validate an RPKI X509 certificate (either a trust
- * anchor or a certificate) as defined in RFC 6487.
- * Returns the parse results or NULL on failure.
- */
-struct cert *
-cert_parse_pre(const char *fn, const unsigned char *der, size_t len)
+static struct cert *
+cert_extract_x509(const char *fn, X509 *x)
{
int extsz;
- int sia_present = 0;
size_t i;
- X509 *x = NULL;
X509_EXTENSION *ext = NULL;
ASN1_OBJECT *obj;
struct parse p;
- /* just fail for empty buffers, the warning was printed elsewhere */
- if (der == NULL)
- return NULL;
-
memset(&p, 0, sizeof(struct parse));
p.fn = fn;
if ((p.res = calloc(1, sizeof(struct cert))) == NULL)
err(1, NULL);
- if ((x = d2i_X509(NULL, &der, len)) == NULL) {
- cryptowarnx("%s: d2i_X509", p.fn);
- goto out;
- }
-
- /* Cache X509v3 extensions, see X509_check_ca(3). */
- if (X509_check_purpose(x, -1, -1) <= 0) {
- cryptowarnx("%s: could not cache X509v3 extensions", p.fn);
- goto out;
- }
-
/* Look for X509v3 extensions. */
if ((extsz = X509_get_ext_count(x)) < 0)
@@ -627,7 +601,6 @@ cert_parse_pre(const char *fn, const uns
goto out;
break;
case NID_sinfo_access:
- sia_present = 1;
if (!sbgp_sia(&p, ext))
goto out;
break;
@@ -647,12 +620,6 @@ cert_parse_pre(const char *fn, const uns
case NID_ext_key_usage:
break;
default:
- /* {
- char objn[64];
- OBJ_obj2txt(objn, sizeof(objn), obj, 0);
- warnx("%s: ignoring %s (NID %d)",
- p.fn, objn, OBJ_obj2nid(obj));
- } */
break;
}
}
@@ -667,54 +634,95 @@ cert_parse_pre(const char *fn, const uns
goto out;
if (!x509_get_expire(x, p.fn, &p.res->expires))
goto out;
- p.res->purpose = x509_get_purpose(x, p.fn);
- /* Validation on required fields. */
+ p.res->x509 = x;
+ return p.res;
+
+ out:
+ cert_free(p.res);
+ X509_free(x);
+ return NULL;
+}
+
+/*
+ * Parse and partially validate an RPKI X509 certificate (either a trust
+ * anchor or a certificate) as defined in RFC 6487.
+ * Returns the parse results or NULL on failure.
+ */
+struct cert *
+cert_parse_pre(const char *fn, const unsigned char *der, size_t len)
+{
+ X509 *x = NULL;
+ struct cert *cert = NULL;
+
+ /* just fail for empty buffers, the warning was printed elsewhere */
+ if (der == NULL)
+ return NULL;
+
+ if ((x = d2i_X509(NULL, &der, len)) == NULL) {
+ cryptowarnx("%s: d2i_X509", fn);
+ goto out;
+ }
+
+ /* Cache X509v3 extensions, see X509_check_ca(3). */
+ if (X509_check_purpose(x, -1, -1) <= 0) {
+ cryptowarnx("%s: could not cache X509v3 extensions", fn);
+ goto out;
+ }
+
+ if (!X509_up_ref(x)) {
+ cryptowarnx("%s: X509_up_ref failed", fn);
+ goto out;
+ }
+
+ if ((cert = cert_extract_x509(fn, x)) == NULL)
+ goto out;
- switch (p.res->purpose) {
+ /* Validation on required fields. */
+ cert->purpose = x509_get_purpose(x, fn);
+ switch (cert->purpose) {
case CERT_PURPOSE_CA:
- if (p.res->mft == NULL) {
- warnx("%s: RFC 6487 section 4.8.8: missing SIA", p.fn);
+ if (cert->mft == NULL) {
+ warnx("%s: RFC 6487 section 4.8.8: missing SIA", fn);
goto out;
}
- if (p.res->asz == 0 && p.res->ipsz == 0) {
- warnx("%s: missing IP or AS resources", p.fn);
+ if (cert->asz == 0 && cert->ipsz == 0) {
+ warnx("%s: missing IP or AS resources", fn);
goto out;
}
break;
case CERT_PURPOSE_BGPSEC_ROUTER:
- p.res->pubkey = x509_get_pubkey(x, p.fn);
- if (p.res->pubkey == NULL) {
- warnx("%s: x509_get_pubkey failed", p.fn);
+ cert->pubkey = x509_get_pubkey(x, fn);
+ if (cert->pubkey == NULL) {
+ warnx("%s: x509_get_pubkey failed", fn);
goto out;
}
- if (p.res->ipsz > 0) {
- warnx("%s: unexpected IP resources in BGPsec cert",
- p.fn);
+ if (cert->ipsz > 0) {
+ warnx("%s: unexpected IP resources in BGPsec cert", fn);
goto out;
}
- if (sia_present) {
+ if (X509_get_ext_by_NID(x, NID_sinfo_access, -1) != -1) {
warnx("%s: unexpected SIA extension in BGPsec cert",
- p.fn);
+ fn);
goto out;
}
break;
default:
- warnx("%s: x509_get_purpose failed in %s", p.fn, __func__);
+ warnx("%s: x509_get_purpose failed in %s", fn, __func__);
goto out;
}
- if (p.res->ski == NULL) {
- warnx("%s: RFC 6487 section 8.4.2: missing SKI", p.fn);
+ if (cert->ski == NULL) {
+ warnx("%s: RFC 6487 section 8.4.2: missing SKI", fn);
goto out;
}
- p.res->x509 = x;
- return p.res;
+ X509_free(x);
+ return cert;
-out:
- cert_free(p.res);
+ out:
X509_free(x);
+ cert_free(cert);
return NULL;
}
@@ -951,6 +959,45 @@ auth_insert(struct auth_tree *auths, str
err(1, "auth tree corrupted");
return na;
+}
+
+/*
+ * Prepend leaf to auth tree for validation purposes.
+ * Returns extended auth tree on success and NULL on failure.
+ */
+struct auth *
+auth_extend(const char *fn, X509 *x, struct auth *parent)
+{
+ struct cert *cert;
+ struct auth *a;
+
+ if (!X509_up_ref(x)) {
+ cryptowarnx("%s: X509_up_ref failed", fn);
+ return NULL;
+ }
+
+ if ((cert = cert_extract_x509(fn, x)) == NULL)
+ return NULL;
+
+ if ((a = calloc(1, sizeof(*a))) == NULL)
+ err(1, NULL);
+ a->parent = parent;
+ a->cert = cert;
+
+ return a;
+}
+
+/*
+ * Free a single auth structure
+ */
+void
+auth_free(struct auth *a)
+{
+ if (a == NULL)
+ return;
+
+ cert_free(a->cert);
+ free(a);
}
static void
Index: extern.h
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
retrieving revision 1.146
diff -u -p -r1.146 extern.h
--- extern.h 9 Aug 2022 09:02:26 -0000 1.146
+++ extern.h 10 Aug 2022 15:41:09 -0000
@@ -350,6 +350,8 @@ RB_HEAD(auth_tree, auth);
struct auth *auth_find(struct auth_tree *, const char *);
struct auth *auth_insert(struct auth_tree *, struct cert *, struct auth *);
+struct auth *auth_extend(const char *, X509 *, struct auth *);
+void auth_free(struct auth *);
enum http_result {
HTTP_FAILED, /* anything else */
@@ -509,7 +511,7 @@ struct auth *valid_ski_aki(const char *,
int valid_ta(const char *, struct auth_tree *,
const struct cert *);
int valid_cert(const char *, struct auth *, const struct cert *);
-int valid_roa(const char *, struct auth *, struct roa *);
+int valid_roa(const char *, struct auth *, X509 *, struct roa *);
int valid_filehash(int, const char *, size_t);
int valid_hash(unsigned char *, size_t, const char *, size_t);
int valid_filename(const char *, size_t);
@@ -517,7 +519,7 @@ int valid_uri(const char *, size_t, co
int valid_origin(const char *, const char *);
int valid_x509(char *, X509_STORE_CTX *, X509 *, struct auth *,
struct crl *, int);
-int valid_rsc(const char *, struct auth *, struct rsc *);
+int valid_rsc(const char *, struct auth *, X509 *, struct rsc *);
int valid_econtent_version(const char *, const ASN1_INTEGER *);
/* Working with CMS. */
Index: filemode.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/filemode.c,v
retrieving revision 1.7
diff -u -p -r1.7 filemode.c
--- filemode.c 11 May 2022 14:42:01 -0000 1.7
+++ filemode.c 10 Aug 2022 15:40:25 -0000
@@ -392,9 +392,9 @@ proc_parser_file(char *file, unsigned ch
if ((status = valid_x509(file, ctx, x509, a, c, 0))) {
if (type == RTYPE_ROA)
- status = valid_roa(file, a, roa);
+ status = valid_roa(file, a, x509, roa);
else if (type == RTYPE_RSC)
- status = valid_rsc(file, a, rsc);
+ status = valid_rsc(file, a, x509, rsc);
}
if (status)
printf("OK");
Index: parser.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v
retrieving revision 1.73
diff -u -p -r1.73 parser.c
--- parser.c 21 Apr 2022 12:59:03 -0000 1.73
+++ parser.c 10 Aug 2022 15:40:25 -0000
@@ -144,7 +144,6 @@ proc_parser_roa(char *file, const unsign
roa_free(roa);
return NULL;
}
- X509_free(x509);
roa->talid = a->cert->talid;
@@ -153,8 +152,9 @@ proc_parser_roa(char *file, const unsign
* the code around roa_read() to check the "valid" field itself.
*/
- if (valid_roa(file, a, roa))
+ if (valid_roa(file, a, x509, roa))
roa->valid = 1;
+ X509_free(x509);
/*
* Check CRL to figure out the soonest transitive expiry moment
Index: validate.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/validate.c,v
retrieving revision 1.40
diff -u -p -r1.40 validate.c
--- validate.c 10 Jun 2022 10:36:43 -0000 1.40
+++ validate.c 10 Aug 2022 15:40:25 -0000
@@ -201,11 +201,14 @@ valid_cert(const char *fn, struct auth *
* Returns 1 if valid, 0 otherwise.
*/
int
-valid_roa(const char *fn, struct auth *a, struct roa *roa)
+valid_roa(const char *fn, struct auth *a, X509 *x, struct roa *roa)
{
size_t i;
char buf[64];
+ if ((a = auth_extend(fn, x, a)) == NULL)
+ return 0;
+
for (i = 0; i < roa->ipsz; i++) {
if (valid_ip(a, roa->ips[i].afi, roa->ips[i].min,
roa->ips[i].max))
@@ -214,9 +217,11 @@ valid_roa(const char *fn, struct auth *a
roa->ips[i].afi, buf, sizeof(buf));
warnx("%s: RFC 6482: uncovered IP: "
"%s", fn, buf);
+ auth_free(a);
return 0;
}
+ auth_free(a);
return 1;
}
@@ -442,16 +447,20 @@ valid_x509(char *file, X509_STORE_CTX *s
* Returns 1 if valid, 0 otherwise.
*/
int
-valid_rsc(const char *fn, struct auth *a, struct rsc *rsc)
+valid_rsc(const char *fn, struct auth *a, X509 *x, struct rsc *rsc)
{
size_t i;
uint32_t min, max;
char buf1[64], buf2[64];
+ int rc = 0;
+
+ if ((a = auth_extend(fn, x, a)) == NULL)
+ goto out;
for (i = 0; i < rsc->asz; i++) {
if (rsc->as[i].type == CERT_AS_INHERIT) {
warnx("%s: RSC ResourceBlock: illegal inherit", fn);
- return 0;
+ goto out;
}
min = rsc->as[i].type == CERT_AS_RANGE ? rsc->as[i].range.min
@@ -474,13 +483,13 @@ valid_rsc(const char *fn, struct auth *a
default:
break;
}
- return 0;
+ goto out;
}
for (i = 0; i < rsc->ipsz; i++) {
if (rsc->ips[i].type == CERT_IP_INHERIT) {
warnx("%s: RSC ResourceBlock: illegal inherit", fn);
- return 0;
+ goto out;
}
if (valid_ip(a, rsc->ips[i].afi, rsc->ips[i].min,
@@ -505,10 +514,13 @@ valid_rsc(const char *fn, struct auth *a
default:
break;
}
- return 0;
+ goto out;
}
- return 1;
+ rc = 1;
+ out:
+ auth_free(a);
+ return rc;
}
int
