Subject: [nfsv4] Roman Danyliw's Discuss on draft-ietf-nfsv4-rpc-tls-08: (with DISCUSS and COMMENT)

From: Roman Danyliw via Datatracker <[email-masked]>
Date: Mon, 06 Jul 2020 20:24:15 -0700

Roman Danyliw 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:
----------------------------------------------------------------------

** Despite Section 5.0 stating that only TLS v1.3+ can be used, there are two
references to TLS v1.2 mechanisms:

-- Section 5.0. Per “Implementations MUST support certificate-based mutual
authentication.  Support for TLS-PSK mutual authentication [RFC4279] is
OPTIONAL”.  Shouldn’t Section 2.2.2 or 4.2.11 of RFC8446 be used instead?

-- Section 5.2.4.  The token binding mechanism suggested here, RFC8471, only
applies to TLS v1.2.  The expired draft-ietf-tokbind-tls13 provides the TLS
v1.3 mechanism.

** Section 7.4.  Per “When using AUTH_NULL or AUTH_SYS, both peers are required
to have DNS TLSA records and certificate material …”, what is “certificate
materials”?  Can this guidance please be clarified (and perhaps related to the
options specified in Section 5.2).

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Thank you for addressing the early SECDIR review items (and thank you Derrell
Piper and Alan Alan DeKok for doing them)

** Section 1.  Per “Encryption by Default: Transport encryption can be enabled
without … generating additional keying materials”, I’m not sure that this is
accurate if the server and clients need to be keyed with certificates or PSK
for TLS

** Section 4.1. Per “Policy settings on the RPC-over-TLS-enabled peer determine
whether RPC operation continues without the use of TLS or RPC operation is not
permitted.”, I had trouble parsing this sentence.

** Section 4.1.  Per “RPC operation can continue …”, it might be worth
repeating what was stated earlier that  “[t]he RPC operation can continue if
permitted by local policy …”

** Section 5.2.1.  Per “For services accessed by their network identifiers
(netids) and universal network addresses (uaddr), the iPAddress subjectAltName
SHOULD be present in the certificate and must exactly …”, why not a normative
MUST?

** Section 5.2.1.  Per “For example, if the Issuer is not in a trusted set of
Issuers, the RPC server may decline to perform RPC transactions with this
client.”, wouldn’t the TLS connection fail in this case and not even get to
whatever authorization logic the RPC server might have?

** Section 5.2.1.  Per “As a suggestion, at least the following parameters of
the X.509 client certificate SHOULD be exposed … Originating IP address”, how
exactly should this IP address be exposed in the certificate, or is this
intended to be the IP address of the peer which presented the certificate?

** Section 5.2.2 and 5.2.4.  Both 5.2.1 and 5.2.3 described what information
should be exposed by implementations.  These sections omit that information. 
For example, I would have expected Section 5.2.4 to discuss Token Binding IDs

** Section 5.2.2.  Is there any MTI guidance on the kinds of digests to support
for these fingerprints?

** Section 5.2.4.  Are there any MTI parameters for the token binding to
specify?

** Section 7.3.  Per “… it is RECOMMENDED that when AUTH_SYS is used, every RPC
client should present authentication materials to RPC servers …”, is this the
same thing as saying that when AUTH_SYS is used a mutual authentication
TLS-scheme is RECOMMENDED?  I concur with this approach.  I would just
recommended saying that explicitly.

** Section 7.4.  Is there a reason not to use normative language in this
section?  Specifically, RECOMMENDED for the first bullet and SHOULD for the
second.

** Editorial Nits:
-- Section 5.  Typo. s/Similary/Similarly/

-- Section 7.1.2. Typo. s/connnection/connection/

---

From: Chuck Lever <[email-masked]>
Date: Tue, 7 Jul 2020 11:15:56 -0400

Hi Roman-

Thanks for your review and comments. If I may, I'd like to handle
the DISCUSS first, and then respond to the COMMENTs in a separate
reply.

> On Jul 6, 2020, at 11:24 PM, Roman Danyliw via Datatracker <[email-masked]> wrote:
> 

Good catch!

> OPTIONAL”.  Shouldn’t Section 2.2.2 or 4.2.11 of RFC8446 be used instead?

In fact Section 5.2.3 already cites RFC8446 Section 2.2. I propose changing
Section 5.0 as follows:

OLD:

   *  Implementations MUST support certificate-based mutual
      authentication.  Support for TLS-PSK mutual authentication
      [RFC4279] is OPTIONAL.  See Section 4.2 for further details.

NEW:

   *  Implementations MUST support certificate-based mutual
      authentication.  Support for PSK mutual authentication is
      OPTIONAL; see Section 5.2.3 for further details.

