It was pointed out to Claudio that rpki-client does not enforce
certificate policies.

The diff below does that. It has two parts.

In cert.c we check that the certificate policy extension matches the
specification in RFC 6487, section 4.8.9, as amended by RFC 7318
section 2. That's maybe a bit lengthy but completely straightforward.
If you're curious what's in a CPS URI, it might be this:
https://www.arin.net/resources/manage/rpki/cps.pdf

The second bit is in parser.c and makes sure that the verifier builds a
policy tree enforcing that the certification path uses a policy OID of
id-cp-ipAddr-asNumber (see RFC 6484). This is a bit trickier since it
involves X509_policy_check() under the hood. In short, we set up a
policy containing that OID in the verify parameters and set verifier
flags that ensure that the user set policy is enforced.

If you will, the second part improves the validation. The verifier
doesn't have a mechanism to enforce things like there's exactly one
policy identifier, etc. That's what's done in cert.c

This works for me. Please test.

Index: cert.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
retrieving revision 1.53
diff -u -p -r1.53 cert.c
--- cert.c      20 Jan 2022 16:36:19 -0000      1.53
+++ cert.c      4 Feb 2022 09:09:38 -0000
@@ -29,6 +29,7 @@
 
 #include <openssl/asn1.h>
 #include <openssl/x509.h>
+#include <openssl/x509v3.h>
 
 #include "extern.h"
 
@@ -969,6 +970,77 @@ out:
        return rc;
 }
 
+static int
+certificate_policies(struct parse *p, X509_EXTENSION *ext)
+{
+       STACK_OF(POLICYINFO)            *policies = NULL;
+       POLICYINFO                      *policy;
+       STACK_OF(POLICYQUALINFO)        *qualifiers;
+       POLICYQUALINFO                  *qualifier;
+       int                              nid;
+       int                              rc = 0;
+
+       if (!X509_EXTENSION_get_critical(ext)) {
+               cryptowarnx("%s: RFC 6487 section 4.8.9: certificatePolicies: "
+                   "extension not critical", p->fn);
+               goto out;
+       }
+
+       if ((policies = X509V3_EXT_d2i(ext)) == NULL) {
+               cryptowarnx("%s: RFC 6487 section 4.8.9: certificatePolicies: "
+                   "failed extension parse", p->fn);
+               goto out;
+       }
+
+       if (sk_POLICYINFO_num(policies) != 1) {
+               warnx("%s: RFC 6487 section 4.8.9: certificatePolicies: "
+                   "want 1 policy, got %d", p->fn,
+                   sk_POLICYINFO_num(policies));
+               goto out;
+       }
+
+       policy = sk_POLICYINFO_value(policies, 0);
+       assert(policy != NULL && policy->policyid != NULL);
+
+       if ((nid = OBJ_obj2nid(policy->policyid)) != NID_ipAddr_asNumber) {
+               warnx("%s: RFC 6487 section 4.8.9: certificatePolicies: "
+                   "unexpected policy identifier %d (%s)", p->fn, nid,
+                   OBJ_nid2sn(nid));
+               goto out;
+       }
+
+       /* Policy qualifiers are optional. If they're absent, we're done. */
+       if ((qualifiers = policy->qualifiers) == NULL) {
+               rc = 1;
+               goto out;
+       }
+
+       if (sk_POLICYQUALINFO_num(qualifiers) != 1) {
+               warnx("%s: RFC 7318 section 2: certificatePolicies: "
+                   "want 1 policy qualifier, got %d", p->fn,
+                   sk_POLICYQUALINFO_num(qualifiers));
+               goto out;
+       }
+
+       qualifier = sk_POLICYQUALINFO_value(qualifiers, 0);
+       assert(qualifier != NULL && qualifier->pqualid != NULL);
+
+       if ((nid = OBJ_obj2nid(qualifier->pqualid)) != NID_id_qt_cps) {
+               warnx("%s: RFC 7318 section 2: certificatePolicies: "
+                   "want CPS, got %d (%s)", p->fn, nid, OBJ_nid2sn(nid));
+               goto out;
+       }
+
+       if (verbose > 1)
+               warnx("%s: CPS %.*s", p->fn, qualifier->d.cpsuri->length,
+                   qualifier->d.cpsuri->data);
+
+       rc = 1;
+ out:
+       sk_POLICYINFO_pop_free(policies, POLICYINFO_free);
+       return rc;
+}
+
 /*
  * Parse and partially validate an RPKI X509 certificate (either a trust
  * anchor or a certificate) as defined in RFC 6487.
@@ -1024,6 +1096,9 @@ cert_parse_inner(const char *fn, const u
                case NID_sinfo_access:
                        sia_present = 1;
                        c = sbgp_sia(&p, ext);
+                       break;
+               case NID_certificate_policies:
+                       c = certificate_policies(&p, ext);
                        break;
                case NID_crl_distribution_points:
                        /* ignored here, handled later */
