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

[Christer Holmberg <[e-mail]>]

Hi Paul,

My comments on some of your SIP-related issues (I am not addressing your issues on Sections 2.1.1 and 2.1.2 in this reply). 

>    * 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.
  
I suggest that we expand the first paragraph in the section. Something like:

"The procedures in this section apply when the UAC has received a challenge that contains a Bearer scheme, 
and the UAC has obtained a token as specified in section Section 2.1.1."
  
>    (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.
  
It is important to point out that the same access token might not necessarily be used throughout the lifetime of the registration, if the previously included access token has expired.

---
 
>    * 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.
  
See my comments for 2.1.4.

---

>    * 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.
  
WFM.
  
>    In the second paragraph, please provide a reference:
>    
>        ... using the procedures from [RFC???] associated with the type of
>        access token used.
  
Ok.

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

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

---

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

The section does enhance 'challenge', so I guess we can add text saying that 'challenge' is defined in RFC 3261.

Regarding the other parameters, the text already says where they are defined.

Or, are you saying that you want the references in the syntax, rather than in the normative text?

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

The HTTP WWW-Authenticate header field uses 'error' and 'scope', and I think it would be good to be aligned.
  
>    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?
  
The UAC will get the scope from the AS, and will forward it towards the UAS/registrar.
  
>    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?
  
I see no reason why it can't be presented to the user, and I guess the UAC actions depend on what the error code is.

However, as the parameter is described in RFC 6749, I don't think we need to repeat that in this draft.

---
  
>    * 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.
  
It was Olle who requested it.

However, I don't see any harm in having it. It allows the UAC to inform proxies/UAS/Registrar whether it supports the bearer scheme.

---

>    * Missing:
>    
>    I find no syntax extensions for Authorization and Proxy-Authorization. 
>    These need to be added.
  
Ok.

Regards,

Christer