Subject: Re: draft-gont-6man-nd-opt-validation-00

From: 神明達哉 <[email-masked]>
Date: Tue, 26 Aug 2014 09:57:13 -0700

At Thu, 21 Aug 2014 18:41:07 +0000,
Ronald Bonica <[email-masked]> wrote:

> As instructed by the WG, Fernando and I have expanded the scope of draft- gont-6man-lla-opt-validation to include validation rules for all options used in all ND messages. The new document is called draft-gont-6man-nd-opt- valdation-00.

This should be "validation" ('i' is missing).  Same typo in the
message subject, which I've corrected.

> Please take a look at the draft. Tell us if we have done what the WG requested.

I don't know (or remember) precisely what the "WG requested", so I
can't say anything about it.  I have some comments on the draft
nevertheless:

- I previously commented on lla-opt-validation:
  http://www.ietf.org/mail-archive/web/ipv6/current/msg20389.html
  and I thought we agreed on some updates from that discussion.  But
  it looks like these points have been completely dropped in nd-opt-validation.
  Was that my misunderstanding about the result of the previous
  discussion, or is nd-opt-validation a completely new document mostly
  unrelated to the previous discussion, or something else?

- Related to the previous point, IIRC lla-opt-validation discusses the
  rationale of the validation, but nd-opt-validation generally only
  lists the validations.  Is this change intentional?

- Some of the described validation seem to come from the original
  specification (mostly RFC4861), while some others seem to be newly
  proposed.  I'd like the draft to clarify which ones are the
  former/latter.

- Maybe related to some of the previous bullet points, some of the
  proposed validations (which generally seem to be newly proposed
  ones?) require the entire ND message be dropped if a certain
  validation fails while some others just require that particular
  option be dropped, and it doesn't explain why the former case should
  be handled differently.  I'd at least like to see the rationale, and
  in general it seems to be against the sense of Section 9 of RFC
  4861.

- Section 5: what's the source of this restriction?

   The Prefix Length is an 8-bit unsigned integer that specifies the
   prefix length (i.e., the number of leading bits in the Prefix field
   that are significant).  Its value MUST be greater than or equal to
   32.

- Section 5: is this adjustment a new proposal of this draft or an
  existing one? (If it's the latter I should have missed something).
  And in any case I'm not sure if it really makes sense.  Especially
  if this is a new proposal I'd like to see more discussion on why we
  need this:

   Host SHOULD set the effective Valid Lifetime as follows:

           EffectiveVL = max(Valid Lifetime, MIN_VALID_LIFETIME)

   Where EffectiveVL is the effective 'Valid Lifetime', and
   MIN_VALID_LIFETIME is set to 1800 (seconds).

- Section 5: I think we should be more precise here:

   As stated in [RFC4861] the Preferred Lifetime MUST BE less than or
   equal to the Valid Lifetime.  If an incoming ND message does not pass
   this validation check, the Prefix Information Option MUST be ignored.

  Preferred Lifetime is only used for stateless address
  autoconfiguration, so when we say "PIO MUST be ignored" we should be
  clear whether it's in terms of on-link determination or SAAC.  In
  the context of the latter, RFC 4862 explicitly requires such PIO be
  ignored.  In the context of the former, this is certainly odd but
  doesn't do any actual harm.  So, if the intent is the latter, I'd
  like to see the rationale, just for the sake of paranoia, etc.

- Section 10: it's odd to me that it specifically covers the first two
  bits (of the first byte) of a label in that there are other forms of
  invalid wire format for domain names (e.g., it can't be longer than
  255 octet).

   The domain suffixes included in this option MUST be encoded with the
   simple encoding specified in Section 3.1 of [RFC1035].  Therefore, if
   any of the labels of a domain does not have the first two bits set to
   zero, the corresponding DNSSL option MUST be ignored.

  So I'd suggest revising the second sentence to more generic,
  e.g. "Therefore, if the encoded data are not a valid non-compressed
  form of domain name, the corresponding DNSSL option MUST be
  ignored."

- Editorial nits
  + Section 4: s/an/a/ (there are other instances of this type of typo)
   message contains an TLLA Option, the TLLA Option MUST be ignored.
  + Section 5: s/BE/be/ (ditto)
   The Length field MUST BE set to 4.  If an incoming ND message does

--
JINMEI, Tatuya

---

From: Ronald Bonica <[email-masked]>
Date: Wed, 27 Aug 2014 18:51:44 +0000

Jinmei,

As always, thanks for your thoughtful review. Comments inline.

                              Ron

> -----Original Message-----
> From: [email-masked] [mailto:[email-masked]] On Behalf
> Of ????
> Sent: Tuesday, August 26, 2014 12:57 PM
> To: Ronald Bonica
> Cc: [email-masked]
> Subject: Re: draft-gont-6man-nd-opt-validation-00
> 
> > As instructed by the WG, Fernando and I have expanded the scope of
> draft- gont-6man-lla-opt-validation to include validation rules for all options
> used in all ND messages. The new document is called draft-gont-6man-nd-
> opt- valdation-00.
> 
> This should be "validation" ('i' is missing).  Same typo in the message subject,
> which I've corrected.
> 
> I don't know (or remember) precisely what the "WG requested", so I can't
> say anything about it.  I have some comments on the draft
> nevertheless:
> 
[RPB] 
The intent of the document has changed slightly. Draft- gont-6man-lla-opt-validation described several ND vulnerabilities. It also proposed validations that implementations could impose upon the SLLA and TLLA options to mitigate the vulnerabilities.

 At IETF 89, the WG instructed the authors to expand the scope of the document. The new document (draft-gont-6man-nd-opt-validation) describes all validations that an implementation should enforce upon all ND options. These validations are a superset of the validations described in draft- gont-6man-lla-opt-validation.

While the two validations described in draft- gont-6man-lla-opt-validation are associated with vulnerabilities, many of the new validations are not. So, we dropped discussion of the two vulnerabilities described in draft- gont-6man-lla-opt-validation. In retrospect, I don't think that was a very good idea.

In the next revision, we will add an appendix describing the two known vulnerabilities. We will import that text from the old document.

>   proposed.  I'd like the draft to clarify which ones are the

[RPB] 
In the next revision, we will identify validations that were carried forward from previous RFCs

>   in general it seems to be against the sense of Section 9 of RFC
[RPB] 
We will also catch this in the next revision.
>    that are significant).  Its value MUST be greater than or equal to
[RPB] 
I thought that this validation might be a little controversial ;-)

