Re: [siprec] Ben Campbell's No Objection on draft-ietf-siprec-metadata-20: (with COMMENT)

["Ben Campbell" <[e-mail]>]

Hi, further comments inline. I removed sections that didn't seem to need 
further discussion.
On 9 Mar 2016, at 22:33, Ram Mohan R (rmohanr) wrote:

[...[

>>
>> In any case, I don't get a sense of how this works from the draft. I
>> think it would benefit from a section that describes the high-level
>> operation (unless that's already in another document, in which case a
>> reference would help.)
>
> I don’t see a need for separate section. I will modify 2nd to last 
> para of
> Section 4. So new text will be:
>
> EXISTING:
> The model allows the capture of a snapshot of a recording's metadata
>    at a given instant in time.  Metadata changes to reflect changes in
>    what is being recorded.  For example, if a participant joins a
>    conference, then the SRC sends the SRS a snapshot of metadata 
> having
>    that participant information (with attributes like name/AoR pair 
> and
>    associate-time.)
>
>
> NEW:
> The model allows the capture of a snapshot of a recording's metadata
>    at a given instant in time.  Metadata changes to reflect changes in
>    what is being recorded.  For example, if a participant joins a
>    conference, then the SRC sends the SRS a snapshot of metadata 
> having
>    that participant information (with attributes like name/AoR pair 
> and
>    associate-time.)
>
>

I see you corrected this is a separate email, so I will respond there.

>>

>> [...]
>>
>>>>
>>
>> I think that would help. I don't think people will universally 
>> interpret
>> "One...MUST be present" to mean "Exactly one...MUST be present."
>
> Ok. New text is:
>
> EXISTING:
> The 'recording' element MUST contain an xmlns namespace attribute with
> value as urn:ietf:params:xml:ns:recording:1. One recording element 
> MUST be
> present in every recording metadata XML document.
>
> NEW:
> The 'recording' element MUST contain an xmlns namespace attribute with
> value as urn:ietf:params:xml:ns:recording:1. Only one recording 
> element
> MUST be present in every recording metadata XML document.
>
>

I see Paul beat me to suggesting s/"Only one"/"Exactly one". It's fine 
with that change applied.

>>>
>>
>> [...]
>>
>>>>
>>>> - 6.1.2: Can you elaborate on "persistent recording" and why it
>>>> removes
>>>> the need for a CS/CSG?
>>>
>>> A persistent RS is a SIP dialog that is setup between SRC to SRS 
>>> even
>>> before any CS is setup. So the metadata sent from SRC to SRS at that
>>> time
>>> would not have CS(and the related CSG)
>>>  element as there is no session that is being recorded yet. IIRC the
>>> WG
>>> discussions this is primarily for endpoints based recording where 
>>> the
>>> endpoint (like a SIP phone) will have a
>>> Persistent RS to SRS. As soon as any call is made from that
>>> phone/answered
>>> on that phone the Session will be recorded from that RS.
>>
>> It would be helpful to say that in the draft.
>
> Will add following text to 6.1.2
>
> NEW:
>
> A persistent RS is a SIP dialog that is setup between SRC to SRS even
> before any CS is setup. So the metadata sent from SRC to SRS when the 
> RS
> is setup will not have have CS(and the related CSG) elements in the 
> XML as
> there is no session that is associated to the RS yet.  For e.g; a 
> phone
> acting
> as SRC can setup a RS with SRS even before phone is part of a CS. Once 
> the
> phone
> joins a CS, the same RS would be used to convey the CS metadata.
>
> Does this look ok ?

Yes, thanks.

>
>
>>
>>>>
>>>> - 6.2: What does a CSG model? What does it mean for a CS to be part
>>>> of a
>>>> group or not part of a group?
>>>
>>> A CSG is used to group all related CS. For e.g, in a contact centre
>>> flow a
>>> call may get transferred to multiple agents/SIP elements. Each of
>>> these
>>> may trigger setup of new CS.
>>> In cases where the SRC knows the related CSs it can group them using
>>> the
>>> CSG element. Even though each CS has its only identifier (UUID) all
>>> will
>>> be associated to same CSG element.
>>> This is expected to provide useful information to SRS.
>>>
>>
>> Some text to that effect would be helpful.
>
> Ok, Will add the following text at the beginning of 6.2
>
> EXISTING:
> One instance of a Communication Session Group(CS-Group) class namely
>    the Communication Session Group object provides association or
>    linking of Communication Sessions.
>
>
> NEW:
> One instance of a Communication Session Group(CS-Group) class namely
>    the Communication Session Group object provides association or
>    linking of Communication Sessions. A CS-Group is used to group all
> related CS. For e.g, in a contact centre flow a call may get 
> transferred
> to multiple agents. Each of these may trigger setup of new CS.
> In cases where the SRC knows the related CSs it can group them using
> The CS-Group element.
>
> Is this better ?

Yes, thanks.

>
>
>>
>> [...]
>>
>>>
>>>>

[...]

>>
>> In the UML is that media streams are only indirectly associated to an 
>> RS
>> via the CS, i.e. if the CS doesn't exist, then it looks like the 
>> media
>> stream is not associated with an RS, either. But in the XML, the
>> <stream> element is always enclosed in a <recording> element.
>>
>> (I'm probably being overly pedantic in expecting the UML and XML
>> associations to formally match. But since the semantics are mainly
>> explained in terms of the UML model, I find it at least a little bit
>> confusing.)
>
> All the other elements are contained inside the <recording> element by 
> its
> association via a CS.
> I agree that media stream since it can be outside a CS can have a 
> direct
> linkage to RS. All the other elements link via CS
> I will add a line from MS block to RS in the model.

That helps, thanks.

[...]

>
>>
>>>>
>>>> -6.11: Do I understand correctly that means  no extensions are
>>>> allowed
>>>> without changing the version number? Is that really the intent?
>>>
>>> Yes. If any new extensions to the schema defined in this draft is
>>> done, it
>>> has to be done by means of a new version number to the namespace
>>> defined
>>> here.
>>>
>>
>> I'm not going to push on this, but I'm surprised this doesn't allow 
>> at
>> least "informational" extensions (where nothing breaks if the 
>> receiver
>> doesn't understand it) using normal XML extension mechanisms.
>
> The versioning was added after discussions with few XML experts who
> suggested it is always a best practice.
> Even if its a informational extension I would expect the ver to be
> incremented, otherwise a SRS parsing the metadata
> Would not understand the new elements. It can choose to ignore but I 
> see
> no harm in having a new ver for new extensions of this draft.

Okay. I don't consider myself an XML expert, and certainly won't argue 
real ones. :-)

[...]

>>
>> Now that you've added the reference, you may not need the normative
>> language here. Is the RECOMMENDED something new beyond what is 
>> already
>> required in siprec-protocol? (I am pretty sure the MUST is already
>> covered there.)
>
> Right. I have fixed this after discussion with Stephen. New text for 
> para
> 1 in Security Consideration is:
> Para 2 is removed.
>
> NEW:
> This document describes an extensive set of metadata that may be 
> recorded
> by the SRS. Most of the
> metadata could be considered private data.   The procedures mentioned 
> in
> security consideration
> section of <xref target="I-D.ietf-siprec-protocol"/> MUST be 
> implemented
> by SRC and SRS for
>  mutual authentication and to protect the content of the metadata in 
> the
> RS.
>
> Some implementations may have the SRC choose parts of metadata that 
> can be
> sent to the SRS.
> In other cases, SRCs may send metadata that is not appropriate for the 
> SRS
> to record. Which
>  metadata is actually recorded by the SRS must be carefully considered 
> to
> balance privacy
> concerns with usability. Implementations MUST control what metadata is
> recorded, and MUST NOT
>  save metadata sent by the SRC that does not conform to the recording
> policy of the SRS.
> Metadata in storage needs to be provided with a level of security that 
> is
> comparable to that
> of the recording session.

Perhaps s/"control what metadata is recorded"/"limit what metadata is 
recorded" ?

>
>>
>>>
>>>>
>>>> - 13.2: I think RFCs 3325 and 3326 need to be normative references.
>>>> That's an issue for 3325, but section 6.5.1 normatively states that
>>>> P-AID
>>>> is a potential source for nameID. (which should probably be scoped 
>>>> to
>>>> only be true for deployments where P-AID makes sense.)
>>>
>>> Fine. We can make reference to 3325 and 3326 normative.
>>
>> On reflection, I think I've changed my mind on suggesting 3325 be
>> normative. In the description of the nameID attribute, it would be
>> better to say that nameID is the AoR of the participant, and then
>> non-normatively mention examples of where that might come from
>> (including P-AID as one of those examples.)
>
> WFM. I will modify the text to make it non-normative.
>
> NEW:
> The AoR is drawn from From header or P-Asserted-Identity header or
> Remote-Party-ID header.

I suggest going a little further, such as "... For example, the AoR may 
be drawn from the From header field or P-Asserted-Identity header 
field." (eliding RPID as mentioned earlier)

>
>>
>> And now that I look at that paragraph again, I wonder if an
>> RFC4424/4424bis identity assertion should be listed as an example
>> source?
>
> I am fine with adding RFC4424 Identity assertion header as one source 
> for
> nameID.

On reflection, the actual AoR would still come from From, etc, even with 
4474/4474bis. So the mention is probably not needed.

>
> Ram
>
>>
>> [...]