[pim] AD review of draft-ietf-pim-sm-linklocal-08.txt

[Adrian Farrel <Adrian.Farrel at huawei.com>]

Hi,

A useful and well-written I-D. Good to see people paying attention to
the security requirements of routing. Thanks.

My review serves two purposes:
1. To catch any nits or confusion (if I'm confused, the average
  dunderhead will also be confused)
2. To smooth the path through the IESG review that will follow

Although you will need to perform a revision of the draft, I don't
think any of the issues is particularly large, and I would expect
that what you need to do is basically editorial.

If you want to query or contest any of the points I made, that is
fine with me.

Thanks,
Adrian

===

The IESG is still trying to work out the rules for Experimental I-Ds :-(

However, given the very low level of support expressed for this document
and the fact that (in theory) an Experimental I-D cannot update a
Proposed Standard, I think that you should:
- mark this as Experimental
- remove the "Updates: 4601" tag
- rely on the existing text to indicate the relevance to 4601

If/when you have implementations and more support in the future, you
should come back and move this to Standards Track and re-insert the
"updates" tag.

===

idnits shows:
 -- Obsolete informational reference (is this intentional?): RFC 2401
    (Obsoleted by RFC 4301)

I assume from the text that this is a deliberate reference. Please
check you are comfortable with this.

===

Section 4 para 2

s/authentication to PIM-SM/authentication of PIM-SM/

===

Section 4

I agree the silent discard. But would it not be advisable to maintain
a count to help diagnose attempted attacks and control traffic
congestion?

The sort of text I would have in mind is...

  When authentication for PIM-SM link-local messages is enabled,

  o  PIM-SM link-local packets that are not protected with AH or ESP
     MUST be silently discarded, although an implementation MAY
     maintain a counter of such packets.

  o  PIM-SM link-local packets that fail the authentication checks MUST
     be silently discarded, although an implementation is RECOMMENDED
     to maintain a counter of such packets.

Similar issue in Section 5.

===

Section 5

This section ends with "NOT permitted."
This is not RFC 2119 language.

I think you need to rephrase as either:

"not permitted"

or

  Note that since authentication MUST be supported by a conforming
  implementation, an implementation MUST NOT generate the combination
  of NON-NULL Encryption and NULL Authentication.

===

Section 6

OLD
  Interface ID tagging
     The implementation MUST be able to tag the inbound packets with
     the ID of the interface (physical or virtual) via which it
     arrived.
NEW
  Interface ID tagging
     The implementation MUST be able to tag the inbound packets with
     the ID of the interface (physical or virtual) on which they
     arrived.

===

Section 6

OLD
  Manual key support
     Manually configured keys MUST be able to secure the specified
     traffic.
NEW
  Manual key support
     It MUST be possible to use manually configured keys to secure the
     specified traffic.

===

Section 7.1

  The Network Administrator will configure a router manually
  during its boot up process.

This surprised me somewhat. Is it not possible to manually configure
a router (with its SAs) at any time, including adding new SAs to
running routers?

===

Figure 2

I found this quite hard to parse because I thought it was a topological
view. Only when I read the text did I realise it was a cross between a
list and a topology.

Would the following be easier to read?


                                       |
                +++++                  |
                + B + SAb ------------>|
                +   + SAa <------------|
                +++++                  |
                                       |
                +++++ SAb <------------|
                +   +             ---->|
                +   +            /
                + A + SAa -------
                +   +            \
                +   +             ---->|
                +++++ SAc <------------|
                                       |
                +++++                  |
                + C + SAc ------------>|
                +   + SAa <------------|
                +++++                  |


Similar mods to figure 3 would be helpful.

===

Section 8

Right at the very end you have
  This
  document RECOMMENDS the above method for manual key configuration.
  However, it MAY also be used with automated key configuration.

This is slightly ambiguous as both methods are "above". Can you clarify
in the text?

===

Section 9

I think it would be worth adding a reference to RFC 4107. I don't
believe this changes what you have written (in fact your text seems
to be perfectly aligned with RFC 4107), but including the reference
makes this clear (and will pacify the Security ADs).

You may find, however, that after reading 4107 you want to:
- clarify key change-over as expressed in 9.1
- explain the 90 day key change interval in 9.3

===

Section 9.1

You have a "SHOULD" at the start of the paragraph, but no indication of
the option or motivation that makes this not a "MUST". Can you include
some guidance or upgrade to "MUST".

Also
  Note that all routers on the link must complete step 1 before any
  begin step 2.  Likewise, all the routers on the link must complete
  step 2 before any begin step 3.
Are these two "must" actually "MUST"?

And then, to combine these two questions, you have...
  In order to achieve smooth key transition, all routers on a link
  should use the same value for KeyRolloverInterval and should initiate
  the key rollover process within this time period.
should/SHOULD/MUST ?

===

Section 9.2
  The configured value of KeyRolloverInterval should be long enough to

s/should/needs to/

===