lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 19 Jul 2023 09:47:13 +0200
From: Hannes Reinecke <hare@...e.de>
To: Chuck Lever <cel@...nel.org>, davem@...emloft.net, edumazet@...gle.com,
 kuba@...nel.org, pabeni@...hat.com
Cc: netdev@...r.kernel.org, kernel-tls-handshake@...ts.linux.dev
Subject: Re: [PATCH net-next v1 3/7] net/handshake: Add API for sending TLS
 Closure alerts

On 7/18/23 21:00, Chuck Lever wrote:
> From: Chuck Lever <chuck.lever@...cle.com>
> 
> This helper sends an alert only if a TLS session was established.
> 
> Signed-off-by: Chuck Lever <chuck.lever@...cle.com>
> ---
>   include/net/handshake.h   |    1 +
>   net/handshake/Makefile    |    2 +
>   net/handshake/alert.c     |   62 +++++++++++++++++++++++++++++++++++++++++++++
>   net/handshake/handshake.h |    4 +++
>   net/handshake/tlshd.c     |   23 +++++++++++++++++
>   5 files changed, 91 insertions(+), 1 deletion(-)
>   create mode 100644 net/handshake/alert.c
> 
> diff --git a/include/net/handshake.h b/include/net/handshake.h
> index 2e26e436e85f..bb88dfa6e3c9 100644
> --- a/include/net/handshake.h
> +++ b/include/net/handshake.h
> @@ -40,5 +40,6 @@ int tls_server_hello_x509(const struct tls_handshake_args *args, gfp_t flags);
>   int tls_server_hello_psk(const struct tls_handshake_args *args, gfp_t flags);
>   
>   bool tls_handshake_cancel(struct sock *sk);
> +void tls_handshake_close(struct socket *sock);
>   
>   #endif /* _NET_HANDSHAKE_H */
> diff --git a/net/handshake/Makefile b/net/handshake/Makefile
> index 247d73c6ff6e..ef4d9a2112bd 100644
> --- a/net/handshake/Makefile
> +++ b/net/handshake/Makefile
> @@ -8,6 +8,6 @@
>   #
>   
>   obj-y += handshake.o
> -handshake-y := genl.o netlink.o request.o tlshd.o trace.o
> +handshake-y := alert.o genl.o netlink.o request.o tlshd.o trace.o
>   
>   obj-$(CONFIG_NET_HANDSHAKE_KUNIT_TEST) += handshake-test.o
> diff --git a/net/handshake/alert.c b/net/handshake/alert.c
> new file mode 100644
> index 000000000000..999d3ffaf3e3
> --- /dev/null
> +++ b/net/handshake/alert.c
> @@ -0,0 +1,62 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Handle the TLS Alert protocol
> + *
> + * Author: Chuck Lever <chuck.lever@...cle.com>
> + *
> + * Copyright (c) 2023, Oracle and/or its affiliates.
> + */
> +
> +#include <linux/types.h>
> +#include <linux/socket.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/skbuff.h>
> +#include <linux/inet.h>
> +
> +#include <net/sock.h>
> +#include <net/handshake.h>
> +#include <net/genetlink.h>
> +#include <net/tls.h>
> +#include <net/tls_prot.h>
> +
> +#include "handshake.h"
> +
> +/**
> + * tls_alert_send - send a TLS Alert on a kTLS socket
> + * @sock: open kTLS socket to send on
> + * @level: TLS Alert level
> + * @description: TLS Alert description
> + *
> + * Returns zero on success or a negative errno.
> + */
> +int tls_alert_send(struct socket *sock, u8 level, u8 description)
> +{
> +	u8 record_type = TLS_RECORD_TYPE_ALERT;
> +	u8 buf[CMSG_SPACE(sizeof(record_type))];
> +	struct msghdr msg = { 0 };
> +	struct cmsghdr *cmsg;
> +	struct kvec iov;
> +	u8 alert[2];
> +	int ret;
> +
> +	alert[0] = level;
> +	alert[1] = description;
> +	iov.iov_base = alert;
> +	iov.iov_len = sizeof(alert);
> +
> +	memset(buf, 0, sizeof(buf));
> +	msg.msg_control = buf;
> +	msg.msg_controllen = sizeof(buf);
> +	msg.msg_flags = MSG_DONTWAIT;
> +
> +	cmsg = CMSG_FIRSTHDR(&msg);
> +	cmsg->cmsg_level = SOL_TLS;
> +	cmsg->cmsg_type = TLS_SET_RECORD_TYPE;
> +	cmsg->cmsg_len = CMSG_LEN(sizeof(record_type));
> +	memcpy(CMSG_DATA(cmsg), &record_type, sizeof(record_type));
> +
> +	iov_iter_kvec(&msg.msg_iter, ITER_SOURCE, &iov, 1, iov.iov_len);
> +	ret = sock_sendmsg(sock, &msg);
> +	return ret < 0 ? ret : 0;
> +}
> diff --git a/net/handshake/handshake.h b/net/handshake/handshake.h
> index 4dac965c99df..af1633d5ad73 100644
> --- a/net/handshake/handshake.h
> +++ b/net/handshake/handshake.h
> @@ -41,6 +41,7 @@ struct handshake_req {
>   
>   enum hr_flags_bits {
>   	HANDSHAKE_F_REQ_COMPLETED,
> +	HANDSHAKE_F_REQ_SESSION,
>   };
>   
>   /* Invariants for all handshake requests for one transport layer
> @@ -63,6 +64,9 @@ enum hp_flags_bits {
>   	HANDSHAKE_F_PROTO_NOTIFY,
>   };
>   
> +/* alert.c */
> +int tls_alert_send(struct socket *sock, u8 level, u8 description);
> +
>   /* netlink.c */
>   int handshake_genl_notify(struct net *net, const struct handshake_proto *proto,
>   			  gfp_t flags);
> diff --git a/net/handshake/tlshd.c b/net/handshake/tlshd.c
> index b735f5cced2f..aad3c5b06b03 100644
> --- a/net/handshake/tlshd.c
> +++ b/net/handshake/tlshd.c
> @@ -18,6 +18,7 @@
>   #include <net/sock.h>
>   #include <net/handshake.h>
>   #include <net/genetlink.h>
> +#include <net/tls_prot.h>
>   
>   #include <uapi/linux/keyctl.h>
>   #include <uapi/linux/handshake.h>
> @@ -100,6 +101,9 @@ static void tls_handshake_done(struct handshake_req *req,
>   	if (info)
>   		tls_handshake_remote_peerids(treq, info);
>   
> +	if (!status)
> +		set_bit(HANDSHAKE_F_REQ_SESSION, &req->hr_flags);
> +
>   	treq->th_consumer_done(treq->th_consumer_data, -status,
>   			       treq->th_peerid[0]);
>   }
> @@ -424,3 +428,22 @@ bool tls_handshake_cancel(struct sock *sk)
>   	return handshake_req_cancel(sk);
>   }
>   EXPORT_SYMBOL(tls_handshake_cancel);
> +
> +/**
> + * tls_handshake_close - send a Closure alert
> + * @sock: an open socket
> + *
> + */
> +void tls_handshake_close(struct socket *sock)
> +{
> +	struct handshake_req *req;
> +
> +	req = handshake_req_hash_lookup(sock->sk);
> +	if (!req)
> +		return;
> +	if (!test_bit(HANDSHAKE_F_REQ_SESSION, &req->hr_flags))
> +		return;
> +	tls_alert_send(sock, TLS_ALERT_LEVEL_WARNING,
> +		       TLS_ALERT_DESC_CLOSE_NOTIFY);
> +}
> +EXPORT_SYMBOL(tls_handshake_close);
> 
Why do we need to check for the 'REQ_SESSION' flag?
Isn't the hash_lookup sufficient here?
And it it safe to just do a 'test_bit' here?
Wouldn't it be better to do

if (test_and_clear_bit()) tls_alert_send()

Hmm?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@...e.de			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Frankenstr. 146, 90461 Nürnberg
Managing Directors: I. Totev, A. Myers, A. McDonald, M. B. Moerman
(HRB 36809, AG Nürnberg)


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