MPLS Generic Associated Channel (G-ACh) Advertisement Protocol
draft-ietf-mpls-gach-adv-08

[Ballot comment]
Thanks for the hash-to-MAC conversion!  Looks much better now.  One minor nit in Section 6.1:
OLD: "the following values MAY supported"
NEW: "the following values MAY be supported"


----Remaining initial comments-----

"""
  Upon receiving the second message, the receiver retains B-TLV1 from
  the first message and adds B-TLV7 to its B-database.
"""

This is the first mention of a "database", much less a per-application one, and there is no mention of databases anywhere else in the document, nor in RFC 5586.  It seems there's an architectural assumption that needs to be stated here.


"""
  Otherwise, the Value field specifies the applications for
  which an update is requested, in the form of a sequence of
  Application IDs:
"""

This might benefit from some clarity on responder behavior.  MAY the responder send a different list of apps?  MUST all requested apps be present in the response?


It would be nice to have a checksum-like mode for the Authentication Data, which provides some degree of integrity without a need for key management.  If you define the above application, you could also have a TLV that follows the same procedures as Authentication Data, but with a fixed set of parameters (HMAC function and key).

[Ballot comment]

Thanks for handling my discuss points.

(I still didn't go through the comments below but I know some of
'em have been addressed.)

- Only specifying how a sender sends but not saying what a
receiver does on receipt seems to result in a fairly incomplete
protocol.  I don't see that the usual "its MPLS, we manage these
devices" response works for that. This isn't a DISCUSS though
since I guess I could accept you saying "the receiver just puts
the TLV into a local database, but its seems fairly weak really.

- I assume that there are no congestion issues that are likely to
arise with the G-ACh due to the use of this protocol?

- section 2: What does the first sentence of the last para of
section 2 mean? I can't parse it anyway.

- section 3, p6: "An error MAY be logged" - does that mean
locally or create some new n/w traffic? If the latter, isn't that
a potential DoS vector?

- s3, p7: MI definition - how is MI expiry handled? You don't say
(here). If that uses the Lifetime from the application specific
body, then what if there are many of those?

- s3, p8: Why is an editor's note still present?

- s3, p8: Lifetime of 16 bits means max is ~18 hours. That seems
oddly short for configuration data for presumably heavily managed
links and liable to make implementation more complex since the
same stuff will have to be sent more than once every day.

- s3, p8: The "source-channel-application tuple" seems ambiguous
to me - I guess you mean expires-at is the higher level
Timestamp+Lifetime, right? Why not say that?

- 4.1: can a source IP address represent a multicast group or
does it have to be "unique" to the sender?

- 4.2: this seems like a nice potential DoS vector too. (Say if
caculation of some TLV for which I ask consumes resources for the
sender of a GAP message containing that TLV.)

- 4.2: So length=0 means "send all" but app-id=0x0000 means
"who's there"? That seems clunky to me fwiw. I also don't get how
the last para of 4.2 is fully specified.

- 5.1, Saying the MI is set at the "sender's discretion" seems
wrong, given you earlier said it has to be unique within some
(not that well) defined scope.

- 6.1: "secret string" - please don't say that as it  might be
interpreted to mean something human readable/memorable which
results in far weaker security. Please say "secret value" or
"secret octet string" and for bonus points say that if this is
human memorable then its basically pointless and that
implementations MUST be able to handle essentially random binary
values of the appropriate length.

- 6.3: Please refer to RFC2104 which is where HMAC is defined.
Following the bad practices from other RFCs that repeat the
definition is really not a good plan.

- I'd encourage you to say that use of GAP authentication is
RECOMMENDED?  That way, it may get used. Otherwise, I suspect
that it may be ignored, and we may be sorry about that later if
it turns out to just be another fig-leaf.

[Ballot comment]
I have cleared under the premise that this extension is deployed in a highly controlled environment and that there is now  a textual warning about the possibility of congesting the link if not properly configured.

[Ballot discuss]

Thanks for handling my discuss points. You have a fix for
point 4 that'll be in -08 I guess, and I've suggested some
text for point 3 via emali, but that needs checking.

(1) cleared

(2) cleared

(3) 6.1: If using karp key tables, how do the SendNotBefore,
SendNotAfter and similar values from that map to the
Timestamp+Lifetime used here? I think you need to say as
implementers will make different assumptions otherwise. (I don't
care much which of the reasonable options you choose.)

(4) 6.2: you need to say what is in the V for the auth data TLV
as input to HMAC. Options are to include L zeros as the value or
for the V octets to be missing on input to HMAC. See also the
secdir review [1] which makes the same point. 

  [1] http://www.ietf.org/mail-archive/web/secdir/current/msg03795.html

[Ballot comment]

(I didn't go through the comments below but I know some of
'em have been addressed.)

- Only specifying how a sender sends but not saying what a
receiver does on receipt seems to result in a fairly incomplete
protocol.  I don't see that the usual "its MPLS, we manage these
devices" response works for that. This isn't a DISCUSS though
since I guess I could accept you saying "the receiver just puts
the TLV into a local database, but its seems fairly weak really.

- I assume that there are no congestion issues that are likely to
arise with the G-ACh due to the use of this protocol?

- section 2: What does the first sentence of the last para of
section 2 mean? I can't parse it anyway.

- section 3, p6: "An error MAY be logged" - does that mean
locally or create some new n/w traffic? If the latter, isn't that
a potential DoS vector?

- s3, p7: MI definition - how is MI expiry handled? You don't say
(here). If that uses the Lifetime from the application specific
body, then what if there are many of those?

- s3, p8: Why is an editor's note still present?

- s3, p8: Lifetime of 16 bits means max is ~18 hours. That seems
oddly short for configuration data for presumably heavily managed
links and liable to make implementation more complex since the
same stuff will have to be sent more than once every day.

- s3, p8: The "source-channel-application tuple" seems ambiguous
to me - I guess you mean expires-at is the higher level
Timestamp+Lifetime, right? Why not say that?

- 4.1: can a source IP address represent a multicast group or
does it have to be "unique" to the sender?

- 4.2: this seems like a nice potential DoS vector too. (Say if
caculation of some TLV for which I ask consumes resources for the
sender of a GAP message containing that TLV.)

- 4.2: So length=0 means "send all" but app-id=0x0000 means
"who's there"? That seems clunky to me fwiw. I also don't get how
the last para of 4.2 is fully specified.

- 5.1, Saying the MI is set at the "sender's discretion" seems
wrong, given you earlier said it has to be unique within some
(not that well) defined scope.

- 6.1: "secret string" - please don't say that as it  might be
interpreted to mean something human readable/memorable which
results in far weaker security. Please say "secret value" or
"secret octet string" and for bonus points say that if this is
human memorable then its basically pointless and that
implementations MUST be able to handle essentially random binary
values of the appropriate length.

- 6.3: Please refer to RFC2104 which is where HMAC is defined.
Following the bad practices from other RFCs that repeat the
definition is really not a good plan.

- I'd encourage you to say that use of GAP authentication is
RECOMMENDED?  That way, it may get used. Otherwise, I suspect
that it may be ignored, and we may be sorry about that later if
it turns out to just be another fig-leaf.

[Ballot comment]
The GenArt review by Martin Thomson raised some points that still need to be discussed.


There seems to be an editors' note embedded in Section 3

[Ballot discuss]
The authors said they would address the GenArt review from Martin
Thomson a week before the telechat, but that seems to have fallen
through.

My assessment is that the changes will not be major, so there is no
need to defer AD evaluation. This Discuss is a place-holder for the
work.

[Ballot discuss]
This DISCUSS overlaps a fair bit with Stephen's, but I think there are a few differences.


Section 6 needs a fair bit of re-writing, but I think the net effect will be to make it simpler.  In particular, Section 6.3. should be eliminated entirely, since it all it does is re-specify HMAC.  Instead, Section 6.2 should simply specify the inputs to HMAC, and that the output of HMAC is set as the Athentication Data payload.  Alternatively, just replace Section 6.3 with something like the following:

NEW:
"""
The contents of the Authentication Data TLV are the result of computing the HMAC value for the Authentication Keystring and a modified version of the GAP message.  In the below, we denote by HMAC(K,T) the HMAC function for the appropriate Key ID.

MAC computation procedure:
Input: Message without Authentication Data TLV, Key ID
1. Add an Authentication Data TLV to the GAP message, with payload of equal length to the HMAC output, filled with the value 0x878FE1F3, repeated enough times to fill the payload.
2. Compute HMAC(K, Message), where K is the Authentication Key String for the chosen Key ID.
3. Change the payload of Authentication Data TLV to be the output of the HMAC.
Output: GAP message with Authentication Data TLV

MAC verification procedure:
1. Copy the value of the Authentication Data TLV to a temporary variable AD.
2. Overwrite the payload of the Authentication Data TLV with the value 0x878FE1F3, repeated enough times to fill the payload.
3. Compute HMAC(K, Message), where K is the Authentication Key String for the chosen Key ID.
4. Verify that the output of the HMAC is equal to the transmitted value AD.
"""


"""
  This is accomplished by attaching a GAP
  Authentication TLV and including, in the Authentication Data field,
  the output of a cryptographic hash function,
"""
The phrase "hash function" is consistently used where "message authentication code" or "MAC" should be.


"""
  These parameters are not sent over the wire; they are
  assumed to be associated, on each node, with the Key ID by external
  means, such as via explicit operator configuration or a separate key-
  exchange protocol.
"""
The document should either state that a given sender/reciever pair MUST have at most one Key ID between them, or adapt the format of the Authentication Data TLV to include the Key ID.    In any case, Key IDs should be scoped to a sender/receiver pair, to prevent a compromised key from one association being used to spoof messages on another channel.


"""
      At
      present, the following values are possible: HMAC-SHA-1, HMAC-SHA-
      224, HMAC-SHA- 256, HMAC-SHA-384, and HMAC-SHA-512.
"""
Does this mean that implementations are REQUIRED to support all of these algorithms?


I'm concerned that there are some indeterminacies related to combinations of the GAP TLVs in Section 4.  For example, what should I do if I get both Suppress and Request TLVs?  Should I reply and then shut up, or just shut up?  Is it dependent on the order?  It seems like the simplest way to address this might be to require that Application-0x0000 TLVs MUST be processed in order.  So (Suppress, Request) would result in silence, while (Request, Suppress) would result in the response being sent, then nothing.

[Ballot comment]
"""
  The
  payload of a GAP message is a collection of Type-Length-Value (TLV)
  objects, organized on a per-application basis
"""

The use of the word "application" in this section is a little confusing.  It would be helpful to note that this is an "application" in the sense of "something that uses GAP", not in the sense of "application layer".


"""
  where the numbers are specific Type values.
"""

Which "numbers"?  The ones following "TLV"?  Suggested: "where the numbered values "TLV#" represent specific Type values.


"""
  Upon receiving the second message, the receiver retains B-TLV1 from
  the first message and adds B-TLV7 to its B-database.
"""

This is the first mention of a "database", much less a per-application one, and there is no mention of databases anywhere else in the document, nor in RFC 5586.  It seems there's an architectural assumption that needs to be stated here.


"""
  Version: Protocol version, currently set to 0
"""

Should this be "MUST be set to 0"?


"""
For TLVs not carrying static data the Lifetime is of no significance.
"""

How does the recipient know when a TLV carries static data?  Application defined?  Lifetime > 0?  Suggest, "For TLVs not carrying static data the Lifetime is no significance.  The sender of a GAP message indicates this by setting the Lifetime field to 0.".


"""
  Otherwise, the Value field specifies the applications for
  which an update is requested, in the form of a sequence of
  Application IDs:
"""

This might benefit from some clarity on responder behavior.  MAY the responder send a different list of apps?  MUST all requested apps be present in the response?


It would be nice to have a checksum-like mode for the Authentication Data, which provides some degree of integrity without a need for key management.  If you define the above application, you could also have a TLV that follows the same procedures as Authentication Data, but with a fixed set of parameters (HMAC function and key).

[Ballot discuss]
No general objection about this document except that the any impact of congestion is not mentioned and not handled.

1) Congestion Control issues:
First a question: Can the GACH take all of the capacity of the link or at least of the share of the link of the associated data channel?
Second question:  What is the capacity of such a GACH in general?

Now the issues in detail:
Section 2., paragraph 11:

>    The rate at which GAP messages are transmitted is at the discretion
>    of the sender, and may fluctuate over time as well as differ per
>    application.  Each message contains, for each application it
>    describes, a lifetime that informs the receiver how long to wait
>    before discarding the data for that application.

This is error prone, as the sender can send GAP messages at a high rate, potentially congest the link and cause also damage to other applications. Especially, if multiple applications send their GAP messages at the same time, for whatever reason.

Section 5.1., paragraph 8:

>    In some cases an application may desire additional reliability for
>    the delivery of some of its data.  When this is the case, the
>    transmitter MAY send several (for example three) instances of the
>    message in succession, separated by a delay appropriate to, or
>    specified by, the application.  For example this procedure might be
>    invoked when sending a flush instruction following device reset.  The
>    expectation is that the receiver will detect duplicate messages using
>    the MI.

This looks like a potential issue in terms of multiple applications intend to send burst of data.
  I can see the goal to have some sort of reliability in GAP message delivery, but sending a burst of data (i.e., three in a row is a burst) will cause trouble at some point.

In general, it might look easy to avoid any congestion control in this setting, but I can see that this lack of congestion control is putting the GACH at risk due to an overload situation.

2) Timing issue:
Section 3., paragraph 13:

