Subject: [sipcore] Doc Shepherd review of draft-ietf-sipcore-digest-scheme, cont.

From: "A. Jean Mahoney" <[email-masked]>
Date: Mon, 1 Jul 2019 14:30:38 -0500

Hi Rifaat,

Below are the rest of my Doc Shepherd review comments, mainly a few 
minor issues and nits. I have a suggested rework of the Introduction to 
clarify that MD5 is not collision resistant and that RFC3261 references 
the RFC obsoleted by RFC7616.

Thanks!

Jean

General: There are some instances of "SIP protocol" and "HTTP protocol". 
These should be replaced with either "SIP"/"HTTP" or "Session Initiation 
Protocol"/"Hypertext Transfer Protocol".

Abstract:  Should explicitly mention that the document updates RFC 3261 
(idnits flagged this). Should mention that MD5 is still available for 
backward availability.

Introduction:  Suggested rewording -

    The Session Initiation Protocol [RFC3261] uses the same mechanism
    that the Hypertext Transfer Protocol (HTTP) uses for authenticating
    users. This mechanism is called Digest Access Authentication, and
    it is a simple challenge-response mechanism that allows a server
    to challenge a client request and allows a client to provide
    authentication information in response to that challenge. The
    version of Digest Access Authentication that RFC 3261 references
    is specified in [RFC2617].

    The default hash algorithm for Digest Access Authentication is MD5.
    However, it has been demonstrated that the MD5 algorithm is not
    collision resistant, and is now considered a bad choice for a hash
    function [RFC6151].

    The HTTP Digest Access Authentication [RFC7616] document obsoletes
    [RFC2617] and adds stronger algorithms that can be used with
    the Digest Authentication scheme, and establishes a registry for
    these algorithms, known as the "Hash Algorithms for HTTP Digest
    Authentication" registry, so that algorithms can be added in the
    future.

    This document updates the Digest Access Authentication scheme used
    by SIP to support the algorithms listed in the "Hash Algorithms
    for HTTP Digest Authentication" registry defined by [RFC7616].

S2 p2:    This section should also mention that the BNF for the URI
           included in the challenge has been changed and now allows
           more schemes than SIP and SIPS.

S2 p2:    UAC and UAS should be expanded on first use.

S2 p2:    s/"qop" option/"qop" parameter

S2.1 p2:  s/any registered algorithm/any algorithm listed in
           the "Hash Algorithms for HTTP Digest Authentication"
           registry.

S2.4 p1:  Remove the space in "Proxy- Authenticate"

S2.5 p1:  s/introduces some clarification to that/clarifies that

S2.5 p2:  s/proxy to the UA./proxy to the UAC.

S2.6:     I suggest changing the title to "HTTP Digest Authentication
           Scheme Modifications" to match better the section title
           in RFC 3261.

S2.6 p1:  There is also a change to the BNF in bullet 1 that allows
           more URI schemes, so "... specified in bullets, 1, 7 and 8."

S2.6 p3:  Add a reference to RFC7616 after HTTP.

S2.6 #8:  s/handle "qop"/handle the "qop" (occurs twice)

S2.6 last p: s/continue/continues

S2.7 p1:  Insert a reference to [RFC5234] after "Augmented BNF".

S3 p1:    s/used to with/used with

S4 p1:    Add an explicit statement such as, "This document has
           no actions for IANA."

S6:       Add references to RFC 5234 and RFC 6151.

---

From: Rifaat Shekh-Yusef <[email-masked]>
Date: Tue, 2 Jul 2019 14:43:27 -0400

Thanks Jean!

I submitted a new version that addresses most of these comments.
See few replies below inline.

Regards,
 Rifaat

>            more schemes than SIP and SIPS.
I am not clear on this comment.
The update to the BNF by this document does not change the schemes, only
the algorithms.

Can you elaborate on what you meant by this?

> Bullet 1 does not introduce any new semantic changes, that is the reason
it is not listed here.

Regards,
 Rifaat

---

From: "A. Jean Mahoney" <[email-masked]>
Date: Tue, 2 Jul 2019 14:43:07 -0500

Hi Rifaat,

On 7/2/19 1:43 PM, Rifaat Shekh-Yusef wrote:
> On Mon, Jul 1, 2019 at 3:30 PM A. Jean Mahoney <[email-masked] 
> <mailto:[email-masked]>> wrote:
> 
>     Hi Rifaat,
> 

I'm talking about the URI scheme used in the challenge. Originally (3261 
and earlier versions of the digest-scheme draft), the URI included in 
the challenge had the following BNF:

    URI  =  SIP-URI / SIPS-URI

Where:

    SIP-URI          =  "sip:" [ userinfo ] hostport
                        uri-parameters [ headers ]

    SIPS-URI         =  "sips:" [ userinfo ] hostport
                        uri-parameters [ headers ]

In the current version, the BNF has been changed to the following:

    URI = Request-URI

Where:

    Request-URI    =  SIP-URI / SIPS-URI / absoluteURI

    absoluteURI    =  scheme ":" ( hier-part / opaque-part )

    scheme         =  ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )

And other schemes can now be used, like TEL or IM or HTTP. It's a change 
that should be called out.

Thanks!

Jean

---

From: Rifaat Shekh-Yusef <[email-masked]>
Date: Tue, 2 Jul 2019 15:56:27 -0400

Hi Jean,

I now understand your comment.

The document does not define any new scheme; I changed to Request-URI to
not prevent that from happening in the future.

I agree that this should be added to the list of bullets that introduce
semantic changes. I will fix that.

Thanks,
 Rifaat