Subject: [netmod] A review of draft-ietf-netmod-node-tags

From: "Adrian Farrel" <[email-masked]>
Date: Sun, 16 Jan 2022 12:23:27 -0000

Hi,

Thanks for this document which I found clear and easy to read. I think
you have defined a small, but particularly valuable feature.

I have a few comments and a large number of nits (sorry).

Best,
Adrian

== Major ==

There is some contradiction between and within sections 4.3 and 4.4

1. If a user tag is defined as any tag that has the prefix "user:"
   how can you then day that users are not required to use the "user:"
   prefix? That would mean that a user tag is any tag that does or
   does not have the prefix "user:"

2. If any tag not starting with "ietf:", "vendor:", or "user:" is 
   reserved, how can a user create a tag that doesn't start with
   "user:"?

3. Section 4.4 could usefully point at Section 9.1 and say that
   "further tags may be assigned by future specifications".

---

Section 9.1 reasonably uses the "Specification Required" assignment 
policy. But, according to 8126, that policy requires the appointment of
a designated expert. According to section 5.3 of 8126...

   When a designated expert is used, the documentation should give clear
   guidance to the designated expert, laying out criteria for performing
   an evaluation and reasons for rejecting a request.

So you need to add a section to cover this. It can be simple if the rule
is "read the specification, check it is permanent and readily available,
and watch for inappropriate use of language." Or it might be more 
complex if you want the DE to check more stuff.

== Minor ==

The Abstract need to explicitly call out "This document updates RFC 8407
by <doing what?>

The Introduction should repeat that statement and add some detail. In fact
we don't even find a mention of 8407 until Section 8, and even then there
is no hint about what the "update" is. Although, in Section 1 you have

   Section 8 provides guidelines for authors of YANG data models.

That would be the ideal place to describe the update.

---

Figure 1 and its text are a little confusing.

1. It would help to lift the top Object Tag one line so there is a pipe
   ('|') to separate it from Data Object 1

2. The term "have" is not explained. Given the direction of the arrow,
   I think this means "is contained by".

3. The text says "In Figure 1, data objects can contain other data
   objects called subobjects." That's fine, but it would be easier to
   read the figure is data objects 2, 3, and 4 were marked as 
   sub-objects.

4. The text has:

   A data object may contain one single object tag, or one single
   property tag, or one single metric tag.  In many cases, a data object
   only contains one single metric tag.  
   
   That's a bit odd. I mean, the first sentence says, "A, B, or C", and
   the second sentence says "In many cases C". How does the second
   sentence add anything?

5. The text has:
   
   the data object tagged
   with the metric tag also can have one or multiple MetricType tags
   and/or one single multi-source tag.

   These additional tags are not shown on the figure.

---

Section 5 has
   As the main consumer of
   data object tags are users, users may also remove any tag from a live
   server, no matter how the tag became associated with a data object
   within a YANG module.

Suppose there are two users accessing the same YANG data objects on a 
live server. This doesn't seem unreasonable in the case of different
users or monitoring tools reading data about the network or devices.

Doesn't this text lead to "warring tag removal" where one user adds a
tag, and another user removes it?

Maybe this is limited to user tags so that each user may have their 
own tags. But, in this case, it needs to be clearer what a user tag
contains and how it is used. 

It would still be pretty annoying is Benoit added user:benoit to some
data objects, and I went and removed them.

(See also section 5.3)

...Reading the YANG module itself, I wonder whether "add" and "remove"
are ambiguous. Sometimes you may mean adding or removing a tag to/from
a data object. Sometimes you may mean adding or removing to/from a 
filter.

---

9.1

OLD
   Other standards organizations (SDOs) wishing to allocate their own
   set of tags should allocate a prefix from this registry.
NEW
   Other standards organizations (SDOs) wishing to allocate their own
   set of tags should request the allocation of a prefix from this
   registry.
END

---

9.2

   Each YANG data object can have one 'opm' tag, zero or one metric-type
   tag, zero or one multi-source tag.

Is this an instruction for IANA for the management of the registry? I 
don't think it is, and so it should be removed from this section.

---

9.3 and 9.4

These sections include the text "the following registration has been
made:", but it hasn't been (yet). You just need to phrase this text as a
request.

---

Should section 10 talk about the risks associated with an attacker 
adding or removing tags so that a requester gets the wrong data?

Should the section also talk about how the presence of tags may reveal
information about the way in which data objects are used and so
providing access to private information or revealing an attack vector?