>      Message Identifier (MI): Unique identifier of this message.  A
>      sender MUST NOT re-use an MI over a given channel until the
>      message lifetime has expired.  The sole purpose of this field is
>      duplicate detection in the event of a message burst (Section 5.1).

Does this mean that the sender can immediately reuse the MI after the timer has expired at the sender's side? If yes, there is the chance for a race condition, as the receiver side might not yet have expired the timer. I would propose to add some safety margin, i.e., so it the MI is expired on both sides.

[Ballot discuss]
There was a Gen-ART review that has not received a response. (As far as I can tell. But I may not have been on all lists or private e-mail, so I could have missed it.) Can the authors respond so that we can be sure the issues and editorial comments from Martin have been taken into account?

[Ballot comment]
Review by Martin:

I am the assigned Gen-ART reviewer for this draft. For background on
Gen-ART, please see the FAQ at

.

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?

[Ballot discuss]



- DISCUSS-DISCUSS

  This document specifies procedures for an MPLS Label Switching Router
  (LSR) to advertise its capabilities and configuration parameters, or
  other application-specific information, to its peers over LSPs,
  pseudowires, and sections.

  ...
  The data elements
  distributed by the GAP are application-specific and, except for those
  associated with the GAP itself, are outside the scope of this
  document.  An IANA registry is created to allow GAP applications to
  be defined as needed.


