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

[Stewart Bryant <[e-mail]>]

Including Sec ADs as this involves a change to the Authentication section.

Section 6.3 now says

The HMAC proceedure described in [RFC2104] is used to compute the hash.
The hash is computed over the entire GAP message as shown in Fig1.
The length of the Authentication Data field is always less than or equal
to the message digest size of the specific hash function that is being
used, however the implementer needs to consider that although this
decreases the size of the message, it results in a corresponding
reduction in the strength of the assurance provided.
Hash truncation is not RECOMMENDED.

I will address any other security issues in another editing pass and
respond to their Discuss email.


On 10/03/2013 17:52, Martin Thomson wrote:
> 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] 
> <mailto:[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.
>

All done please see above.
>
>
>     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."
>
The following was added just before Fig 1

Implementations of this protocol version MUST set reserved fields in the
message formats that follow, to all zero bits when sending and ignore
any value when receiving messages.

I feel that we should just write an update to RFC2119 to automatically
include this or add it to the standard boiler plate.



>     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?
>
The TLV would be deleted (per procedure) at its death date, and
would not be sent again until at least duration has expired. This would
be silly if the receiver wanted that TLV, but entirely reasonable
if it dis not care about the TLV.

>
>     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?
>
One can imagine alternative behaviors such as maintaining a list of
received values. Our goal was not to prejudge the application.

>
>     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.
>
The behaviour described is the action of the data and not of the TLV that
it arrived in, so I don't think the problem you refer to can occur.

>
>     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.
>
Please see new text above.

>
>     Nits:
>
>     Section 3 could use some subheadings to aid navigation (and
>     referencing).
>
Done
>
>     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.
>
Done
>
>
>     I like the text in the editors note on page 8.  Why is it not the
>     actual text already?
>
That was a hang over from AD review that was already addressed in the
text. The note has been removed.
>
>
>     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?
>
>
I have added the following text in each case:

It is not necessary to retain this TLV.

Thanks for the review

Stewart