netdev - Re: [PATCH net-next v1 5/7] net/handshake: Add helpers for parsing incoming TLS Alerts

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] [day] [month] [year] [list]

Date: Thu, 20 Jul 2023 14:22:27 +0000
From: Chuck Lever III <chuck.lever@...cle.com>
To: Hannes Reinecke <hare@...e.de>
CC: Chuck Lever <cel@...nel.org>, "davem@...emloft.net" <davem@...emloft.net>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "kuba@...nel.org"
	<kuba@...nel.org>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "kernel-tls-handshake@...ts.linux.dev" <kernel-tls-handshake@...ts.linux.dev>
Subject: Re: [PATCH net-next v1 5/7] net/handshake: Add helpers for parsing
 incoming TLS Alerts



> On Jul 20, 2023, at 1:44 AM, Hannes Reinecke <hare@...e.de> wrote:
> 
> On 7/19/23 15:36, Chuck Lever III wrote:
>>> On Jul 19, 2023, at 3:52 AM, Hannes Reinecke <hare@...e.de> wrote:
>>> 
>>> On 7/18/23 21:00, Chuck Lever wrote:
>>>> From: Chuck Lever <chuck.lever@...cle.com>
>>>> Kernel TLS consumers can replace common TLS Alert parsing code with
>>>> these helpers.
>>>> Signed-off-by: Chuck Lever <chuck.lever@...cle.com>
>>>> ---
>>>>  include/net/handshake.h |    4 ++++
>>>>  net/handshake/alert.c   |   46 ++++++++++++++++++++++++++++++++++++++++++++++
>>>>  2 files changed, 50 insertions(+)
>>>> diff --git a/include/net/handshake.h b/include/net/handshake.h
>>>> index bb88dfa6e3c9..d0fd6a3898c6 100644
>>>> --- a/include/net/handshake.h
>>>> +++ b/include/net/handshake.h
>>>> @@ -42,4 +42,8 @@ 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);
>>>>  +u8 tls_record_type(const struct sock *sk, const struct cmsghdr *msg);
>>>> +bool tls_alert_recv(const struct sock *sk, const struct msghdr *msg,
>>>> +     u8 *level, u8 *description);
>>>> +
>>>>  #endif /* _NET_HANDSHAKE_H */
>>>> diff --git a/net/handshake/alert.c b/net/handshake/alert.c
>>>> index 999d3ffaf3e3..93e05d8d599c 100644
>>>> --- a/net/handshake/alert.c
>>>> +++ b/net/handshake/alert.c
>>>> @@ -60,3 +60,49 @@ int tls_alert_send(struct socket *sock, u8 level, u8 description)
>>>>   ret = sock_sendmsg(sock, &msg);
>>>>   return ret < 0 ? ret : 0;
>>>>  }
>>>> +
>>>> +/**
>>>> + * tls_record_type - Look for TLS RECORD_TYPE information
>>>> + * @sk: socket (for IP address information)
>>>> + * @cmsg: incoming message to be parsed
>>>> + *
>>>> + * Returns zero or a TLS_RECORD_TYPE value.
>>>> + */
>>>> +u8 tls_record_type(const struct sock *sk, const struct cmsghdr *cmsg)
>>>> +{
>>>> + u8 record_type;
>>>> +
>>>> + if (cmsg->cmsg_level != SOL_TLS)
>>>> + return 0;
>>>> + if (cmsg->cmsg_type != TLS_GET_RECORD_TYPE)
>>>> + return 0;
>>>> +
>>>> + record_type = *((u8 *)CMSG_DATA(cmsg));
>>>> + return record_type;
>>>> +}
>>>> +EXPORT_SYMBOL(tls_record_type);
>>>> +
>>> tls_process_cmsg() does nearly the same thing; why didn't you update it to use your helper?
>> tls_process_cmsg() is looking for TLS_SET_RECORD_TYPE,
>> not TLS_GET_RECORD_TYPE. It looks different enough that
>> I didn't feel comfortable touching it.
> Argl. Totally forgot that we have TLS_GET_RECORD_TYPE and TLS_SET_RECORD_TYPE ...
> But to make it clear, shouldn't we rather name the function
> tls_get_record_type()

Renamed. Thanks for the review! I will post v2 next week, waiting
for more review comments.



--
Chuck Lever