There are various things that are still wrong with the way x509_location()
is used. I think we should get rid of the warnx in x509_location() if
*out != NULL since it results in warnings only once we found an acceptable
URI. This is also an issue in the SIA for EE certs handling and would be
for CRLs if we validated all access location URIs (which we should).

For sbgp_sia(), my reading of RFC 8182, section 3.2 is that there is only
one rpkiNotify URI, although it's not made explicit. Also, I think per
usual, what's not explicitly mentioned in 6487 or subsequent specs is
not allowed, so reject a cert containing such access description OIDs.
None of the other children of id-ad make sense here anyway:
https://oid-rep.orange-labs.fr/get/1.3.6.1.5.5.7.48

For AIA, SIA, and CRL, do not blindly set the out parameter to NULL.
assert that it is NULL, since this smells like a leak. Use a local
variable and assign the first acceptable URI to the out parameter, check
all URIs for validity and thus make the functions match sbgp_sia() more
closely. AIA only has one accessMethod, so it is slightly different.

With all this mess out of the way, we can drop the (*out != NULL) warnx()
and instead assert that *out == NULL in x509_location().

Index: cert.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
diff -u -p -r1.134 cert.c
--- cert.c	4 Jun 2024 04:17:18 -0000	1.134
+++ cert.c	4 Jun 2024 09:26:24 -0000
@@ -506,7 +506,7 @@ sbgp_sia(const char *fn, struct cert *ce
 	ACCESS_DESCRIPTION	*ad;
 	ASN1_OBJECT		*oid;
 	const char		*mftfilename;
-	char			*carepo = NULL, *rpkimft = NULL;
+	char			*carepo = NULL, *rpkimft = NULL, *notify = NULL;
 	int			 i, rc = 0;
 
 	assert(cert->repo == NULL && cert->mft == NULL && cert->notify == NULL);
@@ -560,14 +560,30 @@ sbgp_sia(const char *fn, struct cert *ce
 			rpkimft = NULL;
 		} else if (OBJ_cmp(oid, notify_oid) == 0) {
 			if (!x509_location(fn, "SIA: rpkiNotify",
-			    ad->location, &cert->notify))
+			    ad->location, &notify))
 				goto out;
-			if (strncasecmp(cert->notify, HTTPS_PROTO,
+			if (strncasecmp(notify, HTTPS_PROTO,
 			    HTTPS_PROTO_LEN) != 0) {
 				warnx("%s: non-https uri in rpkiNotify: %s",
 				    fn, cert->notify);
+				free(notify);
 				goto out;
 			}
+			if (cert->notify != NULL) {
+				warnx("%s: unexpected rpkiNotify accessMethod",
+				    fn);
+				free(notify);
+				goto out;
+			}
+			cert->notify = notify;
+			notify = NULL;
+		} else {
+			char buf[128];
+
+			OBJ_obj2txt(buf, sizeof(buf), oid, 0);
+			warnx("%s: RFC 6487 section 4.8.8.1: unexpected"
+			    " accessMethod: %s", fn, buf);
+			goto out;
 		}
 	}
 
Index: x509.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/x509.c,v
diff -u -p -r1.92 x509.c
--- x509.c	4 Jun 2024 04:17:18 -0000	1.92
+++ x509.c	4 Jun 2024 10:52:28 -0000
@@ -17,6 +17,7 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
+#include <assert.h>
 #include <err.h>
 #include <stdlib.h>
 #include <string.h>
