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

[Thomas Stach <[e-mail]>]

Christer,


On 2018-05-07 21:41, Christer Holmberg wrote:
> 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

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