Re: [MMUSIC] Changes for draft-ietf-mmusic-trickle-ice-sip-14 - ABNF

[Christer Holmberg <[e-mail]>]

Hi,


Hi Thomas,



I hope Paul can jump in on this one too, since it’s ABNF related :)



However, I think there are some issue with the ABNF (not related to Adam¹s

issue).



For example, the syntax says:



  ice-pwd-attribute      = %s"a" "=" ice-pwd-att



Then the draft says:



  with ice-pwd-attribute from [I-D.ietf-mmusic-ice-sip-sdp],



which says



ice-pwd-att           = "ice-pwd:" password

i.e. doesn't introduce a second a=



First, I think the structure in draft-ice-sip-sdp is wrong. It shall be something like:



      Attribute Name: ice-pwd



      Attribute Value: ice-pwd-att



      Usage Level: media



      Charset Dependent: No



      Mux Category: TBD



    The Augmented BNF syntax [RFC5234] for the attribute is:



      ice-pwd-att = password

      password = blah blah...



Now, whether we should use that syntax also in trickle-ice I don't know. I would like to hear Paul's opinion.
You are mixing up the ABNF syntax and the IANA registration template.

The ABNF syntax in section 5.4 of draft-ietf-mmusic-ice-sip-sdp-16 and also section 15.4 of RFC 5245
both define the ABNF grammar as


  ice-pwd-att           = "ice-pwd" ":" password



That is the “old” way of defining attributes. My example above is according to the “new” way, that has been used lately (and, no, I didn’t mix up the ABNF syntax and the IANA reg).



Another example is in section 6 of draft-bundle (with the corresponding IANA reg in section 16.3).



Regards,



Christer











I don't think it is a good idea to re-define the ice-pwd-att token as

 ice-pwd-att = password



If you want to add text to the IANA section it MUST read



    The Augmented BNF syntax [RFC5234] for the attribute is:

          ice-pwd-att           = "ice-pwd" ":" password



The ABNF in draft-ietf-mmusic-trickle-ice-sip-14 is correct wrt usage of ice-pwd-att



Regards

Thomas









In my reading, this would result in:



  a=a=ice-pwd:asd88fgpdd777uzjYhagZg



Is this what you want?



No, and with the above syntax we do not get the a=a=



If you want to use the syntax of draft-ietf-mmusic-ice-sip-sdp, why not do

it directly? Something like:



session-level-field = ice-pwd-att /

                      Š

                      Š



;ice-pwd-att as defined in [I-D.ietf-mmusic-ice-sip-sdp]



Or, have I missed something?



The trickle-ice-sdpfrag syntax uses just a subset of SDP attributes and isn't  build as open as  the SDP syntax , the above doesn't work.



I am not suggesting an open SDP syntax - I am suggesting the reference the specific syntax for the ice-pwd attribute.



Regards,



Christer





























On 06/05/18 21:31, "Thomas Stach" <[e-mail]><mailto:[e-mail]> wrote:



All,



during IESG review we got the below proposal by Adam for changing the

ABNF to allow for more encoding flexibility.



Unless somebody objects, I'd provide the proposed change by end of the

week.



It would affect the "session-level-fields" as proposed below and in a

similar way the  "pseudo-media-descriptions" and

"trickle-ice-attribute-fields"



Excerpt from Adam'S IESG review:



§9.2:



The syntax for "session-level-fields", "pseudo-media-descriptions", and

"trickle-ice-attribute-fields" include extremely strict rules around

ordering of

fields (e.g., including ice-ufrag before ice-pwd would be syntactically

invalid). That level of strictness seems unlikely to lead to interoperable

implementations.



If the intention is to be rigid in this fashion, please add prominent

prose

that warns implementors that fields MUST appear in the order specified,

and

that all other orders are invalid and MUST be rejected.



If that's *not* your intention (and I suspect it isn't), then please fix

the

syntax definition to allow for arbitrary ordering of attributes in the

same way

as SDP does. For example:



     session-level-fields = *(session-level-field CRLF)



     session-level-field = bundle-group-attribute /

                           ice-lite-attribute /

                           ice-pwd-attribute /

                           ice-ufrag-attribute /

                           ice-options-attribute /

                           ice-pacing-attribute /

                           end-of-candidates-attribute /

                           extension-attribute-fields

                                  ; for future extensions





Regards



Thomas