Index: parser.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v
retrieving revision 1.58
diff -u -p -r1.58 parser.c
--- parser.c    28 Jan 2022 15:30:23 -0000      1.58
+++ parser.c    4 Feb 2022 00:15:02 -0000
@@ -43,9 +43,10 @@ static void           build_chain(const struct a
 static struct crl      *get_crl(const struct auth *);
 static void             build_crls(const struct crl *, STACK_OF(X509_CRL) **);
 
-static X509_STORE_CTX  *ctx;
-static struct auth_tree  auths = RB_INITIALIZER(&auths);
-static struct crl_tree  crlt = RB_INITIALIZER(&crlt);
+static X509_STORE_CTX          *ctx;
+static STACK_OF(ASN1_OBJECT)   *cert_policies;
+static struct auth_tree                 auths = RB_INITIALIZER(&auths);
+static struct crl_tree          crlt = RB_INITIALIZER(&crlt);
 
 struct parse_repo {
        RB_ENTRY(parse_repo)     entry;
@@ -206,6 +207,7 @@ static int
 valid_x509(char *file, X509 *x509, struct auth *a, struct crl *crl,
     unsigned long flags, int nowarn)
 {
+       X509_VERIFY_PARAM       *params;
        STACK_OF(X509)          *chain;
        STACK_OF(X509_CRL)      *crls = NULL;
        int                      c;
@@ -217,11 +219,17 @@ valid_x509(char *file, X509 *x509, struc
        if (!X509_STORE_CTX_init(ctx, NULL, x509, NULL))
                cryptoerrx("X509_STORE_CTX_init");
 
+       if ((params = X509_STORE_CTX_get0_param(ctx)) == NULL)
+               cryptoerrx("X509_STORE_CTX_get0_param");
+       if (!X509_VERIFY_PARAM_set1_policies(params, cert_policies))
+               cryptoerrx("X509_VERIFY_PARAM_set1_policies");
+
        X509_STORE_CTX_set_verify_cb(ctx, verify_cb);
        if (!X509_STORE_CTX_set_app_data(ctx, file))
                cryptoerrx("X509_STORE_CTX_set_app_data");
-       if (flags != 0)
-               X509_STORE_CTX_set_flags(ctx, flags);
+       flags |= X509_V_FLAG_EXPLICIT_POLICY;
+       flags |= X509_V_FLAG_INHIBIT_MAP;
+       X509_STORE_CTX_set_flags(ctx, flags);
        X509_STORE_CTX_set_depth(ctx, MAX_CERT_DEPTH);
        X509_STORE_CTX_set0_trusted_stack(ctx, chain);
        X509_STORE_CTX_set0_crls(ctx, crls);
@@ -1116,6 +1124,7 @@ parse_file(struct entityq *q, struct msg
 void
 proc_parser(int fd)
 {
+       ASN1_OBJECT     *cp_oid;
        struct entityq   q;
        struct msgbuf    msgq;
        struct pollfd    pfd;
@@ -1130,6 +1139,13 @@ proc_parser(int fd)
        if ((ctx = X509_STORE_CTX_new()) == NULL)
                cryptoerrx("X509_STORE_CTX_new");
 
+       if ((cp_oid = OBJ_nid2obj(NID_ipAddr_asNumber)) == NULL)
+               cryptoerrx("OBJ_nid2obj");
+       if ((cert_policies = sk_ASN1_OBJECT_new_null()) == NULL)
+               cryptoerrx("sk_ASN1_OBJECT_new_null");
+       if (!sk_ASN1_OBJECT_push(cert_policies, cp_oid))
+               cryptoerrx("sk_ASN1_OBJECT_push");
+
        TAILQ_INIT(&q);
 
        msgbuf_init(&msgq);
@@ -1190,6 +1206,7 @@ proc_parser(int fd)
        /* XXX free auths and crl tree */
 
        X509_STORE_CTX_free(ctx);
+       sk_ASN1_OBJECT_pop_free(cert_policies, ASN1_OBJECT_free);
        msgbuf_clear(&msgq);
 
        exit(0);

Reply via email to