Subject: [OSPF] Alvaro Retana's Yes on draft-ietf-ospf-prefix-link-attr-10: (with COMMENT)

From: "Alvaro Retana" <[email-masked]>
Date: Thu, 13 Aug 2015 11:55:10 -0700

Alvaro Retana has entered the following ballot position for
draft-ietf-ospf-prefix-link-attr-10: Yes

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-ospf-prefix-link-attr/

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

This is a good document, but I have some comments (and nits) that I would
really like to see addressed before publication.  I don't think my
comments raise to the level of a DISCUSS, but I am specially interested
in the actions when errors are detected.

1. Sections 2 and 3.  

a. RFC5250 used the name Opaque ID instead of Instance.  Please be
consistent.  If there’s a really good reason to not use the “official”
name, please at least write in the description of Instance that rfc5250
calls it Opaque ID.

b. “…the attributes from the Opaque LSA with the lowest Instance will be
used.”  This sounds to me like a good place to be a little bit more
prescriptive and s/will/MUST

2. Section 2.1. (OSPFv2 Extended Prefix TLV)  

a. For the Route Type, are those the only allowed values?  What happens
if something else is used?

b. Flags
- What are the defined flags used for?  Since (according to Section 5)
they are not supported in all implementations then I’m assuming there’s a
specific application in mind. [I know there are other ID’s in progress
that use the TLVs defined here.  Is the use of these bits general to
those applications?]

- How should the rest of the flags be assigned?  The IANA section says
nothing about that.

c. "Address Prefix     The prefix itself encoded as a 32-bit value.”  It
is variable, not fixed.  I know that only IPv4 is currently supported,
but as explained in the AF section, there may be future extensions.

3. Section 3.1. (OSPFv2 Extended Link TLV)  What should happen if an
unknown/incorrect Link-Type/Link-ID/Link Data value is received?

4. Section 6. (Security Considerations)

a. I think you should also point the readers at the considerations in
rfc5250.

b. “…implementations must assure that malformed TLV and Sub-TLV
permutations do not result in errors that cause hard OSPF failures.” 
What does that mean?  That the software should be resilient?  Or are you
pointing at just ignoring the information if there is a malformed
TLV/sub-TLV (that is somehow salvageable)?  It would be vey nice is this
draft set the example by specifying what to do in some cases (maybe not
malformed, but incorrect/unknown as mentioned above).

5. Section 7.  (IANA Considerations)  Is there a significant reason why
in all cases the assignment policy for the 33024-65535 range is not being
defined upfront?

Nits:
1. In Sections 2 and 3  s/differential/differentiate

2. In Section 2. (OSPFv2 Extended Prefix Opaque LSA), the figure that
shows the TLV format seems to show a Value field of only 32 bits.  The
description (below the figure), which looks like an exact copy from
rfc3630, describes the field correctly.  It would be nice to either copy
over the whole text from rfc3630 (including the figure that shows that
the Value may be longer) or just leave out that text completely (which is
my preference).   The text saying that the TLV format is the same as in
rfc3630 is enough.
- In Section 3 you write: "The format of the TLVs within the body of the
OSPFv2 Extended Link Opaque LSA is the same as described in Section 2.”  
One more level of indirection..

3. 2.1. (OSPFv2 Extended Prefix TLV)  
a. The description of “Flags” is misaligned (it looks like a
sub-paragraph of AF).
b. s/originated by the different/originated by different

---

From: "Acee Lindem (acee)" <[email-masked]>
Date: Mon, 17 Aug 2015 13:41:48 +0000

Hi Alvaro, 

I’m glad you are continuing in Adrian’s tradition of fastidious document
reviews ;^) 

>https://datatracker.ietf.org/doc/draft-ietf-ospf-prefix-link-attr/

I agree - we can change this to use Opaque ID consistent with RFC 5250.

>prescriptive and s/will/MUST

We will make this a “SHOULD”.

>if something else is used?

Yes - these are the only route types allowed. If an invalid type is
specified, the attributes will not be correlated with a route.

>that use the TLVs defined here.  Is the use of these bits general to

The “Flags” will be supported by all implementations. The reason they are
not implemented by all implementations is that they have been added and
changed as the draft evolved.

>- How should the rest of the flags be assigned?  The IANA section says

We will add a registry.

>but as explained in the AF section, there may be future extensions.

I already changed this once, since IPv4 is the only AF specified in this
document. 
“For Address Family IPv4, the prefix is encoded as a 32-bit value.
Encoding for 
 other address families is not defined by this specification.”

>unknown/incorrect Link-Type/Link-ID/Link Data value is received?

The attributes will not be correlated to an OSPF Router-LSA link.

>malformed, but incorrect/unknown as mentioned above).

This stems from prior attacks on routing protocols involved carefully
crafted malformed TLVs which could result in software crashes.

>in all cases the assignment policy for the 33024-65535 range is not being

The assignments are deferred until needed in cases we have requirements to
crave them up differently. I believe this is a common practice.

>1. In Sections 2 and 3  s/differential/differentiate

Ok

>my preference).   The text saying that the TLV format is the same as in

Are you not familiar with “…”? I’ll fix this as this rather than omit it.
In this case, the description is brief and allows one to implement the
encoding w/o referring to RFC 3630. I can attest to the fact that
programmers appreciate this.

>One more level of indirection..

As stated above, RFC 3630 need not be consulted.

>a. The description of “Flags” is misaligned (it looks like a

Ok. 

>b. s/originated by the different/originated by different

Ok. 

Thanks,
Acee

---

From: "Alvaro Retana (aretana)" <[email-masked]>
Date: Mon, 17 Aug 2015 14:30:43 +0000

On 8/17/15, 9:41 AM, "Acee Lindem (acee)" <[email-masked]> wrote:

Acee:

Hi!

Thanks for looking into my comments!  Just one more thing below.

Alvaro.

. . .
>specified, the attributes will not be correlated with a route.

I¹m glad you brought this up ‹ the correlation.  I should¹ve commented on
this before..

I don¹t think the draft talks about how to correlate the additional
information back to the ³normal² LSAs.  Given that it is important for the
route type and the prefix to match (anything else?), it would be good if
it was called out somewhere.  I¹m assuming that the fact that the
correlation is not made will just result in lost information ‹ but some
application may really want this information, so loosing it could be
importante for them.  Explaining the potential impact would be very good.

Same comment for the other TLV..

. . .
>>b. ³Šimplementations must assure that malformed TLV and Sub-TLV
>>permutations do not result in errors that cause hard OSPF failures.²

Ahh..ok.

Still, explaining what to do with malformed information would be good.
This obviously goes back to the loss of information.

---

From: "Acee Lindem (acee)" <[email-masked]>
Date: Mon, 17 Aug 2015 20:16:59 +0000

Hey Alvaro, 

>Same comment for the other TLV..

Actually, our intent is not to limit these mechanisms to the existing
prefix and link types even though all the existing drafts dependent on the
mechanisms do require correlation. I will word the description of
route-type and link as such.

>Still, explaining what to do with malformed information would be good.

In general, an LSA that is malformed is not acknowledged, stored in the
LSDB, or flooded. There were some attack where implementations accesses
beyond the end of an OSPF packet and crashed (not my implementation ;^).
I’ll write to this.

I will reference the RFC 5250 security considerations. I will also note
that an applications using these extensions must describe the
considerations specific to that application.

Thanks,
Acee