Re: AD review of draft-ietf-nfsv4-rpcrdma-cm-pvt-data

On Wed, 2019-11-06 at 17:00 -0500, Chuck Lever wrote:
> Hi Magnus-
> 
> > On Nov 5, 2019, at 4:46 AM, Magnus Westerlund <
> > [email protected]> wrote:
> > 
> > On Mon, 2019-11-04 at 11:36 -0500, Chuck Lever wrote:
> > > Hi Magnus, thanks for your review and moving forward with the AD write-up.
> > > 
> > > > On Nov 4, 2019, at 9:56 AM, Magnus Westerlund <
> > > > [email protected]> wrote:
> > > > 
> > > > Hi,
> > > > 
> > > > I have started my AD review. It was such a short document that I thought
> > > > I
> > > > read through it to figure out what more I need to read. So I have not
> > > > yet
> > > > read like RFC 8166. So from a context of having little context
> > > > understanding
> > > > the below are my comments and questions. I think it may be faster if you
> > > > help answering them then I read a lot of documents and still have
> > > > question
> > > > marks. 
> > > > 
> > > > 
> > > > 	1. So this private data field is only existing in Infinite Band
> > > > Connection manger? So what this document is defining a structure to a
> > > > field
> > > > which currently don’t have any structure but are part of a protocol
> > > > defined
> > > > by another body? 
> > > > 		a. Have this work been formally agreed with IBTA? 
> > > > 		b. Or is the relationship to the connection manager used
> > > > to
> > > > establish the RPC over RDMA different, if that is the case it needs to
> > > > be
> > > > better explained. 
> > > 
> > > The private data field is for use by Upper Layer Protocols. It's existence
> > > is defined by the IBTA, but not its content. It's opaque to the CM, hence
> > > the name "private".
> > > 
> > > Both iSER and iWARP itself (both IETF-defined) can place data in this
> > > field.
> > > That's why there
> > > is a 32-bit format ID field: if that doesn't match the constant defined in
> > > this document, that means some other ULP is using the private data field,
> > > and its contents are then ignored by RPC-over-RDMA.
> > 
> > So with new text as proposed in other email to clarify that this extensions
> > apply to several different protocols will help making it clear that this is
> > useful for IETF to define and not stepping on anyone's particular toes. 
> > 
> > > 
> > > 
> > > > 	3. Section 5: Is there only going to be one object of private
> > > > data
> > > > forever in the relevant communication? If not, why isn’t this a well-
> > > > specified TLV so that unknow type entries can be skipped to the nest
> > > > object?
> > > 
> > > As I understand it, the private data field is used by one ULP at a time.
> > > I'm not aware of a TLV. There are already existing ULP implementations
> > > that do not assume a TLV, so that might be a tough sell.
> > > 
> > 
> > That helps a bit, but basically what we end up with is that the ULP need to
> > cordinate between the end-point about which private data format(s) it will
> > use.
> > And if is to include multiple formats it need to know that the peer can
> > delimit
> > all of them. That is what I see as the issue. Maybe this assumption and
> > responsibility of the ULP can be made clearer in the document. 
> > 
> > > 
> > > > 	4. Even if there ever is going to be one entry, can you be more
> > > > formal
> > > > in description of what is a valid message using another format
> > > > identifier,
> > > > is that only the 32-bit field of the format Identifier, or also the
> > > > version.
> > > 
> > > I can cite the use of this field by other protocols, if that would help.
> > 
> > As I said yes. 
> > > 
> > > 
> > > > 	5. Isn’t the version field just unnecessary? If one needs
> > > > version using
> > > > another format identifier is more easy than the identifier + version
> > > > construct.
> > > 
> > > It's possibly superfluous. But:
> > > 
> > > - the two fields serve different purposes, even though they could be
> > > combined.
> > > - there's already an implementation that uses both fields.
> > 
> > So, I understand that your down the road on this one and you don't need to
> > implement any change. But, it was a reflection I had after commenting on
> > issue
> > 4). 
> > 
> > > 
> > > 
> > > > 	6. Section 6: The IANA consideration is confusing. If the
> > > > specification
> > > > for a CM private data format requires IESG approval, then why have the
> > > > expert review policy. In that case one can simply use the IESG approval
> > > > policy instead. However, requiring any type of IESG approval here appear
> > > > to
> > > > be a way to high bar. I think expert review is likely appropriate, but
> > > > the
> > > > guidance to the Experts needs to be clearer. For example what happens if
> > > > IBTA want to use this to add some format?
> > > 
> > > I would be OK with Expert Review only, but:
> > > 
> > > 1. I will need suggestions about expert guidance. Will look at other RFCs.
> > 
> > Basically the WG can specify what requirements you consider needed for a
> > registration request. Publicly availble specification, contact information,
> > what
> > criterias the expert should make judgment on. Like if there are sensitive
> > information or not. Etc. 
> > 
> > > 2. IMO WG consensus is needed on the replacement text of this section..
> > 
> > Sure, and if the WG want to choice IESG approval or Specification Required
> > as
> > policy for the registry that is also fine with me. However, I am currently
> > not
> > fine with the strange mix that is currently documented. 
> > 
> > > 
> > > 
> > > > 	7. Section 7: The security consideration. I am missing any
> > > > discussion
> > > > about the security requirements that the actual extension. It appears
> > > > that
> > > > integrity and source authenticity are required for safe operation of
> > > > these
> > > > two extensions.
> > > 
> > > Integrity is a property of RDMA Reliable Connection transports.
> > > 
> > > Not clear about the source authenticity requirement. Can you elaborate?
> > 
> > What I am trying to say, is that security consideration sections should make
> > a
> > security requirement analysis. And what I can see this private data when
> > delivered to receiver, the receiver need to know that the data was not
> > modified
> > and it needs to know from whom it came. Thus integrity and source
> > authentication. Then you can simply add a statement that the protocol you
> > know
> > to carry this field will provide that security service. 
> > 
> > The other aspect to consider is really if one can get any impact on the peer
> > by
> > being malicous in setting these fields. For example can I get the peer
> > entity to
> > consume more resources? To my understanding the answer is partially yes, but
> > not
> > without having to similarily promise to allocate resources. And if I don't
> > do
> > things will fail. And the point is that there is other methods that will be
> > more
> > efficient to do Denile of Service attacks on the peer in the protocol. 
> > 
> > It is usually good to document what aspects of attacks that has been
> > considered
> > and what level of an issue they are on.
> 
> The latest:
> 
> 
https://protect2.fireeye.com/v1/url?k=86105e9f-da9a7c49-86101e04-0cc47ad93dcc-4cde713b365d5029&q=1&e=1de128de-db49-4c9d-8b49-89fbe4823d29&u=https%3A%2F%2Fchucklever.github.io%2Fi-d-rpcrdma-cm-pvt-data%2Fdraft-ietf-nfsv4-rpcrdma-cm-pvt-data.html
> 
> And a convenience URL for diffing with -04:
> 
> 
https://tools.ietf.org/rfcdiff?url1=https://tools.ietf.org/id/draft-ietf-nfsv4-rpcrdma-cm-pvt-data.txt&url2=https://chucklever.github.io/i-d-rpcrdma-cm-pvt-data/draft-ietf-nfsv4-rpcrdma-cm-pvt-data.txt
> 
> 
> I've tried to address the following issues that you called out after your
> attempt at an AD write-up of -04:
> 
> - Now a Proposed Standard updating RFC 8166
> - Replaced the stale link to the IB specification
> - Additional text explaining precedents and the purpose of the Format
> Identifier
> - Revised the IANA Considerations section to remove "specification required"
> language
> - Revised the Security Considerations section
> 
> I can take this further if you would like more changes before the next steps.
> Otherwise I can provide a draft-ietf-nfsv4-rpcrdma-cm-pvt-data-05.xml for you
> to submit manually.
> 
> 
> --
> Chuck Lever
> 
> 
> 
-- 
Cheers

Magnus Westerlund 


----------------------------------------------------------------------
Networks, Ericsson Research
----------------------------------------------------------------------
Ericsson AB                 | Phone  +46 10 7148287
Torshamnsgatan 23           | Mobile +46 73 0949079
SE-164 80 Stockholm, Sweden | mailto: [email protected]
----------------------------------------------------------------------

_______________________________________________
nfsv4 mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/nfsv4