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

[Adam Roach <[e-mail]>]

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 <[e-mail]> wrote:
> 
> 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 <[e-mail]> wrote:
>> 
>> 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.
>> 
>> 
>> _______________________________________________
>> sidr mailing list
>> [e-mail]
>> https://www.ietf.org/mailman/listinfo/sidr
>