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

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

Inline...

On Tue, May 5, 2020 at 2:46 PM Benjamin Kaduk via Datatracker <
[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