Re: [nfsv4] Benjamin Kaduk's Discuss on draft-ietf-nfsv4-rpc-tls-08: (with DISCUSS and COMMENT)

[Benjamin Kaduk <[e-mail]>]

On Mon, Aug 31, 2020 at 11:06:51AM -0400, Chuck Lever wrote:
> Hi-
> 
> I've now addressed Roman's DISCUSS/COMMENT position. For reference, the
> current Editor's Copy with these changes can be found here:

Hmm, not quite -- the RFC 5487 ciphers are not compatible with TLS 1.3,
since TLS 1.3 redefines what a ciphersuite is (which also means that TLS
1.3 provides native support for PSK handshakes), so you still need to
update Section 5.2.2 of this draft to refer to RFC 8446, removing the 4279
and 5487 references.

> https://chucklever.github.io/i-d-rpc-tls/draft-ietf-nfsv4-rpc-tls.html
> 
> In this e-mail I will try to address some of the simpler-to-manage
> DISCUSS comments from your position. I plan to address the more complex
> issues in subsequent e-mails.
> 
> I will also follow up with questions and proposals for addressing
> COMMENT points in subsequent e-mails.

Sure; these messages get to be kind of a marathon if we don't split things
up.

> 
> > On Jul 8, 2020, at 2:56 AM, Benjamin Kaduk via Datatracker <[e-mail]> wrote:
> > 
> > Benjamin Kaduk has entered the following ballot position for
> > draft-ietf-nfsv4-rpc-tls-08: Discuss
> > 
> > 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-rpc-tls/
> > 
> > 
> > 
> > ----------------------------------------------------------------------
> > DISCUSS:
> > ----------------------------------------------------------------------
> > 
> > I support Roman's Discuss points.
> 
> I believe I've now addressed his DISCUSS points to his satisfaction,
> and welcome your review of the new version of the document at the
> link above.
> 
> 
> > (1) I don't think that the claim in Section 4.2 that "[b]oth RPC and TLS have
> > peer and user authentication" is correct, at least given my understanding of
> > those terms.  Using this document's definition of RPC "peer authentication"
> > as analogous to TLS server authentication, the functionality that TLS calls
> > "mutual authentication" is more analogous to RPC client authentication,
> > though it is sometimes repurposed for use for user authentication, with
> > concommittant bad user experience.  This analogy does not seem critical to
> > the mechanisms of this document, so I believe we should remove or modify it.
> 
> Perhaps it's a matter of terminology. RPCSEC_GSS "peer" authentication
> is truly mutual. For AUTH_SYS, both peers identify themselves by a name
> string in the RPC headers, though this is not cryptographically strong
> and thus is typically ignored by receivers.
> 
> I'm open to specific suggestions about how to modify the text.

I think it is a matter of terminology, yes -- RPCSEC_GSS has GSS
credentials for the user, specifically, but the TLS client credentials are
for the client, not the user, so it is misleading to say that TLS has user
authentication (in this context).

My suggestion would be something like:

Both RPC and TLS have provisions for both communication endpoints to
authenticate to each other, with some overlap in the authentication
semantics between RPC and TLS.

> 
> > (2) The mention of using RPCSEC_GSS with GSS channel bindings to TLS is
> > quite underspecified. Unfortunately, this is largely the fault of other
> > specifications, but we have to deal with the fallout.  On first glance (but
> > subject to further clarification/change), it seems like we should:
> 
> 
> I'm deferring a response to (2) until I've reviewed the kitten documents
> you've mentioned. IMO introducing specific instructions for handling GSS
> channel binding is appropriate, and I intend to propose text in a
> subsequent e-mail.

Okay, thanks.

