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

[Yehoshua Gev <[e-mail]>]

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.

>
>
>>
>> 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.

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?
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.


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