It's clear that PIO with ::/0 is pathological. The same for fe00::/8.

But it's not clear what the largest sane prefix length is. Do you have a suggestion?

>    MIN_VALID_LIFETIME is set to 1800 (seconds).
[RPB] 
Agree. Will add in next version.
>   like to see the rationale, just for the sake of paranoia, etc.
[RPB] 
Agree. Will catch this in the next version.

>   form of domain name, the corresponding DNSSL option MUST be
[RPB] 
Also agree.

---

From: Fernando Gont <[email-masked]>
Date: Wed, 27 Aug 2014 16:51:47 -0300

Hi, Jinmei,

Thanks so much for your feedback! -- Please find my comments in-line...
[quite a few comments ommited, since Ron since to ahve responded some of
them]

On 08/26/2014 01:57 PM, ç¥æéå wrote:
>  I have some comments on the draft
> nevertheless:
> 
> http://www.ietf.org/mail-archive/web/ipv6/current/msg20389.html and I
> thought we agreed on some updates from that discussion. 

It seems that I missed your post -- I'll respond in a separate thread.

> But it looks
> like these points have been completely dropped in nd-opt-validation. 

nd-opt-validation is a superset of lla-opt-validation.

> - Related to the previous point, IIRC lla-opt-validation discusses
> the rationale of the validation, but nd-opt-validation generally
> only lists the validations.  Is this change intentional?

Yes. The rationale was that, since we're covering many ND options, it
was better to stick to the validation checks instead of delving into the
details of the associated attack vectors.

> in general it seems to be against the sense of Section 9 of RFC 

Not sure what you mean. In this document we try to validate ND options,
and if an option seems to be malformed, we drop the packet. OTOH,
Section 9 of RFC4861 just notes that if you *don't recognize* an option,
you should still parse the other options.

In our case, its not that we couldn't recognize an option, but rather
that we recognized it as malformed.

That said, I understand that an alternative approach would be to just
drop the option rather than the whole packet. Me 8not necessarily my
co-atuhors) could live with that approach, too.

Thoughts?

> that are significant).  Its value MUST be greater than or equal to 

Of the top of my head, we incorporated a more relaxed sanity check that
that being employed by other stacks (length == 64 in some cases).

But I do agree that the sanity check is debatable. Thoughts?

> And in any case I'm not sure if it really makes sense.  Especially if
> this is a new proposal I'd like to see more discussion on why we need
> this:
> 

When sending the RA, RFC4861 specifies a lower limit. Here we're
enforcing such lower limit, but on the receive side.

This avoids e.g. having a node sending packet with a Valid lifetime of 1
second, which would make you configure your network interface and right
after drop such configuration.

> equal to the Valid Lifetime.  If an incoming ND message does not
> pass this validation check, the Prefix Information Option MUST be
> ignored.
> 
> clear whether it's in terms of on-link determination or SAAC.

FWIW, since the Preferred Lifetime MUST (always) be less than or equal
to the
valid lifetime, if the option does not pass this check, we consider it
to be malformed and hence advice implementation to drop them

>  In the
> context of the latter, RFC 4862 explicitly requires such PIO be 

It's a MUST that is not being complied to, essentially.

Thanks so much!

Best regards,
-- 
Fernando Gont
SI6 Networks
e-mail: [email-masked]
PGP Fingerprint: 6666 31C6 D484 63B2 8FB1 E3C4 AE25 0D55 1D4E 7492

