Reviewer: Francesca Palombini Review result: Almost Ready I am the assigned Gen-ART reviewer for this draft. The General Area Review Team (Gen-ART) reviews all IETF documents being processed by the IESG for the IETF Chair. Please treat these comments just like any other last call comments. For more information, please see the FAQ at <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>. Document: draft-ietf-insipid-logme-marking-11 Reviewer: Francesca Palombini Review Date: 2018-07-06 IETF LC End Date: 2018-07-10 IESG Telechat date: Not scheduled for a telechat Summary: This draft is on the right track but has open issues, described in the review. Major issues: - Minor issues: The following are, in order of importance, the issues I have. Following, I tried to point out the specific details in the doc, which, when specified, might refer to one of those more general issues. *A) Although the scenarios shown (4.1.2.x and 5.1.x) are very descriptive and useful, it is quite difficult to extrapolate from the examples what is the required behavior of UAs or proxies supporting the "log me" marking. For example, Section 4.5.2.5 really gives requirements/rules on Proxy 1 (from the originating Network), although from the way the document is structured, at first glance one could think the section is about terminating network requirements. Some rules are given in Section 4.2 and 4.3, but they seem to cover only the stateless interaction. I think some additional text (possibly introducing stateful processing, i.e. the first paragraph of 4.5.2 at the same time) with rules for stateful processing is missing and could be added there. *B) It is said in the doc that UAs or intermediaries MAY mark dialogs that are "related" to the originally marked dialog. A pointer to what dialogs are related to each other would be useful: from the doc, the example is REFER relates to INVITE, and INVITE relates to REFER, any else? is there a section in a specification that could be referenced, or a "rule" given (for example dependent on local-UUID value, or other)? *C) About error cases (section 5) I have several concerns: - I don't see anywhere that those two described (marker missing from a previously marked dialog and marker appearing mid dialog) are the comprehensive list of error cases. Is that correct? What if a marker disappear and then reappears? Is the intermediary supposed to keep logging (because the marker did not appear mid-dialog) or not? - The document explains that detecting error cases is difficult, and only possible for stateful intermediaries. I'd like to see explicit requirements for stateful intermediaries to be able to detect errors, and that such requirements cover the comprehensive list of error cases described above. - I seem to understand that the only consequence of an "error" case (for the only 2 cases listed so far) is "stop logging and stop marking". Is that correct? What about previously logged messages in the dialog? Should those be kept? Deleted? It is not specified. This could be part of some security considerations. *D) I don't think Section 4.5.2.1.1 is at the right place. I think such a section would fit better in the error handling section (Section 5) to explain why some "missing markers" are errors while others aren't. Also, it actually shows an example where log in marking is in fact supported by the originating UA, so it should definitely not be under section 4.5.2.1, at worse it should be under 4.5.2.2 (that is probably just an overlook). *E) Terminology section missing: the drafts including the terminology are referenced in the document, but it would have been good to have a section in the beginning (sub-section of introduction is common) mentioning which ones these are. At least RFC7989, possibly also RFC8123. *F) I would suggest a reformulation of the rules in 4.2 and 4.3 to have a separation: originating endpoint rules, then terminating endpoint rules for 4.2 and stateless intermediary, stateful intermediary in 4.3. This is just a suggestion for better readability in my opinion, feel free to disregard. Detailed Comments: This document defines a new header field parameter "logme" for the "Session-ID" header field. Good with a reference for "Session-ID" header field, at the end of this sentence. --- as a "log me" parameter since the session identifier is typically passed through SIP B2BUAs or other intermediaries, as per the Session-ID requirement REQ3 in ([RFC7206]). The "logme" parameter For B2BUA, see point E. --- Marking starts with a dialog-initiating request and continues for the lifetime of the dialog, and applies to each request and response in that dialog. Can a ref be added for which are the "dialog-initiating request"? Or put it in terminology (point E). --- A user agent or intermediary adds a "log me" marker in a request or response in two cases: firstly because it is configured to do so, or secondly because it has detected that a dialog is being "log me" marked, causing it to maintain state to ensure that all requests and responses in the dialog are similarly "log me" marked. Once the "log Can we change to: "A user agent or intermediary adds a "log me" marker in an unmarked request or response in two cases: firstly because it is configured to do so, or ...". --- Section 3.2: Can you add some text about an intermediary that does not support marking? It seems hinted from the following examples that it would just remove it. Can it be made explicit in this section? --- If a request or response is "log me" marked, then all re- transmissions of the request or response MUST be similarly "log me" marked. Likewise, re-transmissions of a request or response that was not "log me" marked MUST NOT be "log me" marked. What if the MUST NOT is not complied with? What should the endpoint or intermediary do in that case? I assume error, but it should be specified. (See point C.2 above) --- marking extends to the lifetime of the dialog. In addition, as discussed in Section 3.7, "log me" marked dialogs that create related dialogs (REFER) may transfer the marking to the related dialogs. In such cases, the entire "session", identified by the Session-ID header, is "log me" marked. See point B. --- The local Universally Unique Identifier (UUID) portion of Session-ID [RFC7989] in the initial SIP request of a dialog is used as a random test case identifier. This provides the ability to collate all Although it is pretty much clear what it means, it sounds like test case identifier is defined somewhere? Ref? Terminology? (point E) --- creation and ends when the dialog ends. However, dialogs related to a "log me" marked dialog MAY also be "log me" marked. An example is I guess that MAY depend on a policy in place. Can that be explicit? --- transfer. Alice's terminal inserts a "logme" marker in the REFER request and 200 OK responses to NOTIFY requests in dialog2. Both See point B. --- dialog3 is not logged by Alice's terminal, however the test case identifier ab30317f1a784dc48ff824d0d3715d86 is also the test case identifier local-uuid) in INVITE F5. Also, the test case identifier of dialog2, which is logged by Alice's terminal, can be linked to dialog1 and dialog3 because the remote-uuid component of dialog2 is the test case identifier ab30317f1a784dc48ff824d0d3715d86. This is more for my own understanding: is this the case all the time in SIP? Or is this what this doc specify? If it is the latter, then it should be probably clarified. Maybe it would be good to clarify anyway. --- Section 4.2 and 4.3: Don't see any rule for related dialogs. I understand that is optional, but I think it should be here (stated as optional). (see point B) --- The retention time period for logged messages should be the minimum needed for each particular troubleshooting or testing case. The retention period is configured based on the data retention policies of service providers and enterprises. should -> SHOULD? --- Consent to turn on "log me" marking for a given session must be provided by the end user or by the network administrator. It is may -> MAY? --- Nits/editorial comments: when this dialog and it's related dialogs end. It is considered an it's -> its --- A SIP intermediary MUST copy the "log me" marker into forked requests. Can a ref be added to the spec describing forked requests? --- o The originating user agent itself logs signaling. Change to: The originating user agent itself logs marked signaling requests. Additionally, see point F. --- echoed by a terminating UA. SIP intermediaries on the signaling path between these UAs that do not perform the tasks described in Section 4.5.2 can simply log any request or response that contains a "log me" marker in a stateless manner, if it is allowed per local policy. That can should be a MUST, I think. --- Section 4.5.2.1: F12 - Proxy 1 receives ACK with with no "log me" marker. It doesn't consider this as an error since it is configured to "log me" mark on behalf of Bob's UA. Bob -> Alice --- 4.5.2.3: Typo in the figure: after F2, missing characters on the lines of Proxy 2 and Bob. Same just before F19. --- requests and responses entering network B. However, Proxy 2 supports maintains the marking state of the dialog and "log me" marks requests remove supports (or maintains) --- 4.5.2.4: Typo in the figure: wrongly aligned characters under F9. --- 4.5.2.4: F2 - Proxy 2 removes "log me" marker in the INVITE request F2 before forwarding it as F7. F7 -> F4. Also, maybe add "The same applies to F13 and F19", to be consistent. --- 4.5.2.5: Typo in the figure: wrongly aligned characters under F9. --- 4.5.2.5: F2 - Proxy 2 removes "log me" marker in the INVITE request F2 before forwarding it as F7. F7 -> F4. Also, maybe add "The same applies to F13 and F19", to be consistent. --- 7.4.4: "Log me" in the beginning of the paragraph, but 7.4.5 second paragraph starts with "log me". Which one is it? (I personally have no preference). Same for "log me" marking and "log me" Marking (in 7.4.5) --- (please keep my address in the to or cc in responses to this review, for easier tracking) Best regards, Francesca
