public inbox for kernel-tls-handshake@lists.linux.dev
 help / color / mirror / Atom feed
From: Scott Mayhew <smayhew@redhat.com>
To: Chuck Lever <cel@kernel.org>
Cc: Chuck Lever <chuck.lever@oracle.com>,
	kernel-tls-handshake@lists.linux.dev
Subject: Re: [PATCH] tlshd: Send fatal alert to client when there are server config issues
Date: Wed, 4 Feb 2026 15:39:10 -0500	[thread overview]
Message-ID: <aYOubhCOKmh77fCe@aion> (raw)
In-Reply-To: <d155013f-b9e6-4c51-bccc-f4e73cda5dff@app.fastmail.com>

On Wed, 04 Feb 2026, Chuck Lever wrote:

> 
> 
> On Tue, Feb 3, 2026, at 6:12 PM, Scott Mayhew wrote:
> > Currently if a client attempts an x.509 handshake and the server is
> > misconfigured (no certificates, no private keys, etc), the server simply
> > closes the connection.  Prior to b010190 ("tlshd: Pass ETIMEDOUT from
> > gnutls to kernel"), this would result in a quick failure on the client.
> > Now the client keeps retrying until the mount program times out, which
> > takes several minutes.
> >
> > A misconfigured server isn't a self-correcting problem, so send a fatal
> > alert to the client when this occurs so the client stops retrying
> > immediately.  This requires some minor refactoring of
> > tlshd_tls13_server_x509_handshake() so that the session is initialized
> > before attempting to load the certs and keys (otherwise it is not
> > possible to send an alert).  Also add some debug logging to help
> > the admin take corrective action.
> >
> > Finally add some logging when an alert is received during the handshake.
> > Following suit with handshake completions, alerts will only be logged if
> > debug logging is enabled.
> >
> > Signed-off-by: Scott Mayhew <smayhew@redhat.com>
> > ---
> >  src/tlshd/handshake.c |  4 ++++
> >  src/tlshd/log.c       | 22 +++++++++++++++++++++
> >  src/tlshd/server.c    | 46 +++++++++++++++++++++++++++----------------
> >  src/tlshd/tlshd.h     |  1 +
> >  4 files changed, 56 insertions(+), 17 deletions(-)
> >
> > diff --git a/src/tlshd/handshake.c b/src/tlshd/handshake.c
> > index e78f78f..007339a 100644
> > --- a/src/tlshd/handshake.c
> > +++ b/src/tlshd/handshake.c
> > @@ -115,6 +115,10 @@ void tlshd_start_tls_handshake(gnutls_session_t 
> > session,
> >  			tlshd_log_error("Handshake timeout, retrying");
> >  			parms->session_status = ETIMEDOUT;
> >  			break;
> > +		case GNUTLS_E_WARNING_ALERT_RECEIVED:
> > +		case GNUTLS_E_FATAL_ALERT_RECEIVED:
> > +			tlshd_log_alert(session, ret);
> > +			break;
> >  		default:
> >  			tlshd_log_gnutls_error(ret);
> >  		}
> > diff --git a/src/tlshd/log.c b/src/tlshd/log.c
> > index b70d4af..bab390a 100644
> > --- a/src/tlshd/log.c
> > +++ b/src/tlshd/log.c
> > @@ -188,6 +188,28 @@ void 
> > tlshd_log_cert_verification_error(gnutls_session_t session)
> >  			       tlshd_cert_status_names[i].name);
> >  }
> > 
> > +/**
> > + * @brief Report a TLS alert
> > + * @param[in]     session  Controlling GnuTLS session
> > + * @param[in]     level    TLS alert level (warning or fatal)
> 
> A TLS alert level would be GNUTLS_AL_WARNING or GNUTLS_AL_FATAL (protocol values 1 and 2). The actual @level values passed are GnuTLS error codes like -25 and -12 (negative integers indicating error conditions).
> 

Ah, that's correct.  I think I should just drop the level altogether.
Initially I had in mind that we may want to do something different based
no the alert level, but right now it's serving no purpose there.

> 
> > + */
> > +void tlshd_log_alert(gnutls_session_t session, int level)
> > +{
> > +	gnutls_alert_description_t alert;
> > +
> > +	if (level != GNUTLS_E_WARNING_ALERT_RECEIVED &&
> > +			level != GNUTLS_E_FATAL_ALERT_RECEIVED) {
> > +		tlshd_log_debug("%s called with invalid level", __func__);
> > +		return;
> > +	}
> > +
> > +	if (!tlshd_debug)
> > +		return;
> > +
> > +	alert = gnutls_alert_get(session);
> > +	tlshd_log_notice("Received alert: %s", gnutls_alert_get_name(alert));
> > +}
> > +
> >  /**
> >   * @brief Emit "library call failed" notification
> >   * @param[in]     error  GnuTLS error code to log
> > diff --git a/src/tlshd/server.c b/src/tlshd/server.c
> > index 4850210..3a72fc5 100644
> > --- a/src/tlshd/server.c
> > +++ b/src/tlshd/server.c
> > @@ -397,30 +397,48 @@ static void 
> > tlshd_tls13_server_x509_handshake(struct tlshd_handshake_parms *parm
> >  	gnutls_session_t session;
> >  	int ret;
> > 
> > +	ret = gnutls_init(&session, GNUTLS_SERVER);
> > +	if (ret != GNUTLS_E_SUCCESS) {
> > +		tlshd_log_gnutls_error(ret);
> > +		return;
> > +	}
> > +	gnutls_transport_set_int(session, parms->sockfd);
> > +	gnutls_session_set_ptr(session, parms);
> > +
> > +	ret = tlshd_gnutls_priority_set(session, parms, 0);
> > +	if (ret) {
> > +		tlshd_log_gnutls_error(ret);
> 
> Are you leaking the session handle in this error flow?
> Might need a gnutls_deinit(session);

Yep.  I'll fix it up in v2.

> 
> 
> > +		return;
> > +	}
> > +
> >  	ret = gnutls_certificate_allocate_credentials(&xcred);
> >  	if (ret != GNUTLS_E_SUCCESS) {
> >  		tlshd_log_gnutls_error(ret);
> 
> Ditto.
> 
> 
> >  		return;
> >  	}
> >  	ret = tlshd_server_get_truststore(xcred);
> > -	if (ret != GNUTLS_E_SUCCESS)
> > +	if (ret != GNUTLS_E_SUCCESS) {
> > +		tlshd_log_debug("Failed to initialize server trust store - check the 
> > configuration");
> > +		gnutls_alert_send(session, GNUTLS_AL_FATAL, GNUTLS_A_ACCESS_DENIED);
> > +		gnutls_deinit(session);
> >  		goto out_free_creds;
> > +	}
> > 
> > -	if (!tlshd_x509_server_get_certs(parms))
> > +	if (!tlshd_x509_server_get_certs(parms)) {
> > +		tlshd_log_debug("No usable server certificates were found - check 
> > the configuration");
> > +		gnutls_alert_send(session, GNUTLS_AL_FATAL, GNUTLS_A_ACCESS_DENIED);
> > +		gnutls_deinit(session);
> >  		goto out_free_creds;
> > -	if (!tlshd_x509_server_get_privkey(parms))
> > +	}
> > +	if (!tlshd_x509_server_get_privkey(parms)) {
> > +		tlshd_log_debug("No usable private keys were found - check the 
> > configuration");
> > +		gnutls_alert_send(session, GNUTLS_AL_FATAL, GNUTLS_A_ACCESS_DENIED);
> > +		gnutls_deinit(session);
> >  		goto out_free_certs;
> > +	}
> >  	gnutls_certificate_set_retrieve_function2(xcred,
> >  						  tlshd_x509_retrieve_key_cb);
> > 
> > -	ret = gnutls_init(&session, GNUTLS_SERVER);
> > -	if (ret != GNUTLS_E_SUCCESS) {
> > -		tlshd_log_gnutls_error(ret);
> > -		goto out_free_certs;
> > -	}
> > -	gnutls_transport_set_int(session, parms->sockfd);
> > -	gnutls_session_set_ptr(session, parms);
> > -
> >  	ret = gnutls_credentials_set(session, GNUTLS_CRD_CERTIFICATE, xcred);
> >  	if (ret != GNUTLS_E_SUCCESS) {
> >  		tlshd_log_gnutls_error(ret);
> > @@ -430,12 +448,6 @@ static void 
> > tlshd_tls13_server_x509_handshake(struct tlshd_handshake_parms *parm
> >  					       tlshd_tls13_server_x509_verify_function);
> >  	gnutls_certificate_server_set_request(session, GNUTLS_CERT_REQUEST);
> > 
> > -	ret = tlshd_gnutls_priority_set(session, parms, 0);
> > -	if (ret) {
> > -		tlshd_log_gnutls_error(ret);
> > -		goto out_free_certs;
> > -	}
> > -
> >  	tlshd_start_tls_handshake(session, parms);
> > 
> >  	if (tlshd_debug &&
> > diff --git a/src/tlshd/tlshd.h b/src/tlshd/tlshd.h
> > index 75d9a4a..686d861 100644
> > --- a/src/tlshd/tlshd.h
> > +++ b/src/tlshd/tlshd.h
> > @@ -118,6 +118,7 @@ extern void tlshd_log_perror(const char *prefix);
> >  extern void tlshd_log_gai_error(int error);
> > 
> >  extern void tlshd_log_cert_verification_error(gnutls_session_t session);
> > +extern void tlshd_log_alert(gnutls_session_t session, int level);
> >  extern void tlshd_log_gnutls_error(int error);
> >  extern void tlshd_gnutls_log_func(int level, const char *msg);
> >  extern void tlshd_gnutls_audit_func(gnutls_session_t session, const char *msg);
> > -- 
> > 2.52.0
> 
> -- 
> Chuck Lever
> 


      reply	other threads:[~2026-02-04 20:39 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-03 23:12 [PATCH] tlshd: Send fatal alert to client when there are server config issues Scott Mayhew
2026-02-04 18:47 ` Chuck Lever
2026-02-04 20:39   ` Scott Mayhew [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aYOubhCOKmh77fCe@aion \
    --to=smayhew@redhat.com \
    --cc=cel@kernel.org \
    --cc=chuck.lever@oracle.com \
    --cc=kernel-tls-handshake@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox