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

[Ted Hardie <[e-mail]>]

Hi Christer,

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

Ted

On Thu, May 24, 2018 at 1:19 AM, Christer Holmberg <
[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
>