Re: [mile] Genart last call review of draft-ietf-mile-jsoniodef-09

["Takeshi Takahashi" <[e-mail]>]

Hi Robert,

Thank you very much for your kind feedbacks.
All comments are very much appreciated.
The revised version of the draft is available at: https://github.com/milewg/draft-ietf-mile-jsoniodef
Below are the replies to each of your comments.

> Minor Issues:
> 
> * Section 2.2.5 says "base64 ... should be used for encoding". Why is this (a
>   lower case) should? What happens if something doesn't base64 encode embedded
>   raw data?

Changed to "SHOULD." Thanks.

> * In the table in section 3.1 (and I suspect the corresponding cddl and json
>   schema, but I haven't verified that), in the block for 'Incident',
>   'Indictator' is marked with a *. Looking at RFC7970, I think it should be
>   marked with a ?. (0..1 instead of 0..*).

Thanks for the careful review. We appreciate this comment very much.
Indeed, this was intentionally changed so.
As the 5th bullet in Section 3.2 says, "IndicatorData class is deleted, and classes with its instances now directly have the instances of Indicator class that used to belong to the IndicatorData class."

> * There are places in the prose of RFC7970 that add restrictions to conformant
>   XML documents that aren't captured in the schema. See "An instance of one of
>   these children MUST be present." in section 3.11 of that document for example.
>   That constraint is not present in this document (or did I miss it)? 

The sentence in RFC7970 could be misleading (but the sentence in RFC7970 is correct).
"An instance of one of these children MUST be present" (in section 3.11) means that at least one of "Reference", "Description", "sci:AttackPattern", "sci:Vulnerability", "sci:Weakness", "AdditionalData" classes need to be presented, and it does not mean either "restriction" or "ext-restriction" needs to be presented.

> * In section 3.2, the 7th bullet is likely an artifact of an earlier idea that
>   was replaced by the one in the 8th bullet. I suggest deleting the 7th bullet
>   (The one that says "Record class is replaced by RecordData class, and
>   RecordData class is renamed to Record class."

Removed. Thanks.

> Nits:
> 
> * I suggest removing 'abstract' from 'abstract IODEF JSON' in section 2.

Changed. Thanks.

> * Section 3.2 uses "CBOR/JSON IODEF" in some places (see the title and several
>   bullets). The Introduction introduces this as just "JSON IODEF". I suggest
>   removing the "CBOR/" wherever it occurs.

All of "CBOR/" were removed or changed. Thanks.

> * The example in Figure 6 is broken at the end. There is a BulkObservable list
>   that says it is supposed to consist of ipv6-addr objects, but the list
>   consists of one e-mail object.  (Same happens in Figure 7 of course).

Thank you for pointing this out.
The type is now changed to "domain-name."

Once again, thank you very much for your reviews.

Kind regards,
Take


-----Original Message-----
From: Robert Sparks via Datatracker <[e-mail]> 
Sent: Tuesday, July 16, 2019 11:36 PM
To: [e-mail]
Cc: [e-mail]; [e-mail]; [e-mail]
Subject: Genart last call review of draft-ietf-mile-jsoniodef-09

Reviewer: Robert Sparks
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-mile-jsoniodef-09
Reviewer: Robert Sparks
Review Date: 2019-07-16
IETF LC End Date: 2019-08-01
IESG Telechat date: Not scheduled for a telechat

Summary: Almost ready for publication as a proposed standard RFC, but with minor issues and nits to address before publication

I skimmed the various formal definitions. I have not verified they are complete or correct. If someone in the working group hasn't found a tool to do that, then the chairs should wrangle some intense volunteer labor to make sure these things are what you intend them to be.

Minor Issues:

* Section 2.2.5 says "base64 ... should be used for encoding". Why is this (a
  lower case) should? What happens if something doesn't base64 encode embedded
  raw data?

* In the table in section 3.1 (and I suspect the corresponding cddl and json
  schema, but I haven't verified that), in the block for 'Incident',
  'Indictator' is marked with a *. Looking at RFC7970, I think it should be
  marked with a ?. (0..1 instead of 0..*).

* There are places in the prose of RFC7970 that add restrictions to conformant
  XML documents that aren't captured in the schema. See "An instance of one of
  these children MUST be present." in section 3.11 of that document for example.
  That constraint is not present in this document (or did I miss it)? 

* In section 3.2, the 7th bullet is likely an artifact of an earlier idea that
  was replaced by the one in the 8th bullet. I suggest deleting the 7th bullet
  (The one that says "Record class is replaced by RecordData class, and
  RecordData class is renamed to Record class."

Nits:

* I suggest removing 'abstract' from 'abstract IODEF JSON' in section 2.

* Section 3.2 uses "CBOR/JSON IODEF" in some places (see the title and several
  bullets). The Introduction introduces this as just "JSON IODEF". I suggest
  removing the "CBOR/" wherever it occurs.

* The example in Figure 6 is broken at the end. There is a BulkObservable list
  that says it is supposed to consist of ipv6-addr objects, but the list
  consists of one e-mail object.  (Same happens in Figure 7 of course).