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: