Re: [rtcweb] SDP directorate review of SDP Identity attribute (draft-ietf-rtcweb-security-arch-14)

[Christer Holmberg <[e-mail]>]

Hi,

>Can you let us know which of those you believe are still outstanding?

I believe all except Q2 are still outstanding.

Sean did have some input on Q0, but I am not sure there was ever an outcome.

Regards,

Christer



On Thu, May 24, 2018 at 1:19 AM, Christer Holmberg <[e-mail]<mailto:[e-mail]>> wrote:
Hi,

Below is my SDP directorate review of the SDP Identity attribute, defined in draft-ietf-rtcweb-security-arch-14.

Most of the comments have previously been given as individual comments by Paul K and myself. Some have also been addressed by Sean T, but I will include them for completeness.

---

Q0 (Scope):

As the SDP attribute definition should be able to stand on its own legs, there should be a description/reference to the IdP solution that it is used for.

Also, if the usage of the attribute is scoped to devices implementing the WebRTC specification, that should be indicated.

---

Q1 (Structure):

The document lacks the offer/answer procedure structure used for SDP attributes.

X.1  Generating the Initial Offer
X.2  Generating the Answer
X.3  Offerer Processing of the Answer
X.4  Modifying the Session

Based on discussion in MMUSIC, it is also good to clarify that “initial offer” refers to the offer where the extension is first introduced.

The O/A procedures also need to cover the case when an offer/answer from a peer does not include an Identity attribute.

---


Q2 (Missing information):



The mux category is missing.



---



Q3 (Grammar):



The current ABNF says:



    identity-attribute  = "identity:" identity-assertion

                                         [ SP identity-extension

                                         *(";" [ SP ] identity-extension) ]



This mixes the usage of space and semicolon for separating identity assertions. Please use one:



    identity-attribute  = "identity:" identity-assertion

                                        *( SP identity-extension )



…or:



    identity-attribute  = "identity:" identity-assertion

                                        *(";" identity-extension)



---



Q4 (Attribute definition):



Historically there have been problems with the definitions of new SDP attributes. Especially, the manner of defining the syntax was inconsistent. RFC4566 gave insufficient guidance on how to do this. draft-ietf-mmusic-rfc4566bis (especially section 8.2.4.1) has provided more guidance on how to do this. Please do your definitions in that style. For instance:



    5.6.4.2  a=identity Attribute



    The identity attribute is session level only.  It contains an

    identity assertion, encoded as a base-64 string [RFC4648].



      Attribute Name: identity



      Attribute Value: identity-value



      Usage Level: session



      Charset Dependent: No



      Mux Category: ???



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



    identity-attribute  = "identity:" identity-assertion

                                        *( SP identity-extension )



    [continue on with remainder of section.]



---



Q5:

The draft says that, at minimum, the fingerprint needs to be bound to the identity.

First, I assume this means that, for each assertion, the associated SDP fingerprint attribute must be included in the offer/answer. If so, please include text about that.

Second, it would be good to indicate that, in the SDP, there is no link between a given assertion and the associated fingerprint.

---



Q6:

Section 5.6.4.1 says:



   "The "a=identity" attribute MUST include all

   fingerprint values that are included in "a=fingerprint" lines."

It is unclear what “attribute MUST include all fingerprint values” means. I assume that the text should say something like:


   "The "a=identity" attribute MUST include an assertion for each

   a=fingerprint attribute value that are included in the SDP offer/answer.”



It should also be clarified that, if the attribute is used on media-level in an m- section, it contains the assertion for each fingerprint associated with that m- section.



---



Q7:



Section 5.6.4.2 says:



  “The semantics of multiple identity attributes are undefined.”



First, I don’t see the difference in having a single attribute with multiple assertions, or multiple attributes with single assertions. Having said that, if we only want to allow one way, I would suggest to simply forbid multiple attributes.



Second, it needs to be clarified that the rule apply to a given m- section.



---



Q8:


In the example in Section 5.6.4.1 the SDP fingerprint attribute is included as a session-level attribute. However, it is currently only defined as a media-level attribute.

---

Regards,

Christer