Subject: [secdir] secdir review of draft-ietf-mpls-gach-adv-06

From: Leif Johansson <[email-masked]>
Date: Mon, 18 Feb 2013 14:08:15 +0100

I have reviewed this document as part of the security directorate's
ongoing effort to review all IETF documents being processed by the IESG.
These comments were written primarily for the benefit of the security
area directors.  Document editors and WG chairs should treat these
comments just like any other last call comments.

The technology is somewhat outside my area of expertise but I found
the document relatively easy to follow anyway.

I'm a fan of writing crypto-related algorithms with very little left 
for the imagination of the reader. To that end I would strongly suggest 
specifying what goes into the GAP message hash even more clearly. 

In this case I suspect the intent is that the inner hash is over all 
bytes of GAP message except the GAP authentication TLV which is added 
to the message _after_ the hash is computed.

Conversely the validation phase needs to clearly say what bits of the
message are to be included in computing the hash.

Also I would change the timestamp verification step to use normative
language, eg: "... the receiver MUST, upon successfully authenticating
a message verify that the timestamp field corresponds... The receiver 
MUST silently discard a GAP message that fails timestamp verification."

---

From: Dan Frost <[email-masked]>
Date: Mon, 18 Feb 2013 14:31:42 +0000

Hi Leif,

> the document relatively easy to follow anyway.

Thanks for your review.

> to the message _after_ the hash is computed.

No need to suspect; the current text is explicit.  ;)  It says:

   2.  First Hash

          First, the Authentication Data field is filled with the value
          Apad.

          Then, a first hash, also known as the inner hash, is computed
          as follows:

             First-Hash = H(Ko XOR Ipad || (GAP Message))

          Here the GAP Message is the portion of the packet that follows
          the Associated Channel Header.

The last paragraph specifies that the hash is computed over all bytes of
the GAP Message that follow the Associated Channel Header.  This
includes the Authentication TLV, which is not a problem because "First,
the Authentication Data field is filled with the value Apad."

> message are to be included in computing the hash.

The text states that "When the message is received, the receiver
computes a hash for it as described below", where "below" is Section 6.3
(we should probably s/below/Section 6.3/ here).  In other words, the
transmitter and receiver perform the same computation with the same
input (modulo the Authentication Data field, which is normalised by the
computation itself).

> MUST silently discard a GAP message that fails timestamp verification."

Well, use of the replay mitigation mechanism is not required, so I'm not
sure that adding "MUST" language is appropriate.

Cheers,
-d

---

From: Leif Johansson <[email-masked]>
Date: Mon, 18 Feb 2013 16:07:23 +0100

> the Authentication Data field is filled with the value Apad."
if you would just add that last sentence it would have been that
much clearer to me at least.
> input (modulo the Authentication Data field, which is normalised by the
So in the verification phase you first fill the Authentication Data
field with Apad? That wasn't clear to me from reading the text.
> sure that adding "MUST" language is appropriate.
I guess I'm then questioning the judgement of not making replay
checking a requirement since it seems fairly simple to do.