> 
> > (3) Please check this reference in Section 5.1.1:
> > 
> >   Reverse-direction operation occurs only on connected transports such
> >   as TCP (see Section 2 of [RFC8166]).  [...]
> > 
> > It seems likely that RFC 8167 was intended...
> 
> I agree. I'll fix this.
> 
> 
> > (4) I don't think it's particularly safe to suggest that non-protected RPCs
> > should be exchanged on the same 5-tuple that just terminated a DTLS
> > association, since neither DTLS nor UDP provide in-order delivery, so there
> > is ambiguity as to whether a datagram should be interpreted as DTLS
> > protected or not.  This is particularly problematic in the face of the three
> > different DTLS record headers (DTLSPlaintext, DTLSCiphertext(full), and
> > DTLSCiphertext(minimal)) with something like 10 or 11 different possible
> > values for the first byte that might be in flight, with limited "magic
> > number" verification fields available.  I think I need some input from the
> > TSV ADs about what the options are, though -- while a cooling-off period
> > might be fine if an ephemeral port is in use, it seems problematic for cases
> > where fixed port numbers are used for both source and destination.
> 
> This is a more complex issue, but I'm replying here to inquire if
> you've yet been able to circle back with the TSV ADs. Additional
> input is welcome.

The main output of my conversation with the TSV ADs was that we wanted to
get your (and the WG's) thoughts about how RPC is actually used, both for
NFSv4 and elsewhere.  If there are not any known/current deployments that
would be affected, then we can probably just document the risk and move on,
but if there are existing deployments that could trigger such a scenario,
we'll need to think through things more carefully.

> 
> > (5) Section 5.2.1 requires that:
> > 
> >   *  Implementations SHOULD indicate their trusted Certification
> >      Authorities (CAs).
> > 
> > Indicate to whom?
> 
> The text is vague, but I assume based on context that the RPC-over-TLS
> implementation indicates the list of trusted CAs to the RPC server to
> make final decisions about authorization.

I'm still not entirely sure I am understanding properly.  This is sounding
(in some sense) like "one software component indicates to another software
component the list of trust anchors", but whether this is an instruction
("use these") or a notification ("this is what I did") could be clarified.

> 
> > (6) The usage of RFC 6125 procedures in Section 5.2.1 seems counter to its
> > intent.
> 
> I'm deferring a response to (6) until I've reviewed RFC 6125 again.

Okay.

> 
> > (7) Section 5.2.1 uses the phrase "renegotiate the TLS session".
> > Renegotiation is not defined or allowed for TLS 1.3; generally one would
> > need to either remember the presented certificate and re-run the validation
> > process on it or shutdown the TLS connection and make a new one, though in
> > theory one could try to define a mechanism using post-handshake
> > authentication.  (I don't recommend the latter, though; it's not widely
> > implemented/used.)
> 
> OLD:
> 
> implementations MAY renegotiate the TLS session to reassess the
> connecting peer’s continued authorization.
> 
> NEW:
> 
> implementations MUST terminate the TLS session.

That is okay from the perspective of failing safe, but may be overly
restrictive.  I think it would be okay, if an implementation had the
ability to do so, to merely reevaluate the certificate originally presented
in the context of the new configuration.
Terminating the TLS session is a disruptive event, and this requirement
would require us to boot everyone's session every time the CA/CRL
configuration changes -- that is giving operators a huge disincentive to
ever change the CA/CRL configuration, which is arguably bad for the overall
security of the system.

> 
> > (8) Can we clarify the status of DNSSEC (or DANE) requirements? Section 1
> > assumes that support for DNSSEC is already available in an RPC
> > implementation, but Section 7.1.1 says that "Clients [sic] implementors can
> > choose from" a list of things including DANE TLSA records.  Why would we
> > require DNSSEC support but not using the records if they're present?
> 
> 
> I'm deferring a response to (8) until I've reviewed specifications of
> other consumers of DNSSEC/DANE.

Okay.

> 
> > (9) I agree with Roman('s comment) that Sections 5.2.2 and 5.2.4 should give
> > a minimum amount of information to be exposed to the administrator for
> > implementing the trust mode.
> 
> Section 5.2.2 (Certificate Fingerprints) has been removed because of a
> variety of unresolvable issues.
> 
> Section 5.2.4 (Token Binding) has been removed because there is currently
> no active Internet-Draft or plan to support token binding in TLS 1.3.
> 
> We are leaving these areas open for future standards work.

That works for me.  (I do note that, with respect to certificate
fingerprints, we had some discussion in SAAG at IETF 108 that's continuing
on the mailing list, starting at
https://mailarchive.ietf.org/arch/msg/saag/Md1Yiier0B0WXqN1suRU4r3p9zc/ ,
but please don't feel obligated to read the thread.)

Thanks,

Ben