[nfsv4] Benjamin Kaduk's No Objection on draft-ietf-nfsv4-rpcrdma-cm-pvt-data-07: (with COMMENT)

[Benjamin Kaduk via Datatracker <[e-mail]>]

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.

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"?

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.

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.

   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).

   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.


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.

   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.

Section 7

Should the registry also include the length of the private data?

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.