Re: [nfsv4] AD review of draft-ietf-nfsv4-rpc-tls

[Chuck Lever <[e-mail]>]

> On Apr 1, 2020, at 4:27 AM, Magnus Westerlund <[e-mail]> wrote:
> 
> NFSv4 WG,
>  
> Sorry my AD review took longer time than it was supposed to. Anyway here are my comments. And as usual I am not an expert on NFS or RPC. So don’t hesitate to push back or educate me with relevant context if what I comment appears to be just strange.

Magnus, thank you for your thoughts. These all seem like reasonable
questions. I can answer a few of these right now, and as I make
progress towards a fresh revision of the document, the remaining
questions can be addressed by further discussion.


> 1. Section Status of this memo:
>  
>    This document may contain material from IETF Documents or IETF
>    Contributions published or made publicly available before November
>    10, 2008.  The person(s) controlling the copyright in some of this
>    material may not have granted the IETF Trust the right to allow
>    modifications of such material outside the IETF Standards Process.
>    Without obtaining an adequate license from the person(s) controlling
>    the copyright in such materials, this document may not be modified
>    outside the IETF Standards Process, and derivative works of it may
>    not be created outside the IETF Standards Process, except to format
>    it for publication as an RFC or to translate it into languages other
>    than English.
>  
> What material in this document are not provided according to BCP 78 and the TLP? I just wonder if you are over careful, or is something verbatim copied from an old RFC? 

I don't recall exactly, but I did copy from old RFCs in a few spots.
The RPC authentication flavor enum, for example. I should have added
a comment, but neither the XML source nor the git log appears to
indicate exactly why I added that disclaimer. I assume it's because
of the bits of RFC 5531 that are included herein.

How can we resolve this question to everyone's satisfaction?


>  2. Section 3:
>  
>    Note also that the NFS community uses the term "privacy" where other
>    Internet communities use "confidentiality".  In the current document
>    the two terms are synonymous.
>  
> Can this usage of privacy be avoided. Confidentiality is a tool to help provide privacy. So I think you should try to conform to more mainstream usage of privacy and use confidentiality where that is actually what is provided. See later comment also where this distinction becomes relevant.

The NFS community's usage of "privacy" is baked into existing documents,
which is why Section 3 mentions this confusion. I could make that more
clear here by changing it to read:

> Note also that the NFS community has historically used the term "privacy"
> where other Internet communities ...

I read your comment to further recommend that the rpc-tls document adopt
the use of "confidentiality" and entirely shun the use of "privacy". I
can look into doing that, but cannot promise it will be sensible to
replace every last use of "privacy".


> 3. Section 4.1:
>  
>   <CODE BEGINS>
> enum auth_flavor {
>            AUTH_NONE       = 0,
>            AUTH_SYS        = 1,
>            AUTH_SHORT      = 2,
>            AUTH_DH         = 3,
>            AUTH_KERB       = 4,
>            AUTH_RSA        = 5,
>            RPCSEC_GSS      = 6,
>            AUTH_TLS        = 7,
> /* and more to be defined */
>    };
> <CODE ENDS>
>  
> In this particular case it might not be a major issue, but it is recommended that not put the numbers from the IANA registry into to the document until it has been assigned to the document. In cases where one need a number for testing during development, one can in many registries actually issue an early assignment. 
>  
> To me the right way of doing this table would have been to replace the 7 on the AUTH_TLS line with a [IANA_TBA]. And add an RFC-editor note in this section, and the necessary IANA sub-section requesting assignment. And if an number would have been needed for the implementation an early assignment request could have been done, and then converted to a permanent one using the IANA section. 

Two issues:

- I was not previously aware of the IANA registry for RPC auth
flavors, and interestingly no-one else caught this during WGLC!

- As Dave points out, we have some prototypes that already use
the value 7 for this purpose.

Can someone suggest appropriate phrasing to deal with this
situation, and I will update the IANA considerations section to
request the addition of AUTH_TLS.

In fact, such a change might enable the removal of "updates RFC
5531" and the problematic IPR boilerplate.


