Subject: [Gen-art] Genart last call review of draft-ietf-i2nsf-nsf-monitoring-data-model-12

From: Dale Worley via Datatracker <[email-masked]>
Date: Sun, 28 Nov 2021 15:03:49 -0800

Reviewer: Dale Worley
Review result: Ready with Issues

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

Document:  draft-ietf-i2nsf-nsf-monitoring-data-model-12
Reviewer:  Dale R. Worley
Review Date:  2021-11-28
IETF LC End Date:  2021-12-01
IESG Telechat date:  not known

Summary:

    This draft is on the right track but has open issues, described in
    the review.  It is clear that all of these issues can be fixed
    appropriately, but they need to be fixed before publication.

Major issues:

This document presents a data model for data being passed between
various I2NSF entities.  It appears that the author has a thorough
understanding of the I2NSF architecture and so has made various
references to it in the document.  But since the data model definition
does not depend on the overall architecture, the document should be
revised to either (1) remove unnecessary references to the overall
architecture, (2) segregate them in ways that show they are not needed
to understand the data model, or (3) carefully referenced back to the
documents that define them.

There are also a few points where there seems to be technical issues
regarding the definitions of specific data items.

Details:

1.  Introduction

Why do we have both "administrative entities" and "Security
Controller" here, and "NSF data collector" in section 3?  Naively, I
would expect that in regard to the definition of the data model
presented by the "server side", all "client side" processes would be
considered an amorphous group covered by one generic term.

2.  Terminology

   This document uses the terminology described in [RFC8329].

Given that RFC 8329 doesn't define the terminology, it would be better
to expand on this to "This document uses the terminology described in
[RFC8329], much of which is defined in the I2NSF terminology document
[I2NSF-TERMS]."  Indeed, since I2NSF-TERMS is
draft-ietf-i2nsf-terminology-05, presumably part of the same effort as
this document, why is RFC 8329 being mentioned?

--

There seems to be trouble with terms used in this document.  Some of
them are mentioned in section 2.2 of RFC 8329, which simply refers to
I2NSF-TERMS.  Others (e.g. "I2NSF Record") seem like they should be
listed in RFC 8329, but aren't, and seem to be entirely undefined.
Some of those terms appear in text that may as well be omitted from
this document.  Ideally, the specialized vocabulary in this document
should be listed in this section and proper definitions or references
provided for them.

3.  Use Cases for NSF Monitoring Data

   *  The security administrator with I2NSF User can configure a policy

"I2NSF User" is not listed in RFC 8329.  Also, the placement of "with
I2NSF User" suggests that that phrase is some aspect of "security
administrator", and you might want to say "The I2NSF User that is the
security administrator ...".  OTOH, if "with I2NSF User" is some
aspect of "can configure", it should probably be placed after "can
configure".  (Is an I2NSF User a type of "user", as that word is
normally used?)

4.  Classification of NSF Monitoring Data

   This enables security administrators to assess the state of
   the networks and in a timely fashion.

Likely should delete "and".

   In essence, these types of monitoring data can be leveraged to

Probably can be simplified to "This monitoring data ...".

   As with I2NSF components, every generic system entity can include a
   set of capabilities that creates information about some context with
   monitoring data (i.e., monitoring information), composition,
   configuration, state or behavior of that system entity.

I am sure this could be clarified if it was simplified.  I think the
meaning is "Every system entity creates information about some context
with defined I2NSF monitoring data, and so every entity can be an
I2NSF component."

   This
   information is intended to be provided to other consumers of
   information and in the scope of this document, which deals with NSF
   monitoring data in an automated fashion.

I think this means "This information is can be consumed by other I2NSF
components."

4.1.  Retention and Emission

   I2NSF Event:  I2NSF Event is defined as an important occurrence over
      time,

This should be "an important occurrence at a particular time,".  "over
time" means that there is an extended period of time over which the
event occurs, but I'm sure that I2NSF Events specify only a single
instant for "when it happened".

      Records can be
      continuously processed by a system entity as an I2NSF Producer and