---

From: 神明達哉 <[email-masked]>
Date: Thu, 28 Aug 2014 08:56:13 -0700

At Wed, 27 Aug 2014 18:51:44 +0000,

> But it's not clear what the largest sane prefix length is. Do you have a suggestion?

I'm not sure if we can effectively impose a simple restriction on the
prefix length.  Even a prefix length of 0 (which means a prefix of
::/0) with the L bit on might make sense if this is a single-link
network with a "router" effectively acting as an RA server to tell
hosts that all unicast addresses are on-link by default (while ::/0
covers others addresses such as multicast addresses, but hosts should
handle these as an exception anyway).

Maybe this is a good example of why we'd generally need a discussion
on the rationale of the restrictions.  Depending on why we want to
impose such a limitation, we may be able to find a reasonable value or
we may find it ineffective to impose a limitation on some specific
cases.

--
JINMEI, Tatuya

---

From: 神明達哉 <[email-masked]>
Date: Thu, 28 Aug 2014 10:14:06 -0700

At Wed, 27 Aug 2014 16:51:47 -0300,

> details of the associated attack vectors.

In some cases the rationale is not really clear.  Some specific cases
of lla-opt as discussed before are one example, and the prefix length
in the PIO is another.  I don't think it a good idea to completely omit
background discussions unconditionally.  If we find we need such
additional discussions in too many cases and rather want to keep the
document body concise, one option would be to discuss them in an
appendix as Ron proposed.

> co-atuhors) could live with that approach, too.

I see there may be a logic leap in my original comment.  Let me
rephrase it: I saw two issues in the mixture of dropping a specific
option and dropping the entire ND message:

1. it's not clear why we have two different actions
2. in the case of dropping the entire message (as this seems to be
   newly introduced in the draft), it may be an overkill

I now understand the intent that would answer issue #1; this draft
considers a certain type of validation failures to be "malformed", and
proposes dropping the entire message for these types of failures.
The intent wasn't obvious to me, and it would help if it explicitly
states the general idea somewhere in the draft.

The second issue is related to my original comment referring to
Section 9 of RFC4861.  I imagined a possibility that a future
extension of the protocol allows, e.g., the length field of PIO >
4 to include some additional information.  Obviously, a host that
doesn't know this extension can't handle this version of PIO, but
should still be able to parse and process other options (which may
contain another PIO in the old format for compatibility) as long as
the entire integrity of the message is preserved.  If it's really a
malicious attempt or data corruption, the host should be able to
detect it once it completes parsing the whole options (e.g., it may
find one ND option is too long for the entire message), at which point
it can/should drop the entire message.

I don't know if RFC4861 Section 9 intended to include this type of
extension, but that's what I tried to say by referring to that section
in this context.

I also think this is another example of why it's generally better to
discuss the rationale of specific restrictions.  If there's a concrete
bad scenario when a node just skips an option instead of dropping the
entire message, such a less likely possibility of option extensions
wouldn't justify the leniency.  Without such a discussion, we'd always
encounter the question of "why".

> But I do agree that the sanity check is debatable. Thoughts?

See my response to Ron.  I don't have a specific idea right now,
including whether we can really impose a specific restriction.

> enforcing such lower limit, but on the receive side.

??? I'm afraid I'm now totally confused.  Could you specify the exact
part of RFC 4861 that defines the "lower limit"?  As far as I can see,
the only occurrence of the magic number of "1800" in RFC4861 is this:

      MaxRtrAdvInterval
                     The maximum time allowed between sending
                     unsolicited multicast Router Advertisements from
                     the interface, in seconds.  MUST be no less than 4
                     seconds and no greater than 1800 seconds.

Is this where this "MIN_VALID_LIFETIME" comes from?

> after drop such configuration.

But I thought the ND spec actually intends to allow a legitimate
administrator to advertise such a low value of Valid Lifetime, even 0
(maybe to force hosts to send all packets to the router from now on).
RFC4682 has its own defense against too small values for Valid
Lifetime in the context SLAAC, and in my memory we intentionally
didn't do it for the case of on-link determination.

Anyway, more detailed discussion may reveal the need for such a
restriction.  Yet another example of why "why" is generally necessary.

> It's a MUST that is not being complied to, essentially.

I don't get this...do you mean, by "not being complied", that this
MUST NOT of RFC4861

      Preferred Lifetime
...
                     Note that the value of this field MUST NOT exceed
                     the Valid Lifetime field to avoid preferring
                     addresses that are no longer valid.

is not complied because hosts do not check the preferred lifetime (and
do "allow" pltime > vltime as a result) as part of on-link determination?
Clearly it's really enforced in the context RFC4862 as described in
step c of Section 5.5.3, and I believe sensible router implementations
prohibit the admin from configuring pltime being larger than vltime.
And, pltime isn't used for on-link determination at all.  I personally
don't regard this situation as "not being complied".

--
JINMEI, Tatuya