On Wed, Aug 10, 2022 at 06:16:30PM +0200, Theo Buehler wrote:
> 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?
Here's a less intrusive version of this diff that parses the RFC 3779
extensions of the EE cert "by hand". That's all we need for proper
verification of the resources in ROAs and RSCs. Maybe that's preferable.
If we decide to go with this diff, the "no AS numbers for ROA EE certs"
and the "no inheritance" check should be moved or added to the XXX.
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 12 Aug 2022 18:59:31 -0000
@@ -953,6 +953,82 @@ auth_insert(struct auth_tree *auths, str
return na;
}
+/*
+ * Helper for auth_extend(). This only parses the RFC 3779 extensions
+ * since these are necessary for ROA and RSC validation.
+ * Returns cert on success and NULL on failure.
+ */
+static struct cert *
+auth_parse_ee_cert(const char *fn, X509 *x)
+{
+ struct parse p;
+ X509_EXTENSION *ext;
+ int index;
+
+ memset(&p, 0, sizeof(struct parse));
+ p.fn = fn;
+ if ((p.res = calloc(1, sizeof(struct cert))) == NULL)
+ err(1, NULL);
+
+ index = X509_get_ext_by_NID(x, NID_sbgp_ipAddrBlock, -1);
+ if ((ext = X509_get_ext(x, index)) != NULL) {
+ if (!sbgp_ipaddrblk(&p, ext))
+ goto out;
+ }
+
+ index = X509_get_ext_by_NID(x, NID_sbgp_autonomousSysNum, -1);
+ if ((ext = X509_get_ext(x, index)) != NULL) {
+ if (!sbgp_assysnum(&p, ext))
+ goto out;
+ }
+
+ if (!X509_up_ref(x)) {
+ cryptowarnx("%s: X509_up_ref failed", fn);
+ goto out;
+ }
+
+ p.res->x509 = x;
+ return p.res;
+
+ out:
+ cert_free(p.res);
+ return NULL;
+}
+
+/*
+ * Prepend leaf to auth chain for ROA and RSC validation purposes.
+ * Returns extended auth chain 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 ((cert = auth_parse_ee_cert(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
insert_brk(struct brk_tree *tree, struct cert *cert, int asid)
{
Index: extern.h
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
retrieving revision 1.147
diff -u -p -r1.147 extern.h
--- extern.h 10 Aug 2022 10:27:03 -0000 1.147
+++ extern.h 12 Aug 2022 18:15:37 -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 12 Aug 2022 18:15:37 -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 12 Aug 2022 18:15:37 -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 12 Aug 2022 18:15:37 -0000
@@ -201,11 +201,16 @@ 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;
+
+ /* XXX - check no inheritance and no AS numbers for a->cert here. */
+
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 +219,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 +449,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 +485,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 +516,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