Up until this point, the description of "record" could apply to any
database system.  But I suspect that the intended semantics are that
Records are generated at particular instants (and are unchanging
afterward), and thus a set of records has an ordering in time based on
when they are generated.  This is the fundamental characteristic of a
"log file".  In particular, a database of users does not have this
property but a database of user activities does.  If Record is
intended to be constrained to this situation, that should be stated
explicitly.

   I2NSF Counter:  [...] When an NSF data collector asks for the value
      of a counter to it, a system entity emits

Note this sentence is incomplete in the draft.

It might be valuable to note that an I2NSF Counter can be an integer
approximation of a value that is actually continuous.  (All of the
examples that are given are values that are intrinsically integers.)
Perhaps add as the 3rd sentence "Other examples are integer
approximations to continuous values, such as a processor temperature
measured in tenths of a degree or the percentage of a disk that is
used."

Indeed, the first sentence of this paragraph says "continuous value
changes", despite that all of the examples are integer values that
cannot change continuously.  Perhaps a better phrasing is "a specific
representation of an information element whose value changes very
frequently."

   The retention of I2NSF monitoring information listed in Section 9 may

It seems like "in Section 9" could/should be omitted.

4.2.  Notifications, Events, and Records

   In consequence, an I2NSF Event is specified to
   trigger an I2NSF Policy Rule.  Such an I2NSF Event is defined as any
   important occurrence over time in the system being managed, and/or in
   the environment of the system being managed,

This text provides two definitions of "I2NSF Event" which aren't quite
the same.  One is "anything that triggers an I2NSF Policy Rule", a
purely technical face.  The other is "any important occurrence over
time", which is a human fact.  The two definitions coincide only if
the policy rules exactly cover everything that is "important".  This
needs to be tracked back to the source definition of "I2NSF Event" and
these sentences revised to match it.

   which aligns well with
   the generic definition of Event from [RFC3877].

Strictly, this clause says that "an I2NSF Event" "aligns well with the
generic definition of Event", but I think you mean that the *concept*
of an I2NSF Event aligns etc.

4.3.  Unsolicited Poll and Solicited Push

   Ideally, an I2NSF User is accessing every relevant
   information about the I2NSF Component and is emitting I2NSF Events to
   an NSF data collector (e.g., Security Controller) in a timely manner.

OK, what *is* the model of operations?  In this sentence, it seems
that an "I2NSF User" is a process that accesses (by some method)
information about (in?) an I2NSF Component, and then emits (via I2NSF
Events) that data to an NSF data collector.  But none of that is laid
out in the preceding sections.  Indeed "I2NSF User" is not defined,
though here it doesn't sound like the usual definition of "user".

   The actual mechanism
   implemented by an I2NSF Component is out of the scope of this
   document.

In this sentence, it sounds like the Component is the thing that sends
the data, whereas just above, it is the User.

   In some
   cases, the collection of information has to be conducted via a login
   mechanism provided by a system entity.

What is the use of the terms solicited, unsolicited, poll, and push
here?  Usually, the data source is considered a "server", and the
consumer is a "client".  If the client makes a request to the server,
that is called "solicited" "pulling", and if it happens periodically,
it is called "polling".  Whereas if the server initiates an
interaction to send data, that is called "unsolicited" "pushing".
Terminology in this draft doesn't seem to use those conventions, but
it doesn't tell what conventions it does use.

5.  Basic Information Model for Monitoring Data

   *  vendor-name: The name of the NSF vendor.

Generally, the minimum information needed to identify how to interact
with a device is (1) vendor name, (2) device model name/number, (3)
software version identifier.  Vendor name alone isn't particularly
useful.

6.  Extended Information Model for Monitoring Data

   This section covers the additional information associated with the
   system messages.

What is "system messages"?  The term has not been defined or mentioned
previously.  Is it a special class of "messages"?  Indeed, the term
seems to not be used elsewhere.

   The extended information model is only for the
   structured data such as events, record, and counters.  Any
   unstructured data is specified with the basic information model only.

There has been no previous discussion of "structured"
vs. "unstructured" data.

--

The final sentence of this section suggests that the dampening type
can be set by the user of the monitoring system.  But all occurrences
of dampening-type in the below descriptions say either
"dampening-type: on-repetition" or "dampening-type: none", which
implies that for each type of alarm, only a particular value of
dampening-type is allowed.

Also "dampening-type: none" is invalid (as it is undefined in the
model in section 9) and "dampening-type: no-dampening" is probably
intended.