@@ -413,13 +414,14 @@ x509_pubkey_get_ski(X509_PUBKEY *pubkey,
  * (which has to be freed after use).
  */
 int
-x509_get_aia(X509 *x, const char *fn, char **aia)
+x509_get_aia(X509 *x, const char *fn, char **out_aia)
 {
 	ACCESS_DESCRIPTION		*ad;
 	AUTHORITY_INFO_ACCESS		*info;
 	int				 crit, rc = 0;
 
-	*aia = NULL;
+	assert(*out_aia == NULL);
+
 	info = X509_get_ext_d2i(x, NID_info_access, &crit, NULL);
 	if (info == NULL) {
 		if (crit != -1) {
@@ -456,7 +458,7 @@ x509_get_aia(X509 *x, const char *fn, ch
 		goto out;
 	}
 
-	if (!x509_location(fn, "AIA: caIssuers", ad->location, aia))
+	if (!x509_location(fn, "AIA: caIssuers", ad->location, out_aia))
 		goto out;
 
 	rc = 1;
@@ -473,14 +475,14 @@ out:
  * (which has to be freed after use).
  */
 int
-x509_get_sia(X509 *x, const char *fn, char **sia)
+x509_get_sia(X509 *x, const char *fn, char **out_sia)
 {
 	ACCESS_DESCRIPTION		*ad;
 	AUTHORITY_INFO_ACCESS		*info;
 	ASN1_OBJECT			*oid;
-	int				 i, crit, rsync_found = 0;
+	int				 i, crit, rc = 0;
 
-	*sia = NULL;
+	assert(*out_sia == NULL);
 
 	info = X509_get_ext_d2i(x, NID_sinfo_access, &crit, NULL);
 	if (info == NULL) {
@@ -498,6 +500,8 @@ x509_get_sia(X509 *x, const char *fn, ch
 	}
 
 	for (i = 0; i < sk_ACCESS_DESCRIPTION_num(info); i++) {
+		char	*sia;
+
 		ad = sk_ACCESS_DESCRIPTION_value(info, i);
 		oid = ad->method;
 
@@ -522,51 +526,51 @@ x509_get_sia(X509 *x, const char *fn, ch
 			goto out;
 		}
 
-		if (!x509_location(fn, "SIA: signedObject", ad->location, sia))
+		sia = NULL;
+		if (!x509_location(fn, "SIA: signedObject", ad->location, &sia))
 			goto out;
 
-		if (rsync_found)
-			continue;
-
-		if (strncasecmp(*sia, RSYNC_PROTO, RSYNC_PROTO_LEN) == 0) {
-			const char *p = *sia + RSYNC_PROTO_LEN;
+		if (*out_sia == NULL && strncasecmp(sia, RSYNC_PROTO,
+		    RSYNC_PROTO_LEN) == 0) {
+			const char *p = sia + RSYNC_PROTO_LEN;
 			size_t fnlen, plen;
 
-			rsync_found = 1;
-
-			if (filemode)
+			if (filemode) {
+				*out_sia = sia;
 				continue;
+			}
 
 			fnlen = strlen(fn);
 			plen = strlen(p);
 
 			if (fnlen < plen || strcmp(p, fn + fnlen - plen) != 0) {
 				warnx("%s: mismatch between pathname and SIA "
-				    "(%s)", fn, *sia);
+				    "(%s)", fn, sia);
+				free(sia);
 				goto out;
 			}
 
+			*out_sia = sia;
 			continue;
 		}
-
-		free(*sia);
-		*sia = NULL;
+		if (verbose)
+			warnx("%s: RFC 6487 section 4.8.8: SIA: "
+			    "ignoring location %s", fn, sia);
+		free(sia);
 	}
 
-	if (!rsync_found) {
+	if (*out_sia == NULL) {
 		warnx("%s: RFC 6487 section 4.8.8.2: "
 		    "SIA without rsync accessLocation", fn);
 		goto out;
 	}
 
-	AUTHORITY_INFO_ACCESS_free(info);
-	return 1;
+	rc = 1;
 
  out:
-	free(*sia);
-	*sia = NULL;
+
 	AUTHORITY_INFO_ACCESS_free(info);
-	return 0;
+	return rc;
 }
 
 /*
@@ -698,15 +702,16 @@ x509_any_inherits(X509 *x)
  * after use.
  */
 int
-x509_get_crl(X509 *x, const char *fn, char **crl)
+x509_get_crl(X509 *x, const char *fn, char **out_crl)
 {
 	CRL_DIST_POINTS		*crldp;
 	DIST_POINT		*dp;
 	GENERAL_NAMES		*names;
 	GENERAL_NAME		*name;
-	int			 i, crit, rsync_found = 0;
+	int			 i, crit, rc = 0;
+
+	assert(*out_crl == NULL);
 
-	*crl = NULL;
 	crldp = X509_get_ext_d2i(x, NID_crl_distribution_points, &crit, NULL);
 	if (crldp == NULL) {
 		if (crit != -1) {
@@ -760,26 +765,35 @@ x509_get_crl(X509 *x, const char *fn, ch
 
 	names = dp->distpoint->name.fullname;
 	for (i = 0; i < sk_GENERAL_NAME_num(names); i++) {
+		char	*crl = NULL;
+
 		name = sk_GENERAL_NAME_value(names, i);
 
-		if (!x509_location(fn, "CRL distribution point", name, crl))
+		if (!x509_location(fn, "CRL distribution point", name, &crl))
 			goto out;
 
-		if (strncasecmp(*crl, RSYNC_PROTO, RSYNC_PROTO_LEN) == 0) {
-			rsync_found = 1;
-			goto out;
+		if (*out_crl == NULL && strncasecmp(crl, RSYNC_PROTO,
+		    RSYNC_PROTO_LEN) == 0) {
+			*out_crl = crl;
+			continue;
 		}
+		if (verbose)
+			warnx("%s: ignoring CRL distribution point %s",
+			    fn, crl);
+		free(crl);
+	}
 
-		free(*crl);
-		*crl = NULL;
+	if (*out_crl == NULL) {
+		warnx("%s: RFC 6487 section 4.8.6: no rsync URI "
+		    "in CRL distributionPoint", fn);
+		goto out;
 	}
 
-	warnx("%s: RFC 6487 section 4.8.6: no rsync URI "
-	    "in CRL distributionPoint", fn);
+	rc = 1;
 
  out:
 	CRL_DIST_POINTS_free(crldp);
-	return rsync_found;
+	return rc;
 }
 
 /*
@@ -813,6 +827,8 @@ x509_location(const char *fn, const char
 {
 	ASN1_IA5STRING	*uri;
 
+	assert(*out == NULL);
+
 	if (location->type != GEN_URI) {
 		warnx("%s: RFC 6487 section 4.8: %s not URI", fn, descr);
 		return 0;
@@ -823,12 +839,6 @@ x509_location(const char *fn, const char
 	if (!valid_uri(uri->data, uri->length, NULL)) {
 		warnx("%s: RFC 6487 section 4.8: %s bad location", fn, descr);
 		return 0;
-	}
-
-	if (*out != NULL) {
-		warnx("%s: RFC 6487 section 4.8: multiple %s specified, "
-		    "using the first one", fn, descr);
-		return 1;
 	}
 
 	if ((*out = strndup(uri->data, uri->length)) == NULL)