Re: [sipcore] Benjamin Kaduk's Discuss on draft-ietf-sipcore-sip-token-authnz-15: (with DISCUSS and COMMENT) - the pull request

[Christer Holmberg <[e-mail]>]

Hi,

Based on Benjamin’s comments, I created a new pull request:

https://github.com/rifaat-ietf/draft-ietf-sipcore-sip-token-authnz/pull/9

Regards,

Christer


From: Rifaat Shekh-Yusef <[e-mail]>
Date: Tuesday, 5 May 2020 at 22.10
To: Benjamin Kaduk <[e-mail]>
Cc: "[e-mail]" <[e-mail]>, "[e-mail]" <[e-mail]>, "[e-mail]" <[e-mail]>, "[e-mail]" <[e-mail]>, "A. Mahoney" <[e-mail]>
Subject: Re: Benjamin Kaduk's Discuss on draft-ietf-sipcore-sip-token-authnz-15: (with DISCUSS and COMMENT)
Resent from: <[e-mail]>
Resent to: Rifaat Shekh-Yusef <[e-mail]>, Christer Holmberg <[e-mail]>, Victor Pascual <[e-mail]>
Resent date: Tuesday, 5 May 2020 at 22.10

Inline...

On Tue, May 5, 2020 at 2:46 PM Benjamin Kaduk via Datatracker <[e-mail]<mailto:[e-mail]>> wrote:
Benjamin Kaduk has entered the following ballot position for
draft-ietf-sipcore-sip-token-authnz-15: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-sipcore-sip-token-authnz/



----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

Thanks for the updates in the -14 (and -15); they cover most of my points.

Unfortunately, the new security considerations text seems to introduce a
problematic recommendation:

   Because of that, it is critical to make sure that extra security
   measures be taken to safeguard credentials used for Single Sign-On.
   Examples of such measures include long passphrase instead of a
   password, enabling multi-factor factor authentication, and the use of
   embedded browser when possible, as defined in [RFC8252].

Looking at RFC 8252 (Section 8.12), it seems to be rather strongly recommending
to *not* use an embedded browser, which is the opposite of the apparent
recommendation here.  Are we missing a word "avoiding" or similar?

Yeah, this should have been "browser" not "embedded browser".
We will remove the "embedded browser" and replace it with "the native platform browser".


Also, I am not 100% sure my note about refresh tokens was fully addressed;
in Section 2.1.1 we see:

   The refresh token is only used between the UAC and the AS.  If the AS
   provides a refresh token to the UAC, the UAC uses it to request a new
   access token and refresh token from the AS before the currently used
   access token expires ([RFC6749], Section 1.5).  If the AS does not

Is it accurate to say that the refresh token is used "to request a new access
token and refresh token" (specifically the "and refresh token" part)?  I know that
it is not always returned, but am less sure about whether the semantics always
include an (implicit) request for a new one.

Returning a refresh token is based on AS policy.
We will remove the "and refresh token".



----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Some other comments on the new text that do not rise to Discuss-level.

Thanks for adding the mention of a whitelist of trusted ASes; I would consider
also mentioning it in Section 4 for the authz_server parameter, and/or in the
security considerations.
We will add that to the security consideration section.


I also would have liked to see some guidance about when one should/shouldn't
include the realm parameter in a challenge.
I think this is out of scope, as we are not updating or changing the existing SIP behavior on this issue.


Section 2.1.1

   UAC contacts the AS in order to obtain tokens, and includes the
   requested scopes, based on a local configuration (Figure 1).  The UAC
   MUST check the AS URL received in the 401/407 response against a list
   of trusted ASs configured on the UAC, in order to prevent several
   classes of possible vulnerabilities when a client blindly attempt to
   use any provided authorization.

nits: "attempts", and maybe "any provided authorization server".
Will fix it

Section 3

nit: s/claimes/claims/
Will fix it


Section 5

   When a registrar chooses to challenge a REGISTER request, if the
   registrar can provide access to different levels of services, it is
   RECOMMENDED that the registrar includes a scope in the response in
   order to indicate the minimum scope needed to register and access
   basic services.  The access token might include an extended scope
   that gives the user access to more advanced features beyond basic
   services.

Is there anything to say about how the broader scope value might be learned?
In SIP, it is typically controlled by the admin, that controls the AS, and dictates the level of access for the user.
We will add a sentence to that extent.

Regards,
 Rifaat