Re: [apps-discuss] apps-team review of draft-ietf-fecframe-sdp-elements

["Ali C. Begen (abegen)" <[e-mail]>]

Hi Dave,

I am off to PTO till the weekend (about to leave). But, here are my quick comments:

> -----Original Message-----
> From: Dave Cridland [mailto:[e-mail]]
> Sent: Tuesday, September 07, 2010 10:34 PM
> To: General discussion of application-layer protocols; The IESG; Ali C. Begen (abegen)
> Subject: apps-team review of draft-ietf-fecframe-sdp-elements
> 
> Hiya folks,
> 
> I have been selected as the Applications Area Review Team reviewer
> for this draft (for background on apps-review, please see
> http://www.apps.ietf.org/content/applications-area-review-team).
> 
> Please resolve these comments along with any other Last Call comments
> you may receive. Please wait for direction from your document
> shepherd or AD before posting a new version of the draft.
> 
> Document: draft-ietf-fecframe-sdp-elements
> Reviewer: Dave Cridland
> Review Date: 2010-09-07
> 
> Summary: This draft has minor editorial issues, and one potential
> issue. The document could be published as an RFC with the existing
> encoding of repair-window, but some thought should be given to it.
> 
> Major Issues: None
> 
> Minor Issues:
> 
> Section 4.6 describes an SDP attribute "repair-window", which seems
> to have a possibly over-complicated representation. It has two
> different units, one of which is the default. This yields three
> different ways of expressing the same intent:
> 
> 150
> 150ms
> 150000us

Correct.
 
> It's unclear to me what benefit this brings, and the downside could
> be that implementations only handle a subset of the possibilities,
> resulting in a failure to interoperate.
> 
> I would advise serious consideration of restricting this to one unit.
> If microsecond resolutions are required, then obviously this would be
> µs, resulting in the above being written only as:

Personally and as the author, I was pretty much always fine with the ms. I don't think we will see any need for 'us' in the foreseeable future. But, some asked for it and defining it as an alternative unit was the simple solution. 
 
> 150000

This may address your concern but for the current implementations and applications, it might be more confusing.
 
> This is unfortunately not a current option, and if there are existing
> deployments, this may yield further problems.

Right.

However, I am open to suggestions. If majority thinks we should change it, I can do a quick revision.
 
> Nits:
> 
> The term "bytes" is sometimes used where I suspect "octets" would be
> more precise.

OK, will do one more pass in the next revision.
 
> Security:
> 
> I briefly cast my eye over the Security Considerations section -
> please don't grant these comments the same weight as a security
> review.
> 
> There is advice that "It is RECOMMENDED [to] follow the security
> considerations of SDP". I would have thought that following those
> recommendations would be a requirement. I'm quite taken with using
> both RECOMMENDED and SHOULD in the same sentence, mind, but I think
> this could be phrased better.

Will look into this.
 
> It's also not clear what effect the SDP modification would actually
> have - whether this is a denial of service or if it yields some other
> attack. (My suspicion is that if an attacker is in a position to
> perform this attack, then the attacker can subvert the media streams
> entirely and do something much more interesting).

Yes, he can certainly do more interesting and harmful stuff. But, doing harm without being detected would be worse and SDP modification is one way of doing it.
 
Thanks again for the review. I will address these comments in the next revision (I will be back in the weekend).

Cheers, acbegen

> Dave.
> --
> Dave Cridland - mailto:[e-mail] - xmpp:[e-mail]
>   - acap://acap.dave.cridland.net/byowner/user/dwd/bookmarks/
>   - http://dave.cridland.net/
> Infotrope Polymer - ACAP, IMAP, ESMTP, and Lemonade