netdev - Re: [PATCH v7 1/2] net/handshake: Create a NETLINK service for handling handshake requests

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:   Tue, 28 Mar 2023 14:32:57 -0400
From:   Jeff Layton <jlayton@...hat.com>
To:     Chuck Lever III <chuck.lever@...cle.com>
Cc:     Chuck Lever <cel@...nel.org>, Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Eric Dumazet <edumazet@...gle.com>,
        "open list:NETWORKING [GENERAL]" <netdev@...r.kernel.org>,
        "kernel-tls-handshake@...ts.linux.dev" 
        <kernel-tls-handshake@...ts.linux.dev>,
        John Haxby <john.haxby@...cle.com>
Subject: Re: [PATCH v7 1/2] net/handshake: Create a NETLINK service for
 handling handshake requests

On Tue, 2023-03-28 at 18:19 +0000, Chuck Lever III wrote:
> 
> > On Mar 28, 2023, at 2:14 PM, Jeff Layton <jlayton@...hat.com> wrote:
> > 
> > On Sat, 2023-03-18 at 12:18 -0400, Chuck Lever wrote:
> > > From: Chuck Lever <chuck.lever@...cle.com>
> > > 
> > > When a kernel consumer needs a transport layer security session, it
> > > first needs a handshake to negotiate and establish a session. This
> > > negotiation can be done in user space via one of the several
> > > existing library implementations, or it can be done in the kernel.
> > > 
> > > No in-kernel handshake implementations yet exist. In their absence,
> > > we add a netlink service that can:
> > > 
> > > a. Notify a user space daemon that a handshake is needed.
> > > 
> > > b. Once notified, the daemon calls the kernel back via this
> > >   netlink service to get the handshake parameters, including an
> > >   open socket on which to establish the session.
> > > 
> > > c. Once the handshake is complete, the daemon reports the
> > >   session status and other information via a second netlink
> > >   operation. This operation marks that it is safe for the
> > >   kernel to use the open socket and the security session
> > >   established there.
> > > 
> > > The notification service uses a multicast group. Each handshake
> > > mechanism (eg, tlshd) adopts its own group number so that the
> > > handshake services are completely independent of one another. The
> > > kernel can then tell via netlink_has_listeners() whether a handshake
> > > service is active and prepared to handle a handshake request.
> > > 
> > > A new netlink operation, ACCEPT, acts like accept(2) in that it
> > > instantiates a file descriptor in the user space daemon's fd table.
> > > If this operation is successful, the reply carries the fd number,
> > > which can be treated as an open and ready file descriptor.
> > > 
> > > While user space is performing the handshake, the kernel keeps its
> > > muddy paws off the open socket. A second new netlink operation,
> > > DONE, indicates that the user space daemon is finished with the
> > > socket and it is safe for the kernel to use again. The operation
> > > also indicates whether a session was established successfully.
> > > 
> > > Signed-off-by: Chuck Lever <chuck.lever@...cle.com>
> > > ---
> > > Documentation/netlink/specs/handshake.yaml |  122 +++++++++++
> > > MAINTAINERS                                |    8 +
> > > include/trace/events/handshake.h           |  159 ++++++++++++++
> > > include/uapi/linux/handshake.h             |   70 ++++++
> > > net/Kconfig                                |    5 
> > > net/Makefile                               |    1 
> > > net/handshake/Makefile                     |   11 +
> > > net/handshake/genl.c                       |   57 +++++
> > > net/handshake/genl.h                       |   23 ++
> > > net/handshake/handshake.h                  |   82 +++++++
> > > net/handshake/netlink.c                    |  316 ++++++++++++++++++++++++++++
> > > net/handshake/request.c                    |  307 +++++++++++++++++++++++++++
> > > net/handshake/trace.c                      |   20 ++
> > > 13 files changed, 1181 insertions(+)
> > > create mode 100644 Documentation/netlink/specs/handshake.yaml
> > > create mode 100644 include/trace/events/handshake.h
> > > create mode 100644 include/uapi/linux/handshake.h
> > > create mode 100644 net/handshake/Makefile
> > > create mode 100644 net/handshake/genl.c
> > > create mode 100644 net/handshake/genl.h
> > > create mode 100644 net/handshake/handshake.h
> > > create mode 100644 net/handshake/netlink.c
> > > create mode 100644 net/handshake/request.c
> > > create mode 100644 net/handshake/trace.c
> > > 
> > > 
> > 
> > [...]
> > 
> > > diff --git a/net/handshake/request.c b/net/handshake/request.c
> > > new file mode 100644
> > > index 000000000000..3f8ae9e990d2
> > > --- /dev/null
> > > +++ b/net/handshake/request.c
> > > @@ -0,0 +1,307 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * Handshake request lifetime events
> > > + *
> > > + * 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 <linux/fdtable.h>
> > > +#include <linux/rhashtable.h>
> > > +
> > > +#include <net/sock.h>
> > > +#include <net/genetlink.h>
> > > +#include <net/netns/generic.h>
> > > +
> > > +#include <uapi/linux/handshake.h>
> > > +#include "handshake.h"
> > > +
> > > +#include <trace/events/handshake.h>
> > > +
> > > +/*
> > > + * We need both a handshake_req -> sock mapping, and a sock ->
> > > + * handshake_req mapping. Both are one-to-one.
> > > + *
> > > + * To avoid adding another pointer field to struct sock, net/handshake
> > > + * maintains a hash table, indexed by the memory address of @sock, to
> > > + * find the struct handshake_req outstanding for that socket. The
> > > + * reverse direction uses a simple pointer field in the handshake_req
> > > + * struct.
> > > + */
> > > +
> > > +static struct rhashtable handshake_rhashtbl ____cacheline_aligned_in_smp;
> > > +
> > > +static const struct rhashtable_params handshake_rhash_params = {
> > > +	.key_len		= sizeof_field(struct handshake_req, hr_sk),
> > > +	.key_offset		= offsetof(struct handshake_req, hr_sk),
> > > +	.head_offset		= offsetof(struct handshake_req, hr_rhash),
> > > +	.automatic_shrinking	= true,
> > > +};
> > > +
> > > +int handshake_req_hash_init(void)
> > > +{
> > > +	return rhashtable_init(&handshake_rhashtbl, &handshake_rhash_params);
> > > +}
> > > +
> > > +void handshake_req_hash_destroy(void)
> > > +{
> > > +	rhashtable_destroy(&handshake_rhashtbl);
> > > +}
> > > +
> > > +struct handshake_req *handshake_req_hash_lookup(struct sock *sk)
> > > +{
> > > +	return rhashtable_lookup_fast(&handshake_rhashtbl, &sk,
> > 
> > Is this correct? It seems like we should be searching for the struct
> > sock pointer value, not on the pointer to the pointer (which will be a
> > stack var), right?
> 
> I copied this from the nfsd_file and nfs4_file code we added recently.
> rhashtable_lookup_fast takes a pointer to the key, so a pointer to a
> pointer should be correct in this case.
> 

Got it. Thanks for clarifying!
-- 
Jeff Layton <jlayton@...hat.com>