[secdir] SecDir Review of draft-ietf-rtgwg-mrt-frr-algorithm-08

Russ Housley <housley@vigilsec.com> Thu, 21 January 2016 20:06 UTC

Return-Path: <housley@vigilsec.com>
X-Original-To: secdir@ietfa.amsl.com
Delivered-To: secdir@ietfa.amsl.com
Received: from localhost (ietfa.amsl.com [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 8DDE51A90A5; Thu, 21 Jan 2016 12:06:55 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -100.7
X-Spam-Level:
X-Spam-Status: No, score=-100.7 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, J_CHICKENPOX_19=0.6, J_CHICKENPOX_36=0.6, USER_IN_WHITELIST=-100] autolearn=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id biUWc6Sc__pP; Thu, 21 Jan 2016 12:06:54 -0800 (PST)
Received: from odin.smetech.net (x-bolt-wan.smeinc.net [209.135.219.146]) by ietfa.amsl.com (Postfix) with ESMTP id 168B11A909E; Thu, 21 Jan 2016 12:06:54 -0800 (PST)
Received: from localhost (unknown [209.135.209.5]) by odin.smetech.net (Postfix) with ESMTP id 55F799A4020; Thu, 21 Jan 2016 15:06:53 -0500 (EST)
X-Virus-Scanned: amavisd-new at smetech.net
Received: from odin.smetech.net ([209.135.209.4]) by localhost (ronin.smeinc.net [209.135.209.5]) (amavisd-new, port 10024) with ESMTP id dTPSvtd7CUU3; Thu, 21 Jan 2016 15:05:45 -0500 (EST)
Received: from [192.168.2.104] (pool-108-51-128-219.washdc.fios.verizon.net [108.51.128.219]) (using TLSv1 with cipher AES128-SHA (128/128 bits)) (No client certificate requested) by odin.smetech.net (Postfix) with ESMTP id C17389A401F; Thu, 21 Jan 2016 15:06:52 -0500 (EST)
From: Russ Housley <housley@vigilsec.com>
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Date: Thu, 21 Jan 2016 15:06:52 -0500
Message-Id: <3CA8F251-7482-4839-ACF1-39166B8DC905@vigilsec.com>
To: draft-ietf-rtgwg-mrt-frr-algorithm.all@ietf.org
Mime-Version: 1.0 (Apple Message framework v1085)
X-Mailer: Apple Mail (2.1085)
Archived-At: <http://mailarchive.ietf.org/arch/msg/secdir/8F6ZV7GJL9DN7KxvCJusVB9TncQ>
Cc: IESG IESG <iesg@ietf.org>, IETF SecDir <secdir@ietf.org>
Subject: [secdir] SecDir Review of draft-ietf-rtgwg-mrt-frr-algorithm-08
X-BeenThere: secdir@ietf.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: Security Area Directorate <secdir.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/secdir>, <mailto:secdir-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/secdir/>
List-Post: <mailto:secdir@ietf.org>
List-Help: <mailto:secdir-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/secdir>, <mailto:secdir-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 21 Jan 2016 20:06:55 -0000

I 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 authors, document editors, and WG chairs should
treat these comments just like any other IETF Last Call comments.

Version reviewed: draft-ietf-rtgwg-mrt-frr-algorithm-08


I have a few comments.  None of them are related to security, but since
I read the document, I am passing these comment along.


Summary: Ready (from a Security Directorate perspective)


Major Concerns: None


Minor Concern:

I think the Abstract should be reworked so that it does not include a
reference to an Internet-Draft filename.  One solution is to replace it
with an RFC number after the other document is published.  Another
solution is to add a sentence that explains MRT-FRR.  Perhaps something
like:

   MRT-FRR provides link-protection and node-protection with 100%
   coverage in any network topology that is still connected after
   the failure.


Nits:

The Introduction says that the MRT Lowpoint algorithm is required for
implementation with the Default MRT profile.  It should say this once
in the first paragraph; it does not need repeating in several places.

In the Introduction, please talk about Figure 1 before talking about
Figure 2.  I notice that both of these figures are already part of
draft-ietf-rtgwg-mrt-frr-architecture, so it may be possible to
summarize the point being made and then point to the architecture
document for more details.

The phrases "this draft" and "that draft" are used.  I think it would
be better to select words that anticipate publication as an RFC.

Some of the figures contain algorithm pseudocode, but sometimes there
are comments for explanation and sometimes the comments appear to be
part of the code.  For example, see Figure 12:

            Get the current node, s.
            Compute an ear(either through lowpoint inheritance
            or by following dfs parents) from s to a ready node e.
            (Thus, s is not e, if there is such ear.)
            if s is e
               for each node x in the ear that is not s
                   x.localroot = s
            else
               for each node x in the ear that is not s or e
                   x.localroot = e.localroot

I think it would more clear is it looked something like:

            Get the current node, s.
            Compute an ear from s to a ready node e.
            // Compute either through lowpoint inheritance or
            // by following dfs parents.
            // Thus, s is not e, if there is such ear.
            if s is e
               for each node x in the ear that is not s
                   x.localroot = s
            else
               for each node x in the ear that is not s or e
                   x.localroot = e.localroot

It would be event better if this pseudocode used the Construct_Ear
function.