Subject: Re: [sipcore] draft-yusef-sipcore-digest-scheme-02

From: Rifaat Shekh-Yusef <[email-masked]>
Date: Fri, 17 Jan 2014 21:37:00 -0500

Hi Paul,

Thanks for the thorough review.

I have addressed most of your comments in version -02:
http://tools.ietf.org/html/draft-yusef-sipcore-digest-scheme-02

See more inline...

Regards,
 Rifaat

On Thu, Jan 16, 2014 at 6:00 PM, Paul Kyzivat <[email-masked]> wrote:

> Rifaat,
>
> The new version is a definite improvement. Now I have more to comment on.
> :-)
>
> Section 2.1 defines a preference list. How would this be affected by the
> registration of additional algorithms in the future? ISTM that if there is
> reason to have a "standard" preference list, then it ought to be part of
> the registry. (Or perhaps this document should simply state that the the
> challenger should list the algorithms it supports in order of strength of
> the algorithm.
>
> I changed the text to align with what you have between the parenthesis.

Similarly, section 2.2 specifies the size of the digest per algorithm, for
> the currently defined algorithms. Again, shouldn't this be part of the
> registry, or perhaps there should just be a statement that the registry
> references a document that describes the algorithm, and the size of the
> digest must be specified as part of the algorithm description.
>
> I will changes the registry in the HTTP draft to add output size and
preference, and see how far we can go with that.

In section 2.4, it might be helpful to state that if the UAC doesn't
> support any of the algorithms in the response that it should abandon
> attempts to send this request.
>
> Done

> In section 2.5:
>
> You give a separate definition of H(entity-body) for SHA2-256 and
> SHA2-512-256. But what if some other algorithm from the registry is used?
> ISTM that what is needed is a general mechanism, parameterized by the
> selected algorithm.
>
> I am not clear on this. All I provided was hashes of different algorithms
for empty string; how would you parameterize that?

> Also, I think specifying this as informal deltas to 3261 is going to cause
> a great deal of trouble. I think it would be better to restate most or all
> of section 22.4 with the needed changes applied.
>
> Done.

> In section 3:
>
> If there is to be a registry, then I think enumerating the values from the
> registry in the ABNF is counter-productive. It will simply lead to
> confusion about what needs to be done to add another algorithm. I suggest
> something along the lines of:
>
>       algorithm =  "algorithm" EQUAL (
>                    <digest-alg-name-from-IANA-registry>
>                    / token )
>
> (And I'm debating whether 'token' ought to be there. I think not.)
>
> There is then a requirement that new values added to the registry must be
> compatible with the syntax of 'token'. That will need to go into
> [HTTP-DIGEST] that specifies the registry and requirements for adding to it.
>
>         Thanks,
>         Paul
>
>  Thanks for your thorough review.
>>
>> I tried to make the draft short and simple and rely on the HTTP draft
>> for more details, but that did not seem to work.
>>
>> I have just submitted a new version of the draft:
>> http://www.ietf.org/id/draft-yusef-sipcore-digest-scheme-01.txt
>>
>> With this version I added more details and described the changes
>> compared to RFC3261, instead of pointing to the HTTP draft.
>> I hope this is a bit more clear than the -00 version.
>>
>> Regards,
>>   Rifaat
>>
>>
>>
>>
>> On Wed, Jan 15, 2014 at 2:51 PM, Dale R. Worley <[email-masked]
>> <mailto:[email-masked]>> wrote:
>>
>>     It's great that you are writing this!
>>
>>     The draft as it is now written references "[HTTP-DIGEST]" in many
>>     places, but does not specify what that *is*.  In a sense, this is
>>     unavoidable, since the new HTTP-digest RFC isn't out yet.  But if one
>>     is trying to read *this* draft and figure out its significance, one is
>>     blocked by the fact that this draft is written from the point of view
>>     of being the glue that connects an unspecified document to SIP.  One
>>     solution would be to insert a note that tells the reader the current
>>     draft name for HTTP-DIGEST, a note that would be removed in the formal
>>     publication.
>>
>>         Since RFC 2543 is based on HTTP Digest as defined in RFC 2069, SIP
>>         servers supporting [HTTP-DIGEST] MUST ensure they are backwards
>>         compatible with RFC 2069.  Procedures for this backwards
>>         compatibility are specified in [HTTP-DIGEST].  Note, however, that
>>         SIP servers MUST NOT accept or request Basic authentication.
>>
>>     It's not clear to me what "are backwards compatible with RFC 2069"
>>     means, because RFC 2069 deals exclusively with HTTP.  And similarly, a
>>     SIP server doesn't support [HTTP-DIGEST] per se either.  I think you
>>     mean to say, "SIP servers conformant to this specification must be
>>     backwards compatible with SIP digest authentication as specified in
>>     RFC 3261 section 22.4 (which is based on RFC 2069).  The procedures
>>     for backward compatibility in SIP are the same as those specified for
>>     HTTP in [HTTP-DIGEST]."
>>
>>     My larger point is that neither RFC 2069 or [HTTP-DIGEST] applies to
>>     SIP directly; only the modifications of those made by RFC 2543, RFC
>>     3261, and this draft apply to SIP.
>>
>>                Therefore, any
>>                algorithms that have a dependency on the cnonce (including
>>                "MD5-sess", "SHA2-256-sess", and "SHA2-512-256-sess")
>> require
>>                that the qop directive be sent.  Use of the "qop" parameter
>>
>>     I think it would read better as "Therefore, any algorithms that
>>     require that cnonce be sent (including ...".
>>
>>         RFC 2543 did not allow usage of the Authentication-Info header
>> field
>>         (it effectively used RFC 2069). However, we now allow usage of
>> this
>>         header field, since it provides integrity checks over the bodies
>> and
>>         provides mutual authentication.  ...
>>
>>     There are now three specifications regarding authentication, RFC 2543,
>>     RFC 3261, and "RFC 3261 with the extensions in this document", and you
>>     should make clear which spec enables Authentication-Info.  The text
>>     above seems to say that it is the new draft that enables
>>     Authentication-Info, but looking at RFC 3261 clearly shows a
>>     specification for its use.
>>
>>         ... [HTTP-DIGEST] defines mechanisms for
>>         backwards compatibility using the qop attribute in the request.
>>     These
>>         mechanisms MUST be used by a server to determine if the client
>>         supports the new mechanisms in [HTTP-DIGEST] that were not
>> specified
>>         in RFC 2069.
>>
>>     The connection of remainder of this paragraph to the previous part is
>>     unclear to me.  It is also unclear what particular "backwards
>>     compatibility" is supported "using the qop attribute", as there are
>>     now at least two extensions and now two levels of backward
>>     compatibility.
>>
>>     And given that this seems to be a very general statement about how
>>     backwards compatibility (vs. digest auth in 3261) is achieved, it
>>     probably should be a separate paragraph.
>>
>>     Perhaps this would be clearer to me if I knew off the top of my head
>>     the details of the security mechanisms defined in these various
>>     documents, and then I would know which document is implied in each
>>     reference.  But it would be a better exposition if one did not have to
>>     be on top of all the details to be able to grasp the overall
>>     structure.
>>
>>     Also, is "qop" a "directive", a "parameter", or an "attribute"?
>>
>>            request-digest    =  LDQUOT digest-size LHEX RDQUOT
>>            digest-size       = "32" / "64"
>>
>>     This doesn't achieve what you want, since it results in
>>          "32a"
>>          "62f"
>>     being valid request-digest values.  The problem is that that in the
>>     BNF form "<n>element", "n" must be a literal decimal number.  What you
>>     intend is
>>            request-digest    =  LDQUOT 32LHEX RDQUOT
>>                              /  LDQUOT 64LHEX RDQUOT
>>     But I see in RFC 3261 that response-digest (in Authentication-Info) is
>>     syntactically allowed to be variable length:
>>          Authentication-Info  =  "Authentication-Info" HCOLON ainfo
>>                                  *(COMMA ainfo)
>>          ainfo                =  nextnonce / message-qop
>>                                   / response-auth / cnonce
>>                                   / nonce-count
>>          nextnonce            =  "nextnonce" EQUAL nonce-value
>>          response-auth        =  "rspauth" EQUAL response-digest
>>          response-digest      =  LDQUOT *LHEX RDQUOT
>>     Why not allow request-digest to be variable length?
>>            request-digest    =  LDQUOT *LHEX RDQUOT
>>     That prevents the need for further revisions if a larger digest is
>>     ever defined.
>>
>>     Dale
>>
>>
>>
>>
>> _______________________________________________
>> sipcore mailing list
>> [email-masked]
>> https://www.ietf.org/mailman/listinfo/sipcore
>>
>>
> _______________________________________________
> sipcore mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/sipcore
>

---

From: Rifaat Shekh-Yusef <[email-masked]>
Date: Sat, 18 Jan 2014 06:01:14 -0500

I forgot to mention that I added a new section about forking (section 2.5).

Regards,
 Rifaat

On Fri, Jan 17, 2014 at 9:37 PM, Rifaat Shekh-Yusef
<[email-masked]>wrote:

> On Thu, Jan 16, 2014 at 6:00 PM, Paul Kyzivat <[email-masked]>wrote:
>
>> Rifaat,
>>
>> Also, I think specifying this as informal deltas to 3261 is going to
>> cause a great deal of trouble. I think it would be better to restate most
>> or all of section 22.4 with the needed changes applied.
>>
>> Done.
>
>
>
>> In section 3:
>>
>> If there is to be a registry, then I think enumerating the values from
>> the registry in the ABNF is counter-productive. It will simply lead to

---

From: Paul Kyzivat <[email-masked]>
Date: Sat, 18 Jan 2014 13:12:38 -0500

Rifaat,

It is looking better. More comments:

Section 2.6 #6:

This gives three formulas for H(entity-body), using different 
algorithms. This may be confusing to people, especially if they are 
using some other registered algorithm. And it may lead those who want to 
register new algorithms to conclude that they need to update this text 
to include their algorithm.

I think what you really are trying to say is:

    6.  As a clarification to the calculation of the A2 value for
        message integrity assurance in the Digest authentication
        scheme, implementers should assume, when the entity-body is
        empty (that is, when SIP messages have no body) that the hash
        of the entity-body resolves to the hash of an empty
        string:

        H(entity-body) = chosen-algorithm("")

        For example, when the chosen algorithm is SHA2-256, then:

        H(entity-body) = SHA2-256("") =
       "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"

Section 3:

       request-digest    =  LDQUOT 32LHEX RDQUOT / LDQUOT 64LHEX RDQUOT

This isn't sufficient for future algorithms that require strings of 
different length. My suggestion is that you simply write this as

And then note in text that the number of hex digits must be specified by 
the specification of the algorithm used.

And, repeating from before:

If there is to be a registry, then I think enumerating the values from 
the registry in the ABNF is counter-productive. It will simply lead to 
confusion about what needs to be done to add another algorithm. I 
suggest something along the lines of:

There is then a requirement that new values added to the registry must 
be compatible with the syntax of 'token'. That will need to go into 
[HTTP-DIGEST] that specifies the registry and requirements for adding to it.

	Thanks,
	Paul

On 1/17/14 9:37 PM, Rifaat Shekh-Yusef wrote:
> On Thu, Jan 16, 2014 at 6:00 PM, Paul Kyzivat <[email-masked]
> <mailto:[email-masked]>> wrote:
>
>     Rifaat,
>
>     The new version is a definite improvement. Now I have more to
>     comment on. :-)
>
>     Section 2.1 defines a preference list. How would this be affected by
>     the registration of additional algorithms in the future? ISTM that
>     if there is reason to have a "standard" preference list, then it
>     ought to be part of the registry. (Or perhaps this document should
>     simply state that the the challenger should list the algorithms it
>     supports in order of strength of the algorithm.
>
>     Similarly, section 2.2 specifies the size of the digest per
>     algorithm, for the currently defined algorithms. Again, shouldn't
>     this be part of the registry, or perhaps there should just be a
>     statement that the registry references a document that describes the
>     algorithm, and the size of the digest must be specified as part of
>     the algorithm description.
>
>     SHA2-512-256. But what if some other algorithm from the registry is
>     used? ISTM that what is needed is a general mechanism, parameterized
>     by the selected algorithm.
>
> I am not clear on this. All I provided was hashes of different
> algorithms for empty string; how would you parameterize that?
>
>     cause a great deal of trouble. I think it would be better to restate
>     most or all of section 22.4 with the needed changes applied.
>
> Done.
>
>     In section 3:
>
>     If there is to be a registry, then I think enumerating the values
>     from the registry in the ABNF is counter-productive. It will simply
>     lead to confusion about what needs to be done to add another
>     algorithm. I suggest something along the lines of:
>
>                         <digest-alg-name-from-IANA-__registry>
>                         / token )
>
>     There is then a requirement that new values added to the registry
>     must be compatible with the syntax of 'token'. That will need to go
>     into [HTTP-DIGEST] that specifies the registry and requirements for
>     adding to it.
>
>              Thanks,
>              Paul
>
>         http://www.ietf.org/id/draft-__yusef-sipcore-digest-scheme-__01.txt
>         <http://www.ietf.org/id/draft-yusef-sipcore-digest-scheme-01.txt>
>
>         On Wed, Jan 15, 2014 at 2:51 PM, Dale R. Worley
>         <[email-masked] <mailto:[email-masked]>
>         <mailto:[email-masked] <mailto:[email-masked]>>> wrote:
>
>                         "MD5-sess", "SHA2-256-sess", and
>         "SHA2-512-256-sess") require
>                  RFC 2543 did not allow usage of the Authentication-Info
>         header field
>                  (it effectively used RFC 2069). However, we now allow
>         usage of this
>                  header field, since it provides integrity checks over
>         the bodies and
>                  supports the new mechanisms in [HTTP-DIGEST] that were
>         not specified
>                  in RFC 2069.
>
>         _________________________________________________
>         sipcore mailing list
>         [email-masked] <mailto:[email-masked]>
>         https://www.ietf.org/mailman/__listinfo/sipcore
>         <https://www.ietf.org/mailman/listinfo/sipcore>
>
>
>     _________________________________________________
>     sipcore mailing list
>     [email-masked] <mailto:[email-masked]>
>     https://www.ietf.org/mailman/__listinfo/sipcore
>     <https://www.ietf.org/mailman/listinfo/sipcore>
>
>

---

From: Rifaat Shekh-Yusef <[email-masked]>
Date: Sun, 19 Jan 2014 06:43:15 -0500

On Sat, Jan 18, 2014 at 1:12 PM, Paul Kyzivat <[email-masked]> wrote:

> This gives three formulas for H(entity-body), using different algorithms.
> This may be confusing to people, especially if they are using some other
> registered algorithm. And it may lead those who want to register new
> algorithms to conclude that they need to update this text to include their
> algorithm.
>
Sure. I will do that in the next version of the draft.

>       request-digest    =  LDQUOT *LHEX RDQUOT
Yeah, I did that with the HTTP draft, but forgot to that for this draft. I
will fix it.

>                    <digest-alg-name-from-IANA-registry>
Agree

> [HTTP-DIGEST] that specifies the registry and requirements for adding to it.
Here is the IANA registry as defined in the latest HTTP draft:
http://tools.ietf.org/html/draft-ietf-httpauth-digest-02#section-6

I am not clear on what you mean by " ...compatible with the syntax of
'token' ".

Thanks,
 Rifaat