Re: Gen-ART LC review of draft-ietf-siprec-protocol-16

["Charles Eckel (eckelcu)" <[e-mail]>]

Hi Peter,

Thanks for your detailed review and great comments. I¹ve added proposed
resolutions inline.

On 5/16/15, 4:16 AM, "Peter Yee" <[e-mail]> wrote:

>I am the assigned Gen-ART reviewer for this draft. For background on
>Gen-ART, please see the FAQ at
><http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>
>
>Please resolve these comments along with any other Last Call comments you
>may receive.
>
>
>Document: draft-ietf-siprec-protocol-16
>Reviewer: Peter Yee
>Review Date: May-15-2015
>IETF LC End Date: May-15-2015
>IESG Telechat date: TBD
>
>Summary: This draft is almost ready for publication as a proposed standard
>but has open issues, described in the review. [Ready with issues]
>
>The draft specifies entities and a protocol using SIP, SDP, and RTP for
>recording communication sessions.  It provides the ability to notify UAs
>that they are being recorded and for UAs to notify the recording system of
>their recording preference.
>
>The document is well written and has no obvious major technical issues.
>
>
>Major issues: None
>
>Minor issues:
>
>Page 7, section 5.2, 1st paragraph, 1st sentence: are there any concerns
>about the possibility of an SRC forging the metadata?  It seems like the
>SRC could generate whatever metadata it wants and supply that in the RS.

That is true. The SRC is a trusted party in both the CS and the RS. Mutual
TLS authentication is recommended to help ensure the SRC is the
appropriate SRC, but beyond that it is trusted to "do the right thing².

>
>Page 21, section 8.1.4, last sentence: what does ³appropriately².  Specify
>where is the appropriate interpretation defined or provide it here.

I propose replacing
³interpret the CSRC list appropriately when received.²
with
³interpret the CSRC list per RFC 3550 when received.²

>
>Page 21, section 8.1.5, 2nd paragraph, 1st sentence: by ³content² do you
>actually mean ³context²?  Or do you mean to the content of a SIPREC
>recording?

Yes, as discussed in another thread.

>
>Page 27, section 8.3.1, 2nd paragraph, 1st sentence: is there a
>specification for how the SRC is supposed to map each CS CNAME to an RS
>CNAME?  If so, give a pointer to that specification.

No, that is logic internal to the RS.

>
>Page 30, section 9.1, 4th bullet item: unlike the other bullet items, this
>one is not a temporary wait before sending the metadata, but is rather a
>complete refusal to send certain metadata.  As such, although related to
>the others, it would seem to be out of place within the set of bullet
>items.

Good point. I propose removing from the list and treating as separate
sentence immediately after the bulleted list, e.g.

"Cases in which an SRC may be justified in waiting temporarily before
   sending metadata include:

   o  waiting for previous metadata exchange to complete (i.e. cannot
      send another SDP offer until previous offer/answer completes, and
      may prefer not to send an UPDATE during this time either).

   o  constraining the signaling rate on the RS.

   o  sending metadata when key events occur rather than for every event
      that has any impact on metadata.

of at ²


>
>Page 38, section 12, 2nd paragraph, 3rd sentence: perhaps the word
>³effective² would be more appropriate than characterizing it as an
>³automatic² downgrade?

Yes, as discussed in another thread.

>
>Page 38, section 12.1, 1st paragraph, 2nd to last sentence: just because
>an SRS is compromised does not mean that it cannot be authenticated.  It
>may very well be operating ³correctly² and be able to authenticate, yet
>the compromise allows the attacker to obtain the (decrypted) RS.
>Authentication does not imply that the SRS you are talking to is not
>compromised.  It only indicates the SRS possesses some form of credential
>that appears to identify it correctly.

Yes, resolution proposed in another thread.

>
>
>Page 39, last paragraph: I think this paragraph should mention (like the
>previous one) that a comparable level of security is required in this
>scenario as well.  It¹s not just a different key that¹s going to be used,
>but it should be one of equivalent strength.

Good point, will add "In order to maintain a comparable level of security,
the key used in the RS SHOULD of equivalent or greater strength than that
used in the CS."

>
>Nits:
>
>NB: Anything below marked with an asterisk before the line is a technical
>change; the rest are purely editorial and of lesser importance.

Lots of good catches here and all suggested changes made with the few
exceptions noted inline.

