Hi Peter,

Please check inline below.

On Wed, May 3, 2023 at 6:16 PM Peter Psenak <[email protected]> wrote:

> Hi Ketan,
>
> On 03/05/2023 06:09, Ketan Talaulikar wrote:
> > Hello Authors/All,
> >
> > I think there are a couple of issues with this document related to
> > OSPFv2 aspects. My apologies for this last minute notice and not
> > catching them earlier during the WGLC. >
> > 1) The OSPFv2 IP Algorithm Prefix Reachability Sub-TLV is not carrying
> > the indication of Type1/Type2 for External and NSSA-External route
> > advertisements. A way to fix/address this would be to introduce a 1 byte
> > flags in the Reserved space and then introduce the "E -bit" similar to
> > how it is there in the base OSPFv2 External LSAs -
> > https://datatracker.ietf.org/doc/html/rfc2328#appendix-A.4.5
> > <https://datatracker.ietf.org/doc/html/rfc2328#appendix-A.4.5>
>
> can't we live without the Ext. Type-2 metric for the IP algo prefixes
> and treat then as Type 1 ext metric always?
> The real advantage of the Type-2 metric is not really significant.
>

KT> That is an option but it would create deviations from the base OSPF
processing for IP Algo reachability. That may be more of an issue for
implementations and may be even operators used to OSPF. It may be easier to
just fix the encoding to allow the indication of Type 1/2.


>
> >
> > 2) Also for OSPFv2, since we don't have the base OSPFv2 LSA for Algo
> > reachability, we are missing some key sub-TLVs in the OSPFv2 Extended
> > Prefix Sub-TLVs that would be required for IP FlexAlgo reachability -
> > mainly Forwarding Address and Route Tag.
>
> Similarly using the forwarding address is something which has limited
> benefits and we do not need it for the IP Algo prefixes.
>
> The only problem is during the NSSA translation, where the spec mandates
> the non-zero forwarding address. Is that something that we really need?
>

KT> Same as the previous comment. We will hit some corner cases and may
need to carve out these exceptions. It may be simpler to just add these
sub-TLVs.


>
> >
> > 3) Along with the above changes, perhaps some text is required to
> > indicate the use of these new sub-TLVs and how OSPFv2 base route
> > calculations apply for various route types (specifically external and
> NSSA)?
>
> how is that different to regular prefix (RFC2328)?
>

KT> The route calculations don't change - only that everything is done
using only the OSPFv2 Extended Prefix LSAs instead of the base OSPF LSAs.
One can say it is implicit so I wonder if it helps to make this explicit.


