Session Initiation Protocol (SIP) Recording Metadata
draft-ietf-siprec-metadata-22

I don't see how a reader can understand this without understanding what an SRC, an SRS, an RS, and a CS are, and those are defined in RFC 6341.  Why isn't 6341 a normative reference?

Please expand "AoR" on first use.  You might also cite 3261 there as defining it.  (I find it a bit odd that there's no citation to 3261 except in the Security Considerations.)

You refer twice to "Unified Modelling Language(UML)": is there a reference for UML?

On the other hand, while we're on references, I really don't think you need any of the citations in Section 5.1.1 (and can remove their associated references).  No one reading this needs to know the details of what a URN is or how the ietf URN namespace is organized (just like you don't have a reference for "URI", and don't need it).  I would just simplify it to this and take out the three unnecessary references:

NEW
   The namespace URI for elements defined by this specification is
   the following URN:
   
      urn:ietf:params:xml:ns:recording:1
END

A really tiny point (which the RFC Editor might also bring up): In Section 6.1.1 you refer to "a RS class" and "a RS object".  Unless that's actually pronounced "Recording Session" when you read it, it should be "an RS [class|object]".

There are a lot of missing articles ("the", "a", "an") throughout.  The RFC Editor will fix these, but...

In Section 6.3.1, the definition of the "reason" attribute is slightly confusing because "reason" is also an English word that can fit into the sentences also.  So, here:

      The reason XML element has
      'protocol' as an attribute, which indicates the protocol from
      which the reason string is derived.

...I initially read "the reason XML element has 'protocol' as an attribute..." differently to how you meant it.  I suggest making it this:

      The 'reason' XML element has
      'protocol' as an attribute, which indicates the protocol from
      which the reason string is derived.

I also suggest going back through this and consistently quoting the words when they refer to attribute or element names, to distinguish them from just plain English ("the 'session' XML element", "the 'reason' XML element", " 'protocol' attribute", and so on).  You could skip this for the camel-cased items (such as "sipSessionID"), and the hyphenated ones ("group-ref"), because they can't be confused with plain words... but it might be better to be consistent.

In Section 6.3.2, I don't like the two non-parallel lists at the end.  I think the RFC Editor will fix these, so I'll just point them out and let you decide what to do (or not).  (In contrast, the list in Section 6.5.2 is properly parallel.)

In Section 6.9, just checking here:

   As a security measure, the timestamp element SHOULD be included in
   all tuples unless the exact time of the status change cannot be
   determined.

Are there other reasons one might not include a timestamp element?  Or is the fact that you can't determine the time the only one?  If the latter, then it should be "MUST ... unless", rather than "SHOULD ... unless", and I think that's what you mean.

I'm going to ballot no-objection on this, but I do have some comments

= Substantive =

- I'm confused about the associations between an RS (as modeled by <recording>) and a CS or CSG. As far as I can tell, a <recording> element is associated with a given CS/CSG by the fact of enclosing it. But the UML and related text indicates that a CS or a CSG can be associated with more than one RS. How does that work in the XML?  Along the same lines, what would it mean for a CS or CSG to be associated with more than one RS? 

- 5.1: Does the xml doc contain _exactly_ one <recording> element?

- 5.1.2: See question on 5.1. (Also, the normative language seems redundant between the sections.)

- 6.1.2: Can you elaborate on "persistent recording" and why it removes the need for a CS/CSG?

- 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?

-6.2.2: The UML suggests that a given CS cannot be associated with more than 1 CSG. That’s worth mentioning here.

- 6.3: Can a given CS be directly associated with an RS and also a CSG? (I guess that will always be true in the XML, but it's not apparent from the UML).

-6.3.1, sipSessionID: I gather this is the session-id from the media session, not the recording session. If correct, it would be helpful to say it explicitly.

- 6.3.3: "A stream in a persistent RS is not required to be associated with any CS..."
It's not apparent how you would do that in the UML. I can see how you would do it by virture of being contained in a <recording> element. 

- 6.5.1: Remote-Party-ID? Citation needed ;-)

- 6.10: Why not MUST? Can you envision good reasons to not use this method?

-6.11: Do I understand correctly that means  no extensions are allowed without changing the version number? Is that really the intent?

-10, first paragraph:
Why is the SHOULD not a MUST? Also,  what does the SRC need to authenticate/authorize?
-- 2nd paragraph: Do you really expect people to implement s/mime?
-- last paragraph: I'm not sure what "Implementations MUST control what metadata
   is recorded" means. Is the point that the implementation must let someone control that policy? That it must not record unnecessary metadata? (If the later, how is "necessary" determined?)

- 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.) 

= Editorial =

- Figure numbers and captions would be useful.

- I found the organization to be a bit odd. Section 5 starts talking about some but not all XML details, then 6 goes over the UML model, but switches back to XML in the last few sections, then we've got more XML stuff (schema, etc) further down.

- There are a lot of missing articles and commas, especially in the in section 6 and subsections. The RFC editor will fix this, but it would save them work if the authors did a proofreading pass, if they otherwise have reason to update the draft.

- 6.1: It would have been helpful to mention that the <recording> element represented a recording session back when <recording> was first mentioned in section 5.

Thanks for addressing my discuss points.