On 28/08/2020 14:46, Mr. Jaehoon Paul Jeong wrote:
Hi Tom,
I have addressed all your comments in the following revision:
https://tools.ietf.org/html/draft-ietf-i2nsf-consumer-facing-interface-dm-10
Inline
Here are my answers for your comments:
------------------------------------------------------------------------------------
Some more minor tweaks
s.5.1 /gorup/group/
=> The replacement is done.
YANG module
WG Chairs are not usually listed in the module - they used to be
=> The information of WG Chairs is removed.
description is a bit terse - some quote the Abstract
=> I have improved the descriptions in the YANG module.
YARA, SNORT, SURICATA would benefit from references; they are not ones I
see in TLS or SSH!
=> I have added the references to YARA, SNORT, and SURICATA.
typedef time I see in RFC6991bis
=> I used typedef time in RFC6991bis.
See my other note about importing from 6991-bis rather than from 6991
does the ipv6 addresss ever need the interface?
=> Yes, the IPv6 address needs the CFI interface.
I added an XML example using IPv6 addresses.
start/end ipv4/ipv6 could do with a must end > start
=> I put a description that an IPv4/IPv6 start address is lower than
an IPv4/IPv6 end address.
"A range match for IPv4 addresses is provided. Note that the
start IPv4 address must be lower than the end IPv4 address.";
"A range match for IPv6 addresses is provided. Note that the
start IPv6 address must be lower than the end IPv4 address.";
geo-ip could do with a reference
=> I added a reference to geo-ip as follows.
RFC8805: A Format for Self-Published IP Geolocation Feeds
s.9.1 221.159 is not a documentation address - see RFC5737
=> I used documentation addresses for IPv4 from RFC5737.
I also used documentation addresses for IPv6 from RFC3849.
good
IESG often expect an ipv6 example alongside ipv4
=> I added an XML example using IPv6 addresses in Figure 19.
s.12 Registrant should be IESG
=> I modified the IANA considerations section such that Registrant is the
IESG.
prefix is not that of the module
=> I am not sure of this comment. I think we use the correct prefix of
"cfi-policy".
CFI stands for Consumer-Facing Interface.
Yes indeed you do - my mistake. What I had intended to say, looking at
other NSF modules, was that there are a number of NSF modules and the
chosen prefix have nothing in common. Bear in mind that all the YANG
modules get mixed up together on the box so while the prefix need to be
compact, there is something to be said for them to be meaningful so
RTGWG modules could start rt... or MPLS ones mpls... or PCE ones pce..
and so on so you could consider using a prefix of nsf... such as nsfcfi
or if there are several such nsfcfi-p or some such (but that is getting
a bit long)
Tom Petch
------------------------------------------------------------------------------------
Thanks for your valuable comments.
Best Regards,
Paul
On Fri, Jul 17, 2020 at 8:07 PM tom petch <[email protected]> wrote:
On 17/07/2020 10:51, tom petch wrote:
On 11/07/2020 08:44, Mr. Jaehoon Paul Jeong wrote:
Hi Jan and Tom,
I have revised our I2NSF Consumer-Facing Interface (CFI) Data Model
Draft
according to both your comments.
Jan,
I attach the revised draft and the revision letter to explain how I have
reflected your comments one by one.
Tom,
the references to RFC inside our YANG module cannot be cited in my I-D
XML
file, so I cannot include them
in Normative References.
Yes you can and in fact you must:-)
You can put anything you want to in Normative References with a
corresponding [RFC0913] in the text part of the I-D so you add Section
8.1 "This YANG module imports from [RFC6991], ... and makes reference to
[RFC0854], [RFC0913], ......"
Note that all import must have a reference clause and the referenced
work must appear in Normative References; same technique applies.
Some more minor tweaks
s.5.1 /gorup/group/
YANG module
WG Chairs are not usually listed in the module - they used to be
descripton is a bit terse - some quote the Abstract
YARA, SNORT, SURICATA would benefit from references; they are not ones I
see in TLS or SSH!
typedef time I see in RFC6991bis
does the ipv6 addresss ever need the interface?
start/end ipv4/ipv6 could do with a must end > start
geo-ip could do with a reference
s.9.1 221.159 is not a documentation address - see RFC5737
IESG often expect an ipv6 example alongside ipv4
s.12 Registrant should be IESG
prefix is not that of the module
Tom Petch
Tom Petch
Also, the choice of the prefix is i2nsf-cfi.
I put "Note: This section is informative" for Sections 7 and 10, which
include XML configuration examples.
If you have further comments, please let me know by July 12, 2020, in
EST.
If possible, I want to post this revision on July 13, 2020 after
reflecting
your further comments on the revision.
Thanks.
Paul
On Fri, Mar 20, 2020 at 2:25 AM Jan Lindblad <[email protected]> wrote:
Paul,
Thank you for all your work with the module, and for the reminder for
me
to verify all the changes.
I am afraid I think the module is still not ready for last call, even
if
it is better shape than ever thanks to your efforts. I went through the
module from top to bottom, so this is sorted in order of appearance.
Line 107-204: The following identities are declared in the module, but
never referenced. They should either have a common base with
something, or
be referenced somewhere. If not, why are they defined here? They
currently
serve no purpose in this YANG module.
identity ddos {
identity enforce-type {
identity admin {
identity time {
Line 377: Defining a custom date-and-time type seems odd. You should
probably use one that has already been defined
typedef date-and-time {
Line 513: The leaf represents the name of a user, but the format is
undefined. What should be the format for the string value? How would
a user
know what to configure here? Email addresses? If implementation
dependent,
say so.
leaf name {
type string;
description
"This represents the name of a user.";
Line 518: If no IP address information is specified for the user-group,
what happens then? Is the user access accepted, rejected, or
something else?
uses ip-address-info;
Line 658: Key leaf declared mandatory. All keys are mandatory, so
mandatory is not needed on this leaf.
leaf policy-name {
type string;
mandatory true;
Line 664: Users mentioned in the owners-ref should have full CRUD
privileges to the policy. But what about everyone else? Should they
have
R(ead) privileges? Can anyone create new policies? If not, who can? If
someone creates a policy, but does not mention his own name among
owners
(e.g. misspells or does not get the format right), he will not be
able to
modify or remove the policy. If no owner is mentioned, then noone can.
uses owners-ref;
Line 673: Key leaf declared mandatory. All keys are mandatory, so
mandatory is not needed on this leaf.
leaf rule-name {
type string;
mandatory true;
Line 682: Users mentioned in the owners-ref should have full CRUD
privileges to the rule. But what about everyone else? Should they have
R(ead) privileges? Can anyone create new rules, or only those that have
full CRUD privileges for the policy? If someone creates a rule, but
does
not mention his own name among owners (e.g. misspells or does not get
the
format right), he will not be able to modify or remove the rule.
uses owners-ref;
Line 697: Choice enforce-type has a description that I can't
understand.
What does this mean?
choice enforce-type {
description
"There are two different enforcement types;
admin, and time.
It cannot be allowed to configure
admin=='time' or enforce-time=='admin'.";
Line 703: In case of enforce-type admin (whatever that means), a string
value needs to be configured. What are the valid values for this leaf?
case enforce-admin {
leaf admin {
type string;
description
"This represents the enforcement type
based on admin's decision.";
Line 711: In case of enforce-type time, three times can be configured.
What is the relation between enforce-time, and the other two
(begin-time,
end-time)?
case time {
container time-information {
description
"The begin-time and end-time information
when the security rule should be applied.";
leaf enforce-time {
type date-and-time;
description
"The enforcement type is time-enforced.";
}
leaf begin-time {
type date-and-time;
description
"This is start time for time zone";
}
leaf end-time {
type date-and-time;
description
"This is end time for time zone";
}
Furthermore, the locally defined date-and-time type used includes both
a
date and time, which seems to be at odds with the example
configurations in
the draft. Example 9.2:
<rules>
<rule>
<rule-name>block_access_to_sns_during_office_hours</rule-name>
<event>
<time-information>
<begin-time>2020-03-11T09:00:00.00Z</begin-time>
<end-time>2020-03-11T18:00:00.00Z</end-time>
In the example, the rule-name "block_access_to_sns_during_office_hours
"
suggests that the begin-time and end-time should be times of day
between
which the policy should be enforced. E.g. every day between 9.00 and
18.00.
If that is a valid use case, using a time type with a date doesn't make
much sense. In the context of the policy that repeats "daily", how
should
the start date-and-time value "2020-03-11T09:00:00.00Z " be
interpreted?
What if it was "monthly"?
Line 736: In the frequency leaf, the enumeration value only-once is for
rules that don't repeat. But how long do they apply? A single packet? A
single time the rule is triggered? How does a user know if the rule is
still in effect, i.e. if the "once" has happened or not?
enum only-once {
Line 835: Maybe it's just my limited understanding of how threat-feeds
work, but I wonder i this construct with source and destinations for
threat
feeds is meaningful?
container threat-feed-condition {
description
"The condition based on the threat-feed information.";
leaf-list source {
type leafref {
path
"/i2nsf-cfi-policy/threat-preventions/threat-feed-list/name";
}
description
"Describes the threat-feed condition source.";
}
leaf dest-target {
type leafref {
path
"/i2nsf-cfi-policy/threat-preventions/threat-feed-list/name";
}
description
"Describes the threat-feed condition destination.";
Line 920: Location groups can be configured, but there seems to be no
references to them. How are they supposed to be used?
list location-group{
key "name";
uses location-group;
Line 931: Regarding point 16.1 in your revision letter, you say "We
think
list type of threat-feed-list can be configured more than one feed of
the
same type". I'm afraid that is not the case with the current YANG
model. If
you do wish to allow more than one threat-feed-list for the same
threat-feed-type, you need to add an additional key to your
threat-feed-list.
list threat-feed-list {
key "name";
description
"There can be a single or multiple number of
threat-feeds.";
uses threat-feed-info;
...
grouping threat-feed-info {
description
"This is the grouping for the threat-feed-list";
leaf name {
type identityref {
base threat-feed-type;
Generally, the indentation in the module is much improved. Some lines
are
still a bit off, however, so I would recommend using a tool that
indents
consistently.
Generally, I also wonder whether there has been any discussion with
implementors around the admin security model proposed here. As noted
before, it's a bit different from everything else I have seen. Is it
well
thought through? Do implementors feel this is doable and user friendly?
Currently there are no examples involving owner. Perhaps an example
that
sheds some light over how different users create, modify and see the
various rules would shed some light over this.
Best Regards,
/jan
On 18 Mar 2020, at 18:41, Mr. Jaehoon Paul Jeong
<[email protected]>
wrote:
Hi Jan,
Could you update the state of YANGDOCTORS Last Call Review on
I2NSF Consumer-Facing Interface YANG Data Model if the updates are
fine
to you?
https://datatracker.ietf.org/doc/draft-ietf-i2nsf-consumer-facing-interface-dm/
I think your comments are all addressed in this version.
Thanks.
Best Regards,
Paul
On Thu, Mar 12, 2020 at 1:15 AM Mr. Jaehoon Paul Jeong <
[email protected]> wrote:
Hi Jan,
We authors have addressed your comments with the revision:
https://tools.ietf.org/html/draft-ietf-i2nsf-consumer-facing-interface-dm-08
I attach a revision letter to explain how to respond to your comments.
If you have further comments, please let me know.
Thanks.
Best Regards,
Paul
On Tue, Nov 12, 2019 at 1:53 AM Jan Lindblad via Datatracker <
[email protected]> wrote:
Reviewer: Jan Lindblad
Review result: Almost Ready
This is my YD review of
draft-ietf-i2nsf-consumer-facing-interface-dm-07. I
have previously reviewed the -05 revision (~end June). I find the new
revision
much improved, but still with much to discuss. I will call this
"almost
ready".
Generally speaking, I think the YANG module lacks the precision and
descriptions needed to foster interoperability. The examples at the
end
are
very enlightening however, and compensate for much of that, but their
informal
nature can never replace proper YANG. The module usage needs to be
mostly clear
from the module itself.
The management access control model proposed here is, even with its
latest
adaptation towards NACM, is still quite different from NACM author's
original
ideas. I will therefore bring this use case up in the NETMOD WG for
discussion.
1. Network access control principles
Network access control is about which users are able to use the
network
being
managed, for example connect to facebook. The purpose of the NSF
module
is to
control this access. This version of the YANG module is now based on
a
list of
policies.
Each policy has a list of rules. Each rule has an event --
condition --
action
triplet. This resembles traditional firewall management, which is a
good
thing,
because that concept is stable and much tried. This allows
operators to
create
lists of rules in this style:
if pkt.x == 1: drop // Rule 1
elif pkt.y > 2: alert // Rule 2
elif pkt.z == 10: pass // Rule 3
else: drop // Rule 4
This pattern relies heavily on the ability to control the order of
the
rules.
The current model relies on the alphabetical sorting of names rules
for
the
ordering. The YANG trick I would recommend to give operators the
ability
to
insert and move rules as they wish is to add ordered-by user on the
list:
list rule {
ordered-by user; // <== Add this line
leaf rule-name {
Nothing is said about what the system should do in case policies
conflict. What
if one policy says pass, the other drop for the same packet? Please
clarify.
What should happen to packets that do not match any of the policies?
This module also assumes that all users in the operator's
organization
are
listed in one or more NACM groups (e.g. "employees"). That wasn't
really
the
NACM authors' original intent. Even if this could be made to work
maybe,
there
is no strong reason to repurpose the NACM group concept for user
network
access
purposes. It could easily be modeled differently. So in the cases
where
there
are leafrefs to NACM groups when dealing with network access rather
than
management access, don't use NACM groups.
leaf src-target {
type leafref {
path
/nacm:nacm/nacm:groups/nacm:group/nacm:user-name; //
<== Point to some other list of network users
2. Management access control principles
Management access control is about which users are able to
configure/run
actions/the policies and rules. IMO, the most controversial aspect of
this
module has always been its new and creative management access control
model. In
this revision, the management principles have been remodeled
greatly to
fit in
with NACM. I find this redesign very promising, but the result is
still
not
quite ready for publication.
The point where integration with NACM concepts is important is when
it
comes to
allow some users to CRUD the NSF policies and rules themselves.
There is
now a
leaf-list "owners" on each policy and rule which point to a list of
NACM
groups. My understanding is that the idea is that the NSF module
should
be seen
as a service model that translate high level ownership information to
specific
NACM rules. It would be good to mention these ideas somewhere in
the NSF
document.
leaf-list owners {
type leafref {
path /nacm:nacm/nacm:groups/nacm:group/nacm:name;
I expect the intent is that any user listed in a NACM group
mentioned in
the
owners list would get full CRUD privileges for the contents of the
rule
the
owners leaf sits on. That is never spelled out anywhere, however.
It is a little less clear how leaf-list owners on policy objects
should
be
handled. Should owners listed on a policy object get full CRUD powers
over the
entire policy, including all the rules inside? Or would they need
to be
listed
on the rules as well? Not clear. Is the intent that users not
listed on
the
policy object are unable to create new rules, but to be able to
update
rules
they are listed as owners of, if any?
Who is allowed to create new policy objects? Should users that are
not
owners
get read access to all the policies and rules?
Finally, there is an "owner" leaf on each rule with an identityref
allowing
operators to configure a role name like dept-head or sec-admin. It is
marked
mandatory, but never included in any of the examples at the end of
the
document. This makes me think this may be a remnant from bygone
times and
should be removed from the YANG. If not, an explanation of how to use
this
leaf, and how it interacts with "owners" needs to be added, and the
examples
updated.
3. leafrefs crosspointing between policy instances
There are six leafrefs that point to various objects inside a policy,
e.g. a
rule condition pointing to a device group name. None of them restrict
what can
be pointed to so that only names within the current policy are
valid. It
is
therefore possible to configure the name of a device group defined
in a
different policy. I suspect this is not the intention. In order to
restrict the
leafrefs to the same policy, the following predicate can be added:
leaf-list src-target {
type leafref {
path
"/i2nsf-cfi-policy[policy-name=current()/../../../../../policy-name]/endpoint-group/device-group/name";
// <== Add predicate
4. Mandatory to implement all events, conditions, actions
Each rule is defined with a choice of different events (admin, time),
conditions (firewall, ddos, custom, threat-feed) and actions (pass,
drop,
alert, mirror, ...). Is the intent that all of these options should
be
mandatory to implement? The current model requires this. Also,
would it
make
sense to allow additional mechanisms here? If so, it may be good to
explain to
readers how the set of choices and identities can be extended by
implementations.
5. Optional and mandatory elements
In this revision of the module, 8 leafs have been marked mandatory. A
few of
them are list keys, which are conventionally not marked mandatory,
since
list
keys are always mandatory. A few others are skipped in the XML
examples
at the
end of the NSF document, which makes me believe they might not
really be
mandatory after all.
Three leafs have a default, but most leafs are left optional
without any
default. In many cases I think I understand what it means to not set
a
leaf,
but with the ones listed here, I don't think it clear at all.
Either add
a
default to make it clear, make them mandatory if they should be, or
explain in
the leaf description what happens if not set.
493: leaf-list name
513: leaf-list protocol
531: leaf geo-ip-ipv4
541: leaf continent
562: leaf feed-server-ipv4
585: leaf payload-description
590: leaf-list content
600: leaf-list owners
870: leaf method
6. Indentation
The YANG indentation is mostly wrong. Run the YANG text through
pyang or
some
other tool that can indent the content correctly before you put it
into a
document.
7. YANG element naming
The YANG convention is to not have lists on the top level in the YANG
module,
but to surround lists with a container. The surrounding container
often
has a
name in the plural and the list in singluar, like this
container interfaces {
list interface {
To better fit into the world of IETF YANG modules, I'd recommend
turning
the
top level list i2nsf-cfi-policy into this instead:
container i2nsf-cfi-policies {
list policy {
Further down, I would change container rule to rules:
container rules {
list rule {
Finally, it is customary to not repeat the names of parent object
in the
names
of elements. For example, in the following:
list threat-feed-list
leaf feed-name
leaf feed-server-ipv4
leaf feed-server-ipv6
leaf feed-description
all the leafs should normally not repeat "feed-". Just leaf name,
leaf
server-ipv4, leaf server-ipv6, leaf description. There are many more
examples
of this throughout the module.
The condition choice has many containers with a single leaf inside
(e.g.
ddos-source). Their purpose is rather unclear to me. Remove?
container ddos-source {
description
"This represents the source.";
leaf-list src-target {
Also, I find the name "src-target" rather confusing. How about
"source"?
8. No date leaf
The draft text near fig 2 talks about a date leaf. There is no date
object in
this revision of te YANG.
"Date: Date when this object was created or last modified"
9. leaf owner
Near fig.3 leaf Owner is mentioned. Is this leaf still current?
"Owner: This field contains the onwer of the rule. For example,
the person who created it, and eligible for modifying
it."
10. leaf packet-per-second
This is now modeled as uint16. Is this future proof? Many packet
flows
on the
internet exceed 64k pps.
11. container custon-source
Misspelled. Should be custom-source
12. identity ddos
Is ddos a malware file-type? This is not exactly in line with my
intuition.
13. identity protocol-type
There are other modules that already define protocol-types. Would
it be
worth
reusing one of them?
14. identity palo-alto
Is it a good IETF practice to list vendor names in modules? Can we
consider
this a protocol name? Is there perhaps an RFC/specification name
for it
that we
could reference instead?
15. grouping ipsec-based-method
This grouping contains a list which allows listing none of, either
of or
both
of ipsecike and ikeless. Are all valid configurations?
16. leaf feed-name
This leaf is the key in a list, which makes it possible to have at
most
one
feed of each type. If it would make sense to configure more than one
feed of
the same type, the YANG needs to be updated here.
17. leaf-list content
This leaflist is of type string. What is the format of this string?
Does
the
name refer to something?
18. Event types
container event has a choice between enforce-admin and time
alternatives. Each
of those choices have a leaf that allows the operator to configure an
identityref to an enforce-type value. What does that mean? What
would it
mean
if an operator configured admin == 'time' (or enforce-time ==
'admin')?
19. leaf begin-time, end-time
>From the examples, it can be seen that these are meant to be a
time of
day
values. Currently they are modeled as yang:date-and-time, which means
they are
a concrete time a specific day, e.g. 2019-11-11T16:07. This needs
to be
changed
in order to be what the modeler intended. Perhaps like this:
typedef time-of-day {
type string {
pattern '(2[0-3]|[01]?[0-9]):[0-5][0-9]';
}
}
20. leaf frequency
This leaf is now modeled properly from a YANG perspective. But what
does
it
mean? If this leaf is set to 'once-only', what exactly will happen
only
once?
Please write a description that explains this.
21. Example in Fig.17
The example contains XML that refers to "endpoint-group/user-group".
There is
no such element in the YANG.
<endpoint-group
xmlns="urn:ietf:params:xml:ns:yang:ietf-i2nsf-cfi-policy">
<user-group>
Furthermore, there is nothing called range-ip-address,
start-ip-address,
end-ip-address. They are called range-ipv4-address,
start-ipv4-address,
end-ipv4-address.
<range-ip-address>
<start-ip-address>221.159.112.1</start-ip-address>
<end-ip-address>221.159.112.90</end-ip-address>
</range-ip-address>
Finally, there must not be any xmlns attribute on an closing XML
tag. So
</endpoint-group
xmlns="urn:ietf:params:xml:ns:yang:ietf-i2nsf-cfi-policy">
should be
</endpoint-group>
This happens in several of the examples.
22. Example in Fig.18
There is no element called policy any more. It's now
i2nsf-cfi-policy.
<policy
xmlns="urn:ietf:params:xml:ns:yang:ietf-i2nsf-cfi-policy">
<policy-name>security_policy_for_blocking_sns</policy-name>
The rules are modeled in a container and list, both by the name
rule. So
there
needs to be two <rule> tags.
<rule>
<rule-name>block_access_to_sns_during_office_hours</rule-name>
The security-event element is marked mandatory in the YANG, but
missing
in the
example. The times given below may be what is intended, but do not
match
the
date format for the type used (which include a date, etc).
<event>
<time-information>
<begin-time>09:00</begin-time>
<end-time>18:00</end-time>
</time-information>
</event>
Since the example is not mentioning leaf frequency, it will have the
value
'once-only'. Maybe explain what that means in the context of the
example?
The condition/firewall-condition says the src-target is mandatory and
dest-target optional, exactly like below.
condition/custom-destination/dest-target is mandatory and
src-target is
optional, exactly like below. Is this pure luck, or is there a
logical
explanation why exactly those should be mandatory, and the example
use
precisely those?
<condition>
<firewall-condition>
<source-target>
<src-target>employees</src-target>
</source-target>
</firewall-condition>
<custom-condition>
<destination-target>
<dest-target>sns-websites</dest-target>
</destination-target>
</custom-condition>
The current YANG model does not allow setting both a
firewall-condition
and
custom-condition. If that should be allowed, the model needs to
change.
Should
it be possible to have multiple firewall- or other conditions? That
is
not
currently possible.
This example leaves out the mandatory leaf owner. Is that a sign
that it
should
not be mandatory, or perhaps not exist at all?
23. Example in Fig.19
This example lists a firewall-condition with no src-target, which is
mandatory.
<firewall-condition>
<destination-target>
<dest-target>employees</dest-target>
</destination-target>
</firewall-condition>
Under condition, there is a container rate-limit with a leaf
packet-per-second.
Is this a trigger value for the condition, or is it an actual limit
that
the
system is expected to enforce? If it's a trigger, it may be good to
find
a
clearer name. If it's enforced, it's placement under condition is
deceiving.
If a rule's action is set to 'rate-limit', to which rate will it be
limited?
24. Security Considerations
Section 10 in the NSF document under review is the Security
Considerations. I
think it would make sense to mention something about the management
access
control mechanism here, and its relation to NACM.
(End of list)
_______________________________________________
I2nsf mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/i2nsf
--
===========================
Mr. Jaehoon (Paul) Jeong, Ph.D.
Associate Professor
Department of Software
Sungkyunkwan University
Office: +82-31-299-4957
Email: [email protected], [email protected]
Personal Homepage: http://iotlab.skku.edu/people-jaehoon-jeong.php
<http://cpslab.skku.edu/people-jaehoon-jeong.php>
--
===========================
Mr. Jaehoon (Paul) Jeong, Ph.D.
Associate Professor
Department of Software
Sungkyunkwan University
Office: +82-31-299-4957
Email: [email protected], [email protected]
Personal Homepage: http://iotlab.skku.edu/people-jaehoon-jeong.php
<http://cpslab.skku.edu/people-jaehoon-jeong.php>
_______________________________________________
I2nsf mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/i2nsf
