On 27/03/2013 07:22, Jari Arkko wrote:
Thank you for the review, Martin!

The issue with replication of RFC 2104 has been raised as a Discuss by other 
ADs. That will get resolved. But do the authors or chairs have responses for 
the other things in Martin's review?

Jari
Hi Jari & Martin,

On Mar 10, 2013, at 7:52 PM, Martin Thomson <[email protected]> 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 <[email protected]> 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.
This overlaps with the existing comments and we will fix.


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."
We will add text.


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?
Well that would normally be a silly thing to do, but we do not propose to stop
it lest it be something an application actually wants.

This would be functionally equivalent to issuing a short term note
that was only transiently valid, or issuing some sort of synchronization
message. I can't think why you would want to do it, but why would we
stop it?

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?
Yes, on the one hand we do not wish to constrain the applications, and
on the other we trust the application developers and the IETF review
process (required for a code point) to stop silly behaviour.


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.
OK we will look at this when we deal with the security rewrite.

Nits:

Section 3 could use some subheadings to aid navigation (and referencing).
We will look at that
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.
We can do that.

I like the text in the editors note on page 8.  Why is it not the
actual text already?
That is a cut and paste error during AD review.

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?
As I recall this is allowed.

Stewart
_______________________________________________
Gen-art mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/gen-art

Reply via email to