Re: [sipcore] Comments on draft-ietf-sipcore-sip-authn-02

[Rifaat Shekh-Yusef <[e-mail]>]

Inline...

On Sun, Jun 10, 2018 at 7:48 AM Yehoshua Gev <[e-mail]> wrote:

> Thanks Rifaat.
> See inline.
>
>
>
> On Fri, Jun 8, 2018 at 10:16 PM, Rifaat Shekh-Yusef <[e-mail]
> > wrote:
>
>> Hi Yehoshua,
>>
>> Thanks for your review and comments.
>> See my replies inline.
>>
>> Regards,
>>  Rifaat
>>
>>
>> On Thu, Jun 7, 2018 at 3:54 AM, Yehoshua Gev <[e-mail]> wrote:
>>
>>> Hi,
>>>
>>> Several comments:
>>>
>>> Section 2.2.4: There is a reference to RFC 4474 for the list of SIP
>>> headers to be included in digest-string.
>>> I think it would be good to specifically indicate that the digest-string
>>> equals to signed-identity-digest of RFC 4474, but without the enclosing
>>> quotes.
>>>
>>
>> I am not clear on what you mean by "digest-string equals to
>> signed-identity-digest "
>> The goal is to use the shared-key to provide a hash of digest-string as
>> proof that the client has access to the shared-key.
>>
>
> I meant that the current text doesn't include the specific syntax of digest-string,
> it just references RFC 4474, which doesn't mention digest-string.
> RFC 4474 mention another element (signed-identity-digest) which seems
> very similar to digest-string.
> The only difference I see between them, is that signed-identity-digest is
> enclosed by quoted, while digest-string doesn't.
>

digest-string is defined in section 9
https://tools.ietf.org/html/rfc4474#section-9


>>
>>>
>>> Section 3.2: The last paragraph does not indicate what should be done in
>>> case of authentication failure (due to missing authentication token in the
>>> request or usage of invalid token).
>>> RFC 6750 section 3 requires the use of 401 with WWW-Authenticate header,
>>> and details the syntax of that header.
>>> I believe that the draft can add a reference to that section, saying
>>> that the same syntax should be used in SIP.
>>>
>>
>> Yeah, I agree that the document should explicitly cover the failure use
>> case.
>> I will address that in the next version of the document.
>>
>>
>>> Section 4:
>>> a. I think that a note should be added that the syntax of the Bearer
>>> scheme defined here for SIP deviates from the syntax defined by RFC 6750
>>> (in that that the token is given in a name=value format). I believe this is
>>> done to conform to the current SIP ABNF - maybe this should be written as a
>>> reasoning for the deviation.
>>>
>>
>> That is correct. I will add a note.
>>
>>
>>>
>>> b.. Given the proposed syntax, I don't see a reason to forbid
>>> extensions. Maybe the syntax could be written like:
>>>   credentials /= ("Bearer" LWS bearer-response)
>>>   bearer-response = "access_token" EQUAL access_token *(COMMA auth-param)
>>>
>>>
>> We could do that. Do you have a specific parameter in mind?
>>
> I don't have anything in mind.
>
> I will look into the proper syntax for this.
>>
>>
>>
>>> General:
>>> The draft describes how registrations are authenticated.
>>> There are scenarios for which there is no registration done (like
>>> click-to-call, where only outbound calls are done).
>>> Maybe the draft should be expanded to include any SIP request.
>>>
>>>
>> Good point. I will add some text around that.
>>
>>
>>
>>> This might even be a security issue:
>>> In regular SIP Digest authentication, the Authorization header is
>>> included in all requests even after successful registration, probably as
>>> SIP doesn't require that subsequent requests are sent over the same
>>> [authenticated/secured] transport-layer connection.
>>> Section 2 of the draft roughly corresponds to the methods described in
>>> section 4 (Obtaining Authorization) of RFC 6749.
>>> But, in RFC 6749 these methods are only used to obtain an access token
>>> to be used on subsequent requests, whilst in the draft they are used for
>>> achieving access.
>>> I'm not an expert on OAuth, but I believe the only methods to achieve
>>> access are defined in section 7 (Accessing Protected Resources) of RFC
>>> 6749, which require the usage of an access token with Authorization header
>>> (similar to section 3 of the draft).
>>>
>>>
>> Section 2.1.2 defines the idea of a shared-key to be used to establish
>> new connection and re-register to the proxy after a connection is lost,
>> without the need for a new code.
>> This same mechanism could be used with all subsequent requests, not only
>> registration. I will add some text to explicitly state that.
>>
>> After re-reading section 2, I think I can better explain my concern.
> The section defines a way for the SIP proxy to maintain the authenticity
> of the UA without challenging all the requests of the UA.
> Probably it takes the assumption that all (and only) the UA traffic will
> be sent over the same secured transport-layer connection.
> If this is the assumption (which I don't really like), I think that this
> should be defined as a MUST for the proxy to reject requests received on
> other connections.
>
> No, the point of the shared-key idea is to allow the phone to establish
new connections, when needed, to send new requests.


> Also, if the UA "is incapable of maintainings (typo) the confidentiality
> of the user credentials and any obtained tokens." (section 1.4), how
> would it store the shared-key of section 2.1.2?
>

The shared-key is created and maintained in memory, while the other
credentials are usually stored locally for example in flash.
Also, the document provides a way to enable or disable this functionality,
based on the security policy on the server.


> And how come OAuth, as used for HTTP, doesn't rely on proxies, but relies
> on the clients storing a token? Are SIP UAs more insecure than web browsers?
>
>

> I suggest that the mechanisms of section 2 will only be used by the UA to
> acquire a token, which will later be used according to the mechanism of
> section 3.
> This way the behavior of SIP UA will be much similar to the behavior of
> HTTP client.
>

The browser does not store any tokens when the *code flow* is used, and
that is what this flow is trying to do here.
Section 3 defines that mechanism that you are suggesting here.

Regards,
 Rifaat


>
>> Regards,
>>  Rifaat
>>
>>
>>
>>
>>> Thanks,
>>> Yehoshua Gev
>>>
>>>
>>> _______________________________________________
>>> sipcore mailing list
>>> [e-mail]
>>> https://www.ietf.org/mailman/listinfo/sipcore
>>>
>>>
>>
>