Subject: [sipcore] draft-ietf-sipcore-digest-scheme comments

From: "Olle E. Johansson" <[email-masked]>
Date: Fri, 17 May 2019 15:38:35 +0200

Hi!
Sorry to be late in this discussion.

" This document updates the Digest Access Authentication scheme used by
   the Session Initiation Protocol (SIP) to add support for secure
   digest algorithms to replace the broken MD5 algorithm.”

I would suggest changing “for secure” to “for more secure” to be a bit humble.
In XXX years, the schemes suggested here will be less secure than now.
The good thing is that we don’t have to update this document every time
IANA adds a new algorithm to the registry. :-)

section 2: "SHA- 256” - remove the extra space. Also, there’s an extra
quotation mark at the end of the section.

Section 2.1:

"Note that [RFC7616
] defines a -sess variant for each algorithm; the
   -sess variants are not used with SIP.”

Is this already forbidden in 3261 or is this new proposed language? If so, “are not” should propably
be something like “MUST not”

Section 2.2:

Is this an update to 7616 or just an explanation of 7616?

Section 2.4:

"When the UAC receives a response with multiple header fields with the
   same realm it SHOULD use the topmost header field that it supports,
   unless a local policy dictates otherwise.”

Why a SHOULD? I would prefer a MUST.

“When the UAC receives a 401 response with multiple WWW-Authenticate
   header fields with different realms it SHOULD retry and include an
   Authorization header field containing credentials that match the
   topmost header field of any one of the realms.”

If you are disallowing multiple Authorization headers for the same realm,
but with different algorithms I think this should be clearly written. In my
view, that would be a good thing.

 "8.  Servers MUST be able to properly handle "qop" parameter received
   in an authorization header field, and clients MUST be able to
   properly handle "qop" parameter received in WWW-Authenticate and
   Proxy-Authenticate header fields.  Servers MUST always send a "qop"
   parameter in WWW-Authenticate and Proxy-Authenticate header field
   values, and clients MUST send the "qop" parameter in any resulting
   authorization header field.”

This is not clear. If the servers MUST always send “qop” then we
add that to SIP 2.0 with no backwards options or compatibility.
Since you write “clients MUST be able to”… it seems like you
assume that clients have a choise of whether they use it. I think
one has to be a bit more clear so developers understands how
to modify their implementations.

In addition:
Are we ready to require that all SIP 2.0 compliant software support QOP?

I would like to run an online-SIPit when we have software that supports this
so we can test the behaviour, especially looking into downgrade attacks.

And as Dave said, I don’t see any priority in the IANA registry. RFC 7616 mentions
“strongest” algorithm. "A user agent MUST choose to use the strongest auth-scheme it
   understands and request credentials from the user based upon that
   challenge.” and then adds "When the server offers choices of authentication schemes using the
   WWW-Authenticate header field, the strength of the resulting
   authentication is only as good as that of the of the weakest of the
   authentication schemes.”

I don’t find any definition of “strong algorithm” in RFC 7616. 

Note that this document also suggests that UACs remember the “strongest”
algorithm used by a specific server/service and refuse a downgrade attack
- without discussing any implementation issues.

Good work. A small step forward!

Cheers,
/O

---

From: Rifaat Shekh-Yusef <[email-masked]>
Date: Sat, 18 May 2019 14:37:39 -0400

Thanks Olle!

See my replies below.

Regards,
 Rifaat

> IANA adds a new algorithm to the registry. :-)

Ok

> Ok

> I do not think that 3261 forbids the -sess variant, and I do not see the
need to forbid it here either.

> Is this an update to 7616 or just an explanation of 7616?

No update, just an explanation.

> Why a SHOULD? I would prefer a MUST.

I can do that, but the last part of this paragraph states that local policy
can override this recommendations anyway.
So, does it make any difference?

> view, that would be a good thing.

This is allowed.

> Are we ready to require that all SIP 2.0 compliant software support QOP?
Here is a quote form RFC3261:

"Use of the "qop" parameter is optional in RFC 2617 for the purposes
of backwards compatibility with RFC 2069; since RFC 2543 was
based on RFC 2069, the "qop" parameter must unfortunately
remain optional for clients and servers to receive."

That is no longer the case with RFC7616.

> Take a look at the security considerations section for more information
about downgrade attacks recommendations.

> See section 3.7
https://tools.ietf.org/html/rfc7616#section-3.7

I will make it clearer.

> _______________________________________________
> sipcore mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/sipcore
>

---

From: "Olle E. Johansson" <[email-masked]>
Date: Mon, 20 May 2019 07:40:46 +0200

