[apps-discuss] APPSDIR review of draft-kaplan-insipid-session-id-03

["Vijay K. Gurbani" <[e-mail]>]

I have been selected as the Applications Area Directorate reviewer for 
this draft (for background on appsdir, please see ​ 
http://trac.tools.ietf.org/area/app/trac/wiki/ApplicationsAreaDirectorate ).

Please resolve these comments along with any other Last Call comments 
you may receive. Please wait for direction from your document shepherd 
or AD before posting a new version of the draft.

Document: draft-kaplan-insipid-session-id-03
Title: A Session Identifier for the Session Initiation Protocol (SIP)
Reviewer: Vijay K. Gurbani
Review Date: Aug-14-2013
IETF Last Call Date: Aug-30-2013
IESG Telechat Date: Not known

Summary: This draft is almost ready for publication as an Informational
RFC but has a few issues that should be fixed before publication.

As a general comment, I believe that the document can benefit from an
editing session (overuse of commas at some places and using use of
"which" where "that" will suffice).  In some portions, the document
is written in a more colloquial style (e.g., "The following requirements
drive the need...", "In general, B2BUA behavior cannot be dictated by
standards.  They do whatever their owners/operators wish them to do, or
whatever is necessary to make their applications work.").  Such an
approach drives a conversational style that is helpful in getting the
work understood and accepted.  However, as the work heads towards the
RFC stage, it may be better to substitute this style with the more
pedantic and scholarly style better suited to RFCs.

That said, the document was an excellent read.

Major comments: 2
Minor comments: 9
Nits: 9

Major:
======
- S4.1: HMAC-SHA1 normally generates a 160-bit digest:
   $ echo '<?= hash_hmac("sha1", "[e-mail]", 
"5874125986874526") ?>' | php
   6438adc3c8337e75579a2a9b22875444943f09ca
   $

  However, you want to create a 128-bit digest.  So the assumption is
  that implementations just truncate the last 4 bytes.  Maybe being a
  bit explicit here will not hurt.

- S9.2, second paragraph: "One such weakness may be that a UAC
  generates one or more Call-IDs which have a property that makes
  determining the key more likely."  Here, do you mean that the weakness
  is that the UAC uses the same key to generate one or more Session-IDs
  with different Call-IDs thereby allowing the attacker to guess the
  key?  Ergo, we should use different secret keys each time we generate
  a Session-ID?

  I would think that given the cryptographic strength of SHA1 it will
  hard to determine patterns in the digest such that one is able to
  predict anything.  Given today's processors, SHA1 collisions using
  commodity hardware is not foreseen until 2018 the earliest (source
  http://www.schneier.com/blog/archives/2012/10/when_will_we_se.html).

  And furthermore, even if we assume that the same key was used to
  create Session-IDs for multiple Call-IDs, the attacker does not
  know whether the Session-ID seen in a message used the Call-ID
  in the same message or not.  For all the attacker knows, some B2BUA
  upstream may have changed the Call-ID but left the Session-ID
  alone.

  In other words, if the fear is that a new key should be used for
  each generated Session-ID, then I think we are okay.

Minor:
======
- The first paragraph of the Abstract also appears as the first para-
  graph in the Introduction section; I would suggest you remove it
  from the Abstract.

- S1, second paragraph: s/for matching to/for finding/

- S1, third paragraph: why is the end-to-end message path logical?
  It could, and indeed is, physical as well.  I suspect that removing
  the "logical" adjective does not harm to the original sentence.

- S1.1: I am not sure I understand the need to assign probabilities
  to the requirements.  A requirement is either supported or not, it
  is a binary decision and not one that involves a continuous value.

- S1.1, last bullet: I am not sure what the "SIP layer" is --- well,
  I know what it is from having trawled in this space for a while.  But
  a neophyte user who reads this may wonder what a "SIP layer" is.
  Better instead to say, "The identifier must not affect the processing,
  behaviour, or other semantics associated with receiving and sending of
  SIP messages."

- S5, second paragraph: "Furthermore, out-of-dialog REFER and INVITE
  with [RFC3891] Replaces requests use the appropriate Session-ID
  values."  Here, what is "the appropriate Session-ID value"?  Later on
  you go on to say that the Session-ID values for such requests are
  inherited.  May be simpler just to point to the relevant sections or
  say that "Furthermore, out-of-dialog REFER and INVITE with [RFC3891]
  Replaces requests use the appropriate Session-ID values as described
  below."

- S6, last paragraph: The problem with what is described in this para-
  graph is that the Session-ID looses its efficacy.  Since the UAC did
  not support the Session-ID extension and the edge proxy did not
  either, the operator of the UAC has no way to trace the call until it
  gets back a response from the UAS with the Session-ID header.  So,
  at least when the call is being routed from the UAC, the operator has
  no way to track it.  I am not sure whether we need to come up with a
  solution for this, but may be worth mentioning in the text.

- S9.1, "The requirement ...", better to point out which requirement
  you are referring to (REQ2b).

- S9.2, first paragraph: "Because the algorithm..." --- note the lack of
  an "of", as in "Because of the algorithm ...".  But more importantly,
  which algorithm are you referring to?  SHA-1?  HMAC? HMAC is defined
  in rfc2104 and SHA1 is probably in some NIST FIPS publication.  What
  we are defining in this document is using HMAC-SHA1 on a Call-ID plus
  a secret key.  The properties of one-way hash you appear to refer to
  in the quoted text are inherent in HMAC-SHA1, no?

Nits:
=====
- S1, third paragraph, last sentence: why the asterisk in *must*?
  They don't render the ASCII text with any adoration, I would
  suggest that s/*must*/must/

  Same issue with a *not* in S4.5.1.

- S1, last paragraph: s/or have any/or possess any/

- S3, end of first paragraph: s/one which crosses B2BUAs and which has
  no/one that crosses B2BUAs and does not contain any/

- S4.1, I believe the construct in rfc2104 is HMAC-H-t, which when
  applied to SHA-1 with 128 bit digest output yields "HMAC-SHA1-128" and
  not "HMAC-SHA-1-128".

- S5, second paragraph: s/well, in/well in/

- S5, second paragraph: "This assumes, of course, ..." --- clearly any
  UA that supports this document will support Session-ID.  Thus, I think
  that prefacing text with statements of support for the document just
  add clutter.  Unless you are discussing backward compatibility issues,
  it is best to simply describe behaviour and expectations that are
  assumed of agents that support this document.

- S6, last paragraph: s/UAC which/UAC that/
   also, s/B2BUA which/B2BUA that/
   also, s/UASs which/UASs that/

- S9.1, s/indentifier which/identifier that/
  Many such occurrences in S9.2 as well.

- S9.2, second paragraph: s/behavior, by/behaviour by/

Thanks,

- vijay
-- 
Vijay K. Gurbani, Bell Laboratories, Alcatel-Lucent
1960 Lucent Lane, Rm. 9C-533, Naperville, Illinois 60563 (USA)
Email: vkg@{bell-labs.com,acm.org} / [e-mail]
Web: http://ect.bell-labs.com/who/vkg/  | Calendar: http://goo.gl/x3Ogq