We can generalize sbgp_sia_location() and reuse it for AIAs and CRLs.
This makes the checks a bit more stringent, which seems to be fine in
practice. It also ensures that there are no embedded NULs which came
up recently. One thing I'm not sure about is that valid_uri() refuses
URIs with "/." which is an additional check.

I'll also fix the regress Makefile.inc if this goes in.

Index: cert.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
retrieving revision 1.66
diff -u -p -r1.66 cert.c
--- cert.c      11 Apr 2022 10:39:45 -0000      1.66
+++ cert.c      12 Apr 2022 07:28:38 -0000
@@ -125,40 +125,6 @@ sbgp_addr(struct parse *p,
 }
 
 /*
- * Extract and validate a SIA accessLocation, RFC 6487, 4.8.8 and RFC 8192, 
3.2.
- * Returns 0 on failure and 1 on success.
- */
-static int
-sbgp_sia_location(const char *fn, const char *descr, const char *proto,
-    GENERAL_NAME *location, char **out)
-{
-       ASN1_IA5STRING  *uri;
-
-       if (*out != NULL) {
-               warnx("%s: RFC 6487 section 4.8.8: SIA: %s already specified",
-                   fn, descr);
-               return 0;
-       }
-
-       if (location->type != GEN_URI) {
-               warnx("%s: RFC 6487 section 4.8.8: SIA: %s not URI", fn, descr);
-               return 0;
-       }
-
-       uri = location->d.uniformResourceIdentifier;
-
-       if (!valid_uri(uri->data, uri->length, proto)) {
-               warnx("%s: RFC 6487 section 4.8.8: bad %s location", fn, descr);
-               return 0;
-       }
-
-       if ((*out = strndup(uri->data, uri->length)) == NULL)
-               err(1, NULL);
-
-       return 1;
-}
-
-/*
  * Parse "Subject Information Access" extension, RFC 6487 4.8.8.
  * Returns zero on failure, non-zero on success.
  */
@@ -188,15 +154,15 @@ sbgp_sia(struct parse *p, X509_EXTENSION
                oid = ad->method;
 
                if (OBJ_cmp(oid, carepo_oid) == 0) {
-                       if (!sbgp_sia_location(p->fn, "caRepository",
+                       if (!x509_location(p->fn, "SIA: caRepository",
                            "rsync://", ad->location, &p->res->repo))
                                goto out;
                } else if (OBJ_cmp(oid, manifest_oid) == 0) {
-                       if (!sbgp_sia_location(p->fn, "rpkiManifest",
+                       if (!x509_location(p->fn, "SIA: rpkiManifest",
                            "rsync://", ad->location, &p->res->mft))
                                goto out;
                } else if (OBJ_cmp(oid, notify_oid) == 0) {
-                       if (!sbgp_sia_location(p->fn, "rpkiNotify",
+                       if (!x509_location(p->fn, "SIA: rpkiNotify",
                            "https://";, ad->location, &p->res->notify))
                                goto out;
                }
Index: extern.h
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
retrieving revision 1.125
diff -u -p -r1.125 extern.h
--- extern.h    4 Apr 2022 16:02:54 -0000       1.125
+++ extern.h    12 Apr 2022 07:33:20 -0000
@@ -22,6 +22,7 @@
 #include <sys/time.h>
 
 #include <openssl/x509.h>
+#include <openssl/x509v3.h>
 
 enum cert_as_type {
        CERT_AS_ID, /* single identifier */
@@ -589,6 +590,8 @@ char                *x509_get_pubkey(X509 *, const cha
 enum cert_purpose       x509_get_purpose(X509 *, const char *);
 int             x509_get_time(const ASN1_TIME *, time_t *);
 char           *x509_convert_seqnum(const char *, const ASN1_INTEGER *);
+int             x509_location(const char *, const char *, const char *,
+                   GENERAL_NAME *, char **);
 
 /* printers */
 char           *time2str(time_t);
Index: x509.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/x509.c,v
retrieving revision 1.39
diff -u -p -r1.39 x509.c
--- x509.c      8 Apr 2022 15:29:59 -0000       1.39
+++ x509.c      12 Apr 2022 07:33:02 -0000
@@ -306,24 +306,10 @@ x509_get_aia(X509 *x, const char *fn, ch
                    "expected caIssuers, have %d", fn, OBJ_obj2nid(ad->method));
                goto out;
        }
-       if (ad->location->type != GEN_URI) {
-               warnx("%s: RFC 6487 section 4.8.7: AIA: "
-                   "want GEN_URI type, have %d", fn, ad->location->type);
-               goto out;
-       }
 
-       if (ASN1_STRING_length(ad->location->d.uniformResourceIdentifier)
-           > MAX_URI_LENGTH) {
-               warnx("%s: RFC 6487 section 4.8.7: AIA: "
-                   "URI exceeds max length of %d", fn, MAX_URI_LENGTH);
+       if (!x509_location(fn, "AIA: caIssuers", NULL, ad->location, aia))
                goto out;
-       }
 
-       *aia = strndup(
-           ASN1_STRING_get0_data(ad->location->d.uniformResourceIdentifier),
-           ASN1_STRING_length(ad->location->d.uniformResourceIdentifier));
-       if (*aia == NULL)
-               err(1, NULL);
        rc = 1;
 
 out:
@@ -405,23 +391,9 @@ x509_get_crl(X509 *x, const char *fn, ch
        }
 
        name = sk_GENERAL_NAME_value(dp->distpoint->name.fullname, 0);
-       if (name->type != GEN_URI) {
-               warnx("%s: RFC 6487 section 4.8.6: CRL: "
-                   "want URI type, have %d", fn, name->type);
-               goto out;
-       }
 
-       if (ASN1_STRING_length(name->d.uniformResourceIdentifier)
-           > MAX_URI_LENGTH) {
-               warnx("%s: RFC 6487 section 4.8.6: CRL: "
-                   "URI exceeds max length of %d", fn, MAX_URI_LENGTH);
+       if (!x509_location(fn, "CRL distribution point", NULL, name, crl))
                goto out;
-       }
-
-       *crl = strndup(ASN1_STRING_get0_data(name->d.uniformResourceIdentifier),
-           ASN1_STRING_length(name->d.uniformResourceIdentifier));
-       if (*crl == NULL)
-               err(1, NULL);
        rc = 1;
 
 out:
@@ -500,6 +472,40 @@ x509_get_time(const ASN1_TIME *at, time_
                return 0;
        if ((*t = timegm(&tm)) == -1)
                errx(1, "timegm failed");
+       return 1;
+}
+
+/*
+ * Extract and validate an accessLocation, RFC 6487, 4.8 and RFC 8192, 3.2.
+ * Returns 0 on failure and 1 on success.
+ */
+int
+x509_location(const char *fn, const char *descr, const char *proto,
+    GENERAL_NAME *location, char **out)
+{
+       ASN1_IA5STRING  *uri;
+
+       if (*out != NULL) {
+               warnx("%s: RFC 6487 section 4.8: %s already specified", fn,
+                   descr);
+               return 0;
+       }
+
+       if (location->type != GEN_URI) {
+               warnx("%s: RFC 6487 section 4.8: %s not URI", fn, descr);
+               return 0;
+       }
+
+       uri = location->d.uniformResourceIdentifier;
+
+       if (!valid_uri(uri->data, uri->length, proto)) {
+               warnx("%s: RFC 6487 section 4.8: %s bad location", fn, descr);
+               return 0;
+       }
+
+       if ((*out = strndup(uri->data, uri->length)) == NULL)
+               err(1, NULL);
+
        return 1;
 }
 

Reply via email to