Subject: [MSEC] Review of draft-mattsson-mikey-ticket-02

From: Vincent Roca <[email-masked]>
Date: Tue, 23 Mar 2010 03:15:03 +0100

Dear John/Tian,

Here are a few comments, after a careful reading your I-D. Sorry if
it's a bit long... Do not hesitate to tell me if I misunderstood some parts.
I must also confess I'm not a MIKEY expert and therefore I cannot judge
the most technical parts. Finally it's also possible there are some
duplicate items with Tobias' secdir review (I didn't check).

Hope it will be useful, it's the goal.
Regards,

   Vincent

Main comments:
--------------

** The specified intended status is Informational. Is this motivated
   by the fact this is an individual submission?

** Fig. 3, p 12: If I understand well, mode (3) is also for the case
   where I requests in advance some tickets for future use as
   explained in section 5.3, "Ticket reuse". Am I right?
   I'm asking the question since the text refers to a different
   situation.

** Section 4.2.1.4, p15: it is said:
        "the KMS SHOULD send a REQUEST_RESP message."
   I don't agree, if everything is okay, the KMS *MUST* send a
   REQUEST_RESP message.

** Section 4.2.1.5, p15: it is said:
    "The KMS SHALL generate a fresh timestamp [...].
     If COUNTER [...] is used, the timestamp MAY be equal to the one
     in the REQUEST_INIT message."
   There's a contradiction here. You can't say at the same time that
   the timestamp SHALL be a fresh one and that it MAY be equal to a
   previously sent value.

   This remark can be made to Section 4.2.3.5 where the same text
   appears.

** Section 4.2.1.5, p15: it is said:
    "The KEMAC payload SHOULD use the NULL authentication algorithm,
     as a MAC is included in the V payload."
   Okay, I understand the meaning, but why are you saying SHOULD?
   I'm wondering if a MUST/SHALL is not preferable here to save
   useless complexity.

   This remark can be made to Section 4.2.2.3 where the same text
   appears.

** Section 4.2.1.6, p16: it is said:
    "If the Initiator can verify the integrity of the received
     message and the message can be correctly parsed, the ticket
     and the associated session information SHOULD be stored."
   Why are you saying SHOULD whereas, as I understand, "I" has no
   reason not to store this information and use it?

   Later in this section:
    "If not, the Initiator SHALL
     either discard or send a new REQUEST_INIT message suggesting
     a different Ticket Policy than before."
   Instead of "either ... or", I'd rather say "the Initiator SHALL
   discard and send a new REQUEST_INIT...".
   Another point, why is it said that the "Initiator SHOULD check
   the granted Ticket Policy"? Why SHOULD rather than MUST? What's
   the point of launching a Ticket Transfer if the policy is not
   appropriate? There's a risk this incoherency be discovered
   quite late in the whole process, after having wasting
   significant resources.

   This remark can be made to Section 4.2.2.4 where the same text
   appears.

** Section 4.2.2, p 16: it is said:
    "The messages are preferably included in the session setup
     signaling (e.g.  SIP INVITE and 200 OK)."
   There's an ambiguity here. I have problems figuring out which
   messages are concerned here. Please clarify. Thanks.

** Section 4.2.3.6, p22: If I understand well, rather than "MPK",
   you'd better say "with the MPKi received from the KMS".
   Another point: why "SHOULD" the Responder verify the
   TRANSFER_INIT? It is a SHALL/MUST, not a SHOULD.

** Section 5.1: For the first case, I suggest to say:
     "The KEMAC shown is the KEMAC sent from the KMS to the
      Initiator."
    i.e. be symmetric to the similar sentence in the second case.

    Later in Fig 5, what does "ID Data and RAND" refer to? Is
    this the content of the MOD field?

** 5.2, p 27: it is said:
     "The ticket and other static payloads that were
      provided in the initial exchange SHOULD NOT be included."
   Is the "SHOULD NOT" appropriate?
   I have the feeling that the simplest, the better (in particular
   there are fewer security risks). If it's useless, then we'd
   better say that they MUST NOT be included.

** Section 5.4: it is said:
      "If an error occurs, the message SHOULD be discarded and
       the the error SHOULD be reported with an error message."
   Remove the additional "the".
   Besides, to whom SHOULD the error be reported? Is it to
   a local log system, or to the peer, or the KMS? I guess
   it depends on the precise type of error, but I'd like to
   see more precision here.

   Later in this section (2 occurrences):
      Error message =
      HDR, T, {ERR}, [V|SIGNx]
   Since {} means there can be one or more occurrences, it means
   that the Error message may well contain *no* error field!!!
   What's the point, then?

** Table 5.2: I have a problem here... I have the feeling that
  nowhere before it has been mentioned that *_RESP messages
  must include a copy of the initial *_INIT message. It's not
  a small detail! It should be clearly stated before, and not in
  a section that discusses MAC/Signature coverage.
 
** Section 6.1: When describing the "V" field, it is said:
      "It SHALL be ignored in all messages except TRANSFER_INIT
       messages used for CSB updating (see Section 5.2)."
  I don't agree, you must say: "It MUST be set to 0 in all
  messages..." And also, you'd better say:
      "*  V (1 bit): flag to indicate whether a response message is
          expected (V=1) or not (V=0)." to remove any ambiguity.

