Index | Thread | Search

From:
Mischa <bsdnl@mlst.nl>
Subject:
Re: Relayd doesn't like ecdsa
To:
Tech <tech@openbsd.org>
Date:
Wed, 27 May 2026 08:06:50 +0200

Download raw body.

Thread
As a side note, confirmed to be working on 7.9.
Running multiple sites with relayd in production.

Mischa

On 2026-05-27 07:04, Rafael Sadowski wrote:
> On Sat Apr 25, 2026 at 07:10:42PM +0200, Omar Polo wrote:
>> Hello,
>> 
>> Mischa <bsdnl@mlst.nl> wrote:
>> > On 2026-04-23 14:25, Theo Buehler wrote:
>> > > On Thu, Apr 23, 2026 at 02:07:45PM +0200, Mischa wrote:
>> > >> Hi All,
>> > >>
>> > >> When using edcsa within acme-client.conf, relayd is unable to use the
>> > >> key/cert, it seems to be looking for an RSA key/cert specifically. Is
>> > >> there
>> > >> a way to go around this?
>> > >
>> > > No. The privsep stuff has only RSA wired up. Someone motivated could
>> > > probably crib from smtpd's ca.c.
>> >
>> > I wish I had the skilzzz. :/
>> > Willing to incentivize where possible. :)
>> 
>> some time ago while working on smtpd's ca.c I wrote an implementation
>> for relayd, mostly to validate my understanding.  I was too scared to
>> share it, I don't use relayd normally, and I try to stay a little bit
>> away from it in general.  (sorry, I found it confusing!)
>> 
>> Anyway, I tried to resurrect the diff.  It works for me with a stupid
>> small config and an ec key generated with:
> 
> That's really cool; I borrowed a similar approach from smptd, but it
> was still a work in progress.
> 
> A few comments below. I would add this to the tests.
> 
>> 
>> 	key=...
>> 	pem=...
>> 	openssl ecparam -name secp384r1 -genkey -noout -out "${key}"
>> 	openssl req -new -x509 -key "${key}" -out "${pem}" -days 365 \
>> 		-nodes -subj "/CN=localhost"
>> 
>> can you give it a spin?  there are chances it might work =)
>> 
>> I don't like how we reuse the cko struct in ca_dispatch_relay(), but
>> that's what was already done in the RSA case.
>> 
>> diff /usr/src
>> path + /usr/src
>> commit - e268a8ba09fa63295bce8b5f024a710203085e2a
>> blob - c4f527fa96f5b64a702d682f9d59ef92cf46d9d4
>> file + usr.sbin/relayd/ca.c
>> --- usr.sbin/relayd/ca.c
>> +++ usr.sbin/relayd/ca.c
>> @@ -222,9 +222,11 @@ ca_dispatch_relay(int fd, struct privsep_proc *p, 
>> stru
>>  	struct ctl_keyop	 cko;
>>  	EVP_PKEY		*pkey;
>>  	RSA			*rsa;
>> +	EC_KEY			*ecdsa;
>>  	u_char			*from = NULL, *to = NULL;
>>  	struct iovec		 iov[2];
>> -	int			 c = 0;
>> +	int			 ret, c = 0;
>> +	unsigned int		 len;
>> 
>>  	switch (imsg->hdr.type) {
>>  	case IMSG_CA_PRIVENC:
>> @@ -291,6 +293,55 @@ ca_dispatch_relay(int fd, struct privsep_proc *p, 
>> stru
>>  		free(to);
>>  		RSA_free(rsa);
>>  		break;
>> +
>> +	case IMSG_CA_ECDSA_SIGN:
>> +		IMSG_SIZE_CHECK(imsg, (&cko));
>> +		bcopy(imsg->data, &cko, sizeof(cko));
>> +		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))
>> +			fatalx("%s: invalid key operation", __func__);
>> +		from = (u_char *)imsg->data + sizeof(cko);
>> +
>> +		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 ((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("ca_imsg: calloc");
>> +		ret = ECDSA_sign(0, from, cko.cko_flen,
>> +		    to, &len, ecdsa);
>> +
>> +		iov[c].iov_base = &cko;
>> +		iov[c++].iov_len = sizeof(cko);
> 
> I think it makes sense to set "cko.cko_tlen = (ret > 0) ? len : -1;" 
> here ...
> 
>> +		if (ret > 0) {
>> +			cko.cko_tlen = len;
> 
> ... and not here. cko_tlen is now *explicitly* set to -1 in case of an 
> error
> 
>> +			iov[c].iov_base = to;
>> +			iov[c++].iov_len = len;
>> +		}
>> +
>> +		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__);
>> +
>> +		free(to);
>> +		EC_KEY_free(ecdsa);
>> +		break;
>> +
>>  	default:
>>  		return -1;
>>  	}
>> @@ -440,14 +491,11 @@ rsae_priv_dec(int flen, const u_char *from, 
>> u_char *to
>>  	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;
>> 
>> @@ -477,3 +525,186 @@ ca_engine_init(struct relayd *x_env)
>>  	RSA_meth_free(rsae_method);
>>  	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 privsep	*ps = env->sc_ps;
>> +	struct pollfd	 pfd[1];
>> +	struct ctl_keyop cko;
>> +	struct iovec	 iov[2];
>> +	struct imsgbuf	*ibuf;
>> +	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 (0);
> 
> return NULL?
> 
>> +
>> +	iev = proc_iev(ps, PROC_CA, ps->ps_instance);
>> +	ibuf = &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(ibuf, IMSG_CA_ECDSA_SIGN, 0, 0, -1, iov, cnt) == 
>> -1) {
>> +		log_warn("%s: imsg_composev", __func__);
>> +		return NULL;
>> +	}
>> +	if (imsgbuf_flush(ibuf) == -1) {
>> +		log_warn("%s: imsgbuf_flush", __func__);
>> +		return NULL;
>> +	}
>> +
>> +	pfd[0].fd = ibuf->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(ibuf)) == -1)
>> +			fatalx("imsgbuf_read");
>> +		if (n == 0)
>> +			fatalx("pipe closed");
>> +
>> +		while (!done) {
>> +			if ((n = imsg_get(ibuf, &imsg)) == -1)
>> +				fatalx("imsg_get error");
>> +			if (n == 0)
>> +				break;
>> +
>> +			IMSG_SIZE_CHECK(&imsg, (&cko));
>> +			memcpy(&cko, imsg.data, sizeof(cko));
>> +
>> +			/*
>> +			 * 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.hdr.type != 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 (IMSG_DATA_SIZE(&imsg) !=
>> +				    (sizeof(cko) + cko.cko_tlen))
>> +					fatalx("data size");
>> +				toptr = (u_char *)imsg.data + sizeof(cko);
>> +				d2i_ECDSA_SIG(&sig, (const unsigned char **)&toptr,
> 
> too long
> 
>> +				    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();
>> +}
>> commit - e268a8ba09fa63295bce8b5f024a710203085e2a
>> blob - a5363989f4b4cbcca6c899b12de4ccd3cdf4dfcb
>> file + usr.sbin/relayd/relayd.h
>> --- usr.sbin/relayd/relayd.h
>> +++ usr.sbin/relayd/relayd.h
>> @@ -1005,6 +1005,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
>> @@ -1289,6 +1290,7 @@ void	 script_done(struct relayd *, struct 
>> ctl_script *
>>  int	 script_exec(struct relayd *, struct ctl_script *);
>> 
>>  /* ssl.c */
>> +void	 ssl_error(const char *);
>>  char	*ssl_load_key(struct relayd *, const char *, off_t *, char *);
>>  uint8_t *ssl_update_certificate(const uint8_t *, size_t, EVP_PKEY *,
>>  	    EVP_PKEY *, X509 *, size_t *);
>> commit - e268a8ba09fa63295bce8b5f024a710203085e2a
>> blob - 19950b89e56aec83e15bc9635293bd078f331e45
>> file + usr.sbin/relayd/ssl.c
>> --- usr.sbin/relayd/ssl.c
>> +++ usr.sbin/relayd/ssl.c
>> @@ -46,6 +46,18 @@ ssl_password_cb(char *buf, int size, int rwflag, 
>> void
>>  	return (len);
>>  }
>> 
>> +void
>> +ssl_error(const char *where)
>> +{
>> +	unsigned long	code;
>> +	char		errbuf[128];
>> +
>> +	for (; (code = ERR_get_error()) != 0 ;) {
>> +		ERR_error_string_n(code, errbuf, sizeof(errbuf));
>> +		log_debug("debug: SSL library error: %s: %s", where, errbuf);
>> +	}
>> +}
>> +
>>  char *
>>  ssl_load_key(struct relayd *env, const char *name, off_t *len, char 
>> *pass)
>>  {
>> @@ -181,6 +193,7 @@ ssl_load_pkey(char *buf, off_t len, X509 
>> **x509ptr, EV
>>  	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) {
>> @@ -196,21 +209,47 @@ ssl_load_pkey(char *buf, off_t len, X509 
>> **x509ptr, EV
>>  		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 */
>> +	RSA_free(rsa);
>> +	EC_KEY_free(eckey);
>>  	*pkeyptr = pkey;
>>  	if (x509ptr != NULL)
>>  		*x509ptr = x509;
>>