Subject: [secdir] Secdir last call review of draft-wu-l3sm-rfc8049bis-05

From: Rifaat Shekh-Yusef <[email-masked]>
Date: Wed, 27 Sep 2017 06:28:15 -0700

Reviewer: Rifaat Shekh-Yusef
Review result: Has Issues

I have reviewed this document as part of the security directorate's
ongoing effort to review all IETF documents being processed by the
IESG.  These comments were written primarily for the benefit of the
security area directors.  Document editors and WG chairs should treat
these comments just like any other last call comments.

Summary: Ready with issues

The document defines a YANG data model that defines service configuration
elements that can be used in communication protocols between customers
and network operators. Those elements can also be used as input to
automated control and configuration applications.

Issues
======

* Section 6.9, Security

This section has an Authentication and Encryption sub-sections that apply
to the site.

As per section 6:
   "Authorization of traffic exchange is done through what we call a VPN
    policy or VPN service topology defining routing exchange rules
    between sites."

It might be useful to add an Authorization sub-section to section 6.9 to
capture that security aspect of the model.

* Section 10, Security Considerations

"..., and the server MUST authenticate client access to any protected resource."

There is a need to differentiate between authentication and authorization.
How about the following:
    "..., and the server MUST authenticate the client and authorize access
    to any protected resource."

* Section 10, Security Considerations

"The data nodes defined in the "ietf-l3vpn-svc" YANG module MUST be
carefully created, read, updated, or deleted as appropriate."

I think the above statement is too general, and need to be more specific.
I am assuming that the above statement is trying to say that the identity
of the requestor must be authenticated, and the operations on the model
must be controlled based on authorization associated with the
authenticated entity.

If that is the case, then this should be clearly spelled out.

Nits
====

* Section 6.9.2. Encryption

"A hitless key-change mechanism may be added through augmentation."
Replace "key-change" with "key-exchange"

Regards,
 Rifaat

---

From: Qin Wu <[email-masked]>
Date: Thu, 28 Sep 2017 03:58:48 +0000

Thank Rifaat to review this document, please see my replies inline below.

-Qin
-----邮件原件-----
发件人: Rifaat Shekh-Yusef [mailto:[email-masked]] 
发送时间: 2017年9月27日 21:28
收件人: [email-masked]
抄送: [email-masked]; [email-masked]
主题: Secdir last call review of draft-wu-l3sm-rfc8049bis-05

I have reviewed this document as part of the security directorate's ongoing effort to review all IETF documents being processed by the IESG.  These comments were written primarily for the benefit of the security area directors.  Document editors and WG chairs should treat these comments just like any other last call comments.

The document defines a YANG data model that defines service configuration elements that can be used in communication protocols between customers and network operators. Those elements can also be used as input to automated control and configuration applications.

Issues
======

* Section 6.9, Security

This section has an Authentication and Encryption sub-sections that apply to the site.

As per section 6:

It might be useful to add an Authorization sub-section to section 6.9 to capture that security aspect of the model.

[Qin]: As described above, authorization of traffic exchange is done through VPN policy and VPN service topology.
VPN policy and VPN service topology are specified in section 6.2.1 and section 6.5.2.2. It is not necessary to repeatedly
Discuss it in a new section. 

[Qin]: Sounds a good suggestion. Thanks.

"The data nodes defined in the "ietf-l3vpn-svc" YANG module MUST be carefully created, read, updated, or deleted as appropriate."

I think the above statement is too general, and need to be more specific.
I am assuming that the above statement is trying to say that the identity of the requestor must be authenticated, and the operations on the model must be controlled based on authorization associated with the authenticated entity.

[Qin]: I think this statement convey two meanings:
For writable/creatable/deletable data nodes defined in this YANG module, these data nodes may be considered sensitive or vulnerable in some network environments. Write operations (e.g., edit-config) to these data nodes without proper protection can have a negative effect on network operations.
For the readable data nodes in this YANG module, these data nodes may be considered sensitive or vulnerable in some network environments. It is thus important to control read access (e.g., via get, get-config, or notification) to these data nodes.

Secondly, if you keep on reading the subsequent sentence after this statement in the paragraph 2 of section 10, you will see this is exactly what you proposed to do.
BTW, this is bis of RFC8049 and we didn't propose any new change to RFC8049.

Nits
====

[Qin]: regarding "key-change", one example given in the section 6.9.2 is about
customer changes the pre-shared key on a regular basis. So I think this is not about
key exchange between two parties, two way or three way handshakes. Using key-change is consistent with the example in the section 6.9.2.
Maybe we can change "key-change" into "key change". 
BTW, I believe hitless key change is related to keychain
and can also be referred to as hitless key rollover.

Regards,
 Rifaat