From: Brent Cook Subject: Re: [PATCH] Use tls_set_errorx when errno isn't set on failure To: Theo Buehler Cc: Michael Forney , tech@openbsd.org, jsing@openbsd.org, bcook@openbsd.org Date: Wed, 15 Apr 2026 20:39:31 -0500 On Wed, Apr 15, 2026 at 5:05 PM Theo Buehler wrote: > > On Wed, Apr 15, 2026 at 11:55:21AM -0700, Michael Forney wrote: > > In these cases, the current errno value may not be relevant. > > > > A few calls with TLS_ERROR_OUT_OF_MEMORY were normalized to setx/errorx > > since that was the most common throughout the codebase. > > > > One malloc error path was changed to use TLS_ERROR_OUT_OF_MEMORY > > to match the others. > > --- > > I noticed a couple of these, so went looking for others. Some are > > pretty clear they should use the x variant (e.g. tls_ocsp_asn1_parse_time > > errors), but I wasn't sure about something like BIO_new_mem_buf, > > which probably does set errno = ENOMEM on failure. I changed those > > to tls_set_errorx to match the other BIO_new_mem_buf error paths. > > Thanks. > > Changing to the x variant after BIO_new_mem_buf() looks like the right > thing to do to me. > > I think there are three places (one in tls_keypair_load_cert() and > two in tls_signer_add_keypair_mem()) where the size passed to > BIO_new_mem_buf() could in principle be larger than INT_MAX. This can > overflow to be negative or be truncated to remain positive. In the first > case we'd have no relevant errno set. > > We should fix that at least by adding errors for length > INT_MAX > before BIO_new_mem_buf(), and perhaps we should also consider > not loading files of such preposterous size in the first place. > > > Please disregard any of these I may have gotten wrong if they are > > correct as-is. > > I agree with all these changes and while they are cosmetic, I think > they would be nice to have in the upcoming release -- especially if > people want to update libretls to support OpenSSL 4 without carrying > unnecessary patches on top of libressl 4.3's libtls. > > I will need an ok or someone can commit with mine (probably want > an additional ok from deraadt since the release lock is imminent). > > Diff re-inlined for convenience. This looks correct to me. ok bcook@ > > src/lib/libtls/tls.c | 2 +- > src/lib/libtls/tls_client.c | 2 +- > src/lib/libtls/tls_config.c | 2 +- > src/lib/libtls/tls_keypair.c | 6 +++--- > src/lib/libtls/tls_ocsp.c | 10 +++++----- > src/lib/libtls/tls_server.c | 8 ++++---- > src/lib/libtls/tls_signer.c | 6 +++--- > 7 files changed, 18 insertions(+), 18 deletions(-) > > diff --git a/src/lib/libtls/tls.c b/src/lib/libtls/tls.c > index 41bb06d85..fd0e9929e 100644 > --- a/src/lib/libtls/tls.c > +++ b/src/lib/libtls/tls.c > @@ -686,7 +686,7 @@ tls_configure_ssl_verify(struct tls *ctx, SSL_CTX *ssl_ctx, int verify) > if (xi->crl == NULL) > continue; > if (!X509_STORE_add_crl(store, xi->crl)) { > - tls_set_error(ctx, TLS_ERROR_UNKNOWN, > + tls_set_errorx(ctx, TLS_ERROR_UNKNOWN, > "failed to add crl"); > goto err; > } > diff --git a/src/lib/libtls/tls_client.c b/src/lib/libtls/tls_client.c > index 97e1d4021..5763eab6f 100644 > --- a/src/lib/libtls/tls_client.c > +++ b/src/lib/libtls/tls_client.c > @@ -115,7 +115,7 @@ tls_connect_servername(struct tls *ctx, const char *host, const char *port, > hints.ai_family = AF_UNSPEC; > hints.ai_flags = AI_ADDRCONFIG; > if ((s = getaddrinfo(h, p, &hints, &res0)) != 0) { > - tls_set_error(ctx, TLS_ERROR_UNKNOWN, > + tls_set_errorx(ctx, TLS_ERROR_UNKNOWN, > "%s", gai_strerror(s)); > goto err; > } > diff --git a/src/lib/libtls/tls_config.c b/src/lib/libtls/tls_config.c > index 848117a91..16d896685 100644 > --- a/src/lib/libtls/tls_config.c > +++ b/src/lib/libtls/tls_config.c > @@ -65,7 +65,7 @@ tls_config_load_file(struct tls_error *error, const char *filetype, > goto err; > *len = (size_t)st.st_size; > if ((*buf = malloc(*len)) == NULL) { > - tls_error_set(error, TLS_ERROR_UNKNOWN, > + tls_error_setx(error, TLS_ERROR_OUT_OF_MEMORY, > "failed to allocate buffer for %s file", > filetype); > goto err; > diff --git a/src/lib/libtls/tls_keypair.c b/src/lib/libtls/tls_keypair.c > index ffda91df8..a6f555063 100644 > --- a/src/lib/libtls/tls_keypair.c > +++ b/src/lib/libtls/tls_keypair.c > @@ -144,13 +144,13 @@ tls_keypair_load_cert(struct tls_keypair *keypair, struct tls_error *error, > *cert = NULL; > > if (keypair->cert_mem == NULL) { > - tls_error_set(error, TLS_ERROR_UNKNOWN, > + tls_error_setx(error, TLS_ERROR_UNKNOWN, > "keypair has no certificate"); > goto err; > } > if ((cert_bio = BIO_new_mem_buf(keypair->cert_mem, > keypair->cert_len)) == NULL) { > - tls_error_set(error, TLS_ERROR_UNKNOWN, > + tls_error_setx(error, TLS_ERROR_UNKNOWN, > "failed to create certificate bio"); > goto err; > } > @@ -158,7 +158,7 @@ tls_keypair_load_cert(struct tls_keypair *keypair, struct tls_error *error, > NULL)) == NULL) { > if ((ssl_err = ERR_peek_error()) != 0) > errstr = ERR_error_string(ssl_err, NULL); > - tls_error_set(error, TLS_ERROR_UNKNOWN, > + tls_error_setx(error, TLS_ERROR_UNKNOWN, > "failed to load certificate: %s", errstr); > goto err; > } > diff --git a/src/lib/libtls/tls_ocsp.c b/src/lib/libtls/tls_ocsp.c > index c65911920..1cc167eaa 100644 > --- a/src/lib/libtls/tls_ocsp.c > +++ b/src/lib/libtls/tls_ocsp.c > @@ -85,7 +85,7 @@ tls_ocsp_fill_info(struct tls *ctx, int response_status, int cert_status, > ctx->ocsp->ocsp_result = NULL; > > if ((info = calloc(1, sizeof (struct tls_ocsp_result))) == NULL) { > - tls_set_error(ctx, TLS_ERROR_OUT_OF_MEMORY, "out of memory"); > + tls_set_errorx(ctx, TLS_ERROR_OUT_OF_MEMORY, "out of memory"); > return -1; > } > info->response_status = response_status; > @@ -102,19 +102,19 @@ tls_ocsp_fill_info(struct tls *ctx, int response_status, int cert_status, > info->revocation_time = info->this_update = info->next_update = -1; > if (revtime != NULL && > tls_ocsp_asn1_parse_time(ctx, revtime, &info->revocation_time) != 0) { > - tls_set_error(ctx, TLS_ERROR_UNKNOWN, > + tls_set_errorx(ctx, TLS_ERROR_UNKNOWN, > "unable to parse revocation time in OCSP reply"); > goto err; > } > if (thisupd != NULL && > tls_ocsp_asn1_parse_time(ctx, thisupd, &info->this_update) != 0) { > - tls_set_error(ctx, TLS_ERROR_UNKNOWN, > + tls_set_errorx(ctx, TLS_ERROR_UNKNOWN, > "unable to parse this update time in OCSP reply"); > goto err; > } > if (nextupd != NULL && > tls_ocsp_asn1_parse_time(ctx, nextupd, &info->next_update) != 0) { > - tls_set_error(ctx, TLS_ERROR_UNKNOWN, > + tls_set_errorx(ctx, TLS_ERROR_UNKNOWN, > "unable to parse next update time in OCSP reply"); > goto err; > } > @@ -305,7 +305,7 @@ tls_ocsp_process_response_internal(struct tls *ctx, const unsigned char *respons > if (resp == NULL) { > tls_ocsp_free(ctx->ocsp); > ctx->ocsp = NULL; > - tls_set_error(ctx, TLS_ERROR_UNKNOWN, > + tls_set_errorx(ctx, TLS_ERROR_UNKNOWN, > "unable to parse OCSP response"); > return -1; > } > diff --git a/src/lib/libtls/tls_server.c b/src/lib/libtls/tls_server.c > index 42a697327..47c711421 100644 > --- a/src/lib/libtls/tls_server.c > +++ b/src/lib/libtls/tls_server.c > @@ -242,12 +242,12 @@ tls_configure_server_ssl(struct tls *ctx, SSL_CTX **ssl_ctx, > > if (SSL_CTX_set_tlsext_servername_callback(*ssl_ctx, > tls_servername_cb) != 1) { > - tls_set_error(ctx, TLS_ERROR_UNKNOWN, > + tls_set_errorx(ctx, TLS_ERROR_UNKNOWN, > "failed to set servername callback"); > goto err; > } > if (SSL_CTX_set_tlsext_servername_arg(*ssl_ctx, ctx) != 1) { > - tls_set_error(ctx, TLS_ERROR_UNKNOWN, > + tls_set_errorx(ctx, TLS_ERROR_UNKNOWN, > "failed to set servername callback arg"); > goto err; > } > @@ -298,7 +298,7 @@ tls_configure_server_ssl(struct tls *ctx, SSL_CTX **ssl_ctx, > SSL_CTX_clear_options(*ssl_ctx, SSL_OP_NO_TICKET); > if (!SSL_CTX_set_tlsext_ticket_key_cb(*ssl_ctx, > tls_server_ticket_cb)) { > - tls_set_error(ctx, TLS_ERROR_UNKNOWN, > + tls_set_errorx(ctx, TLS_ERROR_UNKNOWN, > "failed to set the TLS ticket callback"); > goto err; > } > @@ -306,7 +306,7 @@ tls_configure_server_ssl(struct tls *ctx, SSL_CTX **ssl_ctx, > > if (SSL_CTX_set_session_id_context(*ssl_ctx, ctx->config->session_id, > sizeof(ctx->config->session_id)) != 1) { > - tls_set_error(ctx, TLS_ERROR_UNKNOWN, > + tls_set_errorx(ctx, TLS_ERROR_UNKNOWN, > "failed to set session id context"); > goto err; > } > diff --git a/src/lib/libtls/tls_signer.c b/src/lib/libtls/tls_signer.c > index 2573803ec..70160cd98 100644 > --- a/src/lib/libtls/tls_signer.c > +++ b/src/lib/libtls/tls_signer.c > @@ -137,7 +137,7 @@ tls_signer_add_keypair_mem(struct tls_signer *signer, const uint8_t *cert, > } > > if ((skey = calloc(1, sizeof(*skey))) == NULL) { > - tls_error_set(&signer->error, TLS_ERROR_OUT_OF_MEMORY, > + tls_error_setx(&signer->error, TLS_ERROR_OUT_OF_MEMORY, > "out of memory"); > goto err; > } > @@ -223,7 +223,7 @@ tls_sign_rsa(struct tls_signer *signer, struct tls_signer_key *skey, > return (-1); > } > if ((signature = calloc(1, rsa_size)) == NULL) { > - tls_error_set(&signer->error, TLS_ERROR_OUT_OF_MEMORY, > + tls_error_setx(&signer->error, TLS_ERROR_OUT_OF_MEMORY, > "out of memory"); > return (-1); > } > @@ -271,7 +271,7 @@ tls_sign_ecdsa(struct tls_signer *signer, struct tls_signer_key *skey, > return (-1); > } > if ((signature = calloc(1, signature_len)) == NULL) { > - tls_error_set(&signer->error, TLS_ERROR_OUT_OF_MEMORY, > + tls_error_setx(&signer->error, TLS_ERROR_OUT_OF_MEMORY, > "out of memory"); > return (-1); > } > -- > 2.49.0 >