== Nits ==

Throughout

You need to choose between "self describing" and "self-describing".
The hyphenated version is probably correct.

---

Abstract

s/input, instruction, indication/input, instructions, indications/
s/selection filter/selection filters/
s/is very large/are very large/

---

1.

s/associated only with a module/associated with a module/

---

1.

   At the time of writing this document (2020), there are many data
   models that have been specified or are being specified by various
   SDOs and the Open Source community.  

How about...

   Many data models have been specified by various SDOs and the Open
   Source community and it is likely that many more will be specified.

---

1.

OLD
   management categories information at different locations in various
   different ways. 
NEW
   management information at different locations and categorised in
   various different ways. 
END

OLD
   Let alone lack consistent classification criteria
   and representation for a specific service, feature, or data source.
NEW
   Further, there is no consistent classification criteria or
   representation for a specific service, feature, or data source.
END

---

1.

s/Indicate relationship/Indicate a relationship/
s/identify characteristics data/identify characteristic data/
s/In addition, it can provide/In addition, they can provide/
s/input, instruction, indication/input, instructions, indications/
s/selection filter/selection filters/

---

1.

   e.g., return specific object type of operational state
   related to system-management.

I think this may be meant to say

   e.g., return specific objects containing operational state
   related to system-management.

---

1.

s/of <get-schema>/of the <get-schema>/

---

1.

   or some place where offline document are kept

I don't think a "place" advertises. Maybe "an application or server 
where offline documents are kept"

---

1.

   This document defines a YANG module [RFC7950] which augments the
   module tag model 

Would another reference to 8819 be appropriate?

---

2.2

   OPM   Object Property Metric

Should that be

   OPM   Object, Property, Metric

---

3.

OLD
   Among data object tags, the 'opm' (object, property, metric) tags can
   be used to tackle massive data objects collection, indicate
   relationship between data objects and only capture YANG data objects
   associated with performance metrics data modelled with YANG
   (Figure 1).
NEW
   Among data object tags, the 'opm' (object, property, metric) tags can
   be used to tackle massive data object collections, indicate
   relationships between data objects, and capture YANG data objects
   associated with performance metrics data modelled with YANG
   (as shown in Figure 1).
END

OLD
         Figure 1: The Relation between Object, Property and Metric
NEW
         Figure 1: The Relation between Object, Property, and Metric
END

---

3.

s/In Figure 2, 'tunnel-/In Figure 2, the 'tunnel-/

---

3.

OLD
   Consider
   'tunnel-svc' data object and tunnel-svc/name data object as an
   example, 'tunnel-svc' data object has one single object tag (i.e.,
   'ietf:object') while tunnel-svc/name data object has one single
   property subobject tag (i.e., 'ietf:property').
NEW
   Consider
   the 'tunnel-svc' data object and the tunnel-svc/name data object as
   an example: the 'tunnel-svc' data object has one single object tag 
   (i.e., 'ietf:object') while the tunnel-svc/name data object has one
   single property subobject tag (i.e., 'ietf:property').
END

OLD
   e.g., only specific category
   such as loss-related metric subobjects need to be tagged with metric-
   type tag which can further reduce amount data to be fetched.
NEW
   e.g., specific categories,
   such as loss-related metric subobjects, need to be tagged with a
   metric-type tag which can further reduce amount data to be fetched.
END

---

5.

s/the need of live server/the need of a live server/
s/main consumer of/main consumers of/

---

3. has "MetricType tags"
5.1 has "Metric type tag"
9.2 has "IETF Metric Type Tags"

---

5.1

s/data objects classification/data object classifications/
s/related to metric subobject/related to a metric subobject/
s/. 'aggregated' multi-source/.  The 'aggregated' multi-source/
s/being combined/to be combined/
s/.  'non-aggregated'/.  The 'non-aggregated'/
s/interface) be/interface) to be/
s/correlate data object/correlate data objects/
s/defined in Section 9/defined in Section 9.2/

---

5.2

s/or vendor specific tags/or vendor tags/

---

7.

Given that the YANG module is extractable and standalone, you should
probably expand "opm" somewhere near the top.

---

7. opm-tag

OLD
          Opm Tag is used to
          classify operation and management data object into object,
          property, and metric three categories.
NEW
          'opm-tag' is used to
          classify operation and management data objects into the
          three categories, object, property, and metric.
END