>
> thanks,
> Peter
>
> >
> > 4) A nit: in a few places in sec 6.3, the OSPFv2 IP Algorithm Prefix
> > Reachability is being referred to as TLV instead of sub-TLV. Similar
> > issue in sec 6.4 for OSPFv3.
> >
> > Thanks,
> > Ketan
> >
> >
> > On Wed, May 3, 2023 at 12:01 AM John Scudder
> > <[email protected] <mailto:[email protected]>>
>
> > wrote:
> >
> >     Hi Peter,
> >
> >     All good, I figured it was something like that. Two residual nits —
> >
> >     1. One “datapalne” got left in. I guess you need something to seed
> >     version 11 after all…
> >
> >     2. It looks like this one got omitted:
> >
> >     @@ -579,8 +592,18 @@
> >          receiver.
> >
> >          The metric value in the parent TLV is RECOMMENDED to be set to
> >     -   LSInfinity [RFC2328].  This recommendation only servers for
> >     debugging
> >     +   LSInfinity [RFC2328].  This recommendation only serves for
> debugging
> >          purposes and does not impact the functionality.
> >     +---
> >     +jgs: Thanks for adding the additional explanation. I made a minor
> >     editing
> >     +correction in-line, but I also have a slightly more extensive
> >     revision to
> >     +suggest:
> >     +
> >     +NEW:
> >     +     This recommendation is provided as a network troubleshooting
> >     +     convenience; if it is not followed the protocol will still
> >     +     function correctly.
> >     +—
> >
> >     Obviously, I don’t insist on the proposed rewrite. But even if you
> >     don’t use it you presumably should take the s/servers/serves/
> >     proofreading correction.
> >
> >     I’m going to go ahead and request IETF Last Call, but feel free to
> >     push a revision with corrections if you want.
> >
> >     —John
> >
> >      > On May 2, 2023, at 6:06 AM, Peter Psenak
> >     <[email protected]
> >     <mailto:[email protected]>> wrote:
> >      >
> >      >
> >      > Hi John,
> >      >
> >      > I apologize for the misses, likely the result of multiple editors
> >      > updating the draft in parallel.
> >      >
> >      > I also fixed the nits and updated the security sections as you
> >     proposed.
> >      >
> >      > Version 10 has been published.
> >      >
> >      > thanks,
> >      > Peter
> >      >
> >      >
> >      >
> >      >
> >      >
> >      > On 01/05/2023 20:54, John Scudder wrote:
> >      >> Hi Peter (and Shraddha),
> >      >>
> >      >>> On Apr 28, 2023, at 9:13 AM, Peter Psenak
> >     <[email protected]
> >     <mailto:[email protected]>> wrote:
> >      >>>
> >      >>> Shradha and I have worked to address your comments.
> >      >>> The new version of the draft has been published.
> >      >>
> >      >> Thanks for that. I’ve reviewed the diffs in 09. I’ve attached a
> >     short review of it; there are some minor proofreading changes, but
> >     also one place a substantive edit was overlooked that I’ve flagged
> >     in Section 6.2. I also made a further suggestion on your Security
> >     Considerations.
> >      >>
> >      >> I think one more revision and we will be ready for IETF Last
> Call.
> >      >>
> >      >> Thanks,
> >      >>
> >      >> —John
> >      >>
> >      >> --- draft-ietf-lsr-ip-flexalgo-09.txt 2023-05-01
> >     13:21:34.000000000 -0400
> >      >> +++ draft-ietf-lsr-ip-flexalgo-09-jgs-comments.txt    2023-05-01
> >     14:47:16.000000000 -0400
> >      >> @@ -138,9 +138,9 @@
> >      >>     result, traffic for different sessions is destined to a
> >     different
> >      >>     destination IP address.
> >      >>
> >      >> -   IP address allocated to the UPF can be associated with an
> >     algoritm.
> >      >> +   The IP address allocated to the UPF can be associated with
> >     an algorithm.
> >      >>     The mobile user traffic is then forwarded along the path
> >     based on the
> >      >> -   algorithm specific metric and constraints.  As a result,
> >     traffic can
> >      >> +   algorithm-specific metric and constraints.  As a result,
> >     traffic can
> >      >>     be sent over a path that is optimized for minimal latency or
> >     highest
> >      >>     bandwidth.  This mechanism is used to achieve SLA (Service
> Level
> >      >>     Agreement) appropriate for a user session.
> >      >> @@ -186,9 +186,9 @@
> >      >>
> >      >>     Advertisement of participation in IP Flex-Algorithm does not
> >     impact
> >      >>     the router participation signaled for other data-planes.  For
> >      >> -   Example, it is possible that a router participates in a
> >     particular
> >      >> -   flex-algo for IP datapalne but does not participate in the
> >     same flex-
> >      >> -   algo for SR dataplane.
> >      >> +   example, it is possible that a router participates in a
> >     particular
> >      >> +   flex-algo for the IP dataplane but does not participate in
> >     the same flex-
> >      >> +   algo for the SR dataplane.
> >      >>
> >      >>     The following sections describe how the IP Flex-Algorithm
> >      >>     participation is advertised in IGP protocols.
> >      >> @@ -196,6 +196,11 @@
> >      >>  5.1.  The IS-IS IP Algorithm Sub-TLV
> >      >>
> >      >>     The ISIS [ISO10589] IP Algorithm Sub-TLV is a sub-TLV of the
> >     IS-IS
> >      >> +---
> >      >> +jgs: Was it deliberate that you didn't accept the suggestion to
> >      >> +hyphenate "ISIS" above, or an oversight? If deliberate, how
> come?
> >      >> +If accidental, please change in next rev.
> >      >> +---
> >      >>     Router Capability TLV [RFC7981] and has the following format:
> >      >>
> >      >>          0                   1                   2
> >           3
> >      >> @@ -302,9 +307,9 @@
> >      >>  6.  Advertising IP Flex-Algorithm Reachability
> >      >>
> >      >>     To be able to associate the prefix with the Flex-Algorithm,
> the
> >      >> -   existing prefix reachability advertisements can not be used,
> >     because
> >      >> +   existing prefix reachability advertisements cannot be used,
> >     because
> >      >>     they advertise the prefix reachability in default algorithm
> 0.
> >      >> -   Instead, a new IP Flex-Algorithm reachability advertisements
> are
> >      >> +   Instead, new IP Flex-Algorithm reachability advertisements
> are
> >      >>     defined in IS-IS and OSPF.
> >      >>
> >      >>     The M-flag in the FAD is not applicable to IP Algorithm
> >     Prefixes.
> >      >> @@ -410,6 +415,11 @@
> >      >>     all of them do not advertise the same algorithm, it MUST
> >     ignore all
> >      >>     of them and MUST NOT install any forwarding entries based on
> >     these
> >      >>     advertisements.  This situation SHOULD be logged as an error.
> >      >> +---
> >      >> +jgs: Thanks for these rewrites. Unfortunately there is a
> >     similar case
> >      >> +later (Section 6.2) which was missed. It needs a similar
> rewrite,
> >      >> +I will flag it below, please refer back to this section.
> >      >> +---
> >      >>
> >      >>     In cases where a prefix advertisement is received in both a
> IPv4
> >      >>     Prefix Reachability TLV and an IPv4 Algorithm Prefix
> >     Reachability
> >      >> @@ -434,6 +444,9 @@
> >      >>     with a different Algorithm, MUST ignore all of them and MUST
> NOT
> >      >>     install any forwarding entries based on these
> >     advertisements.  This
> >      >>     situation SHOULD be logged as an error.
> >      >> +---
> >      >> +jgs: These two paragraphs need a rewrite similar to Section 6.1.
> >      >> +---
> >      >>
> >      >>     In cases where a prefix advertisement is received in both an
> >     IPv6
> >      >>     Prefix Reachability TLV and an IPv6 Algorithm Prefix
> >     Reachability
> >      >> @@ -579,8 +592,18 @@
> >      >>     receiver.
> >      >>
> >      >>     The metric value in the parent TLV is RECOMMENDED to be set
> to
> >      >> -   LSInfinity [RFC2328].  This recommendation only servers for
> >     debugging
> >      >> +   LSInfinity [RFC2328].  This recommendation only serves for
> >     debugging
> >      >>     purposes and does not impact the functionality.
> >      >> +---
> >      >> +jgs: Thanks for adding the additional explanation. I made a
> >     minor editing
> >      >> +correction in-line, but I also have a slightly more extensive
> >     revision to
> >      >> +suggest:
> >      >> +
> >      >> +NEW:
> >      >> +     This recommendation is provided as a network
> troubleshooting
> >      >> +     convenience; if it is not followed the protocol will still
> >      >> +     function correctly.
> >      >> +---
> >      >>
> >      >>     An OSPFv3 router receiving multiple OSPFv3 IP Algorithm
> Prefix
> >      >>     Reachability Sub-TLVs in the same parent TLV, MUST select
> >     the first
> >      >> @@ -932,13 +955,47 @@
> >      >>     This document inherits security considerations from
> [RFC9350].
> >      >>
> >      >>     This document introduces one additional way to disrupt
> Flexible
> >      >> -   algorithm based networks.  If the node that is authenticated
> >     is taken
> >      >> -   over by an attacker, such rogue node can advertise a prefix
> >      >> +   Algorithm based networks.  If a node that is authenticated
> >     is taken
> >      >> +   over by an attacker, such a rogue node can advertise a prefix
> >      >>     reachability for a particular IP Flexible-algorithm X while
> that
> >      >>     prefix has been advertised in algorithm Y.  This kind of
> >     attack makes
> >      >> -   the prefix unreachable.  Such attack is not preventable
> through
> >      >> +   the prefix unreachable.  Such an attack is not preventable
> >     through
> >      >>     authentication, and it is not different from advertising any
> >     other
> >      >>     incorrect information through IS-IS or OSPF.
> >      >> +---
> >      >> +jgs: Thanks for this. I think you should provide a reference to
> >      >> +illustrate what you're talking about, e.g. "This kind of attack
> >     makes
> >      >> +the prefix unreachable (to see why this is, consider, for
> >     example, the
> >      >> +rule given in the second-last paragraph of Section 6.1)".
> >      >> +
> >      >> +I see you cribbed the text from RFC 9350, which is not a bad
> idea
> >      >> +considering that was recently approved by the IESG so
> >     presumably they
> >      >> +like the look of it. But in that case, I think it would be a
> >     good idea
> >      >> +to copy the 9350 section more comprehensively. Something like
> this:
> >      >> +
> >      >> +   This document adds one new way to disrupt IGP networks that
> >     are using
> >      >> +   Flex-Algorithm: an attacker can suppress reachability for a
> >     given
> >      >> +   prefix whose reachability is advertised by a legitimate node
> >     for a
> >      >> +   particular IP Flex-Algorithm X, by advertising the same
> >     prefix in
> >      >> +   Flex-Algorithm Y from another, malicious node. (To see why
> >     this is,
> >      >> +   consider, for example, the rule given in the second-last
> >     paragraph of
> >      >> +   Section 6.1.)
> >      >> +
> >      >> +   This attack can be addressed by the existing security
> >     extensions, as
> >      >> +   described in [RFC5304] and [RFC5310] for IS-IS, in [RFC2328]
> and
> >      >> +   [RFC7474] for OSPFv2, and in [RFC4552] and [RFC5340] for
> OSPFv3.
> >      >> +
> >      >> +   If a node that is authenticated is taken over by an
> >     attacker, such a
> >      >> +   rogue node can perform the attack described above.  Such an
> >     attack is
> >      >> +   not preventable through authentication, and it is not
> >     different from
> >      >> +   advertising any other incorrect information through IS-IS or
> >     OSPF.
> >      >> +
> >      >> +I was tempted to rewrite further (I was bugged that "node that
> is
> >      >> +authenticated" isn't a well-defined term) but I think the
> >     argument that
> >      >> +this text already passed IESG review recently, is pretty
> >     compelling, so
> >      >> +the above is just a minimal substitution into the RFC 9350
> security
> >      >> +considerations.
> >      >> +---
> >      >>
> >      >>  13.  Acknowledgements
> >      >>
> >      >>
> >      >>
> >      >>
> >      >>
> >      >
> >
> >     _______________________________________________
> >     Lsr mailing list
> >     [email protected] <mailto:[email protected]>
> >     https://www.ietf.org/mailman/listinfo/lsr
> >     <https://www.ietf.org/mailman/listinfo/lsr>
> >
>
>
_______________________________________________
Lsr mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/lsr

Reply via email to