> On 18 May 2019, at 20:37, Rifaat Shekh-Yusef <[email-masked]> wrote:
> 
> Thanks Olle!
> 
> See my replies below.
> 
> Regards,
>  Rifaat
> 
> 
> 
> 
> On Fri, May 17, 2019 at 9:38 AM Olle E. Johansson <[email-masked] <mailto:[email-masked]>> wrote:
> Hi!
> I do not think that 3261 forbids the -sess variant, and I do not see the need to forbid it here either
So then I suggest we remove the statement that the -sess variants are not used with SIP.

> I can do that, but the last part of this paragraph states that local policy can override this recommendations anyway.
Should we allow that? Why would local policy enforce a downgrade?
> That is no longer the case with RFC7616.
Since this is a change from RFC 3261 we propably want clarify that for developers. 
I think that if we want to modify RFC7616 for SIP, we can. The question still
stands - are we ready to change the way current auth works today in SIP/2.0. There’s
a lot of implementations out there that will suddenly not be standard-following.

I’m not saying it’s a bad thing, just that we have to understand the implication.
> Take a look at the security considerations section for more information about downgrade attacks recommendations.
> 
>  
> https://tools.ietf.org/html/rfc7616#section-3.7 <https://tools.ietf.org/html/rfc7616#section-3.7>
> 
> I will make it clearer.
But that is not part of the registry, so what happens if I somewhere in the future add Edvina-9042 to
the registry? What’s the order then? This section just says what’s mandatory to implement but doesn’t
say anything about priority or which one that is the “strongest” one.

I like the idea of a prioritized list for developers with some “strength” weight but fail to find one
in these documents.  There has to be some advice in some RFC somewhere.
…at this point I can’t find anything out there.

Cheers,
/O

> [email-masked] <mailto:[email-masked]>
> https://www.ietf.org/mailman/listinfo/sipcore <https://www.ietf.org/mailman/listinfo/sipcore>

---

From: Christer Holmberg <[email-masked]>
Date: Tue, 21 May 2019 20:57:29 +0000

Hi,

...
 
>> Section 2.1:
>> 
>> "Note that [RFC7616] defines a -sess variant for each algorithm; the

If -sess variant was defined in RFC 7616, it was obviously not mentioned in RFC 3261 __

What is the difference between the session variant and the non-session variant? What is meant by "session"? RFC 7616 doesn't give any explanation, and I couldn't find anything by googling either.

I do think we need to say *something*.
 
... 

>> view, that would be a good thing.
 
RFC 3261 does not say anything about using the topmost header, does it?

>> I’m not saying it’s a bad thing, just that we have to understand the implication.

The typical way to solve this is to say that endpoints compliant with this specification must do this and that, but for backward compatibility also needs to be able to not do it.

Regards,

Christer

---

From: "Olle E. Johansson" <[email-masked]>
Date: Wed, 22 May 2019 10:19:36 +0200

> On 21 May 2019, at 22:57, Christer Holmberg <[email-masked]> wrote:
> 
> Hi,
> 
> ...
>  
>>> Section 2.1:
>>> 

Which ends up being a non-statement because no developer will get any resources to fix this with that kind of openended statement.

/O

---

From: Christer Holmberg <[email-masked]>
Date: Wed, 22 May 2019 08:39:44 +0000

Hi,

    >> What is the difference between the session variant and the non-session variant? What is meant by "session"? RFC 7616 doesn't 
    >> give any explanation, and I couldn't find anything by googling either.
    >> 
    >> The typical way to solve this is to say that endpoints compliant with this specification must do this and that, but for 
    >> backward compatibility also needs to be able to not do it.
    >

    The peer endpoint may not even have implemented this draft, so for that reason an endpoint needs to be able to receive without "qop".

    Regards,

    Christer

---

From: Rifaat Shekh-Yusef <[email-masked]>
Date: Sat, 25 May 2019 07:27:23 -0400

Inline...

On Mon, May 20, 2019 at 1:40 AM Olle E. Johansson <[email-masked]> wrote:

> need to forbid it here either
>
> I can do that, but the last part of this paragraph states that local
> policy can override this recommendations anyway.

This is meant to provide the client with the ability to decide on the
priority of the algorithm to use when the client supports more than one
strong algorithm.

> Here is the following part of the quote:

          remain optional for clients and servers to receive.  *However,
          servers MUST always send a "qop" parameter in WWW-Authenticate
          and Proxy-Authenticate header field values.  If a client
          receives a "qop" parameter in a challenge header field, it
          MUST send the "qop" parameter in any resulting authorization
          header field.*

RFC3261 already requires the servers to include "qop" in the challenge and
requires the client to include the "qop" in the response.
The change is that the client must now send "qop" in the Authorize header
in the initial request, not only in response to a challenge.
Are you saying that clients do not do that today?

