Subject: [sidr] Adam Roach's Discuss on draft-ietf-sidr-slurm-07: (with DISCUSS and COMMENT)

From: Adam Roach <[email-masked]>
Date: Wed, 04 Apr 2018 12:22:45 -0700

Adam Roach has entered the following ballot position for
draft-ietf-sidr-slurm-07: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-sidr-slurm/

----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

Thanks to everyone who worked on this document. The mechanism seems useful.

I'm concerned that the document doesn't describe the file format itself;
rather, it relies on examples to provide vital, nonsupplemental information
such as the names of JSON object members, expected encodings (e.g., strings
versus numbers), and distinction between arrays and objects. I'm making this a
DISCUSS because I think the ambiguity here -- and, in particular the ambiguity
about IP address prefix notation -- will lead to non-interoperable
implementations.

Using section §3.2 as an example:

>   A SLURM file consists of:
>
>   o  A SLURM Version indication that MUST be 1
>
>   o  A slurmTarget element (Section 3.3) consisting of:
>
>      *  Zero or more target elements.  In this version of SLURM, there
>         are two types of values for the target: ASN or Fully Qualified
>         Domain Name(FQDN).  If more than one target line is present,
>         all targets MUST be acceptable to the RP.
>
>   o  Validation Output Filters (Section 3.4), consisting of:
>
>      *  An array of zero or more Prefix Filters, described in
>         Section 3.4.1
>
>      *  An array of zero or more BGPsec Filters, described in
>         Section 3.4.2
>
>   o  Locally Added Assertions (Section 3.5), consisting of:
>
>      *  An array of zero or more Prefix Assertions, described in
>         Section 3.5.1
>
>      *  An array of zero or more BGPsec Assertions, described in
>         Section 3.5.2
>

As this is the normative description of the structure, I would have expected an
indication that the file contains a JSON object (rather than, say, a JSON
array), an indication that the version is to be encoded as a number (rather than
a string), and clarification of what value members are expected to contain.

For example, the following JSON object is in compliance with the preceding
normative description (and, as far as I can tell, all other normative text
in the document):