> 4. Section 4.1: 
>  
> In Section 5.1.1 is clear that the RPC receiving entity should reject requests that are not sent over TLS. However, there appear to be no requirement to actually send all subsequent requests over TLS made in section 4.1. I think such a statement should be added. 
>  
> 5. Section 4.2.1: 
>  
>    A TLS-capable RPC implementation uses
>    GSS channel binding to determine when GSS integrity or privacy is
>    unnecessary.  See Section 2.5 of [RFC7861] for details.
>  
>    RFC 7861 Section 2.5 says: 
>  
>    2.5.  RPCSEC_GSS_BIND_CHANNEL Operation
>  
>    RPCSEC_GSSv3 provides a channel-binding assertion that replaces the
>    RPCSEC_GSSv2 RPCSEC_GSS_BIND_CHANNEL operation.
>  
>    The RPCSEC_GSS_BIND_CHANNEL operation is not supported on RPCSEC_GSS
>    version 3 handles.  If a server receives an RPCSEC_GSS_BIND_CHANNEL
>    operation on an RPCSEC_GSSv3 handle, it MUST return a reply status of
>    MSG_ACCEPTED with an accept_stat of PROC_UNAVAIL [RFC5531].
>  
> I don't see how the details in 2.5 provides an answer to how each peer can determine that TLS Is present by using the GSS channel binding. Maybe this is understandable by someone that understands GSS in detail.  However, I think some more details are needed. 
>  
>  
> 6. Section 5.1.1: 
>    After establishing a TLS session, an RPC server MUST reject with a
>    reject_stat of AUTH_ERROR any subsequent RPC requests over a TLS-
>    protected connection that are outside of a TLS session.
>  
> I assume this is actually bound to a host or a user identity? Because reading the above sentence immediately made me ask how can the RPC server determine that the RPC request is coming from an entity that already has an TLS session? Can you please clarify this question? 
>  
> 7. Section 5.1.2.  Operation on UDP
>    
>     RPC over UDP is protected using DTLS [RFC6347].
>  
> DTLS 1.3 specification appears to have been publication requested so it is not significantly later than your document in the process. Thus, I wonder if it wouldn't be better to require DTLS 1.3 to match version with TLS 1.3? I also recommend to be explicit about version in this sentence. I have also asked the responsible SEC AD about the status of DTLS 1.3 so I hopefully have more input into this question soon.

Good catch. Yes this text should call for DTLS 1.3 or later, if
DTLS 1.3 is nearing readiness. It might have been pretty distant
two years ago when I started authoring rpc-tls.

Please let me know of any additional commentary from the SEC AD.


> 8. Section 5.1.2:
>  
>    As soon as a client
>    initializes a socket for use with an unfamiliar server, it uses the
>    mechanism described in Section 4.1 to discover DTLS support and then
>    negotiate a DTLS session.
>  
> So first of all is the usage of TLS for TCP completely separate from using DTLS over UDP? So having determined TLS support for TCP does not indicate the same for UDP? And is it in any cases possible to skip the initial RPC null request with AUTH_TLS authentication and go directly to negotiate TLS after support has been determined with a server? 
>  
> 9. Section 6. 
>  
> Should there be an RFC-editor note that the whole of Section 6 should be remove prior to publication, or do you intended to have this section included in the RFC? I do note that RFC 7942 do indicate removal of this section.

The XML source for this document contains this comment preceding the
Implementation Status section:

> <!-- RFC Editor:
> Please remove the following section and the reference to RFC 7942
> before this document is published.
> -->


I hope that is adequate.


> 10. Section 7: 
>  
> Privacy leakage due to STARTLS procedure. So what is sent in clear text over the TCP connection prior to the TLS session establishment? Is there information here that provides any information that enable tracking of client, or the user on the client? If there are anything that may be sent that could allow tracking that should be mentioned in the security consideration.
> 
>  
> 11. Section 8: 
>  
> ALPN label. 
>  
> So the IANA section requests the ALPN label "sunrpc" however the document fails to discuss how this is to used.

The ALPN label is part of the TLS protocol. This might need a bit
of text to explain that context and refer the reader to the TLS
document that explains its usage. But maybe more is needed?

Maybe Lars E. can comment on what might be necessary in this
section.


> If it is to be used, then I think there need to be a requirement on including it and verifying that it is present. I also wonder if the WG expect that this label can be used also for a future TLS based solution that requires usage of TLS, or if you actually need to have a separate label between them to identify on TLS Level the difference in policy and intention of the request by the client. 
>  
> 12. Section 8.
>  
> I am missing a sub-section requesting the assignment of the AUTH_TLS value from the 
> RPC Authentication Flavor Numbers registry. What I can see that request has not yet been named as value 7 is unassigned. 
>  
> https://www.iana.org/assignments/rpc-authentication-numbers/rpc-authentication-numbers.xhtml#flavor

I agree, it's missing. See above.


> 13. Requirement on implementation
>  
> Should this document actually update any or all of the versions of NFS 4 to mandate implementation support? From the WG's perspective doesn't it make sense to start demand implementation support. The mechanism is clearly opportunistic in its establishment, however the goal here needs to be to get support in as many places is as possible. Thus, sending a clearer signal that NFS 4.x servers and client are expected to support this should be sent. If not can you clarify what the concerns are? Because we are going to get this question again in the IESG evaluation. 
>  
> To me the reasonable plan towards always used transport security (something I expect the full updates, for example of NFS v4.1 to require) is to require implementation but not usage now. Then next step to require its usage.

Dave covered this in his e-mail reply earlier in this thread. What I
know is that, because rpc-tls is an RPC-layer document, we are
planning separate NFS-specific document(s) to address these issues,
as is appropriate to the more "cemented" status of those minor versions.

We should discuss this during our interim meeting. I will add it to
my slides.


--
Chuck Lever