s/list. Data Object/list. The Data Object/
s/node. Data Object/node. The Data Object/
s/belonging to specific/belonging to a specific/
s/contain One single object tag or/contain one single object tag, or/
s/tag (These/tag (these/
s/As such the/As such, the/

---

7. multi-source-tag 

s/identify  multi-source/identify a multi-source/
s/'aggregated' multi-source/The 'aggregated' multi-source/
s/being combined/to be combined/
s/and report as/and reported as/
s/'non-aggregated' multi-source/The 'non-aggregated' multi-source/
s/interface) be/interface) to be/

---

8.1

There needs to be a mention of Figure 3 in the text.

s/defined below, see Section Section 9.2/defined in Section 9.2/

---

Appendix C

The figure should have a figure number and caption, and should be
referred to by the figure number from the text.

---

Appendix D

I think you want to include a note to the RFC Editor to remove this
appendix before publication.

---

From: Qin Wu <[email-masked]>
Date: Wed, 19 Jan 2022 03:47:30 +0000

Hi, Chris:
Adrian and I talk about user tag management during his review of draft-ietf-netmod-node-tags. One issue raised by Adrian is about whether we will face a situation where one user want to add a user tag while the other user intends to remove such tag.
So the question related to RFC8819 is:
Has RFC8819 missed to talk about the risks associated with an attacker adding or removing tags so that a requester gets the wrong data?
See more details below.

-Qin
>>live
>server. This doesn't seem unreasonable in the case of different users or monitoring tools reading data about the network or devices.
>>
>>Maybe this is limited to user tags so that each user may have their own
>tags. But, in this case, it needs to be clearer what a user tag contains and how it is used. 
>>

> [Qin Wu] Yes, I believe it is limited to user tags, since IETF tags are design time tags, so does implementation tags. It is unlikely to face the situation "warring tag removal".
>But for user tag, your are right, user has its own tags and each user may have different privilege therefore. User with low privilege can not remove the tag owned by high privilege user.
>But I am not sure this is the scope of this document, It seems to me implementation specific and should not in the scope of this document, agree?
> (See also section 5.3)

[AF] Agree about implementation. But maybe, "An implementation MAY include mechanisms to stop users' removing each other's tags or to apply privilege levels to different users."

>>adding
>or removing tags so that a requester gets the wrong data?

> [Qin Wu] User tag is not registered, how user tag is defined and removed is not scope of this document, in my opinion. Take a look at RFC8819, RFC8819 also doesn't flag this as a issue, do you think we should do this?

[AF] Hmmm. Maybe 8819 missed this?
I agree this refers to the previous point.
Actually: 
1. Just go back and check that it is clear that only user tags can be added/removed dynamically 2. Add a note to section 10 to say
   "Note that appropriate privilege and security levels need to be applied to the addition and removal of user tags to ensure that a user receives the correct data."

---

From: Qin Wu <[email-masked]>
Date: Thu, 20 Jan 2022 12:01:44 +0000

Hi, Adrian:
>-----邮件原件-----
>发件人: Adrian Farrel [mailto:[email-masked]] 
>发送时间: 2022年1月19日 22:29
>收件人: Qin Wu <[email-masked]>; [email-masked]
>抄送: [email-masked]
>主题: RE: A review of draft-ietf-netmod-node-tags

>Hi again, Qin.

>Just closing on the one remaining point.

>>>   how can you then say that users are not required to use the "user:"
>> [Qin Wu] Correct.
>> [Qin Wu] :I understand your concern, but my understanding on reserved 
>> tag,
>upon such reserved tag is allocated, it should start with "xxx:",
>>         Therefore such reserved tag can not be seen as user tag. It 
>> seems
>unlikely there is contraction if my understanding is correct.
>>         Correct me if I am wrong.
>>
>> [AF] Ah, I missed the point!

>>A user tag is:
>>- any tag with the prefix "user" 
>>or
>>- any tag that has no prefix at all

>>Thus, and for the avoidance of confusion, the colon (":") when it 
>>appears
>for the first time, is always assumed to be the separator between a prefix and the rest of the tag. And so, when a user tag does not have a prefix, it MUST NOT contain a colon. 
> [Qin Wu] Your understanding is correct, if you think I should add some text to make this more clear, I am happy to do so.

>Maybe just a sentence. I know that my confusion is not indicative of the state of the average reader, but maybe there are others who would be just as confused.

[Qin Wu] Okay, I will add a sentence in section 3.4. Thank for all these good suggestions.