Re: [core] Benjamin Kaduk's Discuss on draft-ietf-core-sid-16: (with DISCUSS and COMMENT)

[Carsten Bormann <[e-mail]>]

Hi Ben,

here is our response, but to the 2021-10-16 updated version in the datatracker (thanks for this).
We have prepared an updated core-sid document at:

https://datatracker.ietf.org/doc/draft-ietf-core-sid/
https://www.ietf.org/archive/id/draft-ietf-core-sid-18.html
https://www.ietf.org/rfcdiff?url2=draft-ietf-core-sid-18.txt

> Discuss (2021-10-26)
> (1) I think there is a new security consideration with this work that is
> important to document clearly -- not only do we define a new type of
> identifier, but we define a file format and other mechanisms for
> disseminating that information.  An entity that's processing
> application/yang-data+cbor; id=sid information needs to ensure that the
> .sid files (or other source of SID information) it uses for such
> processing came from a trustworthy authority (or at least the same
> source as the data file).  It would be possible for malicious
> manipulation of .sid file contents to cause a message recipient to
> mis-interpret the received message without any indication of such
> tampering.
> [ed. there seems to be some proposed text in
> 
> https://mailarchive.ietf.org/arch/msg/core/JS0uD9aUNwim_fwhBpGnZut9kns/
> ]

Indeed.  We have slightly improved on the text in that discussion; 
https://github.com/core-wg/yang-cbor/pull/102

> (2) Per §7.4.2, YANG SID range registries with public ranges MUST
> include a reference to the ".sid" file for such ranges, but the
> IANA-managed YANG SID range registry established by §7.5 does not, in
> and of itself, make such a provision.  This function seems to be served
> by the "IETF YANG SIG Registry" created by §7.6, so we may just need to
> point to the one registry from the other in order to remain internally
> consistent.

Right, we added more explanatory text and a pointer.
https://github.com/core-wg/yang-cbor/pull/99

> (3) There may be another inconsistency to look into; Section 7.6.2 says
> that:
> 
>   *  If another ".sid" file has already allocated SIDs for this YANG
>      module (e.g. for older or newer versions of the YANG module), the
>      YANG items are assigned the same SIDs as in the other ".sid" file.
> 
> But we are supposed to allocate a new SID for a YANG item if its
> semantics change in a revision of the YANG module.  Perhaps it's just
> the "for older or newer versions of the YANG module" phrase that needs
> tweaking?

We toned down the need for allocating a new SID quite a bit; see updated appendix B.
The point of the text here is to say that existing allocations stay in place; the exceptions listed in Appendix B notwithstanding.

> Comment (2021-10-26)
> 
> Would RFC 8792 folding be suitable for Appendix A rather than using
> a custom scheme/disclaimer?

We don’t think so.  The appendix is an illustration, not something one would extract as sourcecode to work on.
The line-breaking scheme we used is noiseless; you cannot say this of RFC 8792.
We added a script that removes the added newlines and indentation in https://github.com/core-wg/yang-cbor/pull/116

> I see that most mention of what to do with SID assignments if the
> semantics of a node (name) changes, or a node name changes without
> changing semantics, has been moved to Appendix B.  I'm not sure if
> it's helpful to retain some mention that this discussion exists,
> in the main body text (e.g., Section 1).

Section 1 now points there and mentions manual interventions.

https://github.com/core-wg/yang-cbor/pull/97

> [The rest of this COMMENT section populated by taking the comment section from the -16
> and removing a small handful of things that are clearly addressed.
> Some items that are retained may have been addressed as well and no
> longer applied]
> 
> The yangdoctors review mentioned the structure extension from RFC 8791,
> and the authors (understandably) expressed reluctance to make such a
> large change at this stage in the process.  I'll note that the DOTS WG
> has recently completed work on a "bis" of RFC 8782, with primary aim of
> replacing yang-data with structures and minimal other changes.  (A
> yangdoctors review of a related document was sufficiently insistent that
> structures are the right way to do this that the WG was convinced to put
> in the effort.)

We changed the ietf-sid-files module to use sx:structure.
https://github.com/core-wg/yang-cbor/pull/116
(I don’t think we can validate the sid file example any more after that change.
Can we?)

So this is about the documents’s *use* of rc:yang—data migrating to sx:structure.

As far as showing sx:structure examples with encoding from YANG-CBOR:
We plan to set up another document with that, metadata, etc.

> The yangdoctors review also asked why sid-file/module-name is optional,
> which was tagged for follow-up.  I have the same question and am
> interested in the outcome of the discussion amongst the authors.

The authors agree that it needs to be mandatory.
Also in https://github.com/core-wg/yang-cbor/pull/116

> If SIDs are supposed to be globally unique identifiers, the reference in
> the YANG module to "namespace" identifiers for individual allocations is
> puzzling to me.  The presence of a namespace would typically allow for
> identifier reuse across namespaces (e.g., using the same SID integer value
> for an identity and a data node within the same module).  I see how the
> current list structure makes it easy to have a single flat list of all
> identifier/sid mappings, but if the SIDs truly are globally unique, then
> the "namespace" should not be a list key, and might be less confusingly
> described as just indicating the type for which the SID is used.  (It
> would also be possible to have separate lists for module SIDs, identity
> SIDs, etc., but it's not clear that doing so is actually the right
> approach.)

