Index | Thread | Search

From:
Rafael Sadowski <rafael@sizeofvoid.org>
Subject:
[5/7] relayd: use imsg_get_ibuf() for variable-length CA key operations
To:
tech@openbsd.org
Date:
Sun, 7 Jun 2026 09:03:34 +0200

Download raw body.

Thread
  • Rafael Sadowski:

    [5/7] relayd: use imsg_get_ibuf() for variable-length CA key operations

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 81d0e5b17456f16bdadfeaed58e7ae5c4995beee
Author: Rafael Sadowski <rafael@sizeofvoid.org>
Date:   Sat Jun 6 21:00:09 2026 +0200

    relayd: use imsg_get_ibuf() for variable-length CA key operations
    
    The IMSG_CA_PRIVENC/PRIVDEC messages carry a ctl_keyop header followed
    by cko_flen (request) or cko_tlen (response) trailing bytes, so the
    exact-size imsg_get_data() cannot be used. Read the header with
    imsg_get_ibuf() + ibuf_get() and take the payload from the same ibuf
    via ibuf_data()/ibuf_size().

diff --git a/ca.c b/ca.c
index 9d4fc65..082f411 100644
--- a/ca.c
+++ b/ca.c
@@ -219,6 +219,7 @@ ca_dispatch_parent(int fd, struct privsep_proc *p, struct imsg *imsg)
 int
 ca_dispatch_relay(int fd, struct privsep_proc *p, struct imsg *imsg)
 {
+	struct ibuf		 ibuf;
 	struct ctl_keyop	 cko;
 	EVP_PKEY		*pkey;
 	RSA			*rsa;
@@ -229,11 +230,19 @@ ca_dispatch_relay(int fd, struct privsep_proc *p, struct imsg *imsg)
 	switch (imsg_get_type(imsg)) {
 	case IMSG_CA_PRIVENC:
 	case IMSG_CA_PRIVDEC:
-		IMSG_SIZE_CHECK(imsg, (&cko));
-		bcopy(imsg->data, &cko, sizeof(cko));
+		if (imsg_get_ibuf(imsg, &ibuf) == -1) {
+			log_warn("%s: imsg_get_ibuf", __func__);
+			return (-1);
+		}
+
+		if (ibuf_get(&ibuf, &cko, sizeof(cko)) == -1) {
+			log_warn("%s: ibuf_get", __func__);
+			return (-1);
+		}
+
 		if (cko.cko_proc > env->sc_conf.prefork_relay)
 			fatalx("%s: invalid relay proc", __func__);
-		if (IMSG_DATA_SIZE(imsg) != (sizeof(cko) + cko.cko_flen))
+		if (ibuf_size(&ibuf) != (size_t)cko.cko_flen)
 			fatalx("%s: invalid key operation", __func__);
 
 		if ((pkey = pkey_find(env, cko.cko_hash)) == NULL) {
@@ -256,7 +265,8 @@ ca_dispatch_relay(int fd, struct privsep_proc *p, struct imsg *imsg)
 		DPRINTF("%s:%d: key hash %s proc %d",
 		    __func__, __LINE__, cko.cko_hash, cko.cko_proc);
 
-		from = (u_char *)imsg->data + sizeof(cko);
+		from = ibuf_data(&ibuf);
+
 		if ((to = calloc(1, cko.cko_tlen)) == NULL)
 			fatalx("%s: calloc", __func__);
 
@@ -309,13 +319,14 @@ static int
 rsae_send_imsg(int flen, const u_char *from, u_char *to, RSA *rsa,
     int padding, u_int cmd)
 {
+	struct ibuf	 ibuf;
 	struct privsep	*ps = env->sc_ps;
 	struct pollfd	 pfd[1];
 	struct ctl_keyop cko;
 	int		 ret = 0;
 	char		*hash;
 	struct iovec	 iov[2];
-	struct imsgbuf	*ibuf;
+	struct imsgbuf	*imsgbuf;
 	struct imsgev	*iev;
 	struct imsg	 imsg;
 	int		 n, done = 0, cnt = 0;
@@ -326,7 +337,7 @@ rsae_send_imsg(int flen, const u_char *from, u_char *to, RSA *rsa,
 		return 0;
 
 	iev = proc_iev(ps, PROC_CA, ps->ps_instance);
-	ibuf = &iev->ibuf;
+	imsgbuf = &iev->ibuf;
 
 	/*
 	 * XXX this could be nicer...
@@ -348,16 +359,16 @@ rsae_send_imsg(int flen, const u_char *from, u_char *to, RSA *rsa,
 	 * Send a synchronous imsg because we cannot defer the RSA
 	 * operation in OpenSSL.
 	 */
-	if (imsg_composev(ibuf, cmd, 0, 0, -1, iov, cnt) == -1) {
+	if (imsg_composev(imsgbuf, cmd, 0, 0, -1, iov, cnt) == -1) {
 		log_warn("%s: imsg_composev", __func__);
 		return -1;
 	}
-	if (imsgbuf_flush(ibuf) == -1) {
+	if (imsgbuf_flush(imsgbuf) == -1) {
 		log_warn("%s: imsgbuf_flush", __func__);
 		return -1;
 	}
 
-	pfd[0].fd = ibuf->fd;
+	pfd[0].fd = imsgbuf->fd;
 	pfd[0].events = POLLIN;
 	while (!done) {
 		switch (poll(pfd, 1, RELAY_TLS_PRIV_TIMEOUT)) {
@@ -374,19 +385,28 @@ rsae_send_imsg(int flen, const u_char *from, u_char *to, RSA *rsa,
 		default:
 			break;
 		}
-		if ((n = imsgbuf_read(ibuf)) == -1)
+		if ((n = imsgbuf_read(imsgbuf)) == -1)
 			fatalx("imsgbuf_read");
 		if (n == 0)
 			fatalx("pipe closed");
 
 		while (!done) {
-			if ((n = imsg_get(ibuf, &imsg)) == -1)
+			if ((n = imsgbuf_get(imsgbuf, &imsg)) == -1)
 				fatalx("imsg_get error");
 			if (n == 0)
 				break;
 
-			IMSG_SIZE_CHECK(&imsg, (&cko));
-			memcpy(&cko, imsg.data, sizeof(cko));
+			if (imsg_get_ibuf(&imsg, &ibuf) == -1) {
+				log_warn("%s: imsg_get_ibuf", __func__);
+				imsg_free(&imsg);
+				return (-1);
+			}
+
+			if (ibuf_get(&ibuf, &cko, sizeof(cko)) == -1) {
+				log_warn("%s: ibuf_get", __func__);
+				imsg_free(&imsg);
+				return (-1);
+			}
 
 			/*
 			 * Due to earlier timed out requests, there may be
@@ -401,7 +421,7 @@ rsae_send_imsg(int flen, const u_char *from, u_char *to, RSA *rsa,
 				continue;
 			}
 
-			if (imsg.hdr.type != cmd)
+			if (imsg_get_type(&imsg) != cmd)
 				fatalx("invalid response");
 
 			ret = cko.cko_tlen;
@@ -410,10 +430,9 @@ rsae_send_imsg(int flen, const u_char *from, u_char *to, RSA *rsa,
 				    __func__, cmd == IMSG_CA_PRIVENC ?
 				    "enc" : "dec", cko.cko_hash);
 			} else if (ret > 0) {
-				if (IMSG_DATA_SIZE(&imsg) !=
-				    (sizeof(cko) + ret))
+				if (ibuf_size(&ibuf) != (size_t)ret)
 					fatalx("data size");
-				toptr = (u_char *)imsg.data + sizeof(cko);
+				toptr = ibuf_data(&ibuf);
 				memcpy(to, toptr, ret);
 			}
 			done = 1;