On Mon, Apr 11, 2022 at 09:41:05AM +0200, Theo Buehler wrote:
> 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:
>
OK claudio@
--
:wq Claudio