We apologize that we are as confused about namespaces as Section 6.2.1 of RFC 7950 is…
SIDs are needed for some of the names in these (Section 6.2.1)-namespaces, and it is useful to identify which namespace is meant in the SID file.  We can leave out some namespaces such as the ones for grouping names, as these are not directly encoded.
Since the same name can be used in multiple namespaces, we need multiple entries in the list “item” for the same “identifier”, and so we need to include the namespace in the key (which also gives us a check that an identifier is not assigned a SID twice in the same namespace).  The (local) uniqueness of the SID is separately checked by the “unique “sid”” statement.

> Section 4
> 
> If the scope of a sid-file-version-identifier resets when the module
> revision is bumped, it seems highly unlikely that we'll need 64 bits of
> identifier for it.

Fixed (now uint32) in https://github.com/core-wg/yang-cbor/pull/84

> Section 5, 7.3
> 
> It's not entirely clear that we need to mention the
> content-type/content-format in this document, as we only indicate use of
> the JSON encoding for the .sid file and the use of SIDs in the
> application/yang-data+cbor; id=sid case seems adequately described by
> the existing treatment in draft-ietf-core-yang-cbor.

This was meant more as a cross-reference in case one wonders where to find the IANA registration, but IANA doesn’t need that, so it should not be in the IANA Considerations section.  We removed it.
https://github.com/core-wg/yang-cbor/pull/117

> Section 7.4.1
> 
> I appreciate that the "mega-range" allocations are sized for the
> appropriate SI prefix :)

We discussed decimal vs. binary a lot and decided that, since the registry structure is mainly for humans to look at, we’d stick with decimal ranges.
There appears to be very little coding efficiency that could be gained by going to binary range boundaries.

>   The information associated to the Organization name should not be
>   publicly visible in the registry, but should be available.  This
>   information includes contact email and phone number and change
>   controller email and phone number.
> 
> Available to whom?

We removed this overspecification in https://github.com/core-wg/yang-cbor/pull/98

> Section 7.5.2
> 
>   In case a SID range is required before publishing the RFC due to
>   implementations needing stable SID values, early allocation as
>   defined in [BCP100] can be employed.  As specified in Section 4.6 of
>   [RFC8126], RFCs and by extension documents that are expected to
>   become an RFC fulfill the requirement for "Specification Required"
>   stated in Section 2 of [BCP100], which allows for the early
>   allocation process to be employed.
> 
> While the first bit of this is all true, the registry here doesn't use
> the "Specification Required" policy for any range, and early allocations
> are available for the "RFC Required" range.  So we should probably tweak
> or remove the second sentence.

Indeed, this should have been "RFC Required”.
Addressed in https://github.com/core-wg/yang-cbor/pull/103

> Section 7.6.3
> 
>   Early Allocations are made with a one-year period, after which they
>   are expired.  [BCP100] indicates that at most one renewal may be
>   made.  For the SID allocation a far more lenient stance is desired.
> 
>   1.  An extension of a referencing documents Early Allocation should
>       update any referenced Early Allocations to expire no sooner than
>       the referencing document.
> 
>   2.  The [BCP100] mechanism allows the IESG to provide a second
>       renewal, and such an event may prompt some thought about how the
>       collection of documents are being processed.
> 
> The first point is rather curious, since it can have no normative force
> (this is not a BCP that would override BCP 100) and is merely a
> continnation of how a more lenient stance is desired.  It's rather odd
> to see it written in this way.  The second point seems to be the only
> really useful part, and in practice this IESG approval of second (and
> more!) renewals has become rather common, to the extent where a revision
> of BCP 100 has been discussed to change the discussion of extensions for
> early renewals.
> 
>   This is driven by the very generous size of the SID space and the
>   often complex and deep dependencies of YANG modules.  Often a core
>   module with many dependencies will undergo extensive review, delaying
>   the publication of other documents.
> 
> Having many *dependencies* and taking a while shouldn't delay
> publication of anything else; however, having many other modules
> dependent on a core module would be likely to do so.

This is a useful discussion, but it is not quite clear to the authors what we should change.  Clearly, this is an uncovered check on the future of BCP100, but it is not clear what we should do now.  (Of course, we could change "RFC required" into Expert review and instruct the Expert to act exactly as defined above, but I’m not sure IANA would be happy about Expert-generated provisional registrations.)

> Appendix A
> 
> IIRC the mismatch between "[e-mail]" and the toplevel
> "module-revision" of "2020-02-05" was noted by an earlier reviewer, but
> I'll mention it here just in case I do not remember correctly.

This (together with the module-revision of ietf-sid-file itself) is now updated.
A task to make a final pass through all the version numbers when all the dust has settled is at https://github.com/core-wg/yang-cbor/issues/66

Grüße, Carsten