What's the point of this draft if you don't register the applications that can use this protocol?
I was expecting a registry with the different capabilities.

- DISCUSS
Below is Scott Bradner's OPS Directorate review (part 1)

This is an OPS_DIR review of draft-ietf-mpls-gach-adv-06.
The ID describes using the MPLS Generic Associated Channel (G-ACh ) to permit an LSP endpoint to inform other LSP endpoints of information of its choosing.
The specification is not all that detailed and leaves most of the actual functionality to the applications that wish to exchange information over the G-Ach.
Unlike most IDs this one includes a " Managability Considerations" - having such a section is great concept - but this example is not all that useful.  The section provides a number of MUSTs but does not provide justification for them - it would help the implementers and operators if there were some additional information provided to say why the MUSTs are MUSTs.  This type of thing is also a problem (a more important one imo) in section 3 which includes SHALL NOT and MUST NOT but does not say why such a prohibition is important.  The authors know how to provide such context, they do so in section 5.1, it would be help if they did the same wherever they insist that something MUST be done or MUST NOT be done.

[Ballot comment]
-  OAM  Operations, Administration, and Maintenance
Don't we have a definition somewhere in a recent RFC?


- Below is Scott Bradner's OPS Directorate review (part 2)

Aside from the above I do not see any operations-specific issues but I think there is a problem in section 2.  The ID says:

"The GAP itself provides no fragmentation and reassembly mechanisms. In the event that an application wishes to send larger chunks of data via GAP messages than fall within the limits of packet size, it is the responsibility of the application to fragment its data accordingly. "

I would think that some mention should be made here about not sending too big chunks of data because of the risk of congestion.  A large chunk of information, fragmented into multiple non-congestion responsive fragments, does not sound like a good idea and some warning would seem to be in order.

[Ballot discuss]
This DISCUSS overlaps a fair bit with Stephen's, but I think there are a few differences.


Section 6 needs a fair bit of re-writing, but I think the net effect will be to make it simpler.  In particular, Section 6.3. should be eliminated entirely, since it all it does is re-specify HMAC.  Instead, Section 6.2 should simply specify the inputs to HMAC, and that the output of HMAC is set as the Athentication Data payload.  Alternatively, just replace Section 6.3 with something like the following:

NEW:
"""
The contents of the Authentication Data TLV are the result of computing the HMAC value for the Authentication Keystring and a modified version of the GAP message.  In the below, we denote by HMAC(K,T) the HMAC function for the appropriate Key ID.

MAC computation procedure:
Input: Message without Authentication Data TLV, Key ID
1. Add an Authentication Data TLV to the GAP message, with payload of equal length to the HMAC output, filled with the value 0x878FE1F3, repeated enough times to fill the payload.
2. Compute HMAC(K, Message), where K is the Authentication Key String for the chosen Key ID.
3. Change the payload of Authentication Data TLV to be the output of the HMAC.
Output: GAP message with Authentication Data TLV

MAC verification procedure:
1. Copy the value of the Authentication Data TLV to a temporary variable AD.
2. Overwrite the payload of the Authentication Data TLV with the value 0x878FE1F3, repeated enough times to fill the payload.
3. Compute HMAC(K, Message), where K is the Authentication Key String for the chosen Key ID.
4. Verify that the output of the HMAC is equal to the transmitted value AD.
"""


It's not clear that a TLV is an appropriate place for this, since the MAC covers the entire GAP message and the TLVs are application specific.  Suggest creating a dedicated application for this, and modifying the Section 6.3. computations to use this application. 

It would be nice to have a checksum-like mode, which provides some degree of integrity without a need for key management.  If you define the above application, you could also have a TLV that follows the same procedures as Authentication Data, but with a fixed set of parameters (HMAC function and key).



"""
  This is accomplished by attaching a GAP
  Authentication TLV and including, in the Authentication Data field,
  the output of a cryptographic hash function,
"""
The phrase "hash function" is consistently used where "message authentication code" or "MAC" should be.


"""
  These parameters are not sent over the wire; they are
  assumed to be associated, on each node, with the Key ID by external
  means, such as via explicit operator configuration or a separate key-
  exchange protocol.
"""
The document should either state that a given sender/reciever pair MUST have at most one Key ID between them, or adapt the format of the Authentication Data TLV to include the Key ID.    In any case, Key IDs should be scoped to a sender/receiver pair, to prevent a compromised key from one association being used to spoof messages on another channel.


"""
      At
      present, the following values are possible: HMAC-SHA-1, HMAC-SHA-
      224, HMAC-SHA- 256, HMAC-SHA-384, and HMAC-SHA-512.
"""
Does this mean that implementations are REQURIED to support all of these algorithms?

[Ballot comment]
"""
  The
  payload of a GAP message is a collection of Type-Length-Value (TLV)
  objects, organized on a per-application basis
"""

The use of the word "application" in this section is a little confusing.  It would be helpful to note that this is an "application" in the sense of "something that uses GAP", not in the sense of "application layer".


"""
  where the numbers are specific Type values.
"""

Which "numbers"?  The ones following "TLV"?  Suggested: "where the numbered values "TLV#" represent specific Type values.


"""
  Upon receiving the second message, the receiver retains B-TLV1 from
  the first message and adds B-TLV7 to its B-database.
"""

This is the first mention of a "database", much less a per-application one, and there is no mention of databases anywhere else in the document, nor in RFC 5586.  It seems there's an architectural assumption that needs to be stated here.


"""
  Version: Protocol version, currently set to 0
"""

Should this be "MUST be set to 0"?


"""
For TLVs not carrying static data the Lifetime is of no significance.
"""

How does the recipient know when a TLV carries static data?  Application defined?  Lifetime > 0?  Suggest, "For TLVs not carrying static data the Lifetime is no significance.  The sender of a GAP message indicates this by setting the Lifetime field to 0.".


"""
  Otherwise, the Value field specifies the applications for
  which an update is requested, in the form of a sequence of
  Application IDs:
"""

This might benefit from some clarity on responder behavior.  MAY the responder send a different list of apps?  MUST all requested apps be present in the response?

[Ballot discuss]

First, thanks for section 6. It was a nice surprise and is nearly
but not quite there:-) I hope these discuss points will be easily
resolved.

(1) 6.1: Why is there no mandatory to implement HMAC hash
function? Without that, you won't get interop. I suggest making
HMAC-SHA256 MTI. HMAC-SHA1 would also be ok. (I'd also suggest
just deleting the other options unless you've a specific reason
to want them here.)

(2) 9: How do you know that application specific data won't
require confidentiality? I can imagine keys being sent using
this, as has been done in Diameter. If you don't want to define a
confidentiality service then I suggest that you say that
application or GAP TLVs that require confidentiality (such as
cryptographic keys) MUST NOT be sent in clear using the GAP
protocol because someone will write a draft that does just that
otherwise.