** Section 6.1.1: In the first figure, it's not very clear whether
   padding is needed or not so as to have 32-bit alignment. In theory,
   there's no padding, but this figure shows an SPI that finishes
   on a 32-bit boundary.
   And it's also the case with th Ps figure...
   If there's no alignment consideration, make it explicit in the
   provided examples.

** Section 6.5: I have the feeling that the ID Type field is more an
   *encoding type* of the ID, than an "ID Type".
   Anyway the content of this field is a little bit unclear to me.

** Section 6.6: Why is the ID Len field 16-bit long? Do you really
   expect such long ID fields? (that's a naive question, sure!)
   Also, is this a size in terms of bytes, it's not said.

** Section 6.11: Similar question concerning the Ticket Type. This is a
   16-bit field whereas there are only two entries specified.
   Do you really expect this field to be populated to a point where
   you need 32 bits?

** Section 7: I didn't understand. Can you reformulate please?

** Section 8: How does R know the TEK used by the content provider?
   I guess you assume that the Responder initiated an exchange with
   the content provider to get the right of accessing the content,
   and obtained the TEK as a result. This is not explained however.

** Section 9: the scenario of Fig 7 is not really clear to me.
   Does it use a GTGK? I also suggest you add in Fig 7 caption
   "Centralized signaling around host A".

** Section 10, p43: it is said:
      "The KMSs SHALL be mutually authenticated and signaling
       between them SHALL be integrity protected."
   I don't agree. Signaling MUST be fully protected, i.e. encrypted,
   integrity/replay protected, authenticated. Otherwise it's an
   ideal target for attacks or eavesdropping.
  
   Another question: is there some commonly agreed way of
   naming the various entities (KMSs, I and R)? Because here
   the KMS may belong to different domains which means we need
   a common identification basis.

** Section 12.1, p44: Don't mention "random number generator".
   We are using "pseudo-random number generator" (PRNG)!

** Section 12.1, p45: it is said:
     "In other words, a 2^n work enables an attacker to get all
      k n-bit keys."
   I don't understand what you mean by "work".

** Section 12.3, last line of p46:
     "it is RECOMMENDED to use key forking and TGKs unless required
      by the use case."
   I guess you mean "NOT RECOMMENDED to use..."

** Section 12.4: it is said:
     "To prevent all forms of eavesdropping, only the endpoint that
      answers should get access to the session keys."
   Are you sure it is a should (BTW, it must be an uppercase "SHOULD").
   I think the following wording is more appropriate:
     "...no entity except the endpoint that answers MUST be able to
      get access..."

Typos and other minor comments:
-------------------------------

** p9: "Such scenarios excludes all key management" => "exclude"

** Section 4.2.1.4, p15: Split the first sentence in two sentences,
   please.

** Section 4.2.2.1, p 17: it is said:
     "but IDRi MAY be left
     out when it can be expected that the Responder has a single
     identity, and IDRr MAY be left out if the Responder can
     identify the Initiator by other means."
  I think IDRi and IRDr should be permuted here.

** Section 4.2.3.5: remove the initial ">" character.

** Section 5.2: It is said:
      "The CSB updating MAY be started by either the Initiator
       or the Responder."
   I'd rather say "CAN".

** Table 6.6: is "HMAC-SHA-256-256" the result of a typo or not?

** p40:
    "  An omitted TRs means that the validity period has defined beginning."
   Can you check this sentence. I'm not sure how to interpret it...

** p40: typos:
   "SHALL NOT be longer the the number of seconds" => "than the"

** Section 12.1, p45, last line: "then" -> "than"

---

From: John Mattsson <[email-masked]>
Date: Mon, 29 Mar 2010 10:06:27 +0200

Dear Vincent,

Many thanks for the careful and detailed review. I agree with the vast majority of your comments and will update the draft accordingly. Misunderstandings are often a sign that the text should be rewritten (but you did not misunderstand much at all).

It was very useful.

Regards,
John

-----Original Message-----
From: Vincent Roca [mailto:[email-masked]] 
Sent: den 23 mars 2010 03:15
To: [email-masked]
Cc: Brian Weis; [email-masked]; Vincent Roca; [email-masked]; Rolf Blom J
Subject: Review of draft-mattsson-mikey-ticket-02

> Dear John/Tian,
> 
> Here are a few comments, after a careful reading your I-D. Sorry
> if it's a bit long... Do not hesitate to tell me if I misunderstood
> some parts. I must also confess I'm not a MIKEY expert and therefore
> I cannot judge the most technical parts. Finally it's also possible
> there are some duplicate items with Tobias' secdir review
> (I didn't check).
> 

Main comments:
--------------

>    by the fact this is an individual submission?

Yes, and there was no interest in msec to have it as a WG item. Having it informal is in line with some other 3GPP related RFCs like EAP-SIM and EAP-AKA.

>    I'm asking the question since the text refers to a different

No, text is correct. The case when tickets are fetched in advance by the Initiator follows mode (1) or (2). Mode (3) is when the Initiator is allowed to generate tickets on its own. Maybe this can be clarified better.

>    I don't agree, if everything is okay, the KMS *MUST* send a

Yes, I agree. I'll change it to MUST.

>    This remark can be made to Section 4.2.3.5 where the same text

Yes, this seems like a contradiction and needs to be rewritten.

>    This remark can be made to Section 4.2.2.3 where the same text

OK, let's change it to MUST/SHALL.

>    reason not to store this information and use it?

Yes, I agree that is should be SHALL/MUST. I'll rewrite this sentence. 

>    discard and send a new REQUEST_INIT...".

Yes, I see your point but "the Initiator SHALL discard and send a new REQUEST_INIT..." may lead to an infinite loop. I'll change it to "the Initiator SHALL discard and MAY send a new REQUEST_INIT".

>    This remark can be made to Section 4.2.2.4 where the same text

Agree, let's change to MUST.

>    messages are concerned here. Please clarify. Thanks.

Yes, we will. It should be "The MIKEY messages are preferably ..."

>    TRANSFER_INIT? It is a SHALL/MUST, not a SHOULD.

Yes, Yes.

>     i.e. be symmetric to the similar sentence in the second case.

Yes, I'll change to "The KEMAC shown is the KEMAC sent from the KMS to the Initiator and Responder."

>     this the content of the MOD field?

Yes, they are the relevant fields from the modifier (IDRr, RANDRkms).

>    better say that they MUST NOT be included.

Yes, that's better.

>    Remove the additional "the".

Yes

>    it depends on the precise type of error, but I'd like to

It should be reported to the other party (Initiator, Responder, KMS). I'll make it more precise.

>    that the Error message may well contain *no* error field!!!

Yes, an error inherited from RFC 3830. I'll correct to "HDR, T, (ERR), [V|SIGNx]"

>   a section that discusses MAC/Signature coverage.

The *_RESP messages does not include a copy. It is only the MAC/Signature that covers the *_INIT message. And this is stated before. Is there a specific sentence that should be clarified?

>   messages..."

I'll change to "It MUST be set to 0 and ignored in all messages..."

> And also, you'd better say:

Ok

>    If there's no alignment consideration, make it explicit in the

There is no 32-bit alignment consideration for any MIKEY payload (even in 3830), only octet alignmnet. Maybe I can write a general sentence about this. Is there any better way to illustrate alignment in figures? Currently the figures are as in RFC 3830.

>    Anyway the content of this field is a little bit unclear to me.

Yes, the name is not very good. This is inherited from 3830. Maybe it should be clarified a little.

>    Also, is this a size in terms of bytes, it's not said.

This is inherited from RFC 3830, so it should not be changed. And identities may be very long, so I think it is better to be safe than sorry. I'll add that it is in bytes.

>    Do you really expect this field to be populated to a point where

This first 16 bit is the "global" part of the identifier; like "3GPP", "OMA", "EnterpriseABC" etc. So keeping it at 16-bits is reasonable as it is assigned by "specification required" and that private use can be expected. The private use range should maybe be expanded. 32 bits will clearly not be populated, but some extra fields are desirable to ease the use of subtypes.

> ** Section 7: I didn't understand. Can you reformulate please?

Ok

>    and obtained the TEK as a result. This is not explained however.

Ok, I'll expand and update this section.

>    Does it use a GTGK?

They do not even need to use a group key. I'll try to clarify. 

> I also suggest you add in Fig 7 caption

Good idea.

>    ideal target for attacks or eavesdropping.

Yes, fully protected.

>    a common identification basis.

No, we have not specified this but we assume that a KMS is defined by a FQDN and that I and R have identities based on SIP URIs.

>    We are using "pseudo-random number generator" (PRNG)!

OK. I'll change.

>    I don't understand what you mean by "work".

I'll change to: "In other words, the time needed by an attacker to get all k n-bit keys is proportional to 2^n".

>    I guess you mean "NOT RECOMMENDED to use..."

I'll rewrite the sentence. The beginning is correct: "it is RECOMMENDED to use key forking and TGKs." 

>      "...no entity except the endpoint that answers MUST be able to

Yes I'll rewrite.

> ** p9: "Such scenarios excludes all key management" => "exclude"

Yes

> ** Section 4.2.1.4, p15: Split the first sentence in two sentences,

OK

>   I think IDRi and IRDr should be permuted here.

Yes

> ** Section 4.2.3.5: remove the initial ">" character.

Yes

>       "The CSB updating MAY be started by either the Initiator

OK

> ** Table 6.6: is "HMAC-SHA-256-256" the result of a typo or not?

No, the thought was to have a similar syntax as the already defined HMAC-SHA-1-160.

>    Can you check this sentence. I'm not sure how to interpret it...

Yes, there is a "no" missing. It should be "An omitted TRs means that the validity period has no defined beginning." 

>    "SHALL NOT be longer the the number of seconds" => "than the"

Yes

> ** Section 12.1, p45, last line: "then" -> "than"

Yes