Re: [Gen-art] Gen-ART review of draft-ietf-mpls-gach-adv-06

[Martin Thomson <[e-mail]>]

I sent the following review a few weeks back.  I just wanted to make sure
that it didn't get accidentally stored in /dev/null.


On 15 February 2013 14:33, Martin Thomson <[e-mail]> wrote:

> I am the assigned Gen-ART reviewer for this draft. For background on
> Gen-ART, please see the FAQ at
>
> <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.
>
> Please resolve these comments along with any other Last Call comments
> you may receive.
>
> Document: draft-ietf-mpls-gach-adv-06
> Reviewer: Martin Thomson
> Review Date: 2013-02-15
> IETF LC End Date: 2013-02-27
> IESG Telechat date: (if known)
>
> Summary: The document is almost ready for publication as proposed
> standard.  There is a major issue that should be easy to resolve.
>
> Major issues:
>
> Section 6.3 duplicates the description of HMAC provided in RFC 2104.
> That is likely to cause a bug.
>
> If you reference RFC 2104, then the only requirement is a clear
> specification for what message is input to the HMAC.  Currently, this
> is buried.  It is unclear if the input includes the G-ACh header
> defined in RFC 5586 (it doesn't need to, but this needs to be made
> explicit).
>
> Filling the authentication part with 0x878FE1F3 seems unnecessary busy
> work, but it's harmless as long as the hash function produces a
> multiple of 32 bits of output.
>
> Minor issues:
>
> To avoid forward compatibility issues, reserved fields should come
> with guidance that says: "Implementations of this protocol version
> MUST set reserved fields to all zero bits when sending and ignore any
> value when receiving messages."
>
> In Section 4.4, how does the duration interact with the lifetime?
> What happens when the duration is longer than lifetime such that the
> TLV is expunged before the duration is up?
>
> Section 5.2 states:
>    [...] If one
>    of the received TLV objects has the same Type as a previously
>    received TLV then the data from the new object SHALL replace the data
>    associated with that Type unless the X specification dictates a
>    different behavior.
>
> This leads to different retention characteristics depending on some
> arbitrary application-specific requirements.  It also complicates
> implementations.  Is there a strong motivation for the "unless the X
> specification dictates a different behavior" part of this statement?
>
> If this behaviour is desirable, a note regarding what happens to the
> composed TLV when some of the values that contribute to it might
> expire might be necessary.
>
> Regarding the last paragraph of Section 6.3:
>           This also means that the use of hash functions with larger
>           output sizes will increase the size of the GAP message as
>           transmitted on the wire.
>
> If you want to prevent hash truncation, then use 'MUST'.  Personally,
> I see no reason to do so.  It's a good way to get smaller messages,
> with a corresponding reduction in the strength of the assurance
> provided.
>
> Nits:
>
> Section 3 could use some subheadings to aid navigation (and referencing).
>
> Section 3 describes the size of fields only through ASCII-art.  It's a
> fairly simple thing to add a bit count to the description of each
> field.  That includes the reserved fields, which have no descriptions.
>
> I like the text in the editors note on page 8.  Why is it not the
> actual text already?
>
> Sections 4.2 and 4.3 could probably use a note that notes that
> retention of these TLVs doesn't make any sense.  These could be sent
> with zero lifetime, except that if these are sent along with the
> Source Address TLV, that's not possible... unless you send multiple
> application data blocks for the same application.  Is that possible?
>