(3) 6.1: If using karp key tables, how do the SendNotBefore,
SendNotAfter and similar values from that map to the
Timestamp+Lifetime used here? I think you need to say as
implementers will make different assumptions otherwise. (I don't
care much which of the reasonable options you choose.)

(4) 6.2: you need to say what is in the V for the auth data TLV
as input to HMAC. Options are to include L zeros as the value or
for the V octets to be missing on input to HMAC. See also the
secdir review [1] which makes the same point. 

  [1] http://www.ietf.org/mail-archive/web/secdir/current/msg03795.html

[Ballot comment]

- Only specifying how a sender sends but not saying what a
receiver does on receipt seems to result in a fairly incomplete
protocol.  I don't see that the usual "its MPLS, we manage these
devices" response works for that. This isn't a DISCUSS though
since I guess I could accept you saying "the receiver just puts
the TLV into a local database, but its seems fairly weak really.

- I assume that there are no congestion issues that are likely to
arise with the G-ACh due to the use of this protocol?

- section 2: What does the first sentence of the last para of
section 2 mean? I can't parse it anyway.

- section 3, p6: "An error MAY be logged" - does that mean
locally or create some new n/w traffic? If the latter, isn't that
a potential DoS vector?

- s3, p7: MI definition - how is MI expiry handled? You don't say
(here). If that uses the Lifetime from the application specific
body, then what if there are many of those?

- s3, p8: Why is an editor's note still present?

- s3, p8: Lifetime of 16 bits means max is ~18 hours. That seems
oddly short for configuration data for presumably heavily managed
links and liable to make implementation more complex since the
same stuff will have to be sent more than once every day.

- s3, p8: The "source-channel-application tuple" seems ambiguous
to me - I guess you mean expires-at is the higher level
Timestamp+Lifetime, right? Why not say that?

- 4.1: can a source IP address represent a multicast group or
does it have to be "unique" to the sender?

- 4.2: this seems like a nice potential DoS vector too. (Say if
caculation of some TLV for which I ask consumes resources for the
sender of a GAP message containing that TLV.)

- 4.2: So length=0 means "send all" but app-id=0x0000 means
"who's there"? That seems clunky to me fwiw. I also don't get how
the last para of 4.2 is fully specified.

- 5.1, Saying the MI is set at the "sender's discretion" seems
wrong, given you earlier said it has to be unique within some
(not that well) defined scope.

- 6.1: "secret string" - please don't say that as it  might be
interpreted to mean something human readable/memorable which
results in far weaker security. Please say "secret value" or
"secret octet string" and for bonus points say that if this is
human memorable then its basically pointless and that
implementations MUST be able to handle essentially random binary
values of the appropriate length.

- 6.3: Please refer to RFC2104 which is where HMAC is defined.
Following the bad practices from other RFCs that repeat the
definition is really not a good plan.

- I'd encourage you to say that use of GAP authentication is
RECOMMENDED?  That way, it may get used. Otherwise, I suspect
that it may be ignored, and we may be sorry about that later if
it turns out to just be another fig-leaf.

IESG/Authors/WG Chairs:

IANA has reviewed draft-ietf-mpls-gach-adv-06.  Authors should review the comments and/or questions below.  Please report any inaccuracies and respond to any questions as soon as possible.

We understand that, upon approval of this document, there are four actions which we must complete.

First in the Pseudowire Associated Channel Types subregistry of the Pseudowire Name Spaces (PWE3) registry located at:

http://www.iana.org/assignments/pwe3-parameters

a single new associated channel type will be registered as follows:

Value: [ TBD-at-registration ]
Description: G-ACh Advertisement Protocol
TLV Follows: No
Reference: [ RFC-to-be ]

Second, in the Address Family Numbers registry located at:

http://www.iana.org/assignments/address-family-numbers

three new address family numbers are to be registered from the Standards Action range as follows

Number: [ TBD-at-registration ]
Description: MPLS-TP Section Endpoint Identifier
Reference: [ RFC-to-be ]

Number: [ TBD-at-registration ]
Description: MPLS-TP LSP Endpoint Identifier
Reference: [ RFC-to-be ]

Number: [ TBD-at-registration ]
Description: MPLS-TP Pseudowire Endpoint Identifier
Reference: [ RFC-to-be ]

Third, a new registry called the G-ACh Advertisement Protocol Applications registry is to be created in the Pseudowire Name Spaces (PWE3) registry located at:

http://www.iana.org/assignments/pwe3-parameters

The new reegistry is maintained via IETF Review as defined in RFC 5226. Applications ID are values between 0x0000 ad 0xFFFF inclusive. There are initial registrations in the new registry as follows:

Application ID Description Reference
--------------- ------------------------------- ----------------
0x0000 G-ACh Advertisement Protocol [ RFC-to-be ]

Fourth, a new registry called the G-ACh Advertisement Protocol: GAP TLV Objects (Application ID 0) registry is to be created in the Pseudowire Name Spaces (PWE3) registry located at:

http://www.iana.org/assignments/pwe3-parameters

The new registry is to be maintained via IETF Review as defined in RFC 5226. Type IDs are values between 0 - 255 inclusive. There are initial registrations in this new registry as follows:

Type Name Type ID Reference
------------------ ------- --------------
Source Address 0 [ RFC-to-be ]
GAP Request 1 [ RFC-to-be ]
GAP Flush 2 [ RFC-to-be ]
GAP Suppress 3 [ RFC-to-be ]
GAP Authentication 4 [ RFC-to-be ]