> applies to TLS v1.2.  The expired draft-ietf-tokbind-tls13 provides the TLS

Potential replacement:

OLD:

5.2.4.  Token Binding

   This mechanism is OPTIONAL to implement.  In this mode, a token
   uniquely identifies the RPC peer.

   Versions of TLS after TLS 1.2 contain a token binding mechanism that
   is more secure than using certificates.  This mechanism is detailed
   in [RFC8471].

NEW:

5.2.4.  Token Binding

   This mechanism is OPTIONAL to implement.  In this mode, a token
   uniquely identifies the RPC peer.  The TLSv1.3 token binding
   mechanism is detailed in [I-D.ietf-tokbind-tls13].

Another option would be to remove this section.

> options specified in Section 5.2).

Potential replacement:

OLD:

   *  When using AUTH_NULL or AUTH_SYS, both peers are required to have
      DNS TLSA records and certificate material, and a policy that
      requires mutual peer authentication and rejection of a connection
      when host authentication fails.

NEW:

   *  When using AUTH_NULL or AUTH_SYS, both peers are required to have
      DNS TLSA records, keys with which to perform mutual peer
      authentication using one of the methods described in Section 5.2,
      and a security policy that requires mutual peer authentication and
      rejection of a connection when host authentication fails.

--
Chuck Lever

---

From: Chuck Lever <[email-masked]>
Date: Tue, 7 Jul 2020 13:20:57 -0400

Hi Roman -

I'll address Section 1 and 4 in this reply, and follow up with the other
Sections in subsequent replies.

> On Jul 6, 2020, at 11:24 PM, Roman Danyliw via Datatracker <[email-masked]> wrote:

The full list item reads:

   Encryption By Default:  Transport encryption can be enabled without
      additional administrative tasks such as identifying client systems
      to a trust authority, generating additional keying material, or
      provisioning a secure network tunnel.

All modes of RPC-on-TLS operation require a server key of some type.
The "additional keying material" in this case refers to client keys.
Section 4.2 specifies a mode of operation where RPC-over-TLS-enabled
clients can initiate a TLS session with an RPC-over-TLS-enabled server
even though the clients do not have a certificate.

This is meant to address deployment scalability issues with GSS, which
requires a keytab on every participating peer.

Perhaps instead of "identifying client systems to a trust authority,
generating additional keying material," the text could say "identifying
client systems to a trust authority and providing them with keying
material,".

> permitted.”, I had trouble parsing this sentence.

A previous IESG COMMENT prompted a rewrite of this sentence since -08.

The first paragraph of Section 4.1 now reads:

   The mechanism described in the current document interoperates fully
   with RPC implementations that do not support RPC-over-TLS.  When an
   RPC-over-TLS-enabled peer encounters a peer that does not support
   RPC-over-TLS, policy settings on the RPC-over-TLS-enabled peer
   determine whether RPC operation continues without the use of TLS, or
   RPC operation is not permitted.

> permitted by local policy …”

How about:

OLD:

   Conversely, if the Reply's reply_stat is not MSG_ACCEPTED, if its
   verifier flavor is not AUTH_NONE, or if its verifier does not contain
   the "STARTTLS" token, the RPC client MUST NOT send a "ClientHello"
   message.  RPC operation can continue, however it will be without any
   confidentiality, integrity or authentication protection from (D)TLS.

NEW:

   Conversely, if the Reply's reply_stat is not MSG_ACCEPTED, if its
   verifier flavor is not AUTH_NONE, or if its verifier does not contain
   the "STARTTLS" token, the RPC client MUST NOT send a "ClientHello"
   message.  RPC operation may continue, depending on local policy, but
   without confidentiality, integrity, or peer authentication protection
   from (D)TLS.

[ ... snipped ... ]

> -- Section 7.1.2. Typo. s/connnection/connection/

Fixed, thanks.

--
Chuck Lever

---

From: Chuck Lever <[email-masked]>
Date: Tue, 7 Jul 2020 13:42:51 -0400

Hi Roman-

This reply addresses COMMENTs regarding Section 7.

> On Jul 6, 2020, at 11:24 PM, Roman Danyliw via Datatracker <[email-masked]> wrote:

Possible replacement:

OLD:

   In light of the above, it is RECOMMENDED that when AUTH_SYS is used,
   every RPC client should present host authentication material to RPC
   servers to prove that the client is a known one.  The server can then
   determine whether the UIDs and GIDs in AUTH_SYS requests from that
   client can be accepted.

NEW:

   In light of the above, when AUTH_SYS is used, the use of a TLS mutual
   authentication mechanism is RECOMMENDED to prove that the RPC client
   is known to the RPC server.  The server can then determine whether
   the UIDs and GIDs in AUTH_SYS requests from that client can be
   accepted.

