Re: [sipcore] WGLC draft-ietf-sipcore-sip-token-authnz

[Paul Kyzivat <[e-mail]>]

I've just reviewed the document again, in full.

There are still a number of areas that need work:

* Section 2.1.1 says:

    The method used to authenticate the user and obtain these tokens is
    out of scope for this document, with one potential method is the
    Native App mechanism defined in [RFC8252].  The advantages of using
    the mechanism defined in [RFC8252] is that the user will be directed
    to use a browser to interact with the authorization server.  This
    allows the authorization server to prompt the user for multi-factor
    authentication, redirect the user to third-party identity providers,
    and the use of single-sign-on sessions.

My problem with this is: the challenge gives the UAC an HTTPS URL of an 
AS. It does not normatively state how the UAC is to use this, or what 
usage(s) the AS MUST support. Hence there is no assurance that the UAC 
will be able to successfully access the AS.

This could be resolved by requiring that the UAC and AS MUST support 
RFC8252. Or perhaps there is some more expansive RFC that can be 
referenced. In the end there needs to be something MTI.

* Section 2.1.2:

You talk about local policy here, but which server's local policy are 
you talking about?

I'm guessing that the AS provides a token containing whatever methods 
have been granted for the user, and the registrar can then use policy to 
decide what method(s) it wants to require for registration. Am I correct 
in assuming that the UAC doesn't play any role in this other than to 
relay the token from the AS to the registrar? And in this case why is 
this point being made in the section about *UAC* behavior?

Can you please clarify the text here?

* Section 2.1.4:

This section says that the UAC MUST include an Authorization header 
field with the Bearer scheme. This is overly strong.

This should be an explanation that *if* it has received a challenge 
containing the Bearer scheme then to resolve that particular challenge 
it needs to send a request with an Authorization header field containing 
the response to that challenge, including the Bearer scheme.

(But note that if there were multiple challenges with different schemes 
then it maybe able to successfully retry the request using non-Bearer 
credentials.)

I don't understand what distinction you are trying to draw by 
distinguishing the behavior for creating a binding from that for 
refreshing a binding. AFAIK there should be no difference. And in fact 
the UAC in general cannot know whether the request being send will 
establish or refresh a binding.

* Section 2.1.5:

This section has similar issues to section 2.1.4: The MUST is too 
strong, and the reason for distinguishing between in-dialog requests 
from others is unclear.

* Section 2.2:

Again is overly strong in its use of MUST. I think it is evident that 
the point being made is that a challenge containing Bearer scheme is the 
way to indicate that is with a challenge with Bearer scheme, but the 
text is at least awkward.

How about:

    When a UAS or Registrar receives a request that fails to contain
    authorization credentials acceptable to it, it SHOULD challenge the
    request by sending a 401 (Unauthorized) response. To indicate that
    it is willing to accept an OAuth2 token as a credential the
    UAS/Registrar MUST include a Proxy-Authentication header field in the
    response, indicate "Bearer" scheme and include an address of an
    Authorization Server from which the originator can obtain an access
    token.

In the second paragraph, please provide a reference:

    ... using the procedures from [RFC???] associated with the type of
    access token used.

* Section 2.3:

Similar comment to section 2.2. How about the following for the first 
paragraph:

    When a proxy receives a request that fails to contain
    authorization credentials acceptable to it, it SHOULD challenge the
    request by sending a 407 (Proxy Authentication Required) response.
    To indicate that it is willing to accept an OAuth2 token as a
    credential the proxy MUST include a Proxy-Authentication
    header field in the response, indicating "Bearer" scheme and
    including an address to an Authorization Server from which the
    originator can obtain an access token.

I don't think the second paragraph should condition any behavior on 
whether it has previously challenged. That requires that the proxy be 
stateful, and it isn't a necessary condition. Instead, how about:

    When a proxy wishes to authenticate a received request, it MUST
    search the request for Proxy-Authorization header fields with 'realm'
    parameters that match its realm. It then MUST successfully validate
    the credentials from at least one Proxy-Authorization header field
    for its realm. When the scheme is Bearer the proxy MUST validate the
    access token, using the procedures from [RFC???] associated with the
    type of access token used.

* Section 3:

The following:

    The realm and auth-param parameters are defined in [RFC3261].

is incomplete, because it is missing 'challenge', 'realm', and 
'https-URI. (Where is https-URI defined?) Another way to do this is by 
adding the following to the syntax:

        challenge = <defined in RFC3261>
        realm = <defined in RFC3261>
        auth-param = <defined in RFC3261>
        scope = <defined in RFC6749>
        error = <defined in RFC6749>
        https-URI = <where???>

(This helps those who might try to formally validate the syntax because 
it removes undefined parameters.)

Also, 'scope' and 'error' are very generic rule names being used in a 
very restricted context. While these are not defined in RFC3261, there 
is the potential conflict with other extensions. I'd recommend using 
more specific terms. E.g.,

        challenge  =/  ("Bearer" LWS bearer-cln *(COMMA bearer-cln))
        bearer-cln = realm / bearer-scope / authz-server / bearer-error /
                     auth-param
        authz-server = "authz_server" EQUAL authz-server-value
        authz-server-value = https-URI
        challenge = <defined in RFC3261>
        realm = <defined in RFC3261>
        auth-param = <defined in RFC3261>
        bearer-scope = <defined as scope in RFC6749>
        bearer-error = <defined as error in RFC6749>
        https-URI = <where???>

Regarding the scope parameter:

This parameter is conveyed from the registrar to the UAC. But what is 
the UAC to do with it? How does it have any control over the scope of 
the token it gets from the AS? Is the UAC supposed to convey this to the 
AS? If so, how?

Regarding the error parameter:

Is this just intended to be of help in debugging the UAC? Or is it 
expected that the UAC might take some sort of pre-programmed action in 
response? If the latter, how might that work? Is this information 
suitable for presentation to the user?

* Section 4:

I agree with Dale - I don't see any value in this tag. I tried to search 
for all discussion on the point. The discussion attributes me with 
requesting it, but I don't think I did.

* Missing:

I find no syntax extensions for Authorization and Proxy-Authorization. 
These need to be added.

	Thanks,
	Paul

On 12/2/19 8:44 AM, A. Jean Mahoney wrote:
> Hi all,
> 
> Today starts the Working Group Last Call of 
> draft-ietf-sipcore-sip-token-authnz-06.
> 
> https://datatracker.ietf.org/doc/draft-ietf-sipcore-sip-token-authnz/
> 
> The document has changed quite a bit since its predecessor went through 
> WGLC.
> 
> Please review.  If you submitted feedback on a previous version, make 
> sure that it was addressed and your questions answered.
> 
> Also appreciated would be any information on implementation plans.
> 
> WGLC ends Monday, Dec 16.