Re: [Last-Call] [I2nsf] Genart last call review of draft-ietf-i2nsf-nsf-monitoring-data-model-12

[t petch <[e-mail]>]

On 28/11/2021 23:03, Dale Worley via Datatracker wrote:
> 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.

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


> 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: [e-mail]
>
>     [and others]
>
> For clarity, between the name and the affiliation should be a comma or
> dash.
>
> [END]
>
>
>
> _______________________________________________
> I2nsf mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/i2nsf
> .
>