> …at this point I can’t find anything out there.
I do not think that we need to explicitly prioritize the algorithms,
because every time you add new algorithm the priority will change.
It is up to the server to device on the priority based on the placement of
the algorithm in the response.

Regards,
 Rifaat

---

From: Rifaat Shekh-Yusef <[email-masked]>
Date: Sat, 25 May 2019 07:33:11 -0400

Inline...

On Tue, May 21, 2019 at 4:57 PM Christer Holmberg <
[email-masked]> wrote:

> What is the difference between the session variant and the non-session
> variant? What is meant by "session"? RFC 7616 doesn't give any explanation,
> and I couldn't find anything by googling either.
>
>
See RFC 7616, section 3.4.2 for more details.

> I do think we need to say *something*.

Any suggestion?

> RFC 3261 does not say anything about using the topmost header, does it?

I was referring to this document.

> The typical way to solve this is to say that endpoints compliant with this
> specification must do this and that, but for backward compatibility also
> needs to be able to not do it.
>
> Regards,
>
> Christer
>
>
>

---

From: Christer Holmberg <[email-masked]>
Date: Sat, 25 May 2019 14:53:07 +0000

Hi,
 
>> I do think we need to say *something*.

Not at the moment. But, if allowing both adds something new to what is currently defined/assumed in RFC 3261 I think it would be good to point it out.
 
... 

> I was referring to this document.

So, the should-use-topmost is something new, defined in this document?

Regards,

Christer

---

From: Rifaat Shekh-Yusef <[email-masked]>
Date: Sat, 25 May 2019 11:06:44 -0400

On Sat, May 25, 2019 at 10:53 AM Christer Holmberg <
[email-masked]> wrote:

> Not at the moment. But, if allowing both adds something new to what is
> currently defined/assumed in RFC 3261 I think it would be good to point it
> out.
>
>
The ABNF allows for the -see variant, but my understanding is the the -sess
variant was never implemented in SIP.
This document does not introduce any new -sess specific changes.

> So, the should-use-topmost is something new, defined in this document?
Yes, as per RFC7616.

Regards,
 Rifaat

---

From: Paul Kyzivat <[email-masked]>
Date: Sat, 25 May 2019 11:43:42 -0400

On 5/25/19 7:33 AM, Rifaat Shekh-Yusef wrote:
> Inline...
> 
> On Tue, May 21, 2019 at 4:57 PM Christer Holmberg 

>     What is the difference between the session variant and the
>     non-session variant? What is meant by "session"? RFC 7616 doesn't

BTW: -sess was not introduced in 7616. It also appeared in 2617, though 
only as MD5-sess.

	Thanks,
	Paul

---

From: Christer Holmberg <[email-masked]>
Date: Sat, 25 May 2019 16:24:35 +0000

Hi,

>    BTW: -sess was not introduced in 7616. It also appeared in 2617, though 
  
If that's the case, then I assume "-sess" is already a part of SIP, and we don't need to say anything.

Regards,

Christer

---

From: Christer Holmberg <[email-masked]>
Date: Sat, 25 May 2019 16:31:05 +0000

Hi,
  
... 

>> So, the should-use-topmost is something new, defined in this document?

Perhaps then say "As defined in RFC7617,...."

And, perhaps mention it in section 2, where the changes are listed.

Also, as the remote peer may not have implemented the draft, I think it would be good to point out that one must not assume that the peer will use the topmost header, even if it supports the algorithm in the topmost header.

Regards,

Christer

---

From: Rifaat Shekh-Yusef <[email-masked]>
Date: Sat, 25 May 2019 12:31:25 -0400

I am fine with removing the note about -sess.

Regards,
 Rifaat

---

From: Rifaat Shekh-Yusef <[email-masked]>
Date: Sat, 25 May 2019 14:00:00 -0400

> And, perhaps mention it in section 2, where the changes are listed.

The normative text for SIP is specified in this document, so I do not see
the need to add such a sentence.

> Also, as the remote peer may not have implemented the draft, I think it
> would be good to point out that one must not assume that the peer will use
> the topmost header, even if it supports the algorithm in the topmost header.
>
>
Ok. I will add a sentence to section 2.3.

Regards,
 Rifaat

---

From: Christer Holmberg <[email-masked]>
Date: Sat, 25 May 2019 18:17:07 +0000

Hi,

... 

> The normative text for SIP is specified in this document, so I do not see the need to add such a sentence.

When we update an RFC, it is good to have an overview about what the updates are, so that people don't have to start reading 3261, 7617 and try to figure out themselves. They will obviously have to read the RFCs to figure out the details, but it helps if they know what the updates are about.

Regards,

Christer

---

From: Rifaat Shekh-Yusef <[email-masked]>
Date: Sat, 25 May 2019 14:23:32 -0400

