| 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
| ||
|
Message-ID: <33AFECBC-E0A8-4C87-9625-B0DA22F8D617@oracle.com>
Date: Wed, 19 Jul 2023 13:24:38 +0000
From: Chuck Lever III <chuck.lever@...cle.com>
To: Hannes Reinecke <hare@...e.de>
CC: Chuck Lever <cel@...nel.org>, "David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
Paolo
Abeni <pabeni@...hat.com>,
"open list:NETWORKING [GENERAL]"
<netdev@...r.kernel.org>,
"kernel-tls-handshake@...ts.linux.dev"
<kernel-tls-handshake@...ts.linux.dev>
Subject: Re: [PATCH net-next v1 3/7] net/handshake: Add API for sending TLS
Closure alerts
> On Jul 19, 2023, at 3:47 AM, Hannes Reinecke <hare@...e.de> wrote:
>
> 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?
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
Powered by blists - more mailing lists