public inbox for kernel-tls-handshake@lists.linux.dev
 help / color / mirror / Atom feed
* [PATCH] tlshd: Clean up logic in tlshd_start_tls_handshake()
@ 2025-09-24  0:01 Chuck Lever
  2025-09-24 11:22 ` Benjamin Coddington
  2025-09-24 14:27 ` Chuck Lever
  0 siblings, 2 replies; 3+ messages in thread
From: Chuck Lever @ 2025-09-24  0:01 UTC (permalink / raw)
  To: Ben Coddington, Xin Long; +Cc: kernel-tls-handshake, Chuck Lever

From: Chuck Lever <chuck.lever@oracle.com>

gnutls_handshake() is supposed to return only a GNUTLS_E value, and
the session_status field is supposed to contain only a positive
errno.

GNUTLS_E_PREMATURE_TERMINATION is -110. It turns out that on x86,
-ETIMEDOUT is also -110. Make sure the correct symbolic constants
and auditing functions are utilized.

Fixes: b010190cfed2 ("tlshd: Pass ETIMEDOUT from gnutls to kernel")
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 src/tlshd/handshake.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/tlshd/handshake.c b/src/tlshd/handshake.c
index 5a2893994d4d..44932b7a5311 100644
--- a/src/tlshd/handshake.c
+++ b/src/tlshd/handshake.c
@@ -90,19 +90,19 @@ void tlshd_start_tls_handshake(gnutls_session_t session,
 	} while (ret < 0 && !gnutls_error_is_fatal(ret));
 	tlshd_set_nagle(session, saved);
 	if (ret < 0) {
-		/* Any errors here should default to blocking access: */
+		/* By default, a handshake error is permanent */
 		parms->session_status = EACCES;
 		switch (ret) {
 		case GNUTLS_E_CERTIFICATE_ERROR:
 		case GNUTLS_E_CERTIFICATE_VERIFICATION_ERROR:
 			tlshd_log_cert_verification_error(session);
 			break;
-		case -ETIMEDOUT:
-			tlshd_log_gnutls_error(ret);
-			parms->session_status = -ret;
+		case GNUTLS_E_PREMATURE_TERMINATION:
+			tlshd_log_error("Handshake timeout, retrying");
+			parms->session_status = ETIMEDOUT;
 			break;
 		default:
-			tlshd_log_notice("tlshd_start_tls_handshake unhandled error %d, returning EACCES\n", ret);
+			tlshd_log_gnutls_error(ret);
 		}
 		return;
 	}
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] tlshd: Clean up logic in tlshd_start_tls_handshake()
  2025-09-24  0:01 [PATCH] tlshd: Clean up logic in tlshd_start_tls_handshake() Chuck Lever
@ 2025-09-24 11:22 ` Benjamin Coddington
  2025-09-24 14:27 ` Chuck Lever
  1 sibling, 0 replies; 3+ messages in thread
From: Benjamin Coddington @ 2025-09-24 11:22 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Xin Long, kernel-tls-handshake, Chuck Lever

On 23 Sep 2025, at 20:01, Chuck Lever wrote:

> From: Chuck Lever <chuck.lever@oracle.com>
>
> gnutls_handshake() is supposed to return only a GNUTLS_E value, and
> the session_status field is supposed to contain only a positive
> errno.
>
> GNUTLS_E_PREMATURE_TERMINATION is -110. It turns out that on x86,
> -ETIMEDOUT is also -110. Make sure the correct symbolic constants
> and auditing functions are utilized.
>
> Fixes: b010190cfed2 ("tlshd: Pass ETIMEDOUT from gnutls to kernel")
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>

Its better!

Reviewed-by: Benjamin Coddington <bcodding@redhat.com>

Ben


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] tlshd: Clean up logic in tlshd_start_tls_handshake()
  2025-09-24  0:01 [PATCH] tlshd: Clean up logic in tlshd_start_tls_handshake() Chuck Lever
  2025-09-24 11:22 ` Benjamin Coddington
@ 2025-09-24 14:27 ` Chuck Lever
  1 sibling, 0 replies; 3+ messages in thread
From: Chuck Lever @ 2025-09-24 14:27 UTC (permalink / raw)
  To: Chuck Lever, Ben Coddington, Xin Long; +Cc: kernel-tls-handshake

On 9/23/25 5:01 PM, Chuck Lever wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> 
> gnutls_handshake() is supposed to return only a GNUTLS_E value, and
> the session_status field is supposed to contain only a positive
> errno.
> 
> GNUTLS_E_PREMATURE_TERMINATION is -110. It turns out that on x86,
> -ETIMEDOUT is also -110. Make sure the correct symbolic constants
> and auditing functions are utilized.
> 
> Fixes: b010190cfed2 ("tlshd: Pass ETIMEDOUT from gnutls to kernel")
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  src/tlshd/handshake.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/src/tlshd/handshake.c b/src/tlshd/handshake.c
> index 5a2893994d4d..44932b7a5311 100644
> --- a/src/tlshd/handshake.c
> +++ b/src/tlshd/handshake.c
> @@ -90,19 +90,19 @@ void tlshd_start_tls_handshake(gnutls_session_t session,
>  	} while (ret < 0 && !gnutls_error_is_fatal(ret));
>  	tlshd_set_nagle(session, saved);
>  	if (ret < 0) {
> -		/* Any errors here should default to blocking access: */
> +		/* By default, a handshake error is permanent */

I had forgotten that session_status is initially EIO, so my new
comment makes less sense than what is there now. Will drop the
comment change.


>  		parms->session_status = EACCES;
>  		switch (ret) {
>  		case GNUTLS_E_CERTIFICATE_ERROR:
>  		case GNUTLS_E_CERTIFICATE_VERIFICATION_ERROR:
>  			tlshd_log_cert_verification_error(session);
>  			break;
> -		case -ETIMEDOUT:
> -			tlshd_log_gnutls_error(ret);
> -			parms->session_status = -ret;
> +		case GNUTLS_E_PREMATURE_TERMINATION:
> +			tlshd_log_error("Handshake timeout, retrying");
> +			parms->session_status = ETIMEDOUT;
>  			break;
>  		default:
> -			tlshd_log_notice("tlshd_start_tls_handshake unhandled error %d, returning EACCES\n", ret);
> +			tlshd_log_gnutls_error(ret);
>  		}
>  		return;
>  	}


-- 
Chuck Lever

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-09-24 14:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-24  0:01 [PATCH] tlshd: Clean up logic in tlshd_start_tls_handshake() Chuck Lever
2025-09-24 11:22 ` Benjamin Coddington
2025-09-24 14:27 ` Chuck Lever

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox