Index | Thread | Search

From:
Rafael Sadowski <rafael@sizeofvoid.org>
Subject:
[4/7] relayd: read imsg payloads via the new imsg/ibuf getters
To:
tech@openbsd.org
Date:
Sun, 7 Jun 2026 09:02:37 +0200

Download raw body.

Thread
  • Rafael Sadowski:

    [4/7] relayd: read imsg payloads via the new imsg/ibuf getters

This is a series of commits for an imsg APIv2 rework in relayd. These
were committed and tested individually. They can be reviewed in their
entirety here:

gothub: https://rsadowski.gothub.org/?action=summary&headref=imsg_v2&path=relayd.git
PR: https://codeberg.org/rsadowski/relayd/pulls/1/files

commit a8940b10e7a293b19907c9dbbca397a06d6a01ca
Author: Rafael Sadowski <rafael@sizeofvoid.org>
Date:   Sat Jun 6 20:03:29 2026 +0200

    relayd: read imsg payloads via the new imsg/ibuf getters
    
    Convert the config_get* handlers from IMSG_SIZE_CHECK() + memcpy() to
    the new imsg API. Fixed-size payloads use imsg_get_data(). Functions
    with a fixed header followed by variable-length data use imsg_get_ibuf()
    + ibuf_get() and read the remainder from the same ibuf cursor, since
    imsg_get_data() requires the payload to match the requested size
    exactly.