> section?  Specifically, RECOMMENDED for the first bullet and SHOULD for the

The reason not to use normative language is merely that we believe there are
no interoperability ramifications for choosing not to follow the suggestions.
Combined with the changes made to address your DISCUSS:

OLD:

   RPC-over-TLS implementations and deployments are strongly encouraged
   to adhere to the following policies to achieve the strongest possible
   security with RPC-over-TLS.

   *  RPCSEC GSS provides integrity and privacy services which are
      redundant when TLS is in use.  These services should be disabled
      in that case.

NEW:

   RPC-over-TLS implementations and deployments are strongly encouraged
   to adhere to the following policies to achieve the strongest possible
   security with RPC-over-TLS.

   *  When using AUTH_NULL or AUTH_SYS, both peers are RECOMMENDED to
      have DNS TLSA records, keys with which to perform mutual peer
      authentication using one of the methods described in Section 5.2,
      and a security policy that requires mutual peer authentication and
      rejection of a connection when host authentication fails.

   *  RPCSEC GSS provides integrity and privacy services which are
      redundant when TLS is in use.  These services SHOULD be disabled
      in that case.

--
Chuck Lever

---

From: Chuck Lever <[email-masked]>
Date: Wed, 8 Jul 2020 11:43:57 -0400

Hi Roman-

Here are my responses to your COMMENTs in Section 5.

> On Jul 6, 2020, at 11:24 PM, Roman Danyliw via Datatracker <[email-masked]> wrote:

This language came from Tigran Mkrtchyan. Tigran, can you comment?

> whatever authorization logic the RPC server might have?

I can remove this sentence.

> intended to be the IP address of the peer which presented the certificate?

I believe this is the IP address of the peer which presented the certificate.

> For example, I would have expected Section 5.2.4 to discuss Token Binding IDs

PSK and Token Binding were added on request, and no further details were provided
by the requesters.

> ** Section 5.2.2.  Is there any MTI guidance on the kinds of digests to support

I've had some difficulty with this. Originally the document required SHA-1, as
it is the de facto standard algorithm for certificate fingerprinting. However,
subsequent security review pointed out that SHA-1 is deprecated.

I changed the requirement to SHA-256, but this is problematic: most fingerprint
implementations I'm aware of use SHA-1. I have found no published document that
suggests that SHA-1 is a problem for certificate fingerprinting, and no standard
that specifically discusses certificate fingerprinting algorithms.

During Gen-ART review, the reviewer complained about the comparative:

  Implementations MUST support SHA-256
  [FIPS.180-4] or stronger as the hash algorithm for the fingerprint.

Suggesting that the document would need to provide a fixed list of particular
algorithms here, rather than an open-ended requirement. I punted and removed
the sentence.

I'm not sure how to proceed.

> ** Section 5.2.4.  Are there any MTI parameters for the token binding to

After reviewing RFC 8473, it appears that more changes to the RPC protocol would
be necessary to support Token Binding. Thus perhaps support for token binding in
this context should be deferred to a separate document. Can anyone with more
domain expertise comment?

--
Chuck Lever

---

From: "Mkrtchyan, Tigran" <[email-masked]>
Date: Wed, 8 Jul 2020 22:02:31 +0200 (CEST)

Hi Roman,

----- Original Message -----
> From: "Chuck Lever" <[email-masked]>
> To: "Roman Danyliw" <[email-masked]>, "Tigran Mkrtchyan" <[email-masked]>
> Cc: "The IESG" <[email-masked]>, [email-masked], "nfsv4-chairs" <[email-masked]>, "NFSv4"
> <[email-masked]>
> Sent: Wednesday, July 8, 2020 5:43:57 PM
> Subject: Re: [nfsv4] Roman Danyliw's Discuss on draft-ietf-nfsv4-rpc-tls-08: (with DISCUSS and COMMENT)

> This language came from Tigran Mkrtchyan. Tigran, can you comment?

The MUST implies that protocol require client to reject the connection to a server
which doesen't provide ipAddress, however, with SHOULD protocol delegates the decision
to the application, similar to how one can pass `-k` option to curl to bypass
certificate validation.

@Chuck
Feel free to change it to MUST, if it's more appropriate.

Thanks, 
   Tigran.

---

From: Chuck Lever <[email-masked]>
Date: Mon, 20 Jul 2020 08:32:05 -0400

Hi Roman-

I haven't heard any response on these items. Is there something more
you need from me?

> On Jul 7, 2020, at 11:15 AM, Chuck Lever <[email-masked]> wrote:
> 
> Hi Roman-
> 

--
Chuck Lever