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

[Magnus Westerlund <[e-mail]>]

Hi,

I think this addresses all my issues. I will clear with the assumption that a
new version will be submitted before the AD approves the publication.

Cheers

Magnus

On Tue, 2020-04-28 at 08:51 +0000, Christer Holmberg wrote:
> Hi Magnus,
> 
> >    I think keeping _ in authz is okay to do, and allowed by the syntax. So
> > if the
> >    WG want to correct this to have it align with the norm or leave it as it
> > is is
> >    up to the WG. 
> >    
> >    However, the changes did may wonder one thing about the the inclusion of
> > scope
> >    and error. The ABNF constructs defined in RFC 6749 are only including the
> > value
> >    part. So to my understanding they really should have an parameter-name =
> > value
> >    construct defined. Like this. 
> >    
> >    scope-param = "scope" EQUAL DQUOTE scope DQUTE
> >    scope = <defined in RFC6749>
> >    error-
> >    param = "error" EQUAL DQUOTE error DQUOTE
> >    error = <defined in RFC6749>
> 
>   
> You are right. Good catch! :)
>   
> >    The IANA section looks good. 
> 
>   
> Good :)
> 
> Fixed in this commit: 
> https://github.com/rifaat-ietf/draft-ietf-sipcore-sip-token-authnz/pull/7/commits/fb8a94084c5ef3490f3e6c2ba2f00550d79fbb2b
> 
> Regards,
> 
> Christer
> 
>     
>     
>     On Mon, 2020-04-27 at 19:50 +0000, Christer Holmberg wrote:
>     > Hi,
>     > 
>     > The following pull request commits contains the syntax and IANA changes
> based
>     > on Magnus DISCUSS:
>     > 
>     > 
>     
> https://github.com/rifaat-ietf/draft-ietf-sipcore-sip-token-authnz/pull/7/commits/168086b4f1220620d063af07a3292b667e30ef37
>     >  (Syntax)
>     > 
>     
> https://github.com/rifaat-ietf/draft-ietf-sipcore-sip-token-authnz/pull/7/commits/22f810025d1bf8df45875e92e4d4c11d0f574693
>     >  (IANA Considerations)
>     > 
>     > Please note the parameter name "authz_server". Following the naming
> style of
>     > other header field parameters it should perhaps be "authz-server".
> However,
>     > for backward compatibility I would prefer to not change it at this
> point.
>     > 
>     > Paul, I would appreciate if you could also take a look at these. Thanks!
>     > 
>     > Regards,
>     > 
>     > Christer
>     > 
>     > 
>     > 
>     > On 23/04/2020, 22.51, "Christer Holmberg" <
> [e-mail]>
>     > wrote:
>     > 
>     >     Hi Magnus,
>     >     
>     >     Thank You for the review! Please see inline.
>     >         
>     >         --------------------------------------------------------------
> --------
>     >         DISCUSS:
>     >         --------------------------------------------------------------
> --------
>     >         
>     >         > I think these resolution for this is rather straight forward,
>     > however the
>     >         > implications of one is going to break deployed
> implementations.
>     >         >
>     >         > 1. Section 4:
>     >         >
>     >         > This is rather straight forward to resolve but you do have a
> SIP
>     > syntax
>     >         > violation in these rules.
>     >         >
>     >         >       challenge  =/  ("Bearer" LWS bearer-cln *(COMMA bearer-
> cln))
>     >         >       bearer-cln = realm / scope / authz-server / error /
> auth-param
>     >         >       authz-server = "authz_server" EQUAL authz-server-value
>     >         >       authz-server-value = https-URI
>     >         >       realm = <defined in RFC3261>
>     >         >       auth-param = <defined in RFC3261>
>     >         >       scope = <defined in RFC6749>
>     >         >       error = <defined in RFC6749>
>     >         >       https-URI = <defined in RFC7230>
>     >         >
>     >         > So RFC 3261 defines the Challenge construct as:
>     >         >
>     >         > challenge           =  ("Digest" LWS digest-cln *(COMMA
> digest-
>     > cln))  / other-challenge
>     >         >
>     >         > Where this extension needs to match the syntax of the other-
>     > challenge:
>     >         >
>     >         > other-challenge     =  auth-scheme LWS auth-param  *(COMMA
> auth-
>     > param)
>     >         >
>     >         > Where we need to look at:
>     >         > auth-param        =  auth-param-name EQUAL  ( token / quoted-
> string
>     > )
>     >         >
>     >         > Please note what is included in the "token" rule.
>     >         >      token       =  1*(alphanum / "-" / "." / "!" / "%" / "*"
>     >         >                     / "_" / "+" / "`" / "'" / "~" )
>     >         >
>     >         > the allowed syntax for https-URI in RFC 7230 is:
>     >         >
>     >         >    https-URI = "https:" "//" authority path-abempty [ "?"
> query ]  [
>     > "#" fragment ]
>     >         >
>     >         > Which include both "/", "?" and "#" that are not allowed in
> token.
>     > Thus, the
>     >         > URI included in the authz-server-value  MUST be converted into
> a
>     > quoted-string
>     >         > matching syntax rule.
>     >         
>     >         You are correct. We currently reference https-URI in RFC 7230,
> but the
>     > definition in 7230 does not place quotes around it.
>     >     
>     >         The same applies to scope and error.
>     >     
>     >         So, we need to fix:
>     >     
>     >     OLD:
>     >     
>     >          authz-server = "authz_server" EQUAL authz-server-value
>     >     
>     >          scope = <defined in RFC6749>
>     >           error = <defined in RFC6749>
>     >     
>     >     NEW:
>     >     
>     >          authz-server = "authz_server" EQUAL DQUOTE authz-server-value
> DQUOTE
>     >     
>     >          scope-cln = DQUOTE scope DQUOTE
>     >          scope = <defined in RFC6749>
>     >          error-cln = DQUPTE error DQUOTE
>     >          error = <defined in RFC6749>
>     >     
>     >     
>     >     (I noted that that Benjamin has some comments regarding the
> referenced
>     > RFCs for the parameter values, but I will address that in the reply to
> his
>     > review.)
>     >     
>     >     
>     >     -----
>     >     
>     >         > 2. In addition should not the "authz_server" be registered in
> the
>     >         > 
>     > 
> https://www.iana.org/assignments/sip-parameters/sip-parameters.xhtml#sip-parameters-12
>     >         > registry?
>     >         
>     >         I guess so. And, then I guess we also need to register "scope"
> and
>     > "error".
>     >     
>     >         --------------------------------------------------------------
> --------
>     >         COMMENT:
>     >         --------------------------------------------------------------
> --------
>     >         
>     >         > An additional thing.
>     >         >
>     >         > Is SIP directly using the HTTP Authentication Schemes IANA
> registry
>     >         > (
>     > 
> https://www.iana.org/assignments/http-authschemes/http-authschemes.xhtml#authschemes
>     > )
>     >         > or does it have its own tucked away somewhere? And if it is
> the
>     > former, should
>     >         > its references for the "bearer" add this RFC as a reference?
>     >         
>     >         SIP uses the HTTP registry.
>     >     
>     >        (The SIP registry does register a "digest" value, but that is for
> the
>     > Security-XXX headers defined in RFC 3329)
>     >     
>     >     Regards,
>     >     
>     >     Christer          
>     >         
>     >         
>     >     
>     >     
>     > 
>     -- 
>     Cheers
>     
>     Magnus Westerlund 
>     
>     
>     ----------------------------------------------------------------------
>     Networks, Ericsson Research
>     ----------------------------------------------------------------------
>     Ericsson AB                 | Phone  [phone]
>     Torshamnsgatan 23           | Mobile [phone]
>     SE-164 80 Stockholm, Sweden | mailto: [e-mail]
>     ----------------------------------------------------------------------
>     
>     
-- 
Cheers

Magnus Westerlund 


----------------------------------------------------------------------
Networks, Ericsson Research
----------------------------------------------------------------------
Ericsson AB                 | Phone  [phone]
Torshamnsgatan 23           | Mobile [phone]
SE-164 80 Stockholm, Sweden | mailto: [e-mail]
----------------------------------------------------------------------