On Sat, May 25, 2019 at 2:17 PM Christer Holmberg <
[email-masked]> wrote:

> When we update an RFC, it is good to have an overview about what the
> updates are, so that people don't have to start reading 3261, 7617 and try
> to figure out themselves. They will obviously have to read the RFCs to
> figure out the details, but it helps if they know what the updates are
> about.
>

Section 2 is all about the changes introduced to the Digest mechanism.
If that is not sufficient, can you propose some text?

---

From: Christer Holmberg <[email-masked]>
Date: Sat, 25 May 2019 19:36:03 +0000

Hi,

... 

>> When we update an RFC, it is good to have an overview about what the updates are, so that people don't have to start reading 3261, 7617 
>> and try to figure out themselves. They will obviously have to read the RFCs to figure out the details, but it helps if they know what the updates 
>> are about.
>

Yes, but that doesn't really say anything about what exactly is updated.

> If that is not sufficient, can you propose some text?
 
Something like this:

"2.  Updates RFC 3261

This section replaces the reference to RFC2617 with a reference to RFC7617 in RFC3261, and
describes the modifications to the usage of the Digest mechanism in RFC3261 resulting from 
that reference update. It adds support for the SHA-256 and SHA-512/256 algorithms. It adds
required support for the "qop" option. It provides additional UAC and UAS procedures regarding usage of
multiple SIP Authorization, WWW-Authenticate and Proxy-Authenticate header fields, including 
in which order to insert and process them. It provides guidance regarding forking. Finally, it
updates the SIP protocol BNF as required by the updates."

Feel free to modify, remove - or add if I have forgot something.

In addition, I suggest to change the names of subsections 2.3 and 2.4.

The current name of subsection 2.3 is "The Authenticate Response Header Field". But, there is no such header field described. The section talks about other header fields (with similar names). Could we simply call it "UAS behavior"?

The current name of subsection 2.4 is "The Authorization Request Header Field". But, the section also talks about the WWW-Authenticate header field. Could we simply call it "UAC behavior"?

Regards,

Christer

---

From: Christer Holmberg <[email-masked]>
Date: Sat, 25 May 2019 19:52:42 +0000

....and, of course, in 3261 the reference to 2617 needs to be updated to 7617 too.

On 25/05/2019, 22.36, "sipcore on behalf of Christer Holmberg" <[email-masked] on behalf of [email-masked]> wrote:

    Hi,
    
    ... 

    
     
    Something like this:
    
    "2.  Updates RFC 3261

    

    Regards,
    
    Christer

---

From: Rifaat Shekh-Yusef <[email-masked]>
Date: Sat, 25 May 2019 16:08:14 -0400

On Sat, May 25, 2019 at 3:36 PM Christer Holmberg <
[email-masked]> wrote:

> >>>>>>> I can do that, but the last part of this paragraph states that
> local policy can override this recommendations anyway.
> updates are, so that people don't have to start reading 3261, 7617
> >> and try to figure out themselves. They will obviously have to read the
> RFCs to figure out the details, but it helps if they know what the updates
> >> are about.
> >

I do not see the need for this change. The first page has "Update: 3261"
and the first sentence in section 2 explicitly states that again.

> All of this is specified later in section 2, but if this helps someone, I
do not mind adding this to section 2.

> The current name of subsection 2.3 is "The Authenticate Response Header
> Field". But, there is no such header field described. The section talks
> about other header fields (with similar names). Could we simply call it
> "UAS behavior"?
>
> The current name of subsection 2.4 is "The Authorization Request Header
> Field". But, the section also talks about the WWW-Authenticate header
> field. Could we simply call it "UAC behavior"?
>
>
Sure. I can make these changes.

Regards,
 Rifaat

---

From: Christer Holmberg <[email-masked]>
Date: Sat, 25 May 2019 20:45:31 +0000

Hi,

...

> I do not see the need for this change. The first page has "Update: 3261" and the first sentence in section 2 explicitly states that again.

Yes, but in a draft it's good to see where the updates are, so that implementers etc can jump there. Granted, this is a very short draft, and there aren't too many other sections.

> All of this is specified later in section 2, but if this helps someone, I do not mind adding this to section 2.
 
Again, it's an overview of the changes. And, much of the text is already in section 2, so __

>> The current name of subsection 2.3 is "The Authenticate Response Header Field". But, there is no such header field 
>> described. The section talks about other header fields (with similar names). Could we simply call it "UAS behavior"?
>>
>> The current name of subsection 2.4 is "The Authorization Request Header Field". But, the section also talks about the 
>> WWW-Authenticate header field. Could we simply call it "UAC behavior"?
>

Make sure you name all header fields in the text too. In some cases you have left the header field name from the text, probably because you assume it's the same as in the section name.

Thanks!

Regards,

Christer