We understand that these four actions are the only ones required to be completed upon approval of this document.

Note:  The actions requested in this document will not be completed until the document has been approved for publication as an RFC. This message is only to confirm what actions will be performed.

The following Last Call announcement was sent out:

From: The IESG
To: IETF-Announce
CC:
Reply-To: ietf@ietf.org
Subject: Last Call:  (MPLS Generic Associated Channel (G-ACh) Advertisement Protocol) to Proposed Standard


The IESG has received a request from the Multiprotocol Label Switching WG
(mpls) to consider the following document:
- 'MPLS Generic Associated Channel (G-ACh) Advertisement Protocol'
  as Proposed Standard

The IESG plans to make a decision in the next few weeks, and solicits
final comments on this action. Please send substantive comments to the
ietf@ietf.org mailing lists by 2013-02-27. Exceptionally, comments may be
sent to iesg@ietf.org instead. In either case, please retain the
beginning of the Subject line to allow automated sorting.

Abstract

  The MPLS Generic Associated Channel (G-ACh) provides an auxiliary
  logical data channel associated with a Label Switched Path (LSP), a
  pseudowire, or a section (link) over which a variety of protocols may
  flow.  These protocols are commonly used to provide Operations,
  Administration, and Maintenance (OAM) mechanisms associated with the
  primary data channel.  This document specifies simple procedures by
  which an endpoint of an LSP, pseudowire, or section may inform the
  other endpoints of its capabilities and configuration parameters, or
  other application-specific information.  This information may then be
  used by the receiver to validate or adjust its local configuration,
  and by the network operator for diagnostic purposes.


The file can be obtained via
http://datatracker.ietf.org/doc/draft-ietf-mpls-gach-adv/

IESG discussion can be tracked via
http://datatracker.ietf.org/doc/draft-ietf-mpls-gach-adv/ballot/


No IPR declarations have been submitted directly on this I-D.

AD review
=========
Hi,

I have conducted my usual AD review of your document as part of the
publication request processing. The aim of my review is to catch any
issues that might show up in IETF last call, directorate reviews, or
IESG evaluation. The intent is to resolve the issues by text changes
or through email discussion to smooth the passage of the document
through the later stages of the process.

I have reviewed this document jointly with draft-ietf-mpls-tp-
ethernet-addressing and since the author team is the same for the two
documents, I recommend that you process the comments for the two
documents at the same time.

It will become clear to you as you read this review that I am not
overly impressed with the quality of this protocol specification. It is
possible that the issue lies with documentation details and the level of
review that the working group gave. However, I have not often seen a
better example of why it is valuable to have interworking
implementations of a protocol specification before requesting
publication.

As usual, all my comments are open for disagreement and discussion. I
think that you will probably want to produce a revision of this
document to address my comments, so I have put it into "Revised I-D
Needed" state in the data tracker.

Thanks for the work,
Adrian

===

Please update the captions to the figures as "Figure n : blah, blah"

---

Section 1

  It provides an auxiliary logical data channel
  associated with an MPLS Label Switched Path (LSP), a pseudowire, or a
  section (link) over which a variety of protocols may flow.

Marginally ambiguous because of clause ordering. It is not "a section
over which a variety of protocols may flow." Suggest re-wording...

  It provides an auxiliary logical data channel over which a variety of
  protocols may flow. Each such data channel is associated with an MPLS
  Label Switched Path (LSP), a pseudowire, or a section (link).

---

Section 1

  ...Operations, Administration, and Maintenance (OAM)
  capabilities associated with the underlying LSP, pseudowire, or
  section.

I don't think "underlying" is right, is it?
How about:

  ...Operations, Administration, and Maintenance (OAM)
  capabilities for the associated LSP, pseudowire, or section.

---

Section 1

  The main principle guiding the design of the MPLS G-ACh advertisement
  protocol (GAP) is simplicity.

Capitalise, please.

---

Section 1.1

After...
  The G-ACh advertisement protocol presented in this document
  thus allows LSRs to exchange information of a similar sort to that
  supported by LLDP for Ethernet links.
...could you please a simple sentence explaining why it was not enough
to provide a way to encapsulate LLDP in the G-ACh. [Hint: I think the
answer is that you envisage the GAP being used for a larger set of
MPLS-specific features than is currently defined in LLDP and for which
you do not consider it would be appropriate to extend LLDP.]

This is particularly important given that:
- the only use of the GAP mentioned anywhere is [I-D.ietf-mpls-tp-
  ethernet-addressing]
- you say...
  Where
  it is anticipated that the sole purpose of the GAP will be to provide
  Ethernet MAC address learning, the use of LLDP SHOULD be considered.
From where I am sitting, that looks like "This protocol is not needed
because LLDP does everything we want."

So, the question you need to answer (for me and in the I-D) is why have
you invented a new protocol when LLDP does everything that is needed?

And, BTW, I would like to understand the meaning of "SHOULD" in a 2119
context in this document. Since it is a protocol spec, and since it
references 2119, the implication is that implementations of
[I-D.ietf-mpls-tp-ethernet-addressing] should not be made unless
another I-D using the GAP is published as an RFC *and* implemented by
the same implementation.

Please don't think that deleting this text will get you off the hook!
You have already made the point (and with WG consensus). Now you need to
explain to me why there is a need for either document.   

I guess you should also say why you consider LMP to be inappropriate.

---

It is not really necessary to include LSR in Section 1.2 as the term
is only used before that section.

---