>
>General:
>
>Put a comma after ³e.g.² and ³i.e.² throughout the document ‹ it¹s done
>inconsistently as it is.  Maybe s/e\.g\. /e\.\g., / would do the trick and
>likewise for ³i.e².
>
>Replace ³Recording aware² with ³Recording-aware².
>
>Specific:
>
>Page 4, section 1, 2nd sentence: change ³accordance to² to ³accordance
>with².
>
>Page 5, last bullet item: insert ³a² before ³non-SIP².
>
>Page 7, last paragraph, 2nd sentence: insert ³the² before the 2nd ³SRS².
>
>Page 8, Figure 2, step 1: append ³1² after ³snapshot² for parallel
>construction.
>
>Page 10, section 6.1.2, 1st sentence: change ³recording indication² to
>³recording indications².  Or, alternatively use ³a recording indication²
>if there¹s only one provided to all participants.
>
>Page 11, section 6.3, 1st sentence: insert ³a² before ³recording
>indication².
>
>Page 12, 1st partial paragraph, 1st full sentence: delete an extraneous
>space before ³IVR².
>
>Page 12, 1st partial paragraph, 2nd full sentence: insert ³a² before ³user
>agent².
>
>Page 12, section 7.1.1, 2nd paragraph, last sentence: change ³contributes²
>to ³contribute².
>
>Page 12, section 7.1.1, 3rd paragraph, 1st sentence: insert ³an² before
>³SRC².
>
>Page 14, section 7.1.2, 1st paragraph, last sentence: insert ³a² before
>³CS².
>
>Page 15, section 7.2, 1st paragraph, 2nd sentence: insert ³the² before
>³a=inactive².
>
>Page 15, section 7.2, 1st paragraph, 3rd sentence: insert ³the² before
>³a=recvonly².
>
>Page 15, section 7.2, 1st paragraph, 3rd sentence: Would this sentence be
>more correct if rewritten for clarity as: "When the SRS is ready to
>receive recorded streams, the SRS sends a new SDP offer and sets the
>a=recvonly
>attribute in the media streams.²?

I think either construction gets the point across. It left as is in order
to be consistent with the structure of the previous sentence.

>
>Page 15, section 7.2, 2nd paragraph, 1st sentence: insert ³an² before the
>first ³SDP² and insert ³the² before ³SRS².
>
>Page 16, 2nd paragraph, 1st sentence: insert ³the² before the 2nd ³SRS².
>
>Page 16, 2nd paragraph, 2nd sentence: consider changing ³with recorded
>streams² to ³with the streams to be recorded² if that mRTP/aakes more
>sense.
>
>Page 19, section 8.1.1, 2nd item, 2nd paragraph: change the semicolon to a
>comma.
>
>Page 20, section 8.1.2, 1st paragraph, 1st sentence: move the comma before
>"[RFC5124]² to after that; do the same for ³[RFC4585]².  Change ³non
>encrypted² to ³non-encrypted².
>
>Page 20, section 8.1.2, 1st paragraph, 2nd sentence: move the comma before
>"[RFC3711]² to after that.  Delete the comma after "RTP Profile for Audio
>and Video Conferences with Minimal Control².  Change ³AVP² to "(RTP/AVP)².
>
>Page 20, section 8.1.2, 1st paragraph, 3rd sentence: insert ³RTP/³ before
>each of ³SAVP² and ³AVP².
>
>Page 20, section 8.1.2, 2nd paragraph, 2nd sentence: change the space
>after ³AVPF² to a hyphen.
>
>Page 20, section 8.1.3, 1st paragraph, 1st sentence: append a comma after
>³[RFC3550]².
>
>Page 21, section 8.1.4, last sentence: change ³sets² to ³set².
>
>*Page 22, section 8.1.7, last sentence: change ³AVFP² to ³AVPF².
>
>Page 24, section 8.2, 1st paragraph, 2nd sentence: delete the comma after
>³to².
>
>Page 30, section 9.1, 1st bullet item: insert ³a² before the first
>³previous².  Insert ³the SRC² before ³cannot².  Insert ³the² before the
>2nd ³previous².
>
>Page 30, last paragraph, 1st sentence: change ³an² to ³a² before ³200².
>
>Page 31, 2nd paragraph, 2nd sentence: append ³.,² after ³e.g².
>
>Page 32, section 9.2, 2nd paragraph, 2nd sentence: delete ³for².
>
>Page 33, last paragraph, 2nd sentence: Why not state this in the
>affirmative: ³Any subsequent partial updates will only be dependent on the
>metadata sent in this full metadata snapshot and any intervening partial
>updates.²

I find the current construction to be simpler.

>
>Page 34, section 9.2.1, 1st paragraph: change ³augmented² to ³Augmented².
>Change ³BNF² to ³ABNF².
>
>Page 34, section 9.2.1, definition: change ³RFC3261² to ³RFC 3261².
>
>Page 34, section 10, 1st paragraph, 3rd sentence: insert ³a² before ³new
>SDP².
>
>Page 34, section 10, 2nd paragraph, 3rd sentence: change ³solves² to
>³solve².
>
>Page 35, section 11.1.1, description, 1st sentence: insert ³that² before
>³the SIP².
>
>Page 35, section 11.1.1, description, 3rd sentence: change ³UAS² to ³UA².
>
>Page 35, section 11.1.2, description: change all occurrences of ³media
>level² and ³session level² to ³media-level² and ³session-level²,
>respectively.
>
>Page 35, section 11.2.1, 1st paragraph: change the 2nd ³for² to ³of a².
>
>Page 36, section 11.2.2, 1st paragraph: change the 2nd ³for² to ³of a².
>
>Page 38, section 12, 1st paragraph, 1st sentence: delete the comma after
>³therefore².
>
>Page 40, section 12.4, last sentence: append a comma after ³simple².
>

Thanks so much,
Charles