Re: [nfsv4] Secdir last call review of draft-ietf-nfsv4-mv1-msns-update-04

[Brian E Carpenter <[e-mail]>]

(reducing the CCs by several thousand)

In line...

On 09-Mar-19 14:11, Spencer Dawkins at IETF wrote:
> Hi, David,
> 
> On Fri, Mar 8, 2019 at 5:52 PM David Noveck <[e-mail]> wrote:
> 
>> I previously said:
>>> Will address nits 0)-4) and 6)-10) in -05.
>>
>> However when I looked more closely at 9), I've found that I cannot make
>> the requested change.
>>
>>> 9) s12.2.3: replace ietf.org with example.org (not supposed to use
>>> real domains even if it is “ours”).
>>
>> This is not an example.   The use of ietf,org is intentional and is
>> intended as a way
>> to provide the ability to define globally usable variable definition as
>> well as domain-specific
>> ones.  This is specified in section 22.5 of RFC5661.   I'm not sure about
>> the details of
>> the prohibition of the use of real domain names but I can;t see how it
>> would apply in this
>> case where this form of vatibl specification, including a real domain name
>> has already
>> been adopted.
>>
> 
> Soon, I will not be the IESG Whisperer for this document, but we defined
> test and documentation domains in https://tools.ietf.org/html/rfc2606, a
> LONG time ago. If I'm reading https://tools.ietf.org/html/rfc2606#section-2
> and https://tools.ietf.org/html/rfc2606#section-3 correctly, the use of
> ..example, or .example.com, is intended to avoid people copying a domain
> name from documentation that someone is actually using now, or starts or
> stops using in the future, along with all the possible paths through
> confusion that could result.
> 
> ISTM that ietf.org is in use now, so, picking a name that someone starts
> using later isn't a problem, and if ietf.org stops working or starts
> referring to the International Egret Trading Federation, we may have bigger
> problems than this draft not using example.com ...
> 
> But do the right thing, of course.

I'm commenting only because I spent a little time with this draft as Gen-ART
reviewer.

Of course this draft can't change the domain name used in RFC5661 for the
same purpose, which is already embedded in the IANA registry:
https://www.iana.org/assignments/nfsv4-path-variables/nfsv4-path-variables.xhtml

It isn't used for documentation; I think it's used to provide uniqueness in a
naming structure. You could possibly argue that would be better done using something
in .arpa (like IN-ADDR.ARPA and IP6.ARPA) but you'd have needed to argue that
about ten years ago.

So the right thing to do in this case is nothing ;-)

Unintended side effect: such use of ietf.org ought to trigger the addition
of ietf.org to the "technical" assignments referred to in note (a) on
clause 4.3 of https://tools.ietf.org/html/rfc2860. Which incidentally means
that our domain name is pretty well protected.

     Brian

