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

[Christer Holmberg <[e-mail]>]

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