Index | Thread | Search

From:
Mischa <openbsd@mlst.nl>
Subject:
Re: Relayd doesn't like ecdsa
To:
Rafael Sadowski <rafael@sizeofvoid.org>
Cc:
Tech <tech@openbsd.org>
Date:
Mon, 15 Jun 2026 20:50:49 +0200

Download raw body.

Thread
  • Rafael Sadowski:

    Relayd doesn't like ecdsa

  • Hi Rafael,
    
    Patches applied on -current and running.
    Looking good.
    
    Mischa
    
    On 2026-06-15 17:04, Rafael Sadowski wrote:
    > On Mon Jun 15, 2026 at 04:09:11PM +0200, Rafael Sadowski wrote:
    >> On Wed Jun 03, 2026 at 12:45:27PM +0200, Mischa wrote:
    >> > Hi Rafael,
    >> >
    >> > Not sure if this is relevant in this thread but...
    >> > I got this message yesterday and the relayd process stopped.
    >> > This is with the initial patch which Omar provided, running on 7.9.
    >> >
    >> > Jun  2 21:18:49 obsdams relayd[69181]: ecdsae_send_enc_imsg: priv ecdsa poll
    >> > timeout, keyop #29f3
    >> > Jun  2 21:18:49 obsdams relayd[69181]: fatal in relay: proc_dispatch: relay
    >> > 1 got invalid imsg 61 peerid -1 from ca 1
    >> > Jun  2 21:18:49 obsdams relayd[33605]: lost child: pid 69181 exited
    >> > abnormally
    >> > Jun  2 21:18:50 obsdams relayd[33597]: ecdsae_send_enc_imsg: imsgbuf_flush:
    >> > Broken pipe
    >> >
    >> > Mischa
    >> >
    >> 
    >> I think you ran into the RELAY_TLS_PRIV_TIMEOUT (hard-coded 1sec) and
    >> Omas's diff do not handle IMSG_CA_ECDSA_SIGN in relay_dispatch_ca
    >> properly:
    >> 
    >> diff --git a/relay.c b/relay.c
    >> index 2f6ec5d..0c6dae0 100644
    >> --- a/relay.c
    >> +++ b/relay.c
    >> @@ -1980,6 +1980,7 @@ relay_dispatch_ca(int fd, struct privsep_proc 
    >> *p, struct imsg *imsg)
    >>  	switch (imsg_get_type(imsg)) {
    >>  	case IMSG_CA_PRIVENC:
    >>  	case IMSG_CA_PRIVDEC:
    >> +	case IMSG_CA_ECDSA_SIGN:
    >>  		log_warnx("%s: priv%s result after timeout", __func__,
    >>  		    imsg_get_type(imsg) == IMSG_CA_PRIVENC ? "enc" : "dec");
    >>  		return (0);
    >> 
    > 
    > Of course this was quick and dirty below is a better solution. Full 
    > diff
    > if you want to test it.
    > 
    > https://rsadowski.gothub.org/?action=summary&headref=feature/ecdsa&path=relayd.git
    > 
    > 
    > diff --git a/ca.c b/ca.c
    > index 6ce8626..0fb4877 100644
    > --- a/ca.c
    > +++ b/ca.c
    > @@ -223,9 +223,11 @@ ca_dispatch_relay(int fd, struct privsep_proc *p, 
    > struct imsg *imsg)
    >  	struct ctl_keyop	 cko;
    >  	EVP_PKEY		*pkey;
    >  	RSA			*rsa;
    > +	EC_KEY			*ecdsa;
    >  	u_char			*to = NULL;
    >  	struct iovec		 iov[2];
    > -	int			 c = 0;
    > +	int			 ret = 0, c = 0;
    > +	unsigned int		 len;
    > 
    >  	switch (imsg_get_type(imsg)) {
    >  	case IMSG_CA_PRIVENC:
    > @@ -299,6 +301,64 @@ ca_dispatch_relay(int fd, struct privsep_proc *p, 
    > struct imsg *imsg)
    >  		free(to);
    >  		RSA_free(rsa);
    >  		break;
    > +
    > +	case IMSG_CA_ECDSA_SIGN:
    > +		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 (ibuf_size(&ibuf) != (size_t)cko.cko_flen)
    > +			fatalx("%s: invalid key operation", __func__);
    > +
    > +		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_get_type(imsg), -1, -1, iov,
    > +			    c) == -1)
    > +				log_warn("%s: proc_composev_imsg", __func__);
    > +			break;
    > +		}
    > +
    > +		if ((ecdsa = EVP_PKEY_get1_EC_KEY(pkey)) == NULL)
    > +			fatalx("%s: invalid relay key", __func__);
    > +
    > +		len = ECDSA_size(ecdsa);
    > +
    > +		if ((to = calloc(1, len)) == NULL)
    > +			fatalx("%s: calloc", __func__);
    > +
    > +		ret = ECDSA_sign(0, ibuf_data(&ibuf), ibuf_size(&ibuf), to,
    > +		    &len, ecdsa);
    > +
    > +		cko.cko_tlen = (ret > 0) ? len : -1;
    > +		iov[c].iov_base = &cko;
    > +		iov[c++].iov_len = sizeof(cko);
    > +		if (ret > 0) {
    > +			iov[c].iov_base = to;
    > +			iov[c++].iov_len = len;
    > +		}
    > +
    > +		if (proc_composev_imsg(env->sc_ps, PROC_RELAY, cko.cko_proc,
    > +		    imsg_get_type(imsg), -1, -1, iov, c) == -1)
    > +			log_warn("%s: proc_composev_imsg", __func__);
    > +
    > +		free(to);
    > +		EC_KEY_free(ecdsa);
    > +		break;
    > +
    >  	default:
    >  		return -1;
    >  	}
    > @@ -389,7 +449,7 @@ rsae_send_imsg(int flen, const u_char *from, u_char 
    > *to, RSA *rsa,
    > 
    >  		while (!done) {
    >  			if ((n = imsgbuf_get(imsgbuf, &imsg)) == -1)
    > -				fatalx("imsg_get error");
    > +				fatalx("imsgbuf_get error");
    >  			if (n == 0)
    >  				break;
    > 
    > @@ -455,14 +515,11 @@ rsae_priv_dec(int flen, const u_char *from, 
    > u_char *to, RSA *rsa, int padding)
    >  	return rsae_send_imsg(flen, from, to, rsa, padding, IMSG_CA_PRIVDEC);
    >  }
    > 
    > -void
    > -ca_engine_init(struct relayd *x_env)
    > +static void
    > +rsa_engine_init(void)
    >  {
    >  	const char	*errstr;
    > 
    > -	if (env == NULL)
    > -		env = x_env;
    > -
    >  	if (rsa_default != NULL)
    >  		return;
    > 
    > @@ -494,3 +551,196 @@ ca_engine_init(struct relayd *x_env)
    >  	ssl_error(errstr);
    >  	fatalx("%s: %s", __func__, errstr);
    >  }
    > +
    > +/*
    > + * ECDSA privsep engine (called from unprivileged processes)
    > + */
    > +
    > +const EC_KEY_METHOD *ecdsa_default = NULL;
    > +
    > +static EC_KEY_METHOD *ecdsae_method = NULL;
    > +
    > +static ECDSA_SIG *
    > +ecdsae_send_enc_imsg(const unsigned char *dgst, int dgst_len,
    > +    const BIGNUM *inv, const BIGNUM *rp, EC_KEY *eckey)
    > +{
    > +	struct ibuf	 ibuf;
    > +	struct privsep	*ps = env->sc_ps;
    > +	struct pollfd	 pfd[1];
    > +	struct ctl_keyop cko;
    > +	struct iovec	 iov[2];
    > +	struct imsgbuf	*imsgbuf;
    > +	struct imsgev	*iev;
    > +	struct imsg	 imsg;
    > +	int		 n, done = 0, cnt = 0;
    > +	const u_char	*toptr;
    > +	static u_int	 seq = 0;
    > +
    > +	char		*hash;
    > +	ECDSA_SIG	*sig = NULL;
    > +
    > +	if ((hash = EC_KEY_get_ex_data(eckey, 0)) == NULL)
    > +		return (NULL);
    > +
    > +	iev = proc_iev(ps, PROC_CA, ps->ps_instance);
    > +	imsgbuf = &iev->ibuf;
    > +
    > +	/*
    > +	 * XXX this could be nicer...
    > +	 */
    > +
    > +	memset(&cko, 0, sizeof(cko));
    > +	(void)strlcpy(cko.cko_hash, hash, sizeof(cko.cko_hash));
    > +	cko.cko_proc = ps->ps_instance;
    > +	cko.cko_flen = dgst_len;
    > +	cko.cko_cookie = seq++;
    > +
    > +	iov[cnt].iov_base = &cko;
    > +	iov[cnt++].iov_len = sizeof(cko);
    > +	iov[cnt].iov_base = (void *)(uintptr_t)dgst;
    > +	iov[cnt++].iov_len = dgst_len;
    > +
    > +	/*
    > +	 * Send a synchronous imsg because we cannot defer the ECDSA
    > +	 * operation in OpenSSL's engine layer.
    > +	 */
    > +	if (imsg_composev(imsgbuf, IMSG_CA_ECDSA_SIGN, 0, 0, -1, iov, cnt) ==
    > +	    -1) {
    > +		log_warn("%s: imsg_composev", __func__);
    > +		return (NULL);
    > +	}
    > +	if (imsgbuf_flush(imsgbuf) == -1) {
    > +		log_warn("%s: imsgbuf_flush", __func__);
    > +		return (NULL);
    > +	}
    > +
    > +	pfd[0].fd = imsgbuf->fd;
    > +	pfd[0].events = POLLIN;
    > +
    > +	while (!done) {
    > +		switch (poll(pfd, 1, RELAY_TLS_PRIV_TIMEOUT)) {
    > +		case -1:
    > +			if (errno != EINTR)
    > +				fatal("%s: poll", __func__);
    > +			continue;
    > +		case 0:
    > +			log_warnx("%s: priv ecdsa poll timeout, keyop #%x",
    > +			    __func__,
    > +			    cko.cko_cookie);
    > +			return (NULL);
    > +		default:
    > +			break;
    > +		}
    > +
    > +		if ((n = imsgbuf_read(imsgbuf)) == -1)
    > +			fatalx("imsgbuf_read");
    > +		if (n == 0)
    > +			fatalx("pipe closed");
    > +
    > +		while (!done) {
    > +			if ((n = imsgbuf_get(imsgbuf, &imsg)) == -1)
    > +				fatalx("imsgbuf_get error");
    > +			if (n == 0)
    > +				break;
    > +
    > +			if (imsg_get_ibuf(&imsg, &ibuf) == -1) {
    > +				log_warn("%s: imsg_get_ibuf", __func__);
    > +				imsg_free(&imsg);
    > +				return (NULL);
    > +			}
    > +
    > +			if (ibuf_get(&ibuf, &cko, sizeof(cko)) == -1) {
    > +				log_warn("%s: ibuf_get", __func__);
    > +				imsg_free(&imsg);
    > +				return (NULL);
    > +			}
    > +
    > +			/*
    > +			 * Due to earlier timed out requests, there may be
    > +			 * responses that need to be skipped.
    > +			 */
    > +			if (cko.cko_cookie != seq - 1) {
    > +				log_warnx(
    > +				    "%s: priv ecdsa obsolete keyop #%x",
    > +				    __func__,
    > +				    cko.cko_cookie);
    > +				imsg_free(&imsg);
    > +				continue;
    > +			}
    > +
    > +			if (imsg_get_type(&imsg) != IMSG_CA_ECDSA_SIGN)
    > +				fatalx("invalid response");
    > +
    > +			if (cko.cko_tlen == -1) {
    > +				log_warnx("%s: priv ecdsa failed for key %s",
    > +				    __func__, cko.cko_hash);
    > +			} else if (cko.cko_tlen > 0) {
    > +				if (ibuf_size(&ibuf) != (size_t)cko.cko_tlen)
    > +					fatalx("data size");
    > +				toptr = ibuf_data(&ibuf);
    > +				d2i_ECDSA_SIG(&sig, &toptr, cko.cko_tlen);
    > +			}
    > +			done = 1;
    > +
    > +			imsg_free(&imsg);
    > +		}
    > +	}
    > +	imsg_event_add(iev);
    > +
    > +	return (sig);
    > +}
    > +
    > +static ECDSA_SIG *
    > +ecdsae_do_sign(const unsigned char *dgst, int dgst_len, const BIGNUM 
    > *inv,
    > +    const BIGNUM *rp, EC_KEY *eckey)
    > +{
    > +	ECDSA_SIG *(*psign_sig)(const unsigned char *, int, const BIGNUM *,
    > +	    const BIGNUM *, EC_KEY *);
    > +
    > +	DPRINTF("%s:%d", __func__, __LINE__);
    > +	if (EC_KEY_get_ex_data(eckey, 0) != NULL)
    > +		return (ecdsae_send_enc_imsg(dgst, dgst_len, inv, rp, eckey));
    > +	EC_KEY_METHOD_get_sign(ecdsa_default, NULL, NULL, &psign_sig);
    > +	return (psign_sig(dgst, dgst_len, inv, rp, eckey));
    > +}
    > +
    > +static void
    > +ecdsa_engine_init(void)
    > +{
    > +	int (*sign)(int, const unsigned char *, int, unsigned char *,
    > +	    unsigned int *, const BIGNUM *, const BIGNUM *, EC_KEY *);
    > +	int (*sign_setup)(EC_KEY *, BN_CTX *, BIGNUM **, BIGNUM **);
    > +	const char *errstr;
    > +
    > +	if ((ecdsa_default = EC_KEY_get_default_method()) == NULL) {
    > +		errstr = "EC_KEY_get_default_method";
    > +		goto fail;
    > +	}
    > +
    > +	if ((ecdsae_method = EC_KEY_METHOD_new(ecdsa_default)) == NULL) {
    > +		errstr = "EC_KEY_METHOD_new";
    > +		goto fail;
    > +	}
    > +
    > +	EC_KEY_METHOD_get_sign(ecdsa_default, &sign, &sign_setup, NULL);
    > +	EC_KEY_METHOD_set_sign(ecdsae_method, sign, sign_setup,
    > +	    ecdsae_do_sign);
    > +
    > +	EC_KEY_set_default_method(ecdsae_method);
    > +
    > +	return;
    > +
    > + fail:
    > +	ssl_error(errstr);
    > +	fatalx("%s", errstr);
    > +}
    > +
    > +void
    > +ca_engine_init(struct relayd *x_env)
    > +{
    > +	if (env == NULL)
    > +		env = x_env;
    > +
    > +	rsa_engine_init();
    > +	ecdsa_engine_init();
    > +}
    > diff --git a/relay.c b/relay.c
    > index 2f6ec5d..8db6c32 100644
    > --- a/relay.c
    > +++ b/relay.c
    > @@ -1978,6 +1978,9 @@ int
    >  relay_dispatch_ca(int fd, struct privsep_proc *p, struct imsg *imsg)
    >  {
    >  	switch (imsg_get_type(imsg)) {
    > +	case IMSG_CA_ECDSA_SIGN:
    > +		log_warnx("%s: priv result after timeout", __func__);
    > +		return (0);
    >  	case IMSG_CA_PRIVENC:
    >  	case IMSG_CA_PRIVDEC:
    >  		log_warnx("%s: priv%s result after timeout", __func__,
    > diff --git a/relayd.h b/relayd.h
    > index c772300..dd666b6 100644
    > --- a/relayd.h
    > +++ b/relayd.h
    > @@ -1001,6 +1001,7 @@ enum imsg_type {
    >  	IMSG_CFG_DONE,
    >  	IMSG_CA_PRIVENC,
    >  	IMSG_CA_PRIVDEC,
    > +	IMSG_CA_ECDSA_SIGN,
    >  	IMSG_SESS_PUBLISH,	/* from relay to pfe */
    >  	IMSG_SESS_UNPUBLISH,
    >  	IMSG_TLSTICKET_REKEY
    > diff --git a/ssl.c b/ssl.c
    > index 77f4dd1..c635648 100644
    > --- a/ssl.c
    > +++ b/ssl.c
    > @@ -189,6 +189,7 @@ ssl_load_pkey(char *buf, off_t len, X509 **x509ptr, 
    > EVP_PKEY **pkeyptr)
    >  	X509		*x509 = NULL;
    >  	EVP_PKEY	*pkey = NULL;
    >  	RSA		*rsa = NULL;
    > +	EC_KEY		*eckey = NULL;
    >  	char		*hash = NULL;
    > 
    >  	if ((in = BIO_new_mem_buf(buf, len)) == NULL) {
    > @@ -204,21 +205,47 @@ ssl_load_pkey(char *buf, off_t len, X509 
    > **x509ptr, EVP_PKEY **pkeyptr)
    >  		log_warnx("%s: X509_get_pubkey failed", __func__);
    >  		goto fail;
    >  	}
    > -	if ((rsa = EVP_PKEY_get1_RSA(pkey)) == NULL) {
    > -		log_warnx("%s: failed to extract RSA", __func__);
    > -		goto fail;
    > -	}
    >  	if ((hash = malloc(TLS_CERT_HASH_SIZE)) == NULL) {
    >  		log_warn("%s: allocate hash failed", __func__);
    >  		goto fail;
    >  	}
    >  	hash_x509(x509, hash, TLS_CERT_HASH_SIZE);
    > -	if (RSA_set_ex_data(rsa, 0, hash) != 1) {
    > -		log_warnx("%s: failed to set hash as exdata", __func__);
    > +
    > +	switch (EVP_PKEY_id(pkey)) {
    > +	case EVP_PKEY_RSA:
    > +		if ((rsa = EVP_PKEY_get1_RSA(pkey)) == NULL) {
    > +			log_warnx("%s: failed to extract RSA", __func__);
    > +			goto fail;
    > +		}
    > +		if (RSA_set_ex_data(rsa, 0, hash) != 1) {
    > +			log_warnx("%s: failed to set hash as exdata", __func__);
    > +			goto fail;
    > +		}
    > +		break;
    > +	case EVP_PKEY_EC:
    > +		if ((eckey = EVP_PKEY_get1_EC_KEY(pkey)) == NULL) {
    > +			log_warnx("%s: failed to set extract EC key", __func__);
    > +			goto fail;
    > +		}
    > +		if (EC_KEY_set_ex_data(eckey, 0, hash) == 0) {
    > +			log_warnx("%s: failed to set hash as exdata", __func__);
    > +			goto fail;
    > +		}
    > +
    > +		/* Reset the key to work around caching in OpenSSL 3. */
    > +		if (EVP_PKEY_set1_EC_KEY(pkey, eckey) == 0) {
    > +			log_warnx("%s: failed to set EC key", __func__);
    > +			goto fail;
    > +		}
    > +		break;
    > +	default:
    > +		log_warnx("%s: incorrect key type", __func__);
    >  		goto fail;
    >  	}
    > 
    > -	RSA_free(rsa); /* dereference, will be cleaned up with pkey */
    > +	/* dereference, will be cleaned up with pkey */
    > +	RSA_free(rsa);
    > +	EC_KEY_free(eckey);
    >  	*pkeyptr = pkey;
    >  	if (x509ptr != NULL)
    >  		*x509ptr = x509;
    
    
  • Rafael Sadowski:

    Relayd doesn't like ecdsa