Section 2

  Although one GAP message can contain data for several applications,
  the receiver maintains the data associated with each application
  separately. This enables the sender to transmit a targeted update
  that refreshes the data for a subset of applications without
  affecting the data of other applications.

How the receiver maintains the data is entirely an implementation   
issues. I suggest you rephrase this as:

  Each GAP message can contain data for several applications. A
  sender may transmit a targeted update that refreshes the data for a
  subset of applications without affecting the data of other
  applications sent on a previous message.
                                                         
---

Section 3

After "XXXX" please note "(TBD by IANA)" so that the RFC editor
spots this, correlates it with the IANA section, and updates it
accordingly.

In Section 9.1, please mark the value to be assigned as XXXX
                                                                             
---

Section 3

s/A Gap message/A GAP message/

---

Section 3

Can two ADB elements for the same application be present in the
same message? If so what processing rules apply? If not, how is the
error handled?

Can two TLVs of the same type be present in the same ADB element? If
so what processing rules apply? If not, how is the error handled?

And the combination of cases, if two ADB elements for the same
application can be present in the same message, can a TLV of the same
type be present in each? If so what processing rules apply? If not,
how is the error handled?

---

Section 3 and 5

Although Section 5.1 tells us how the Message Identifier can be set, it
is ambiguous wrt Section 3 since in Section 3 we have:

      Message Identifier: Unique identifier of this message

And in 5.1 we have:

  The Message Identifier (MI) uniquely identifies this message and is
  set at the sender's discretion.

What does "at the sender's discretion" mean? It seems to say: "If the
sender doesn't fancy setting it, that's OK."

Additionally, it is not clear what the uniqueness scope of the Message
Identifier is supposed to be. Presumably not globally unique! Is it per
sender or per data-channel per sender?
                               
The sole purpose of the MI seems to be contained in the final paragraph
of Section 5.1. If that is the case, you seem to have some confusion
between rapid retransmission (same message) and lifetime avoiding
retransmission (different message). All this needs to be cleaned up and
explained. Noting that the last sentence of 5.1 belongs in 5.2.

I have to wonder whether you really need the field.

---

Sections 2, 3, and 5

  The Lifetime field specifies how long,
  in seconds, the receiver should retain the data in this message.  If
  the lifetime is zero the data is immediately marked as expired.

Have you considered offering a lifetime value meaning "retain until   
replaced"? This would *considerably* simplify the job of the sender
which would not need to run multiple timers to ensure that the
information is updated in a timely manner and doesn't accidentally
expire.

Note also that 2*16 seconds is not a very long time. things like the
MAC address of an interface can safely be considered "stable".

In rather a topsy-turvy way you say in Section 5.1...
  Lifetimes SHOULD be set in such a way that at least three updates
  will be sent prior to Lifetime expiration.  For example, if updates
  are sent at least every 60 seconds, a Lifetime of 185 seconds may be
  used.
...Isn't it the other way around? Shouldn't updates be sent according to
the lifetime value that has been set?

Discarding the information when the timer expires needs to be clarified.
I think you mean "revert to the configuration-based state that was held
before the information was received." But you might mean "transition to
the complete absence of this information."

In Section 2 should you discuss message loss and the consequences? Any
mitigation you need to add? Is the retransmission to protect against
lifetime expiry supposed to handle this? If so, it doesn't seem
frequent enough to install the initial state? Do you need a two-speed
timer: rapid on first use and dropping down to slow for state retention?
Or would an Ack be easier? Probably, the final paragraph of Section 5.1
can be adapted to handle this case, and then you can put a forward
pointer in Section 2.

But note that the final paragraph of 5.1 is a bit of a mess!...
  In some cases additional reliability may be desired for the delivery
  of a GAP message.  When this is the case, the RECOMMENDED procedure
  is to send three instances of the message in succession, separated by
  a delay appropriate to the application.  This procedure SHOULD be
  used, if at all, only for messages that are in some sense
  exceptional; for example when sending a flush instruction following
  device reset.  The MI may be used to detect and discard duplicate
  messages.
...What is a "delay appropriate to the application?" Maybe it means
"Each specification of a GAP application must state the appropriate
delay to use for such fast retransmission." Which would be fine, but
would require this document to make the statement for Application 0x0000
and would require [] to make a statement for Application 0x0001.
... Also "The procedure SHOULD be used..." reads like the procedure is
intended to be used. But "...if at all" really means "The procedure
SHOULD NOT be used except..."

I can't understand what is meant by "If the lifetime is zero the data is
immediately marked as expired." Are you trying to say that the data
should be processed, potentially replacing any stored information, and
then should be "discarded" (noting my previous comment about "discarded"
not being clear)? Or are you trying to say, that the ADB element should
be discarded unprocessed?

