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 wait for direction from your document shepherd or AD before posting a new version of the draft. Document: draft-ietf-kitten-sasl-saml-08 Reviewer: Ben Campbell Review Date: 2012-01-13 IESG Telechat date: 2012-01-19 Summary: This draft is almost ready for publication as a proposed standard. There are a few minor issues that should be considered first. Note: This is incremental to my review of version 06 at last call. Version 08 is considerably improved and resolved most of my comments. But a few still remain. Quoted text below is from that previous review. Major issues: None Minor issues: > -- section 3.2, last paragraph: "… needs to correlate the TCP session from the SASL client with the SAML authentication." > > Please elaborate on this correlation The author added text, but the new text is about correlating response with request. The text I mentioned was about correlating a TCP connection to a SAML authentication. > -- section 4, 3rd and 4th paragraph (paragraph a and b) > > These seem like protocol affecting differences. If so, they need elaboration, such as normative statements and formal definitions, or references to such. I did not see a response to this comment. > -- section 5, general: > > The section seems to need further elaboration or references I did not see a response to this comment. > Also, this section compresses the interaction with the identity provider. Why not show the details for those steps like the others? (If you mean them to be out of scope, then why give as much detail as you do?) > I did not see a response to this comment. Nits/editorial comments: > -- Pagination is strange throughout the document. (Mostly blank pages, etc.) It's worse in the PDF version, but still not right in the text version. Pagination is still strange. I see a few mostly blank pages, diagrams split across page breaks, etc. > -- section 3, 1st paragraph: "Recall section 5 of [RFC4422] for what needs to be described here." > > That reads like an author's "to do" note to himself. Has the needed description been completed, or does it still need to be described? Partially fixed. I suggest s/"needs to be"/"is" > -- section 3.1, first paragraph: > > Is "authorization identifier" the same as "IdP-Identifier"? The paragraph has been updated, but I still have the question. > -- section 3.3, 2nd paragraph: "and SHALL be used to set state in the server accordingly, and it shall be used by the server" > > Is this new normative language, or a repeat of language from the SAML spec? > The author changed SHALL to MUST, which leads me to believe my comment was not clear. I did not object to SHALL. My concern was, with the reference to RFC4422, it was not clear if the text was introduction a new normative requirement, or just restating requirements from 4422. If the second, then it's important to make sure the reader knows which doc is authoritative. You can do that by keeping the language descriptive, or by explicitly (and strongly) attributing the language with something like 'RFC4433 says, "…. SHALL…."' If, on the other hand, this is truly a new normative statement, then no change is needed. > -- section 4, 1st paragraph: > > I have difficulty parsing this. The text is updated, but I still have trouble parsing it. In particular, I'm not sure what you mean by the phrase "...and appropriate references of it not referenced elsewhere in this document…". > -- section 7 > > Does the GSS-API description introduce security considerations? If not, please say so. > I did not see a response to this comment. _______________________________________________ Ietf mailing list Ietf@xxxxxxxx https://www.ietf.org/mailman/listinfo/ietf