["1",
  ["65536", "rpki.example.com"],
  [
    ["192.0.2.0/255.255.255.0", "All VRPs encompassed by prefix"],
    ["64496", "All VPRs maching ASN"],
    ["198.51.100.0/255.255.255.0", "64497", "All VRPs encompassed by prefix,
      matching ASN"]
  ],
  [
    ["64496", "All keys for ASN"],
    ["Zm9v", "Key matching Router SKI"],
    ["64497", "YmFy", "Key for ASN 64497 matching Router SKI"],
  ],
  [
    ["64496", "198.51.100.0/255.255.255.0", "My other important route"],
    ["64496", "2001:DB8::/FFFF:FFFF::", "48",
     "My other important de-aggregated routes"],
  ],
  [
    ["64496", "My known key for my important ASN",
     "<some base64 SKI>", "<some base64 public key>"]
  ]
]

Fixing this should be pretty easy; the document simply needs text added that
describes the JSON structure explicitly, with clear indications of how values
are to be encoded. For example, the preceding text I quote becomes:

   A SLURM file consists of a single JSON object containing the following
   members:

   o  A  "slurmVersion" member that MUST be set to 1, encoded as a number

   o  A "slurmTarget" member (Section 3.3) If more than one target line is
      present, all targets MUST be acceptable to the RP. The "slurmTarget"
      member is encoded as an array of zero or more objects. Each object in the
      array contains exactly one member.  In this version of SLURM, the member
      may be named either:

      * "asn", in which case it contains an ASN, or

      * "hostname", in which case it contains a Fully Qualified Domain
         Name (FQDN).

   o  A "validationOutputFilters" member (Section 3.4), whose value is an
      object. The object MUST contain exactly two members:

      *  A "prefixFilters" member, whose value is described in
         Section 3.4.1

      *  A "bgpsecFilters" member, whose value is described in
         Section 3.4.2

   o  A "locallyAddedAssertions" member (Section 3.5), whose value is an
      object. The object MUST contain exactly two members:

      *  A "prefixAssertions" member, whose value is described in
         Section 3.5.1

      *  A "bgpsecAssertions" member, whose value is described in
         Section 3.5.2

Gotchas to watch out for include:

 - If you're using the word "element" to describe something in a JSON object,
   you probably need to find a more specific word. This document, for example,
   uses "element" instead of "member" in most places.

 - Everywhere you use the word "structure," replace it with either "array" or
   "object," as appropriate.

 - When values can be encoded as either a number or a string (e.g., as with
   "slurmVersion" above, or with AS numbers), indicate which encoding is
   expected.

 - For IP prefixes, be clear about acceptable syntax. For example: is
   the RFC 950 syntax ("192.0.2.0/255.255.255.0") acceptable? My suggestion is
   to cite RFC 4632 §3.1 for prefix-length notation (both for IPv4 and IPv6),
   and RFC 5952 for the syntax of IPv6 addresses.

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

The remaining comments are in document order.

---------------------------------------------------------------------------

Title:

It seems odd to use the stylized capitalization (e.g., "nUmber") without
following it by the "SLURM" acronym. Consider adding "(SLURM)" to the title.

---------------------------------------------------------------------------

§3.1:

>  This document describes responses in the JSON [RFC8259] format.

I don't think this means to say "responses," does it? It appears to be
describing a JSON document rather than a request/response protocol.

---------------------------------------------------------------------------

§3.3:

>  A SLURM file MUST specify a "slurmTarget" element that identifies the
>  environment in which the SLURM file is intended to be used.  The
>  "slurmTarget" element MAY have an empty array as its value, which
>  means "applies to all".  The meaning of the "slurmTarget" element, if
>  present, is determined by the user.  If a "slurmTarget" element is
>  present, an RP SHOULD verify that the target is an acceptable value,
>  and reject this SLURM file if the "slurmTarget" element is not
>  acceptable.  Each "slurmTarget" element contains merely one "asn" or
>  one "hostname".  An explanatory "comment" MAY be included in each
>  "slurmTarget" element so that it can be shown to users of the RP
>  software.

When reworking this paragraph in particular, please be careful to distinguish
between the "slurmTarget" member and the elements in the array that constitutes
its value. The preceding text calls both of these things '"slurmTarget"
element,' which is very confusing.

---

From: Adam Roach <[email-masked]>
Date: Wed, 4 Apr 2018 14:33:06 -0500

I realized after I sent this that my suggested text has a couple of 
errors in t.

>        array contains exactly one member.  In this version of SLURM, the member

I copied the "target line" language over without reading it carefully. I 
don't think "line" makes sense here. Perhaps:

    o  A "slurmTarget" member (Section 3.3). The "slurmTarget" member is 
encoded
       as an array of zero or more objects, each representing a target.  If
       more than one target is present, all targets MUST be acceptable 
to the
       RP.  Each object in the array contains exactly one member.  In this

>        * "asn", in which case it contains an ASN, or

This should should say "...an ASN, encoded as a number."

---

From: Tim Bruijnzeels <[email-masked]>
Date: Thu, 5 Apr 2018 12:38:05 +0200

Dear Adam, all,

Thank you for this feedback - indeed we struggled a bit with formally specifying JSON and relied on examples. I believe that with your suggestions we can improve this.

As for IP address prefix notation - yes.. we should follow your suggestion and cite RFC 4632 §3.1 for prefix-length notation (both for IPv4 and IPv6), and RFC 5952 for the syntax of IPv6 addresses. I am so used to doing it this way that it slipped my mind to specify this, but of course it should be unambiguous.

As I did most of the JSON text I will take it on me to re-work this text and ask Di to merge it with the changes he is working on. There should be a -08 version coming soon.

Tim

> On 4 Apr 2018, at 21:22, Adam Roach <[email-masked]> wrote:
> 
> _______________________________________________
> sidr mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/sidr

---

From: Adam Roach <[email-masked]>
Date: Thu, 5 Apr 2018 08:14:00 -0500

Thanks for your quick response! Feel free to reach out to me directly if you find any particular part of the structure challenging to describe. 

/a

> On Apr 5, 2018, at 05:38, Tim Bruijnzeels <[email-masked]> wrote:
> 
> Dear Adam, all,
>

---

From: Di Ma <[email-masked]>
Date: Fri, 27 Apr 2018 10:59:12 +0800

Adam,

The version -08 of SLURM draft has just been submitted after we authors exchanged notes on JSON work in this document.

In addition to some editorial changes based on other ADs’ suggestion, we authors mainly improved the description of the specifications of the SLURM file with JSON format.

According to your suggestion, we have specified the syntax of SLURM file and normalized the usage of ‘object’ and ‘member’ and so on, not merely relying on the instances we provided in this document.

Thanks to Tim’s efforts on JSON work, we believe this version is much more clear and easier to comprehend.

Thanks again to your review and comments.

Di

> 在 2018年4月5日，21:14，Adam Roach <[email-masked]> 写道：
>