From: "Takeshi Takahashi" <takeshi_takahashi@nict.go.jp>
To: "'Robert Sparks'" <rjsparks@nostrum.com>, <gen-art@ietf.org>
Cc: <mile@ietf.org>, <ietf@ietf.org>, <draft-ietf-mile-jsoniodef.all@ietf.org>
References: <156328777358.31683.16943185111751565258@ietfa.amsl.com>
In-Reply-To: <156328777358.31683.16943185111751565258@ietfa.amsl.com>
Date: Sun, 21 Jul 2019 01:26:53 +0900
Message-ID: <00f301d53f17$f279e490$d76dadb0$@nict.go.jp>
MIME-Version: 1.0
Content-Type: text/plain;
	charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Thread-Index: AQCZKsLxGTNqLQwvBZckzaIjcX+9VqlL2F8g
Content-Language: ja
Archived-At: <https://mailarchive.ietf.org/arch/msg/gen-art/7fV-kKBPerIu_SuFD9fU7izezlw>
Subject: Re: [Gen-art] Genart last call review of draft-ietf-mile-jsoniodef-09
Precedence: list

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:
>=20
> * 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)?=20

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:
>=20
> * 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 <noreply@ietf.org>=20
Sent: Tuesday, July 16, 2019 11:36 PM
To: gen-art@ietf.org
Cc: mile@ietf.org; ietf@ietf.org; draft-ietf-mile-jsoniodef.all@ietf.org
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)?=20

* 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).