diff mbox series

[net-next,v1,3/7] net/handshake: Add API for sending TLS Closure alerts

Message ID 168970680139.5330.16891764300979182957.stgit@oracle-102.nfsv4bat.org (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series In-kernel support for the TLS Alert protocol | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next, async
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1344 this patch: 1344
netdev/cc_maintainers warning 1 maintainers not CCed: chuck.lever@oracle.com
netdev/build_clang success Errors and warnings before: 1371 this patch: 1371
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1367 this patch: 1367
netdev/checkpatch warning WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Chuck Lever July 18, 2023, 7 p.m. UTC 
From: Chuck Lever <chuck.lever@oracle.com>

This helper sends an alert only if a TLS session was established.

Signed-off-by: Chuck Lever <chuck.lever@oracle.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

Comments

Hannes Reinecke July 19, 2023, 7:47 a.m. UTC | #1 
On 7/18/23 21:00, Chuck Lever wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> 
> This helper sends an alert only if a TLS session was established.
> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.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@oracle.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
Chuck Lever July 19, 2023, 1:24 p.m. UTC | #2 
> On Jul 19, 2023, at 3:47 AM, Hannes Reinecke <hare@suse.de> wrote:
> 
> On 7/18/23 21:00, Chuck Lever wrote:
>> From: Chuck Lever <chuck.lever@oracle.com>
>> This helper sends an alert only if a TLS session was established.
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.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@oracle.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?

REQ_SESSION indicates that a TLS session was established
successfully.

The hash will contain a handshake context starting before the
tlshd upcall, and will continue to exist even if the handshake
attempt failed. Those are both times when a Close alert should
not be sent.


> 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?

Yes, that makes sense.


--
Chuck Lever
diff mbox series

Patch

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@oracle.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);