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" <peter(_at_)akayla(_dot_)com> 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