* [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