Re: [nfsv4] Benjamin Kaduk's No Objection on draft-ietf-nfsv4-rpcrdma-cm-pvt-data-07: (with COMMENT)
Benjamin Kaduk <kaduk@mit.edu> Thu, 20 February 2020 19:52 UTC
Return-Path: <kaduk@mit.edu>
X-Original-To: nfsv4@ietfa.amsl.com
Delivered-To: nfsv4@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id C32B6120072; Thu, 20 Feb 2020 11:52:04 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -4.199
X-Spam-Level:
X-Spam-Status: No, score=-4.199 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id oHh8ZvT3-8n5; Thu, 20 Feb 2020 11:52:02 -0800 (PST)
Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id DE75B120018; Thu, 20 Feb 2020 11:52:01 -0800 (PST)
Received: from kduck.mit.edu ([24.16.140.251]) (authenticated bits=56) (User authenticated as kaduk@ATHENA.MIT.EDU) by outgoing.mit.edu (8.14.7/8.12.4) with ESMTP id 01KJptVL003726 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 20 Feb 2020 14:51:58 -0500
Date: Thu, 20 Feb 2020 11:51:55 -0800
From: Benjamin Kaduk <kaduk@mit.edu>
To: Chuck Lever <chuck.lever@oracle.com>
Cc: The IESG <iesg@ietf.org>, draft-ietf-nfsv4-rpcrdma-cm-pvt-data@ietf.org, Tom Haynes <loghyr@gmail.com>, Spencer Shepler <spencer.shepler@gmail.com>, Brian Pawlowski <beepee@gmail.com>, nfsv4-chairs@ietf.org, nfsv4@ietf.org
Message-ID: <20200220195155.GJ97652@kduck.mit.edu>
References: <158216101788.17661.6926758160404035704.idtracker@ietfa.amsl.com> <E718C1B8-B96B-4A64-8B92-C4ABEC7B5E5F@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <E718C1B8-B96B-4A64-8B92-C4ABEC7B5E5F@oracle.com>
User-Agent: Mutt/1.12.1 (2019-06-15)
Archived-At: <https://mailarchive.ietf.org/arch/msg/nfsv4/n5Sqq2NKU1YSlbZSZYMT_blqpdA>
Subject: Re: [nfsv4] Benjamin Kaduk's No Objection on draft-ietf-nfsv4-rpcrdma-cm-pvt-data-07: (with COMMENT)
X-BeenThere: nfsv4@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: NFSv4 Working Group <nfsv4.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/nfsv4>, <mailto:nfsv4-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/nfsv4/>
List-Post: <mailto:nfsv4@ietf.org>
List-Help: <mailto:nfsv4-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/nfsv4>, <mailto:nfsv4-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 20 Feb 2020 19:52:05 -0000
Hi Chuck, On Thu, Feb 20, 2020 at 10:13:26AM -0500, Chuck Lever wrote: > Hi Ben, thanks for your comments! Responses and questions below. > > > > On Feb 19, 2020, at 8:10 PM, Benjamin Kaduk via Datatracker <noreply@ietf.org> wrote: > > > > Benjamin Kaduk has entered the following ballot position for > > draft-ietf-nfsv4-rpcrdma-cm-pvt-data-07: No Objection > > > > When responding, please keep the subject line intact and reply to all > > email addresses included in the To and CC lines. (Feel free to cut this > > introductory paragraph, however.) > > > > > > Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html > > for more information about IESG DISCUSS and COMMENT positions. > > > > > > The document, along with other ballot positions, can be found here: > > https://datatracker.ietf.org/doc/draft-ietf-nfsv4-rpcrdma-cm-pvt-data/ > > > > > > > > ---------------------------------------------------------------------- > > COMMENT: > > ---------------------------------------------------------------------- > > > > I agree with Alissa. > > I assume by this you mean that you are seconding the requested change > to the registry policy to "Specification Required" ? I have that change > lined up for the Editor's copy waiting for confirmation that it would > not require another WGLC. See my reply yesterday to Alissa. Yes, that was it, and sounds good. > AI: Chuck > > > > Section 4 > > > > For RPC-over-RDMA version 1, the CM Private Data field is formatted > > as described in the following subsection. RPC clients and servers > > use the same format. If the capacity of the Private Data field is > > too small to contain this message format, the underlying RDMA > > transport is not managed by a Connection Manager, or the underlying > > RDMA transport uses Private Data for its own purposes, the CM Private > > Data field cannot be used on behalf of RPC-over-RDMA version 1. > > > > How will an implementation know if "the underlying RDMA transport uses > > Private Data for its own purposes"? > > That's what the Format Identifier is for. It indicates where the > RPC/RDMA-specific data sits in the CM Private Data buffer. A > compliant RPC/RDMA implementation does not assume it is the only > user of the CM Private Data. Sure ... but now we have some text saying that other uses of CM Private Data can't coexist with RPC-over-RDMA v1 ("the underlying RDMA transport uses Private Data for its own purposes [...] cannot be used on behalf of RPC-over-RDMA version 1") that seems in conflict with the goal of using the Format Identifier "magic word" to identify the RPC-over-RDMA private data within a larger assembly. So we should probably try to resolve the apparent inconsistency. > The "Interoperability Amongst RDMA Transports" section is intended > to explain this. I'm happy to clarify that text if needed. > > > > Section 5 > > > > Although it is possible to reorganize the last three of the eight > > bytes in the existing format, extended formats are unlikely to do so. > > New formats would take the form of extensions of the format described > > in this document with added fields starting at byte eight of the > > format and changes to the definition of previously reserved flags. > > > > I would suggest making it a (mandatory) invariant of this format to > > retain these last three bytes' interpretation, requiring the use of a > > different "magic word" for future versions that need to diverge from it. > > The current text does not really give an implementation anything that it > > can rely on. > > Our experience with RPC/RDMA version 1 itself is that the only field > that needs to be at a fixed location is the Version field. In fact, > we've found the need to change the semantics and data type of one or > more of the first four fields in the RPC/RDMA version 1 header. > > We've had to work around the requirement that they remain the same in > all future versions of RPC/RDMA. The text in Section 5 reflects that > experience. > > Can you elaborate on the risks that might arise if we were to keep > the existing text? Not really; it just seemed like it would make future implementations simpler if they could make that assumption (and the 32-bit Format Identifier should provide enough flexibility to freely make new formats if needed). But the current text doesn't seem flawed, to me. > > > Section 6 > > > > The RPC-over-RDMA version 1 protocol framework depends on the > > semantics of the Reliable Connected (RC) queue pair (QP) type, as > > defined in Section 9.7.7 of [IBA]. The integrity of CM Private Data > > > > It's interesting to see such a reference to [IBA], when IIUC the RFC > > 8166 protocol is not limited to Infiniband as the underlying transport. > > Correct, RPC/RDMA can operate on iWARP and RoCE as well. However: > > - both of those RDMA fabric types also support RC QPs, and > - [IBA] appears to be the only publication where "RC QP type" > is defined, though I could be wrong. > > I don't read this text as suggesting that RPC/RDMA cannot operate > on those alternate fabrics, but perhaps I am "too close to the > code". I don't read it that way, either; I was just noting that it was surprising that this is the only reference available. If it is, then it is, though, so no need to change anything. > > > Additional analysis of RDMA transport security appears in the > > Security Considerations section of [RFC5042]. That document > > > > nit: the actual analysis isn't *in* the security considerations section > > (but is referenced from it). > > I can revisit RFC 5042 and extract those references. I don't think you need to bother pulling in the specific section references; I was just suggesting a minor tweak to something like "are indicated by the Security Considerations section of [RFC5042]". > AI: Chuck > > > > Improperly setting one of the fields in a version 1 Private Message > > can result in an increased risk of disconnection (i.e., self-imposed > > Denial of Service). There is no additional risk of exposing upper- > > layer payloads after exchanging the Private Message format defined in > > the current document. > > > > I'm not entirely sure where or how one might have expected such > > additional exposures to occur. > > Agreed. However, I was asked to make this statement here to make it > clear that the proposed CM Private Data format does not put data > further at risk. I'm open to suggested rewording. I think the key points are: "Exchanging the data contained in the Private Message format defined in this document does not directly expose any upper-layer payloads to an attacker; furthermore, the behavior changes that may occur as a result of processing the Private Data do not introduce any new risk of exposure of upper-layer payload data to an attacker, since the RDMA transport protection continues to apply, and misconfiguration results only in disconnection". > > > We should probably mention the risk that some (other) CM-private data > > item might inadvertenly produce in its payload the "magic number" that > > we use to identify this protocol's data structure. I *think* (but > > please confirm) that erroneously doing so would lead only to (likely) > > RDMA-channel disconnection and could not introduce (e.g.) data > > corruption. > > Yes, my expectation is that it would have the same consequences as > someone tampering with the RPC/RDMA CM Private Data. > > I will see about mentioning this failure mode in the text. Thanks. > AI: Chuck > > > > In addition to describing the structure of a new format version, any > > document that extends the Private Data format described in the > > current document must discuss security considerations of new data > > items exchanged between connection peers. > > > > In a similar vein, future extensions should consider what the risks of > > erroneously identifying "random" data as the new format would be. > > AI: Chuck > > > > Section 7 > > > > Should the registry also include the length of the private data? > > I can add that. > > AI: Chuck > > > > Similarly to the previous section's comments, should prospective > > registrations also be analyzing the risks to their protocol of > > interpreting "random" data as the data structure (as would happen upon > > an inadvertent match of the "magic number")? > > > > Section 7.1 > > > > The new Reference field should contain a reference to that > > documentation. The DE can assign new Format Identifiers at random as > > long as they do not conflict with existing entries in this registry. > > > > Random may not be the best choice for this, if there are ways to produce > > values that are less-likely-than-random to occur inadvertently in the > > payload of any of the registered formats. > > I chose the current Format Identifier for RPC/RDMA CM Private Data > by extracting the middle bits from a randomly-generated UUID. That's > why the text uses the "random" wording. > > IMO we want to avoid choosing values like 0x00000007, precisely because > they are likely to look like a payload. I think we are agreeing here. I agree :) > Another sentence might be necessary to explain that a new Identifier > should be chosen so that it will not look like a payload of other > registered format. That might be challenging for DEs to carry out in > practice. I don't think we need to require the DEs to perform an exhaustive analysis that any given value is provably not possible to occur in a compliant implementation of any other extant payload type (that sounds really hard!). Mostly I just want it written down that that's a risk to keep in mind when making new allocations. > AI: Chuck Thanks, Ben
- [nfsv4] Benjamin Kaduk's No Objection on draft-ie… Benjamin Kaduk via Datatracker
- Re: [nfsv4] Benjamin Kaduk's No Objection on draf… Chuck Lever
- Re: [nfsv4] Benjamin Kaduk's No Objection on draf… Benjamin Kaduk
- Re: [nfsv4] Benjamin Kaduk's No Objection on draf… Chuck Lever
- Re: [nfsv4] Benjamin Kaduk's No Objection on draf… Benjamin Kaduk
- Re: [nfsv4] Benjamin Kaduk's No Objection on draf… Tom Talpey
- Re: [nfsv4] Benjamin Kaduk's No Objection on draf… Chuck Lever
- Re: [nfsv4] Benjamin Kaduk's No Objection on draf… Chuck Lever
- Re: [nfsv4] Benjamin Kaduk's No Objection on draf… Tom Talpey
- Re: [nfsv4] Benjamin Kaduk's No Objection on draf… Chuck Lever