On Sun, Apr 10, 2022 at 12:40:08PM +0200, Claudio Jeker wrote:
> This is a lot cleaner and indeed an improvement. I think some of the rc
> handling can also be simplified. The code in sbgp_sia_resource_entry()
> and sbgp_sia_resource() no longer require cleanup on error so we can just
> return 0 instead of goto out. It is OK to do this cleanup in a 2nd step
> (which you probably already planned).
Yes, I kept the rc dance since it avoids noise in the following steps.
The ones without cleanup will go away eventually.
Here's the next step: merge sbgp_sia() and sbgp_sia_resource(). Now
that both are short and easy, there's no need for a split.
Also: move the .mft extension check out of sbgp_sia_resource_mft() and
use rtype_from_file_extension() instead. The next step will dedup
sbgp_sia_resource_*().
Index: cert.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
retrieving revision 1.63
diff -u -p -r1.63 cert.c
--- cert.c 11 Apr 2022 06:47:38 -0000 1.63
+++ cert.c 11 Apr 2022 07:11:27 -0000
@@ -162,18 +162,12 @@ sbgp_sia_resource_mft(struct parse *p, c
return 0;
}
- /* Make sure it's an MFT rsync address. */
+ /* Make sure it's an rsync address. */
if (!valid_uri(d, dsz, "rsync://")) {
warnx("%s: RFC 6487 section 4.8.8: bad MFT location", p->fn);
return 0;
}
- if (dsz < 4 || strcasecmp(d + dsz - 4, ".mft") != 0) {
- warnx("%s: RFC 6487 section 4.8.8: SIA: "
- "not an MFT file", p->fn);
- return 0;
- }
-
if ((p->res->mft = strndup(d, dsz)) == NULL)
err(1, NULL);
@@ -257,15 +251,28 @@ sbgp_sia_resource_entry(struct parse *p,
}
/*
- * Multiple locations as defined in RFC 6487, 4.8.8.1.
+ * Parse "Subject Information Access" extension, RFC 6487 4.8.8.
* Returns zero on failure, non-zero on success.
*/
static int
-sbgp_sia_resource(struct parse *p, AUTHORITY_INFO_ACCESS *sia)
+sbgp_sia(struct parse *p, X509_EXTENSION *ext)
{
+ AUTHORITY_INFO_ACCESS *sia = NULL;
ACCESS_DESCRIPTION *ad;
int i, rc = 0;
+ if (X509_EXTENSION_get_critical(ext)) {
+ warnx("%s: RFC 6487 section 4.8.8: SIA: "
+ "extension not non-critical", p->fn);
+ goto out;
+ }
+
+ if ((sia = X509V3_EXT_d2i(ext)) == NULL) {
+ cryptowarnx("%s: RFC 6487 section 4.8.8: SIA: "
+ "failed extension parse", p->fn);
+ goto out;
+ }
+
for (i = 0; i < sk_ACCESS_DESCRIPTION_num(sia); i++) {
ad = sk_ACCESS_DESCRIPTION_value(sia, i);
if (!sbgp_sia_resource_entry(p, ad))
@@ -285,34 +292,11 @@ sbgp_sia_resource(struct parse *p, AUTHO
goto out;
}
- rc = 1;
- out:
- return rc;
-}
-
-/*
- * Parse "Subject Information Access" extension, RFC 6487 4.8.8.
- * Returns zero on failure, non-zero on success.
- */
-static int
-sbgp_sia(struct parse *p, X509_EXTENSION *ext)
-{
- AUTHORITY_INFO_ACCESS *sia = NULL;
- int rc = 0;
-
- if (X509_EXTENSION_get_critical(ext)) {
+ if (rtype_from_file_extension(p->res->mft) != RTYPE_MFT) {
warnx("%s: RFC 6487 section 4.8.8: SIA: "
- "extension not non-critical", p->fn);
- goto out;
- }
-
- if ((sia = X509V3_EXT_d2i(ext)) == NULL) {
- cryptowarnx("%s: RFC 6487 section 4.8.8: SIA: "
- "failed extension parse", p->fn);
+ "not an MFT file", p->fn);
goto out;
}
- if (!sbgp_sia_resource(p, sia))
- goto out;
rc = 1;
out:
