Subject: Secdir last call review of draft-ietf-ospf-ospfv3-segment-routing-extensions-16

From: Yaron Sheffer <[email-masked]>
Date: Sun, 04 Nov 2018 07:38:14 -0800

Reviewer: Yaron Sheffer
Review result: Has Nits

Summary: document has non-security related nits.

Details

* The definition of "segment" is different here from the one used in the
architecture RFC. The RFC is more abstract, quoting: A node steers a packet
through an ordered list of instructions, called "segments". Whereas here a
segment is simply a sub-path. This is confusing to a non-expert, and perhaps
indicates a change in the group's thinking.

* SID/Label Sub-TLV: is it Mandatory? If so, please point it out.

* "The SR-Algorithm TLV is optional" - I find this sentence confusing. Maybe
replace by "The SR-Algorithm TLV is mandatory for routers that implement
segment routing"?

* The reference under "IGP Algorithm Type" registry should be to the IANA
registry itself, not to the I-D that defines it. (In particular since the IANA
registry has already been established,
https://www.iana.org/assignments/igp-parameters/igp-parameters.xhtml#igp-algorithm-types).

* OSPFv3 Extended Prefix Range TLV Flags octet: add the usual incantation about
reserved bits.

* In general I agree with the reasoning in the Security Considerations. I would
like to raise the question if, in addition to mis-routing, this adds a threat
of massive denial-of-service on MPLS endpoints, e.g. by allowing an attacker
who has OSPF access to introduce routing loops. (This may be completely bogus,
I am far from expert with either of these protocols).

---

From: Peter Psenak <[email-masked]>
Date: Mon, 05 Nov 2018 09:12:34 +0100

Hi Yaron,

thanks for your comments, please see inline:

> indicates a change in the group's thinking.

the definition in this draft relates to segment as used by IGPs, in 
which case a segment represents the sub-path. There are other segments 
outside of IGPs which can represent other things, but they are not 
covered by this draft.

> * SID/Label Sub-TLV: is it Mandatory? If so, please point it out.

SID/Label Sub-TLV is not advertised on its own. It is advertised as a 
sub-TLV of the:

3.2.  SID/Label Range TLV
3.3.  SR Local Block TLV

Both of these section specify that SID/Label Sub-TLV MUST be included.

> replace by "The SR-Algorithm TLV is mandatory for routers that implement

the text says:

    "If the SR-Algorithm TLV
    is not advertised by the node, such node is considered as not being
    segment routing capable."

Isn't that sufficient?

> https://www.iana.org/assignments/igp-parameters/igp-parameters.xhtml#igp-algorithm-types).

I got another comment from Opsdir last call review to include the I-D 
that defined it. I Added them both, hopefully that satisfy everybody.

> * OSPFv3 Extended Prefix Range TLV Flags octet: add the usual incantation about

Done.

> I am far from expert with either of these protocols).

above is addressed by usage of the usage of the OSPF authentication as 
described in the security section.

thanks,
Peter

> .

---

From: Yaron Sheffer <[email-masked]>
Date: Tue, 6 Nov 2018 08:13:06 +0200

Thank you Peter, this addresses my comments.

	Yaron

---

From: Peter Psenak <[email-masked]>
Date: Fri, 09 Nov 2018 12:44:20 +0100

Hi Yaron,

> Thank you Peter, this addresses my comments.

latest version (17) includes your comments.

thanks,
Peter