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

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

Thanks Ben!

We appreciate all your reviews and feedback.
This document is in a much better shape because of your detailed and
comprehensive reviews.

Regards,
 Rifaat


On Tue, May 5, 2020 at 5:31 PM Benjamin Kaduk <[e-mail]> wrote:

> I think so, yes.
>
> -Ben
>
> On Tue, May 05, 2020 at 05:11:38PM -0400, Rifaat Shekh-Yusef wrote:
> > Ben,
> >
> > Does this pull request address your latest DISCUSS and comments?
> >
> > Regards,
> >  Rifaat
> >
> >
> > On Tue, May 5, 2020 at 3:31 PM Christer Holmberg <
> > [e-mail]> wrote:
> >
> > > 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]> 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
> > >
> > >
> > >
>