diff --git a/config.c b/config.c
index ccf0ef4..7ca37d5 100644
--- a/config.c
+++ b/config.c
@@ -250,9 +250,10 @@ int
 config_getreset(struct relayd *env, struct imsg *imsg)
 {
 	u_int		 mode;
-
-	IMSG_SIZE_CHECK(imsg, &mode);
-	memcpy(&mode, imsg->data, sizeof(mode));
+	if (imsg_get_data(imsg, &mode, sizeof(mode)) == -1) {
+		log_warn("%s: imsg_get_data", __func__);
+		return (-1);
+	}
 
 	config_purge(env, mode);
 
@@ -267,11 +268,12 @@ config_getcfg(struct relayd *env, struct imsg *imsg)
 	struct host		*h, *ph;
 	u_int			 what;
 
-	if (IMSG_DATA_SIZE(imsg) != sizeof(struct relayd_config))
-		return (0); /* ignore */
-
 	/* Update runtime flags */
-	memcpy(&env->sc_conf, imsg->data, sizeof(env->sc_conf));
+	if (imsg_get_data(imsg, &env->sc_conf,
+		    sizeof(struct relayd_config)) == -1) {
+		log_warn("%s: imsg_get_data", __func__);
+		return (-1);
+	}
 
 	what = ps->ps_what[privsep_process];
 
@@ -341,21 +343,26 @@ config_settable(struct relayd *env, struct table *tb)
 int
 config_gettable(struct relayd *env, struct imsg *imsg)
 {
+	struct ibuf		 ibuf;
 	struct table		*tb;
 	size_t			 sb;
-	u_int8_t		*p = imsg->data;
-	size_t			 s;
 
-	if ((tb = calloc(1, sizeof(*tb))) == NULL)
+	if (imsg_get_ibuf(imsg, &ibuf) == -1) {
+		log_warn("%s: imsg_get_ibuf", __func__);
 		return (-1);
+	}
 
-	IMSG_SIZE_CHECK(imsg, &tb->conf);
-	memcpy(&tb->conf, p, sizeof(tb->conf));
-	s = sizeof(tb->conf);
+	if ((tb = calloc(1, sizeof(*tb))) == NULL)
+		return (-1);
 
-	sb = IMSG_DATA_SIZE(imsg) - s;
-	if (sb > 0) {
-		if ((tb->sendbuf = get_string(p + s, sb)) == NULL) {
+	if (ibuf_get(&ibuf, &tb->conf, sizeof(tb->conf)) == -1) {
+		log_warn("%s: ibuf_get", __func__);
+		free(tb);
+		return (-1);
+	}
+	if ((sb = ibuf_size(&ibuf)) > 0) {
+		if ((tb->sendbuf = get_string(ibuf_data(&ibuf), sb)) == NULL) {
+			log_warn("%s: get_string", __func__);
 			free(tb);
 			return (-1);
 		}
@@ -363,6 +370,7 @@ config_gettable(struct relayd *env, struct imsg *imsg)
 	if (tb->conf.check == CHECK_BINSEND_EXPECT) {
 		tb->sendbinbuf = string2binary(tb->sendbuf);
 		if (tb->sendbinbuf == NULL) {
+			free(tb->sendbuf);
 			free(tb);
 			return (-1);
 		}
@@ -389,8 +397,11 @@ config_gethost(struct relayd *env, struct imsg *imsg)
 	if ((host = calloc(1, sizeof(*host))) == NULL)
 		return (-1);
 
-	IMSG_SIZE_CHECK(imsg, &host->conf);
-	memcpy(&host->conf, imsg->data, sizeof(host->conf));
+	if (imsg_get_data(imsg, &host->conf, sizeof(host->conf)) == -1) {
+		log_warn("%s: imsg_get_data", __func__);
+		free(host);
+		return (-1);
+	}
 
 	if (host_find(env, host->conf.id) != NULL) {
 		log_debug("%s: host %d already exists",
@@ -457,8 +468,11 @@ config_getrdr(struct relayd *env, struct imsg *imsg)
 	if ((rdr = calloc(1, sizeof(*rdr))) == NULL)
 		return (-1);
 
-	IMSG_SIZE_CHECK(imsg, &rdr->conf);
-	memcpy(&rdr->conf, imsg->data, sizeof(rdr->conf));
+	if (imsg_get_data(imsg, &rdr->conf, sizeof(rdr->conf)) == -1) {
+		log_warn("%s: imsg_get_data", __func__);
+		free(rdr);
+		return (-1);
+	}
 
 	if ((rdr->table = table_find(env, rdr->conf.table_id)) == NULL) {
 		log_debug("%s: table not found", __func__);
@@ -488,11 +502,14 @@ config_getvirt(struct relayd *env, struct imsg *imsg)
 	struct rdr	*rdr;
 	struct address	*virt;
 
-	IMSG_SIZE_CHECK(imsg, virt);
-
 	if ((virt = calloc(1, sizeof(*virt))) == NULL)
 		return (-1);
-	memcpy(virt, imsg->data, sizeof(*virt));
+
+	if (imsg_get_data(imsg, virt, sizeof(*virt)) == -1) {
+		log_warn("%s: imsg_get_data", __func__);
+		free(virt);
+		return (-1);
+	}
 
 	if ((rdr = rdr_find(env, virt->rdrid)) == NULL) {
 		log_debug("%s: rdr not found", __func__);
@@ -544,8 +561,11 @@ config_getrt(struct relayd *env, struct imsg *imsg)
 	if ((rt = calloc(1, sizeof(*rt))) == NULL)
 		return (-1);
 
-	IMSG_SIZE_CHECK(imsg, &rt->rt_conf);
-	memcpy(&rt->rt_conf, imsg->data, sizeof(rt->rt_conf));
+	if (imsg_get_data(imsg, &rt->rt_conf, sizeof(rt->rt_conf)) == -1) {
+		log_warn("%s: imsg_get_data", __func__);
+		free(rt);
+		return (-1);
+	}
 
 	if ((rt->rt_gwtable = table_find(env, rt->rt_conf.gwtable)) == NULL) {
 		log_debug("%s: table not found", __func__);
@@ -574,8 +594,11 @@ config_getroute(struct relayd *env, struct imsg *imsg)
 	if ((nr = calloc(1, sizeof(*nr))) == NULL)
 		return (-1);
 
-	IMSG_SIZE_CHECK(imsg, &nr->nr_conf);
-	memcpy(&nr->nr_conf, imsg->data, sizeof(nr->nr_conf));
+	if (imsg_get_data(imsg, &nr->nr_conf, sizeof(nr->nr_conf)) == -1) {
+		log_warn("%s: imsg_get_data", __func__);
+		free(nr);
+		return (-1);
+	}
 
 	if (route_find(env, nr->nr_conf.id) != NULL) {
 		log_debug("%s: route %d already exists",
@@ -690,22 +713,28 @@ config_setrule(struct relayd *env, struct protocol *proto)
 int
 config_getproto(struct relayd *env, struct imsg *imsg)
 {
+	struct ibuf		 ibuf;
 	struct protocol		*proto;
-	size_t			 styl;
 	size_t			 s;
-	u_int8_t		*p = imsg->data;
 
-	if ((proto = calloc(1, sizeof(*proto))) == NULL)
+	if (imsg_get_ibuf(imsg, &ibuf) == -1) {
+		log_warn("%s: imsg_get_ibuf", __func__);
 		return (-1);
+	}
 
-	IMSG_SIZE_CHECK(imsg, proto);
-	memcpy(proto, p, sizeof(*proto));
-	s = sizeof(*proto);
+	if ((proto = calloc(1, sizeof(*proto))) == NULL)
+		return (-1);
 
-	styl = IMSG_DATA_SIZE(imsg) - s;
-	proto->style = NULL;
-	if (styl > 0) {
-		if ((proto->style = get_string(p + s, styl - 1)) == NULL) {
+	if (ibuf_get(&ibuf, proto, sizeof(*proto)) == -1) {
+		log_warn("%s: ibuf_get", __func__);
+		free(proto);
+		return (-1);
+	}
+	if ((s = ibuf_size(&ibuf)) > 0) {
+		proto->style = NULL;
+		if ((proto->style = get_string(ibuf_data(&ibuf), s - 1))
+		    == NULL) {
+			log_warn("%s: get_string", __func__);
 			free(proto);
 			return (-1);
 		}
@@ -729,40 +758,48 @@ config_getproto(struct relayd *env, struct imsg *imsg)
 int
 config_getrule(struct relayd *env, struct imsg *imsg)
 {
+	struct ibuf		 ibuf;
 	struct protocol		*proto;
 	struct relay_rule	*rule;
-	size_t			 s, i;
-	u_int8_t		*p = imsg->data;
+	size_t			 i;
 	ssize_t			 len;
 
+	if (imsg_get_ibuf(imsg, &ibuf) == -1) {
+		log_warn("%s: imsg_get_ibuf", __func__);
+		return (-1);
+	}
+
 	if ((rule = calloc(1, sizeof(*rule))) == NULL)
 		return (-1);
 
-	IMSG_SIZE_CHECK(imsg, rule);
-	memcpy(rule, p, sizeof(*rule));
-	s = sizeof(*rule);
-	len = IMSG_DATA_SIZE(imsg) - s;
+	if (ibuf_get(&ibuf, rule, sizeof(*rule)) == -1) {
+		log_warn("%s: ibuf_get", __func__);
+		free(rule);
+		return (-1);
+	}
 
 	if ((proto = proto_find(env, rule->rule_protoid)) == NULL) {
 		free(rule);
 		return (-1);
 	}
 
+
 #define GETKV(_n, _f)	{						\
-	if (rule->rule_ctl.kvlen[_n]._f >= 0) {				\
+	len = rule->rule_ctl.kvlen[_n]._f;				\
+	if (len >= 0) {							\
 		/* Also accept "empty" 0-length strings */		\
-		if ((len < rule->rule_ctl.kvlen[_n]._f) ||		\
+		if (ibuf_size(&ibuf) < (size_t)len ||			\
 		    (rule->rule_kv[_n].kv_##_f =			\
-		    get_string(p + s,					\
-		    rule->rule_ctl.kvlen[_n]._f)) == NULL) {		\
+		    get_string(ibuf_data(&ibuf), len)) == NULL) {	\
+			free(rule);					\
+			return (-1);					\
+		}							\
+		if (len > 0 && ibuf_skip(&ibuf, len) == -1) {		\
 			free(rule);					\
 			return (-1);					\
 		}							\
-		s += rule->rule_ctl.kvlen[_n]._f;			\
-		len -= rule->rule_ctl.kvlen[_n]._f;			\
-									\
 		DPRINTF("%s: %s %s (len %ld, option %d): %s", __func__,	\
-		    #_n, #_f, rule->rule_ctl.kvlen[_n]._f,		\
+		    #_n, #_f, len,					\
 		    rule->rule_kv[_n].kv_option,			\
 		    rule->rule_kv[_n].kv_##_f);				\
 	}								\
@@ -1027,17 +1064,24 @@ config_setrelay(struct relayd *env, struct relay *rlay)
 int
 config_getrelay(struct relayd *env, struct imsg *imsg)
 {
+	struct ibuf		 ibuf;
 	struct privsep		*ps = env->sc_ps;
 	struct relay		*rlay;
-	u_int8_t		*p = imsg->data;
-	size_t			 s;
+	size_t			s;
+
+	if (imsg_get_ibuf(imsg, &ibuf) == -1) {
+		log_warn("%s: imsg_get_ibuf", __func__);
+		return (-1);
+	}
 
 	if ((rlay = calloc(1, sizeof(*rlay))) == NULL)
 		return (-1);
 
-	IMSG_SIZE_CHECK(imsg, &rlay->rl_conf);
-	memcpy(&rlay->rl_conf, p, sizeof(rlay->rl_conf));
-	s = sizeof(rlay->rl_conf);
+	if (ibuf_get(&ibuf, &rlay->rl_conf, sizeof(rlay->rl_conf)) == -1) {
+		log_warn("%s: ibuf_get", __func__);
+		free(rlay);
+		return (-1);
+	}
 
 	rlay->rl_s = imsg_get_fd(imsg);
 	rlay->rl_tls_ca_fd = -1;
@@ -1054,17 +1098,15 @@ config_getrelay(struct relayd *env, struct imsg *imsg)
 		}
 	}
 
-	if ((off_t)(IMSG_DATA_SIZE(imsg) - s) <
-	    (rlay->rl_conf.tls_cakey_len)) {
+	s = ibuf_size(&ibuf);
+	if (s != (size_t)rlay->rl_conf.tls_cakey_len) {
 		log_debug("%s: invalid message length", __func__);
 		goto fail;
 	}
-
-	if (rlay->rl_conf.tls_cakey_len) {
-		if ((rlay->rl_tls_cakey = get_data(p + s,
-		    rlay->rl_conf.tls_cakey_len)) == NULL)
+	if (s > 0) {
+		if ((rlay->rl_tls_cakey = get_data(ibuf_data(&ibuf), s))
+		    == NULL)
 			goto fail;
-		s += rlay->rl_conf.tls_cakey_len;
 	}
 
 	TAILQ_INIT(&rlay->rl_tables);
@@ -1092,10 +1134,14 @@ config_getrelaytable(struct relayd *env, struct imsg *imsg)
 	struct ctl_relaytable	 crt;
 	struct relay		*rlay;
 	struct table		*table;
-	u_int8_t		*p = imsg->data;
 
-	IMSG_SIZE_CHECK(imsg, &crt);
-	memcpy(&crt, p, sizeof(crt));
+	if (imsg_get_data(imsg, &crt, sizeof(crt)) == -1) {
+		log_warn("%s: imsg_get_data", __func__);
+		return (-1);
+	}
+
+	if ((rlt = calloc(1, sizeof(*rlt))) == NULL)
+		goto fail;
 
 	if ((rlay = relay_find(env, crt.relayid)) == NULL) {
 		log_debug("%s: unknown relay", __func__);
@@ -1107,9 +1153,6 @@ config_getrelaytable(struct relayd *env, struct imsg *imsg)
 		goto fail;
 	}
 
-	if ((rlt = calloc(1, sizeof(*rlt))) == NULL)
-		goto fail;
-
 	rlt->rlt_table = table;
 	rlt->rlt_mode = crt.mode;
 	rlt->rlt_flags = crt.flags;
@@ -1133,10 +1176,11 @@ config_getrelayfd(struct relayd *env, struct imsg *imsg)
 	struct ctl_relayfd	 crfd;
 	struct relay		*rlay = NULL;
 	struct relay_cert	*cert;
-	u_int8_t		*p = imsg->data;
 
-	IMSG_SIZE_CHECK(imsg, &crfd);
-	memcpy(&crfd, p, sizeof(crfd));
+	if (imsg_get_data(imsg, &crfd, sizeof(crfd)) == -1) {
+		log_warn("%s: imsg_get_data", __func__);
+		return (-1);
+	}
 
 	switch (crfd.type) {
 	case RELAY_FD_CERT: