Re: [mile] AD Review of RFC5070-bis IODEFv2

["Roman D. Danyliw" <[e-mail]>]

Good evening Kathleen!

Thank you for your detailed review.  Specific comments are inline.  A new -20 draft was released to address this feedback.

> -----Original Message-----
> From: mile [mailto:[e-mail]] On Behalf Of Kathleen Moriarty
> Sent: Tuesday, April 26, 2016 1:51 PM
> To: MILE IETF <[e-mail]>
> Subject: [mile] AD Review of RFC5070-bis IODEFv2

[snip]
 
>  I also see that the IANA changes are
> requested to be done in the RFC editor queue, can we do that sooner to
> prevent multiple reviewers from commenting on the same thing?  I can find
> someone to help if needed.

No problem.  I'll reach out.

> In the introduction, there should be a summary of changes from RFC5070.
> Change logs are usually deleted upon publication and last call reviewers
> usually look for this in the introduction.  A pointer to section 4.4 would be
> good as well here.

Section 1.4 was added to provide such a changelog.

> Section 3.3.1
> Did the WG discuss (sorry if I missed it), just replacing the values for TLP with
> what is there now?  Or are there other groups using what was there in v1?
> Having duplication means that applications have to program in the
> equivalence, which is fine, just one more thing to do or that could cause
> confusion.

There wasn't a detailed conversation to outright replace the @restriction values with TLP.  It was an additive suggestion.  The old values were primarily left for compatibility.  

Any implementers making the v1 to 2 transition have an issue with dropping the old values? 

> Section 3.9 Contact class
> 
> I noticed that nothing is required and wanted to make sure that is what the
> WG wanted for this class.  The class can be used with no required elements.

That's true if one only reads the schema.  The text provides clarifying guidance (a MUST) that some class child class is required.  There are a number of other classes (both from 5070 and this draft) that share this same design.

[snip]

> Section 3.19
> Shouldn't this be TimeDomainWasChecked?
> DateDomainWasChecked
>       Zero or one.  DATETIME.  A timestamp of when the domain listed in
>       the Name class was resolved.
> 
> I had to read the description to realize it was a full timestamp, which is good
> as that's what incident handlers need.

I agree that this name isn't quite right.  This class (and its parent DomainData) was borrowed outright from RFC 5901.  To be consistent with the naming of other timestamp classes, we should rename it to "DomainWasCheckedTime", "CheckedTime" or "DateTime".  

Is that a change we want to make?  It is simple.

[snip]

> Security Considerations section:
> 
> First paragraph - please explicitly mention that the data can be both security
> and privacy sensitive here.  Details can come later in a privacy section that
> needs to be added.
> 
> The privacy section should to include options for handling data in sensitive
> fields, such as using role based contact information instead of individuals
> contact info as well as protecting sensitive data with encryption (object level
> and transport MUST - but is covered in a transport).
> 
> Contact info (email, telephone, postalAddress, etc.), IPs, target/source
> information, any identifiers that can be linked to individuals.
> 
> RFC6973 is a great resource and a quick read.

A new Section 9.2, Privacy, was added to address this need.  Does it cover what's needed?

> Nits:
> 
> Section 2.15.1, "a" is repeated twice at the end of the flowing text:
>  spec-name
>       Required.  ENUM.  Identifies the format and semantics of the
>       element body of this class.  Formal standards and specifications
>       can be referenced as well as a free-form text description with a a
> 
> Section 3.10.1
> change describes to described in the last line of the first paragraph:
> From: "to be describes in either free-form or machine readable form."
> To: "to be described in either free-form or machine readable form."
> 
> Section 3.14, 4th line, EvenData should be EventData "recursive definition of
> EvenData allows for the grouping of related"
> 
> Section 5, line 4, change "ad" to "add"
> There is support to ad additional enumerated values and new classes.

All the above nits were fixed.  Thanks for the detailed reading.

> Schema/UML comparison
> 
> There is a mismatch between PostalAddress in the UML and the Schema in
> the Contact class.  The UML has it as 0-1 and the schema has it as 0-
> unbounded
> 
> <xs:element name="Contact">
>       <xs:complexType>
>         <xs:sequence>
>           <xs:element ref="iodef:ContactName"
>                       minOccurs="0" maxOccurs="unbounded"/>
>           <xs:element ref="iodef:ContactTitle"
>                       minOccurs="0" maxOccurs="unbounded"/>
>           <xs:element ref="iodef:Description"
>                       minOccurs="0" maxOccurs="unbounded"/>
>           <xs:element ref="iodef:RegistryHandle"
>                       minOccurs="0" maxOccurs="unbounded"/>
>           <xs:element ref="iodef:PostalAddress"
>                       minOccurs="0" maxOccurs="unbounded"/>

The text and schema agreed, 0..M.  The UML diagram has been updated to be 0..M.

> DomainData Class -
> This isn't an error, just a point of inconsistency.  MaxOccurs value is explicitly
> set here and not in other places since 1 is the default.

Another cut-and-paste artifact from RFC5901.  The schema was been fixed to be consistent.

Again, thanks!
Roman