Lastly, the recommended multiplier ("at least three updates will be sent
prior to Lifetime expiration") cuts it too fine! You need to allow for
out-queues, transmission, and processing. The normal approach to this
(probably going back to Van Jacobson) is to add a unit of half. Thus,
in your case, Lifetime = 3.5 * Update.

---

Section 3

Are TLV Value fields padded up to a multiple of 4 octets?

---

Section 3

It is clear that each ADB element must contain at least one TLV. What
is the error processing for an empty ADB element?

---

Section 4

Since the Application ID 0x0000 ADB contains "metadata and processing
instructions rather than static data that is meant to be retained"
please explain how the Lifetime field is to be set/interpreted.

---

Does the TLV in Section 4.1 support the identifiers described in
[I-D.ietf-mpls-tp-itu-t-identifiers]? The chain of references for
"non-IP MPLS-TP networks" appears to lead through RFC 6428 to RFC
6370 which (I think) does not include the non-IP identifiers.

I see that you are adding new values to the Address Family registry in
Section 9.2. It is not clear to me where the definitive reference for
those types live. I would like to see more clarity in the text about
which address (and where it is specified) is intended for each new
code point that you are asking IANA to allocate.

---

Section 2 says that this is a one-way protocol, however, Section 4.2
defines an Application ID 0x0000 TLV that forms part of a two-way
exchange and is actually a direct request for information.

Additionally, it seems a little odd to hide this in a TLV when what it
really is is a separate message type. By placing it in the TLVs you
open up the prospect of message thrash as two implementations send
each other updates without clearing out this TLV.

The problem might be amusingly exacerbated by placing 0x0000 in the
list of Application ID for which a refresh is requested.

In any case, please explicitly clarify that setting a length of zero
intends retransmission of the 0x0000 Application data.

---

Section 4.3

Does the flush apply to the 0x0000 Application Data previously sent?

Why do you not allow selective flushing per Application?

---

Section 4.4

  The request is strictly advisory: the
  receiver SHOULD accept and act on the request, but MAY override it at
  any time.

The "SHOULD" that you have used is stronger than "advisory". What is
more you don't mean "advisory" you mean "a request".

I suggest you change this to

  ...the receiver MAY accept and act on the request, MAY ignore the
  request, or MAY resume transmissions at any time according to
  implementation or configuration choices, and depending on local
  pragmatics.

This section is another example of a TLV that violates the statement
in Section 2 that this is a one-way protocol.

Lastly, you need to say what it means if the Duration leaves the
information as expired (i.e. the Duration takes us beyond the end of
the Lifetime). Obviously the sender (of the original information) can
decide that that would be a bad thing and retransmit anyway, but it is
not clear what the receiver (of the information) would do if the
transmission is not made.

Frankly, however, it is not clear why you have this TLV. Having gone
through the hoops of deciding that you need to be able to send the
data, that the data can time out, and that you need to retransmit the
data, I think you need to give better motivation for this TLV.

---

I wish Sections 4.1 through 4.5 would end up containing the actual
TLV Type codes to be used for each TLV. This could be achieved by
simply including the values in the figures. (This is safe because
these are the first allocations from the new registry and there is
no question of IANA not allocating them as requested).

---

5.1 I like that operation of GAP SHOULD be configurable per data
channel. But you appear to have written "SHALL".  This precludes
making an implementation that automatically and always runs GAP.
(I think it still allows implementations that don't support GAP at
all because they don't implement this spec).

---

Paragraph 3 of section 5.2 talks about retaining objects for
application data that can be handled. But this is implementation-
specific. I think it is only the information that has to be retained.
Try to talk about function rather than constrain implementation.

---

The last paragraph of 5.2 is a bit confusing.

  The receiver MAY make use of the application data contained in a GAP
  message to perform some level of autoconfiguration, for example if
  the application is an OAM protocol.  The implementation SHOULD,
  however, take care to prevent cases of oscillation resulting from
  each endpoint attempting to adjust its configuration to match the
  other.  Any such autoconfiguration based on GAP information MUST be
  disabled by default.

I *think* you are really trying to place constraints on future
specifications rather than on implementations (there is nothing in this
specification that could be used for auto-conf). But I note that
[I-D.ietf-mpls-tp-ethernet-addressing] looks like auto-conf and is not
disabled by default.

---

In Section 6.1, you should discuss the operation of key exchange
protocols in the absence of IP protocols in the network. After all, one
of the use-cases (oh, the only use case :-) for GAP is to operate in
networks that don't have IP available. You might also note that GAP is
described as a protocol for exchanging capabilities and configuration:
keys might be described by some as exactly such information.

If your fall-back is that keys have to be manually configured in the
absence of IP, then you desperately need to call this out in
[I-D.ietf-mpls-tp-ethernet-addressing] because manual configuration of
keys seems a whole lot harder than manual configuration of MAC
addresses.

---

Section 6.1

I believe that RFC 4107 says that you need to discuss mechanisms for
dynamic key update, to make a recommendation as to whether such
mechanisms are needed, and to state how often keys need to be updated.

---

The problem I see with processing of the Timestamp described in 6.2 is
that it does not allow for a sender correcting its clock. Can you add
text to help clarify how this case is handled.

---

The final paragraph of Section 6.2 is a bit of a throw-away. What you
are saying, I think is that "Full algorithm flexibility is provided by
the protocol's use of the Authentication Key Identifier which acts as
an indirect reference to the algorithm and key data in use. Implementors
SHOULD consider the use of [I-D.ietf-karp-crypto-key-table] for key
management."

You might move this text to the end of 6.1 where it will sit with the
discussion of the Key ID.

---

How do you distinguish "device discovery" in Section 7 from talking to
a device whose MAC address you don't know in [I-D.ietf-mpls-tp-ethernet-
addressing] ?

Very unclear which MAC address to use when.

---

Maybe Section 8 should point at Section 6 since Section 6 seems to be
all about Security mechanisms.

---
                                                                             
You have used [I-D.ietf-karp-crypto-key-table] in a normative way. Not
a big risk as it is currently in KARP WG last call. Please move it to
Section 10.1.

---

Trivial point, but there are precisely three authors, all marked as
editors. You could safely dispense with the "editor" designation (on
the front page and in the Authors' Addresses section).