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

From: Adrian Farrel <[email-masked]>
Date: Tue, 19 May 2009 14:52:46 +0100

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/

===