Re: rpki-client: check inherit constraint on TAs earlier on

2022-09-03 Thread Theo Buehler
On Sat, Sep 03, 2022 at 01:08:35PM +, Job Snijders wrote:
> RPKI Trust Anchors (self-signed root certificates) MAY NOT contain
> 'inherit' elements in their RFC 3779 resource extensions according to
> RFC 6490 section 2.2.
> 
> We could check way earlier on in the validation process whether the TA
> certificate conforms to this constraint. The below changeset moves the
> check from be applied on a 'struct cert'; to apply on a 'struct X509'.

I'm not against this although I think it makes sense how it curently is.
It is not really that much earlier. We do ta_parse() and almost
immediately afterward valid_ta() in both paths.

In general, the split between parse and validate is not super clear cut,
and more of a result of how filemode was added.

> @@ -391,6 +391,34 @@ x509_inherits(X509 *x)
>  
>   rc = 1;
>   out:
> + ASIdentifiers_free(asidentifiers);
> + sk_IPAddressFamily_pop_free(addrblk, IPAddressFamily_free);
> + return rc;
> +}
> +
> +/*
> + * Check whether at least one RFC 3779 extension is set to inherit.
> + * Return 1 if an inherit element is encountered in AS or IP.
> + * Return 0 otherwise.
> + */
> +int
> +x509_any_inherits(X509 *x)
> +{
> + STACK_OF(IPAddressFamily)   *addrblk = NULL;
> + ASIdentifiers   *asidentifiers = NULL;
> + int  rc = 0;
> +
> + addrblk = X509_get_ext_d2i(x, NID_sbgp_ipAddrBlock, NULL, NULL);
> + if (addrblk != NULL)

This NULL check is not needed. X509v3_addr_inherits() returns 0 on NULL.

> + if (X509v3_addr_inherits(addrblk))
> + rc = 1;
> +
> + asidentifiers = X509_get_ext_d2i(x, NID_sbgp_autonomousSysNum, NULL,
> + NULL);
> + if (asidentifiers != NULL)

Same here.

> + if (X509v3_asid_inherits(asidentifiers))
> + rc = 1;
> +
>   ASIdentifiers_free(asidentifiers);
>   sk_IPAddressFamily_pop_free(addrblk, IPAddressFamily_free);
>   return rc;
> 



rpki-client: check inherit constraint on TAs earlier on

2022-09-03 Thread Job Snijders
RPKI Trust Anchors (self-signed root certificates) MAY NOT contain
'inherit' elements in their RFC 3779 resource extensions according to
RFC 6490 section 2.2.

We could check way earlier on in the validation process whether the TA
certificate conforms to this constraint. The below changeset moves the
check from be applied on a 'struct cert'; to apply on a 'struct X509'.

Kind regards,

Job

Index: cert.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
retrieving revision 1.86
diff -u -p -r1.86 cert.c
--- cert.c  3 Sep 2022 13:01:43 -   1.86
+++ cert.c  3 Sep 2022 13:07:06 -
@@ -861,6 +861,10 @@ ta_parse(const char *fn, struct cert *p,
warnx("%s: BGPsec cert cannot be a trust anchor", fn);
goto badcert;
}
+   if (x509_any_inherits(p->x509)) {
+   warnx("%s: Trust anchor IP/AS resources may not inherit", fn);
+   goto badcert;
+   }
 
EVP_PKEY_free(pk);
return p;
Index: extern.h
===
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
retrieving revision 1.153
diff -u -p -r1.153 extern.h
--- extern.h2 Sep 2022 19:10:36 -   1.153
+++ extern.h3 Sep 2022 13:07:06 -
@@ -710,6 +710,7 @@ char*x509_convert_seqnum(const char *,
 int x509_location(const char *, const char *, const char *,
GENERAL_NAME *, char **);
 int x509_inherits(X509 *);
+int x509_any_inherits(X509 *);
 
 /* printers */
 char   *time2str(time_t);
Index: validate.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/validate.c,v
retrieving revision 1.43
diff -u -p -r1.43 validate.c
--- validate.c  3 Sep 2022 13:01:43 -   1.43
+++ validate.c  3 Sep 2022 13:07:06 -
@@ -106,28 +106,12 @@ valid_ski_aki(const char *fn, struct aut
 }
 
 /*
- * Authenticate a trust anchor by making sure its resources are not
- * inheriting and that the SKI is unique.
+ * Validate a trust anchor by making sure that the SKI is unique.
  * Returns 1 if valid, 0 otherwise.
  */
 int
 valid_ta(const char *fn, struct auth_tree *auths, const struct cert *cert)
 {
-   size_t   i;
-
-   /* AS and IP resources must not inherit. */
-   if (cert->asz && cert->as[0].type == CERT_AS_INHERIT) {
-   warnx("%s: RFC 6487 (trust anchor): "
-   "inheriting AS resources", fn);
-   return 0;
-   }
-   for (i = 0; i < cert->ipsz; i++)
-   if (cert->ips[i].type == CERT_IP_INHERIT) {
-   warnx("%s: RFC 6487 (trust anchor): "
-   "inheriting IP resources", fn);
-   return 0;
-   }
-
/* SKI must not be a dupe. */
if (auth_find(auths, cert->ski) != NULL) {
warnx("%s: RFC 6487: duplicate SKI", fn);
Index: x509.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/x509.c,v
retrieving revision 1.49
diff -u -p -r1.49 x509.c
--- x509.c  3 Sep 2022 13:06:15 -   1.49
+++ x509.c  3 Sep 2022 13:07:06 -
@@ -352,7 +352,7 @@ x509_get_expire(X509 *x, const char *fn,
 }
 
 /*
- * Check whether the RFC 3779 extensions are set to inherit.
+ * Check whether all RFC 3779 extensions are set to inherit.
  * Return 1 if both AS & IP are set to inherit.
  * Return 0 on failure (such as missing extensions or no inheritance).
  */
@@ -391,6 +391,34 @@ x509_inherits(X509 *x)
 
rc = 1;
  out:
+   ASIdentifiers_free(asidentifiers);
+   sk_IPAddressFamily_pop_free(addrblk, IPAddressFamily_free);
+   return rc;
+}
+
+/*
+ * Check whether at least one RFC 3779 extension is set to inherit.
+ * Return 1 if an inherit element is encountered in AS or IP.
+ * Return 0 otherwise.
+ */
+int
+x509_any_inherits(X509 *x)
+{
+   STACK_OF(IPAddressFamily)   *addrblk = NULL;
+   ASIdentifiers   *asidentifiers = NULL;
+   int  rc = 0;
+
+   addrblk = X509_get_ext_d2i(x, NID_sbgp_ipAddrBlock, NULL, NULL);
+   if (addrblk != NULL)
+   if (X509v3_addr_inherits(addrblk))
+   rc = 1;
+
+   asidentifiers = X509_get_ext_d2i(x, NID_sbgp_autonomousSysNum, NULL,
+   NULL);
+   if (asidentifiers != NULL)
+   if (X509v3_asid_inherits(asidentifiers))
+   rc = 1;
+
ASIdentifiers_free(asidentifiers);
sk_IPAddressFamily_pop_free(addrblk, IPAddressFamily_free);
return rc;