Re: Relayd doesn't like ecdsa
Mischa <[email protected]>
| Newsgroups | gmane.os.openbsd.tech |
|---|---|
| Message-ID | <[email protected]> |
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;