From: Chuck Lever III <chuck.lever@oracle.com>
To: Trond Myklebust <trondmy@hammerspace.com>
Cc: Linux NFS Mailing List <linux-nfs@vger.kernel.org>,
"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
"linux-cifs@vger.kernel.org" <linux-cifs@vger.kernel.org>,
"borisp@nvidia.com" <borisp@nvidia.com>,
"simo@redhat.com" <simo@redhat.com>,
"ak@tempesta-tech.com" <ak@tempesta-tech.com>
Subject: Re: [PATCH RFC 08/15] SUNRPC: Add RPC_TASK_CORK flag
Date: Tue, 19 Apr 2022 18:16:42 +0000 [thread overview]
Message-ID: <AE1190F4-EDE4-4C2D-94C9-02A5EDAAFBC6@oracle.com> (raw)
In-Reply-To: <a771c65353d0805fc5f028fa56691ee762d6843f.camel@hammerspace.com>
> On Apr 18, 2022, at 10:57 PM, Trond Myklebust <trondmy@hammerspace.com> wrote:
>
> On Mon, 2022-04-18 at 12:52 -0400, Chuck Lever wrote:
>> Introduce a mechanism to cause xprt_transmit() to break out of its
>> sending loop at a specific rpc_rqst, rather than draining the whole
>> transmit queue.
>>
>> This enables the client to send just an RPC TLS probe and then wait
>> for the response before proceeding with the rest of the queue.
>>
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>> include/linux/sunrpc/sched.h | 2 ++
>> include/trace/events/sunrpc.h | 1 +
>> net/sunrpc/xprt.c | 2 ++
>> 3 files changed, 5 insertions(+)
>>
>> diff --git a/include/linux/sunrpc/sched.h
>> b/include/linux/sunrpc/sched.h
>> index 599133fb3c63..f8c09638fa69 100644
>> --- a/include/linux/sunrpc/sched.h
>> +++ b/include/linux/sunrpc/sched.h
>> @@ -125,6 +125,7 @@ struct rpc_task_setup {
>> #define RPC_TASK_TLSCRED 0x00000008 /* Use
>> AUTH_TLS credential */
>> #define RPC_TASK_NULLCREDS 0x00000010 /* Use
>> AUTH_NULL credential */
>> #define RPC_CALL_MAJORSEEN 0x00000020 /* major
>> timeout seen */
>> +#define RPC_TASK_CORK 0x00000040 /* cork the
>> xmit queue */
>> #define RPC_TASK_DYNAMIC 0x00000080 /* task was
>> kmalloc'ed */
>> #define RPC_TASK_NO_ROUND_ROBIN 0x00000100 /*
>> send requests on "main" xprt */
>> #define RPC_TASK_SOFT 0x00000200 /* Use soft
>> timeouts */
>> @@ -137,6 +138,7 @@ struct rpc_task_setup {
>>
>> #define RPC_IS_ASYNC(t) ((t)->tk_flags &
>> RPC_TASK_ASYNC)
>> #define RPC_IS_SWAPPER(t) ((t)->tk_flags & RPC_TASK_SWAPPER)
>> +#define RPC_IS_CORK(t) ((t)->tk_flags & RPC_TASK_CORK)
>> #define RPC_IS_SOFT(t) ((t)->tk_flags &
>> (RPC_TASK_SOFT|RPC_TASK_TIMEOUT))
>> #define RPC_IS_SOFTCONN(t) ((t)->tk_flags & RPC_TASK_SOFTCONN)
>> #define RPC_WAS_SENT(t) ((t)->tk_flags &
>> RPC_TASK_SENT)
>> diff --git a/include/trace/events/sunrpc.h
>> b/include/trace/events/sunrpc.h
>> index 811187c47ebb..e8d6adff1a50 100644
>> --- a/include/trace/events/sunrpc.h
>> +++ b/include/trace/events/sunrpc.h
>> @@ -312,6 +312,7 @@ TRACE_EVENT(rpc_request,
>> { RPC_TASK_TLSCRED, "TLSCRED"
>> }, \
>> { RPC_TASK_NULLCREDS, "NULLCREDS"
>> }, \
>> { RPC_CALL_MAJORSEEN, "MAJORSEEN"
>> }, \
>> + { RPC_TASK_CORK, "CORK"
>> }, \
>> { RPC_TASK_DYNAMIC, "DYNAMIC"
>> }, \
>> { RPC_TASK_NO_ROUND_ROBIN, "NO_ROUND_ROBIN"
>> }, \
>> { RPC_TASK_SOFT, "SOFT"
>> }, \
>> diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c
>> index 86d62cffba0d..4b303b945b51 100644
>> --- a/net/sunrpc/xprt.c
>> +++ b/net/sunrpc/xprt.c
>> @@ -1622,6 +1622,8 @@ xprt_transmit(struct rpc_task *task)
>> if (xprt_request_data_received(task) &&
>> !test_bit(RPC_TASK_NEED_XMIT, &task-
>>> tk_runstate))
>> break;
>> + if (RPC_IS_CORK(task))
>> + break;
>> cond_resched_lock(&xprt->queue_lock);
>> }
>> spin_unlock(&xprt->queue_lock);
>>
>>
>
> This is entirely the wrong place for this kind of control mechanism.
I'm not sure I entirely understand your concern, so bear with
me while I try to clarify.
> TLS vs not-TLS needs to be decided up front when we initialise the
> transport (i.e. at mount time or whenever the pNFS channels are set
> up). Otherwise, we're vulnerable to downgrade attacks.
Downgrade attacks are prevented by using "xprtsec=tls" because
in that case, transport creation fails if either the AUTH_TLS
fails or the handshake fails.
The TCP connection has to be established first, though. Then the
client can send the RPC_AUTH_TLS probe, which is the same as the
NULL ping that it already sends. That mechanism is independent
of the lower layer transport (TCP in this case).
Therefore, RPC traffic must be stoppered while the client:
1. waits for the AUTH_TLS probe's reply, and
2. waits for the handshake to complete
Because an RPC message is involved in this interaction, I didn't
see a way to implement it completely within xprtsock's TCP
connection logic. IMO, driving the handshake has to be done by
the generic RPC client.
So, do you mean that I need to replace RPC_TASK_CORK with a
special return code from xs_tcp_send_request() ?
> Once we've decided that TLS is the right thing to do, then we shouldn't
> declare to the RPC layer that the TLS-enabled transport is connected
> until the underlying transport connection is established, and the TLS
> handshake is done.
That logic is handled in patch 10/15.
Reconnecting and re-establishing a TLS session is handled in
patches 11/15 and 12/15. Again, if the transport's policy setting
is "must use TLS" then the client ensures that a TLS session is in
use before allowing more RPC traffic on the new connection.
--
Chuck Lever
next prev parent reply other threads:[~2022-04-19 18:23 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-18 16:51 [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS Chuck Lever
2022-04-18 16:51 ` [PATCH RFC 01/15] SUNRPC: Replace dprintk() call site in xs_data_ready Chuck Lever
2022-04-18 16:51 ` [PATCH RFC 02/15] SUNRPC: Ignore data_ready callbacks during TLS handshakes Chuck Lever
2022-04-18 16:51 ` [PATCH RFC 03/15] SUNRPC: Capture cmsg metadata on client-side receive Chuck Lever
2022-04-18 16:51 ` [PATCH RFC 04/15] SUNRPC: Fail faster on bad verifier Chuck Lever
2022-04-18 16:51 ` [PATCH RFC 05/15] SUNRPC: Widen rpc_task::tk_flags Chuck Lever
2022-04-18 16:51 ` [PATCH RFC 06/15] SUNRPC: Add RPC client support for the RPC_AUTH_TLS authentication flavor Chuck Lever
2022-04-18 16:51 ` [PATCH RFC 07/15] SUNRPC: Refactor rpc_call_null_helper() Chuck Lever
2022-04-18 16:52 ` [PATCH RFC 08/15] SUNRPC: Add RPC_TASK_CORK flag Chuck Lever
2022-04-19 2:57 ` Trond Myklebust
2022-04-19 18:16 ` Chuck Lever III [this message]
2022-04-19 19:04 ` Trond Myklebust
2022-04-19 19:40 ` Chuck Lever III
2022-04-19 22:08 ` Trond Myklebust
2022-04-20 0:34 ` Chuck Lever III
2022-04-18 16:52 ` [PATCH RFC 09/15] SUNRPC: Add a cl_xprtsec_policy field Chuck Lever
2022-04-18 16:52 ` [PATCH RFC 10/15] SUNRPC: Expose TLS policy via the rpc_create() API Chuck Lever
2022-04-18 16:52 ` [PATCH RFC 11/15] SUNRPC: Add infrastructure for async RPC_AUTH_TLS probe Chuck Lever
2022-04-18 16:52 ` [PATCH RFC 12/15] SUNRPC: Add FSM machinery to handle RPC_AUTH_TLS on reconnect Chuck Lever
2022-04-18 16:52 ` [PATCH RFC 13/15] NFS: Replace fs_context-related dprintk() call sites with tracepoints Chuck Lever
2022-04-18 16:52 ` [PATCH RFC 14/15] NFS: Have struct nfs_client carry a TLS policy field Chuck Lever
2022-04-18 16:52 ` [PATCH RFC 15/15] NFS: Add an "xprtsec=" NFS mount option Chuck Lever
2022-04-19 3:31 ` [PATCH RFC 00/15] Prototype implementation of RPC-with-TLS Trond Myklebust
2022-04-19 16:00 ` Chuck Lever III
2022-04-19 18:48 ` Trond Myklebust
2022-04-19 18:53 ` Chuck Lever III
2022-04-19 20:49 ` Rick Macklem
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=AE1190F4-EDE4-4C2D-94C9-02A5EDAAFBC6@oracle.com \
--to=chuck.lever@oracle.com \
--cc=ak@tempesta-tech.com \
--cc=borisp@nvidia.com \
--cc=linux-cifs@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=linux-nvme@lists.infradead.org \
--cc=netdev@vger.kernel.org \
--cc=simo@redhat.com \
--cc=trondmy@hammerspace.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.