From 91fddc0d9387446137c6d47183a3b3ee2ef2b30d Mon Sep 17 00:00:00 2001 From: job Date: Mon, 15 Jun 2026 14:30:53 +0000 Subject: [PATCH] Tighten well-formedness checks on AIA & SIA extensions in certs Valid Rsync URIs always contain a module component. To avoid duplication of URI validation code, refactor rsync_base_uri() to optionally allocate & dup the base URI portion. Thanks to Ties de Kock for reporting. OK tb@ claudio@ --- regress/usr.sbin/rpki-client/Makefile.inc | 4 +- usr.sbin/rpki-client/extern.h | 4 +- usr.sbin/rpki-client/repo.c | 6 +-- usr.sbin/rpki-client/rsync.c | 47 +++++++++++++---------- usr.sbin/rpki-client/validate.c | 8 +++- 5 files changed, 40 insertions(+), 29 deletions(-) diff --git a/regress/usr.sbin/rpki-client/Makefile.inc b/regress/usr.sbin/rpki-client/Makefile.inc index 86342c48376..e43944f1538 100644 --- a/regress/usr.sbin/rpki-client/Makefile.inc +++ b/regress/usr.sbin/rpki-client/Makefile.inc @@ -1,4 +1,4 @@ -# $OpenBSD: Makefile.inc,v 1.47 2026/05/01 11:23:57 tb Exp $ +# $OpenBSD: Makefile.inc,v 1.48 2026/06/15 14:30:53 job Exp $ .PATH: ${.CURDIR}/../../../../usr.sbin/rpki-client @@ -29,7 +29,7 @@ CLEANFILES+= *.out *.err *.txt TEST_COMMON= as.c aspa.c ccr.c cert.c cms.c constraints-dummy.c crl.c \ encoding.c io.c ip.c json.c mft.c print.c repo-dummy.c \ - rfc3779.c roa.c validate.c x509.c asn1_bit_string.c + rfc3779.c roa.c rsync.c validate.c x509.c asn1_bit_string.c SRCS_test-ip += test-ip.c ${TEST_COMMON} run-regress-test-ip: test-ip diff --git a/usr.sbin/rpki-client/extern.h b/usr.sbin/rpki-client/extern.h index 7788c46a597..e5709158c97 100644 --- a/usr.sbin/rpki-client/extern.h +++ b/usr.sbin/rpki-client/extern.h @@ -1,4 +1,4 @@ -/* $OpenBSD: extern.h,v 1.281 2026/05/18 16:26:41 tb Exp $ */ +/* $OpenBSD: extern.h,v 1.282 2026/06/15 14:30:53 job Exp $ */ /* * Copyright (c) 2019 Kristaps Dzonsons * @@ -859,7 +859,7 @@ void proc_filemode(int) __attribute__((noreturn)); /* Rsync-specific. */ -char *rsync_base_uri(const char *); +int rsync_base_uri(const char *, char **); void proc_rsync(char *, char *, int) __attribute__((noreturn)); /* HTTP and RRDP processes. */ diff --git a/usr.sbin/rpki-client/repo.c b/usr.sbin/rpki-client/repo.c index 073fa85f2d3..80b81532955 100644 --- a/usr.sbin/rpki-client/repo.c +++ b/usr.sbin/rpki-client/repo.c @@ -1,4 +1,4 @@ -/* $OpenBSD: repo.c,v 1.82 2026/06/08 12:12:00 job Exp $ */ +/* $OpenBSD: repo.c,v 1.83 2026/06/15 14:30:53 job Exp $ */ /* * Copyright (c) 2021 Claudio Jeker * Copyright (c) 2019 Kristaps Dzonsons @@ -482,7 +482,7 @@ rsync_get(const char *uri, const char *validdir) struct rsyncrepo *rr; char *repo; - if ((repo = rsync_base_uri(uri)) == NULL) + if (!rsync_base_uri(uri, &repo)) errx(1, "bad caRepository URI: %s", uri); SLIST_FOREACH(rr, &rsyncrepos, entry) @@ -1227,7 +1227,7 @@ repo_lookup(int talid, const char *uri, const char *notify) struct repo *rp; char *repouri; - if ((repouri = rsync_base_uri(uri)) == NULL) + if (!rsync_base_uri(uri, &repouri)) errx(1, "bad caRepository URI: %s", uri); /* Look up in repository table. */ diff --git a/usr.sbin/rpki-client/rsync.c b/usr.sbin/rpki-client/rsync.c index 6cb45f77d87..756e7d0034f 100644 --- a/usr.sbin/rpki-client/rsync.c +++ b/usr.sbin/rpki-client/rsync.c @@ -1,4 +1,4 @@ -/* $OpenBSD: rsync.c,v 1.62 2026/05/27 09:42:19 claudio Exp $ */ +/* $OpenBSD: rsync.c,v 1.63 2026/06/15 14:30:53 job Exp $ */ /* * Copyright (c) 2019 Kristaps Dzonsons * @@ -54,22 +54,22 @@ struct rsync { static TAILQ_HEAD(, rsync) states = TAILQ_HEAD_INITIALIZER(states); /* - * Return the base of a rsync URI (rsync://hostname/module). The - * caRepository provided by the RIR CAs point deeper than they should - * which would result in many rsync calls for almost every subdirectory. - * This is inefficient so instead crop the URI to a common base. - * The returned string needs to be freed by the caller. + * Check whether the provided URI contains a valid rsync base URI. + * (rsync://hostname/module/). Optionally copy it into out_base_uri, which must + * be freed by the caller. This is useful because the caRepository provided by + * the CAs point deeper than they should which would result in many rsync + * executions for almost every subdirectory. This is too inefficient, so crop + * the URI to a common base. Returns 1 on success, 0 on failure. */ -char * -rsync_base_uri(const char *uri) +int +rsync_base_uri(const char *uri, char **out_base_uri) { const char *host, *module, *rest; - char *base_uri; /* Case-insensitive rsync URI. */ if (strncasecmp(uri, RSYNC_PROTO, RSYNC_PROTO_LEN) != 0) { warnx("%s: not using rsync schema", uri); - return NULL; + return 0; } /* Parse the non-zero-length hostname. */ @@ -77,32 +77,37 @@ rsync_base_uri(const char *uri) if ((module = strchr(host, '/')) == NULL) { warnx("%s: missing rsync module", uri); - return NULL; + return 0; } else if (module == host) { warnx("%s: zero-length rsync host", uri); - return NULL; + return 0; } /* The non-zero-length module follows the hostname. */ module++; - if (*module == '\0') { + if (*module == '\0' || *module == '/') { warnx("%s: zero-length rsync module", uri); - return NULL; + return 0; } /* The path component is optional. */ if ((rest = strchr(module, '/')) == NULL) { - if ((base_uri = strdup(uri)) == NULL) - err(1, NULL); - return base_uri; + if (out_base_uri != NULL) { + if ((*out_base_uri = strdup(uri)) == NULL) + err(1, NULL); + } + return 1; } else if (rest == module) { warnx("%s: zero-length module", uri); - return NULL; + return 0; } - if ((base_uri = strndup(uri, rest - uri)) == NULL) - err(1, NULL); - return base_uri; + if (out_base_uri != NULL) { + if ((*out_base_uri = strndup(uri, rest - uri)) == NULL) + err(1, NULL); + } + + return 1; } /* diff --git a/usr.sbin/rpki-client/validate.c b/usr.sbin/rpki-client/validate.c index 9c82deda048..beaab8f207a 100644 --- a/usr.sbin/rpki-client/validate.c +++ b/usr.sbin/rpki-client/validate.c @@ -1,4 +1,4 @@ -/* $OpenBSD: validate.c,v 1.83 2026/05/16 07:27:03 job Exp $ */ +/* $OpenBSD: validate.c,v 1.84 2026/06/15 14:30:53 job Exp $ */ /* * Copyright (c) 2019 Kristaps Dzonsons * @@ -241,6 +241,7 @@ valid_filename(const char *fn, size_t len) * Validate a URI to make sure it is pure ASCII and does not point backwards * or doing some other silly tricks. To enforce the protocol pass either * https:// or rsync:// as proto, if NULL is passed no protocol is enforced. + * If it's rsync, check that the URI contains a module component. * Returns 1 if valid, 0 otherwise. */ int @@ -268,6 +269,11 @@ valid_uri(const char *uri, size_t usz, const char *proto) if (strstr(uri, "/.") != NULL) return 0; + if (strncasecmp(uri, RSYNC_PROTO, RSYNC_PROTO_LEN) == 0) { + if (!rsync_base_uri(uri, NULL)) + return 0; + } + return 1; }