> 
> Spencer
> 
> 
>> On Thu, Feb 28, 2019 at 8:30 PM David Noveck <[e-mail]> wrote:
>>
>>>> These
>>>> instructions will really help when work on a fully revised NFS RFC gets
>>>> going.
>>>
>>> That's the intention.  The working group is now discussing the
>>> possibility of an rfc5661bis
>>> that would use those instructions.   Note that the current document would
>>> be the source of
>>> a large set of the changes but there are also others: The
>>> internationalization section needs to
>>> be updated as it was in RFC7530.  Section 12 needs to be updated to
>>> reflect the work done by
>>> RFC8434.  Also, there are a lot of erratta that would need to be applied..
>>>
>>>> 0) The big issue being addressed in the security considerations is the
>>> fact
>>>> that RPSEC_GSS is must support not must use
>>>
>>> That reflects choices made for RFC3530 and carried over until now.   This
>>> has resulted in
>>> a situation in which AUTH_SYS is very commonly used despite its obvious
>>> security weaknesses.
>>>
>>>> So what do you do when RFC 5661 says:
>>>
>>>>  The fetching of attributes containing file system location
>>>>  information SHOULD be performed using RPCSEC_GSS with integrity
>>>>  protection,
>>>
>>> Not much, since there is not much I can do about that.
>>>
>>>> And, you don’t now change to MUST use RPSEC_GSS?
>>>
>>> No I don't.   Currently use of AUTH_SYS is allowed and the working
>>> group has not made any decision to change that.
>>>
>>>> Well I think at a minimum you should do some 2119 language:
>>>
>>> OK, but note that this is a widely-deployed protocol and we are not
>>> in a position to invalidate deployment choices made in line with RFC5661,
>>> even if we think they were ill-advised.
>>>
>>>> 0) s16 (after the bullets): 2119 this should:
>>>
>>> If you are suggesting changing "should" to "SHOULD" in this parapgraph,
>>> I'm OK with making that change in -05, although I will raise the issue
>>> with
>>> the working group to check about existing implemtations.
>>>
>>>> 1) s16 (2nd para after the bullets): 2199 this SHOULD NOT:
>>>
>>> I intend to shift this to "SHOULD NOT"  in -05.
>>>
>>>> 2) s16 (last bullet): 2119 this should:
>>>
>>> Given that this bullet appears in a summary, I think the "should" is
>>> misleading.   Converting it to "SHOULD" would lead to recommemmending
>>> filtering without specifying the filtering to be done.  What I propose to
>>> do in -05 is replace "should be subect to filtering" by "is made subject to
>>> filtering as described above".
>>>
>>>> The use of requests issued without RPCSEC_GSS (i.e. using
>>>>  the known to be insecure AUTH_SYS), while undesirable and
>>>>  Insecure, may not be avoidable in all cases.
>>>
>>> While I approve of drawing more attention to the weaknesses of
>>> AUTH_SYS, I feel the use of the word "insecure" is too vague.
>>> "Insecure" covers a range of problems whereby traffic is either
>>> exposed to prying eyes, subject to corruption in transit, or acted
>>> upon without authentiction.   Given the context, I think we need to
>>> be clear about what the actual problem is even though AUTH_SYS,
>>> in what has to be considered a virtuoso performance :-), manages
>>> to acomplish the hat trick of protocol insecurity.
>>>
>>> So I propose writing this as follows:
>>>
>>> The use of requests issued without RPCSEC_GSS (i.e. using AUTH_SYS which
>>> has no provision to avoid corruption of data in flight), while undesirable
>>> and a potential security exposure, may not be avoidable in all cases.
>>>
>>>
>>>> It’s probably also worth discussing whether s14.3 should drop
>>>> support for includes id-sha1
>>>
>>> The text you are referring to was carried over from RFC5661 as-is since
>>> there was no issue known with it.
>>>
>>> Regarding the potential deletion of id-sha1, the issue, given that server
>>> support is currently REQUIRED, is whether there are clients using this who
>>> might be impacted by a change.   I'll check with the working group.
>>>
>>>> 4) s4.3 (1st sentence): Not sure the 2119-RECOMMENDED is needed.
>>>
>>> These are really OPTIONAL althouh the word "RECOMMENDED" is (confusingly)
>>> used.
>>> Will delete.
>>>
>>>> 5) s4.3 (penultimate para): Seems like r/should/SHOULD.
>>>
>>> It is probably the case that RFC5661 made a poor choice here.  However, I
>>> don't think we can change it now.
>>>
>>>> 10) s16 (3rd bullet): Why is requirement is caps?
>>>
>>> It's because I was under the misapprehension that statement that things
>>> were REQUIRED or RECOMMENDED could be referred to as "REQUIREMENTS" or
>>> "RECOMMENDATIONS".  It turns out not to be so.
>>>
>>> Will address nits 0)-4) and 6)-10) in -05.
>>>
>>> On Wed, Feb 27, 2019 at 12:38 PM Sean Turner <[e-mail]> wrote:
>>>
>>>> Reviewer: Sean Turner
>>>> Review result: Has Issues
>>>>
>>>> This draft updates NFSv4.1 to correct handling of the fs_locations and
>>>> fs_locations_info attributes; NFSv4.1 weighs in at 617 pages and this
>>>> draft weighs in a relatively spry 106; let’s say I was really, really
>>>> hoping
>>>> for some multi-page ASCII art, but also no pretty much all text.  The
>>>> draft does a good job of explaining what’s being replaced and why, but
>>>> I have to say that I did not check all of section pointers.  These
>>>> instructions will really help when work on a fully revised NFS RFC gets
>>>> going.  Unfortunately for me, I do not have NFSv4.1 in my head so let’s
>>>> just say that I did my best …
>>>>
>>>> Summary: This draft is probably worthy of some type of DISCUSS based
>>>> on the following:
>>>>
>>>> 0) The big issue being addressed in the security considerations is the
>>>> fact
>>>> that RPSEC_GSS is must support not must use and the other mechanism
>>>> referred to by RFC 5661 are:
>>>>
>>>> - AUTH_NONE - nada authentication
>>>>
>>>> - AUTH_SYS as described in Appendix A is known to be insecure due to
>>>>    the lack of a verifier to permit the credential to be validated.
>>>>    AUTH_SYS SHOULD NOT be used for services that permit clients to
>>>>    modify data.  AUTH_SYS MUST NOT be specified as RECOMMENDED or
>>>>    REQUIRED for any Standards Track RPC service.
>>>>
>>>> - AUTH_DH as mentioned in Sections 8.2 and 13.4.2 is considered
>>>>    obsolete and insecure; see [RFC2695].  AUTH_DH SHOULD NOT be used for
>>>>    services that permit clients to modify data.  AUTH_DH MUST NOT be
>>>>    specified as RECOMMENDED or REQUIRED for any Standards Track RPC
>>>>    service.
>>>>
>>>> So what do you do when RFC 5661 says:
>>>>   The fetching of attributes containing file system location
>>>>   information SHOULD be performed using RPCSEC_GSS with integrity
>>>>   protection,
>>>> And, you don’t now change to MUST use RPSEC_GSS?  Well I think at a
>>>> minimum you should do some 2119 language:
>>>>
>>>> 0) s16 (after the bullets): 2119 this should:
>>>>
>>>>   In light of the above, a server should present file system
>>>>   location entries that correspond to file systems on other servers
>>>>   using a host name.
>>>>
>>>> 1) s16 (2nd para after the bullets): 2199 this SHOULD NOT:
>>>>
>>>>    As a
>>>>    result, the client should not use such unverified location entries
>>>>    as a basis for migration, even though RPCSEC_GSS might be
>>>>    available on the destination.
>>>>
>>>> 2) s16 (last bullet): 2119 this should:
>>>>
>>>>   Where the use of the returned information cannot be
>>>>   avoided, it should be subject to filtering to eliminate the
>>>>   possibility that the client would treat an invalid address as
>>>>   if it were a NFSv4 server.
>>>>
>>>> And, I guess since AUTH_SYS is insecure can I suggest a friendly
>>>> amendment to the following sentence:
>>>>
>>>> OLD:
>>>>   The use of requests issued without RPCSEC_GSS (i.e. using
>>>>   AUTH_SYS), while undesirable, may not be avoidable in all
>>>>   cases.
>>>>
>>>> NEW:
>>>>
>>>>   The use of requests issued without RPCSEC_GSS (i.e. using
>>>>   the known to be insecure AUTH_SYS), while undesirable and
>>>>   Insecure, may not be avoidable in all cases.
>>>>
>>>> It’s probably also worth discussing whether s14.3 should drop
>>>> support for includes id-sha1.
>>>>
>>>>
>>>> These are the nits:
>>>>
>>>> 0) Update to use new 2119 paragraph:
>>>>
>>>>   The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
>>>>   "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY", and
>>>>   "OPTIONAL" in this document are to be interpreted as described in BCP
>>>>   14 [RFC2119] [RFC8174] when, and only when, they appear in all
>>>>   capitals, as shown here.
>>>>
>>>> 1) s3.1 (wordy): r/since the
>>>>   ports to be used for various types of connections might be
>>>>   required to be different/since the
>>>>   ports to be used for various types of connections might be
>>>>   different.
>>>>
>>>> 2) s3.1 (missing period): r/Two such addresses support the use of
>>>> clientid
>>>>   ID trunking, as described in [RFC5661]/Two such addresses support the
>>>> use of clientid
>>>>   ID trunking, as described in [RFC5661].
>>>>
>>>> 3) s4.2 (missing word): r/which may accessed/which may be accessed
>>>>
>>>> 4) s4.3 (1st sentence): Not sure the 2119-RECOMMENDED is needed.
>>>>
>>>> 5) s4.3 (penultimate para): Seems like r/should/SHOULD.
>>>>
>>>> 6) s4.5: r/present , is/present, is
>>>>
>>>> 7) s4.5: Note sure you need the parenthetical in the following:
>>>>   a server may (but is not required to)
>>>>
>>>> 8) s4.5.4: r/In the event that server failures,/In the event that
>>>> server fails,
>>>> or
>>>> r/In the event that server failures,/In the event of server failures,
>>>>
>>>> 9) s12.2.3: replace ietf.org with example.org (not supposed to use
>>>> real domains even if it is “ours”).
>>>>
>>>> 10) s16 (3rd bullet): Why is requirement is caps?
>>>>
>>>>
>