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

[Adam Roach <[e-mail]>]

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.