Last Call Review of draft-ietf-drinks-spp-framework-09
review-ietf-drinks-spp-framework-09-genart-lc-yee-2015-01-23-00

Request Review of draft-ietf-drinks-spp-framework
Requested rev. no specific revision (document currently at 12)
Type Last Call Review
Team General Area Review Team (Gen-ART) (genart)
Deadline 2015-01-22
Requested 2015-01-09
Authors Kenneth Cartwright, Vikas Bhatia, Syed Ali, David Schwartz
Draft last updated 2015-01-23
Completed reviews Genart Last Call review of -09 by Peter Yee (diff)
Genart Telechat review of -09 by Peter Yee (diff)
Secdir Early review of -?? by Paul Hoffman
Assignment Reviewer Peter Yee
State Completed
Review review-ietf-drinks-spp-framework-09-genart-lc-yee-2015-01-23
Reviewed rev. 09 (document currently at 12)
Review result Ready with Nits
Review completed: 2015-01-23

Review
review-ietf-drinks-spp-framework-09-genart-lc-yee-2015-01-23

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-drinks-spp-framework-09
Reviewer: Peter Yee
Review Date: Jan-22-2015
IETF LC End Date: Jan-22-2015
IESG Telechat date: TBD

Summary: This draft is basically ready for publication as a Standards Track
RFC, but has some nits that should be fixed before publication. [Ready with
nits.]

The draft specifies a framework for use in provisioning session
establishment data into Session Data Registries and SIP Service Provider
data stores, just like it says in the abstract.  It does not provide a
concrete protocol for these operations.  Most of my comments have to do with
niggling readability issues, not the substance of the document.  It seems
quite reasonable for its stated purpose.

Major issues: None

Minor issues: 

I'm not sure I wholly comfortable with how the framework describes the
protocol be created from it as a "transport protocol".  That term is
somewhat loaded in network and in the Internet is generally thought of as
meaning a protocol such as TCP or UDP.  Furthermore, the operations that are
carried out by the protocol specified in the framework are all that
applications layer and rely upon other application layer protocols to
perform security vital functions such as authentication and authorization.
Perhaps a less loaded term could be used?

Section 7.2: Is the "Delete" operation meant to be atomic?  Should that be
specified in that section?

Section 9.7: this section discusses how the "transport protocol" provides
connection protection services and then says that therefore a
man-in-the-middle attack is possible.  If that's the case, then the
"transport protocol" is not (adequately) providing connection protection.
And without connection protection, a man-in-the-middle attack would of
course be possible, so saying that because there is connection protection, a
man-in-the-middle attack is therefore possible seems misleading.

Nits:

General:

There are a few occurrences of "e.g.  XYZ" (that is, "e.g." followed by two
spaces and something (XYZ)).  Replace the first space with a comma, which
should be easy with a general search-and-replace operation.

Change the few contrary occurrences of "source based" to "source-based".

Insert "the" consistently before <xyz> when talking about an element.

Specific:

Page 3, section 1, 2nd paragraph, 1st sentence: consider replacing "need"
with "ability".

Page 3, section 1, 3rd paragraph, 1st sentence: change "role" to "roles".
Change "applies" to "apply".

Page 10, section 3.2, 1st paragraph, 3rd sentence: insert "a" before "time".

Page 11, section 4.1: do you really mean "point-to-point" or "end-to-end"?
In any case, insert "a" or "an" before the correct choice, as appropriate.

Page 12, section 4.8, 1st sentence: change "large number of dataset" to
"large datasets".  Insert "of" between "millions" and "records".

Page 12, section 4.8, 2nd sentence: change "dataset" to "datasets".

Page 12, section 4.10, 2nd sentence: change "A" to "An" unless SPPF is
actually pronounced as a word and not spelled out.

Page 13, section 4.11, 1st sentence: insert "the" before "SPP".

Page 13, section 5.1, 2nd sentence: would "modified" be a better word than
"updated" so as to type to the attribute name "mDate"?

Page 13, section 5.1, last paragraph, last sentence: change "IANA
Consideration" to "IANA Considerations".

Page 15, SedGrpOfferKeyType specification, comment: change "a object offer"
to "an object offer".

Page 15, 1st paragraph after the SedGrpOfferKeyType specification, 2nd
sentence: append "to" after "Refer".  Append "a" after "for".  Insert "the"
before "SED Group Offer object".

Page 15, PubIdKeyType bullet, 2nd sentence: insert "the" before "abstract".

Page 16, "Request syntax invalid" table entry: change "a" to "the" before
"syntax".

Page 17, "System temporarily unavailable" table entry: insert "the" before
"client".

Page 18, last table entry: delete "the" before "said" in both occurrences.

Page 19, 1st partial paragraph: insert "to" between "refer" and "the".

Page 19, section 6.2, 1st paragraph, 4th sentence: replace "whom" with
"which".

Page 19, section 6.2, 2nd paragraph, 1st sentence: replace "URI" with
"URIs".

Page 20, 1st paragraph: delete "the section".

Page 20, 1st paragraph after the PubIdType specification, 2nd sentence:
insert "a" before "member".

Page 20, 1st paragraph after the PubIdType specification, 3rd sentence:
change "SED" to "SEDs".

Page 22, 1st partial paragraph: change "the" to "a" before "TN add
operation".  Consider changing "add" to "Add".

Page 22, last paragraph, 2nd sentence: is the span meant to be inclusive or
exclusive of the starting/ending TNs?  I'm guessing inclusive, but it wasn't
utterly clear from the text and might be worth spelling out.

Page 22, last paragraph, last sentence: Insert "The" before "TNRType".

Page 23, last paragraph, 2nd sentence: insert "the" before "open plan".

Page 24, "uri" description: append "as" after "acts".

Page 25, 1st partial paragraph, 1st full sentence: delete the space after
"SED Record".

Page 26, "dgName" description: change "side affect" to "side effect".

Page 27, 2nd paragraph, 1st sentence: change "source base" to
"source-based".

Page 27, 2nd paragraph, 3rd sentence: change "organizations" to
"organization's".

Page 29, "sedFunction" description: change "his or her" to "its".

Page 31, "IPAddrType" specification: change to default value to "IPv4" from
"v4".

Page 32, "regx" description: change "Repl" to "repl".

Page 32, "repl" description: change "Regex" to "regex".

Page 32, "ipAddr" description: change "IP v6" to "IPv6".

Page 32, Section 6.5, 1st sentence: change "know" to "known".

Page 32, Section 6.5, last sentence: insert "to" between "refer" and "the".

Page 33, "status" description: change "when ever" to "whenever".

Page 34, section 6.6., 3rd paragraph, 1st sentence: delete "Lets" or replace
it with "Let's".  Change "his" to "its".

Page 34, section 6.6., 3rd paragraph, 2nd sentence: append "the" after
"rewrite".

Page 34, section 6.6, 4th paragraph, 1st sentence: append "to" after
"refer".

Page 36, section 7, 1st sentence: change "operation specific" to
"operation-specific".  Change "that" to "which".

Page 36, section 7.1, 2nd paragraph: append "the" after "defined in".

Page 36-37, section 7.2, each bullet item: I think the "as part of
fulfilling the deletion request" can be safely left out in all cases.  It
should be obvious from the lead-in material that these rules only apply in
that case.

Page 37, "SED Groups" rule, 3rd sentence: append "to" after "relating".

Page 38, section 7.4, 1st paragraph, 1st sentence: replace "whom" with
"which".  Append "to" after "refer".

Page 38, section 7.4, 2nd paragraph, 3rd sentence: replace "the the" with
just one "the".

Page 38, section 7.5, 1st paragraph, 1st sentence: replace "whom" with
"which".  Append "to" after "refer".

Page 38, section 7.5, 1st paragraph, 3rd sentence: replace "Offers" with
"Offer".

Page 39, section 7.6, 1st paragraph, 1st sentence: insert "the" before
"Get".  Delete the comma after "status" and replace it with "and".

Page 39, section 7.6, 2nd paragraph, 2nd sentence: append "the" after
"defined in".

Page 41, section 9.3.1, 1st paragraph, 1st sentence: insert "An" before the
sentence.  

Page 41, section 9.3.1, 1st paragraph, 2nd sentence: you should probably put
references here ([XML], [SOAP]) instead of raw URLs to their specifications.
Change "on-the wire" to "on-the-wire".

Page 42, 1st partial paragraph, 1st whole sentence: delete "and" after
"completing one".

Page 42, section 9.5: change the section title to "Non-repudiation" in line
with RFC 5280 usage.

Page 43, section 9.7, 3rd sentence: insert "a" before "man-in-the-middle".
However, see related item in the "Minor Issues" section.

Page 44, 2nd paragraph, 1st sentence: delete the comma after "string".

Page 44, 3rd paragraph: change "well known" to "well-known".

Page 52, section 13: put a paragraph on the end of the section.