Index | Thread | Search

From:
Rafael Sadowski <rafael@sizeofvoid.org>
Subject:
Re: relayd reload race crash
To:
Theo Buehler <tb@theobuehler.org>
Cc:
Nick Owens <mischief@offblast.org>, tech@openbsd.org, Claudio Jeker <cjeker@diehard.n-r-g.com>
Date:
Tue, 3 Mar 2026 15:28:46 +0100

Download raw body.

Thread
On Tue Mar 03, 2026 at 03:03:58PM +0100, Theo Buehler wrote:
> > 
> > diff --git a/ca.c b/ca.c
> > index 167fb1c..3515977 100644
> > --- a/ca.c
> > +++ b/ca.c
> > @@ -234,11 +234,33 @@ ca_dispatch_relay(int fd, struct privsep_proc *p, struct imsg *imsg)
> >  			fatalx("%s: invalid relay proc", __func__);
> >  		if (IMSG_DATA_SIZE(imsg) != (sizeof(cko) + cko.cko_flen))
> >  			fatalx("%s: invalid key operation", __func__);
> > -		if ((pkey = pkey_find(env, cko.cko_hash)) == NULL)
> > -			fatalx("%s: invalid relay hash '%s'",
> > +
> > +		if ((pkey = pkey_find(env, cko.cko_hash)) == NULL) {
> > +			log_warnx("%s: invalid relay hash '%s'",
> >  			    __func__, cko.cko_hash);
> > -		if ((rsa = EVP_PKEY_get1_RSA(pkey)) == NULL)
> > -			fatalx("%s: invalid relay key", __func__);
> > +			/* Signal failure to the waiting relay worker. */
> > +			cko.cko_tlen = -1;
> > +			iov[c].iov_base = &cko;
> > +			iov[c++].iov_len = sizeof(cko);
> > +			if (proc_composev_imsg(env->sc_ps, PROC_RELAY,
> > +			    cko.cko_proc, imsg->hdr.type, -1, -1, iov,
> > +			     c) == -1)
> > +				log_warn("%s: proc_composev_imsg", __func__);
> > +			break;
> > +		}
> > +
> > +		if ((rsa = EVP_PKEY_get1_RSA(pkey)) == NULL) {
> 
> How does a partially initialized (or !RSA pkey end up in the sc_pkeys?

Thanks Theo,

I know what you mean, it should never happen, and if EVP_PKEY_get1_RSA
fails, the message is sent.

As I see it, we only support RSA.

diff --git a/ca.c b/ca.c
index 167fb1c..33e3003 100644
--- a/ca.c
+++ b/ca.c
@@ -234,9 +234,21 @@ ca_dispatch_relay(int fd, struct privsep_proc *p, struct imsg *imsg)
 			fatalx("%s: invalid relay proc", __func__);
 		if (IMSG_DATA_SIZE(imsg) != (sizeof(cko) + cko.cko_flen))
 			fatalx("%s: invalid key operation", __func__);
-		if ((pkey = pkey_find(env, cko.cko_hash)) == NULL)
-			fatalx("%s: invalid relay hash '%s'",
+
+		if ((pkey = pkey_find(env, cko.cko_hash)) == NULL) {
+			log_warnx("%s: invalid relay hash '%s'",
 			    __func__, cko.cko_hash);
+			/* Signal failure to the waiting relay worker. */
+			cko.cko_tlen = -1;
+			iov[c].iov_base = &cko;
+			iov[c++].iov_len = sizeof(cko);
+			if (proc_composev_imsg(env->sc_ps, PROC_RELAY,
+			    cko.cko_proc, imsg->hdr.type, -1, -1, iov,
+			     c) == -1)
+				log_warn("%s: proc_composev_imsg", __func__);
+			break;
+		}
+
 		if ((rsa = EVP_PKEY_get1_RSA(pkey)) == NULL)
 			fatalx("%s: invalid relay key", __func__);
 
@@ -392,7 +404,11 @@ rsae_send_imsg(int flen, const u_char *from, u_char *to, RSA *rsa,
 				fatalx("invalid response");
 
 			ret = cko.cko_tlen;
-			if (ret > 0) {
+			if (ret == -1) {
+				log_warnx("%s: priv%s failed for key %s",
+				    __func__, cmd == IMSG_CA_PRIVENC ?
+				    "enc" : "dec", cko.cko_hash);
+			} else if (ret > 0) {
 				if (IMSG_DATA_SIZE(&imsg) !=
 				    (sizeof(cko) + ret))
 					fatalx("data size");