6.1.1.  Memory Alarm

   *  severity: The severity of the alarm such as critical, high,
      medium, and low.

"such as" implies that there may be other values, whereas section 5
states that there are exactly 4 severities and section 9 agrees.  You
need to decide what the rule is and align all descriptions of
"security" data to that rule.

6.2.2.  Configuration Change

Should there be components of the event that describe what change was
made to the configuration?  The examples for "message" only
distinguish creating a new configuration vs. modifying an existing
configuration, but that information seems to me to be inadequate for
any significant security monitoring.

6.2.3.  Session Table Event

   The following information should be included in a Session
   Table Event:

Is "session table event" a known term of art?

6.2.4.  Traffic Flows

   *  arrival-rate: Arrival rate of packets of the traffic flow.

Most data for "packets per second" have a twin datum for "bytes per
second".  Should there be an "arrival-speed" datum for traffic flows?

6.3.1.  DDoS Detection

   *  attack-type: Any one of SYN flood, ACK flood, SYN-ACK flood, FIN/
      RST flood, TCP Connection flood, UDP flood, ICMP flood, HTTPS
      flood, HTTP flood, DNS query flood, DNS reply flood, SIP flood,
      SSL flood, and NTP amplification flood.

The module definition gives a fixed set of attack-types, but given
that there are 14 described types, it seems likely that additional
types will be defined.  Some extension mechanism needs to be used,
either a catch-all extension type or recogition that users will define
additional types.

   *  end-time: The time stamp indicating when the attack ended.  If the
      attack is still undergoing when sending out the alarm, this field
      can be empty.

The Yang definition seems to make this field mandatory and provide no
null value.  Perhaps making it optional in the model is the best way
of modeling the desired semantics.

6.3.2.  Virus Event

It's not clear whether this event is for when a virus is found within
a packet flow or for when it is found within a host system.  Are there
two different types of virus events for these?  Or does each type use
a subset of the fields of one common event schema?

   *  virus: Type of the virus. e.g., trojan, worm, macro virus type.

It seems like this datum should be named "virus-type".  Also, it seems
unlikely that virus types form a definitive taxonomy, so this field
should be considered less important than "virus-name" (which is likely
to be a key into a database of known viruses).

6.3.3.  Intrusion Event

   *  event-name: The name of the event. e.g., detection-intrusion.

Why is there not a single, definitive event-name value for intrusion
events?  Or was "i.e." meant rather than "e.g."?

   *  raw-info: The information describing the flow triggering the
      event.

Given there are 8 defined fields that describe the flow, what
additional information can raw-info contain?  Also, the semantics of
this raw-info is different from that in 6.3.2.

Indeed, there is some disalignment in the description of this field:

In section 6, raw-info is listed for:
    6.3.2 Virus Event
    6.3.3 Intrustion event

In section 8, raw-info is listed for:
    i2nsf-nsf-detection-ddos
    i2nsf-nsf-detection-virus
    i2nsf-nsf-detection-intrusion
    i2nsf-nsf-detection-web-attack
    i2nsf-nsf-detection-voip-volte

In the model in section 9, raw-info is listed as a component of:
    i2nsf-nsf-detection-ddos
    grouping i2nsf-nsf-event-type-content, which is used in
        i2nsf-nsf-detection-virus
        i2nsf-nsf-detection-intrusion
        i2nsf-nsf-detection-web-attack
	i2nsf-nsf-detection-voip-volte

Only in section is raw-info described as "describing the flow
triggering the event".

6.3.4.  Web Attack Event

   *  event-name: The name of event. e.g., detection-web-attack.

Why is there not a single, definitive event-name value for intrusion
events?  Or was "i.e." meant rather than "e.g."?

   *  cookies: The HTTP Set-Cookie header field of the response.

I would think the cookies header in the request would be of more
interest than the cookies header in the response.

6.3.5.  VoIP/VoLTE Event

This event type has no event-type field.  Is that correct?

6.4.3.  User Activity Log

6.4.1 and 6.4.3 are only weakly aligned with each other, despite that
they describe login and activities of two types of users (administrators
and ordinary users).  Should these types be unified, or at least their
fields compared to better align them?

6.7.2.  Policy Hit Counter

   *  hit-times: The hit times that the security policy matches the
      specified traffic.

Given the Yang definition, I think the wording you want here is "The
number of times that the security policy ...".

7.  NSF Monitoring Management in I2NSF

It's not clear to me that any of this is needed for the definition of
the data model.  It seems more to be a higher-level description of the
entire I2NSF system, but the details of the data model aren't directly
relevant to the higher-level description (as long as the data model
provides the required fields) and that the data model isn't directly
affected by the higher-level I2NSF system.

9.  YANG Data Model

        The key words 'MUST', 'MUST NOT', 'REQUIRED', 'SHALL',
        'SHALL NOT', 'SHOULD', 'SHOULD NOT', 'RECOMMENDED',
        'NOT RECOMMENDED', 'MAY', and 'OPTIONAL' in this
        document are to be interpreted as described in BCP 14
        (RFC 2119) (RFC 8174) when, and only when, they appear
        in all capitals, as shown here.

This module contains the full RFC 8174 text, but the only use of it is
the instance of MUST in nsf-name.

10.  I2NSF Event Stream

   The following example

It seems like this should start a new paragraph.  The preceding text
is an overview of the event stream, but the following text is a single
example.  The example is not what I would expect; it is not an event.
I think this text would better state the purpose:

   The following example XML shows the capabilities of the event
   streams generated by an NSF (e.g., "NETCONF" and "I2NSF-Monitoring"
   event streams) for the subscription of an NSF data collector.  The
   XML examples in this document ...

--
             <replayLogCreationTime>
               2021-04-29T09:37:39+00:00
             </replayLogCreationTime>

It's not clear to me "2021-04-29T09:37:39+00:00" is a value that is
applicable to all NSF event streams.

15.  Contributors

   Chaehong Chung Department of Electronic, Electrical and Computer
   Engineering Sungkyunkwan University 2066 Seo-ro Jangan-gu Suwon,
   Gyeonggi-do 16419 Republic of Korea EMail: [email-masked]

   [and others]

For clarity, between the name and the affiliation should be a comma or
dash.

[END]

---

From: t petch <[email-masked]>
Date: Mon, 29 Nov 2021 10:33:41 +0000

> like any other last call comments.

This I-D is one of six closely related documents with much overlap 
between them; with hindsight, the strucure might have been different 
with a 'common' I-D covering more of what is common, information model, 
data model and text.

The trouble with the way that Last Call is organised is that the changes 
suggested below will pull this I-D out of line with the others 
potentially leading to contradiction and confusion.  I tend to see 
'capability' as the starting point from which the others -'nsf-facing', 
'consumer-facing', 'nsf-monitoring', 'registration' - are derived.  I 
would like 'capability' to receive the same review as this and then the 
changes to 'capabiity' can be carried across to the others.

I have a vested interest in this in that I put a lot of effort into 
making the set coherent earlier in the year so 'improving' 
'nsf-monitoring' without similar changes to the others will invalidate 
that work.

Tom Petch

> _______________________________________________
> I2nsf mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/i2nsf
> .
>

---

From: "Mr. Jaehoon Paul Jeong" <[email-masked]>
Date: Mon, 29 Nov 2021 22:09:25 +0900

Hi Dale,
I will address your comments while this draft is well-synchronized with
other I2NSF YANG data model drafts.

Thanks.

Best Regards,
Paul

-- 
===========================
Mr. Jaehoon (Paul) Jeong, Ph.D.
Associate Professor
Department Head
Department of Computer Science and Engineering
Sungkyunkwan University
Office: +82-31-299-4957
Email: [email-masked], [email-masked]
Personal Homepage: http://iotlab.skku.edu/people-jaehoon-jeong.php
<http://cpslab.skku.edu/people-jaehoon-jeong.php>

---

From: "Mr. Jaehoon Paul Jeong" <[email-masked]>
Date: Mon, 29 Nov 2021 22:17:35 +0900

Hi Tom,
I will fully consider your concerns on the revision of this monitoring data
model draft based on Dale's
comments.

As long as I know, this monitoring data model draft is less coupled with
the capability data model draft than
the other data model drafts such as the nsf-facing interface,
consumer-facing interface, and registration
interface data model drafts.

Thanks.

Best Regards,
Paul