Re: [Rift] AD Review of draft-ietf-rift-rift-12 (Part 2a)

[Tony Przygienda <[e-mail]>]

+1

thanks for review Alvaro and sorry for any omissions from previous reviews

-- tony

On Fri, Jul 22, 2022 at 8:55 AM Jordan Head <[e-mail]> wrote:

> Thanks, Alvaro,
>
> I have the necessary changes per your previous review implemented, no
> submission as the draft upload tool isn't enabled yet.
>
> I'll start working through this round of comments with Tony.
>
> On 7/22/22, 8:00 AM, "Alvaro Retana" <[e-mail]> wrote:
>
>     [External Email. Be cautious of content]
>
>
>     Hi!
>
>     I have a couple of replies and then some comments on the updated text
> in -15.
>
>     Thanks!
>
>     Alvaro.
>
>
>     ...
>     > > Part 2a starts with §4.2 (Specification) and goes just through
> §4.2.2 -- I
>     > > just started reading the LIE FSM/§4.2.2.1. I included a couple of
> comments
>     > > on the schema itself.
>     ...
>
>
>     For the Chairs/Shepherd:
>
>     > > - §8.1 includes a set of suggested UDP ports, but they are not
> reflected
>     > > in the registry. Early allocation has not been requested -- you
> should
>     > > consider doing so. See rfc7120.
>
>     The same comment applies to the multicast addresses.
>
>
>
>     ...
>     > > [minor] "IPv4 Time to Live (TTL) / IPv6 Hop Limit (HL) of 1" Was
> GTSM
>     > > (rfc5082) considered? Several documents (for example, rfc8085 and
>     > > draft-ietf-opsec-v6) suggest its use. You may be asked about it
> later. It
>     > > would be good to preempt those questions by adding some text in the
>     > > Security Considerations about any risks...or using it.
>     >
>     > There is a religious war about 255 vs. 1 since a long time. I
> personally in
>     > deployment was hurt @ least twice by implementations ending up not
> checking
>     > TTL/misprocessing and forwarding one-hop packets with 254 on unicast/
>     > multicast. Also, having a buggy FIB looping tightly packets with TTL
> 255 is
>     > seldom fun. I never saw a thing not decrementing TTL and dropping on
> 1. I
>     > prefer the 1 with possibly a multi-hop spoof (never saw that in my
> life
>     > frankly, such an attack is really, really hard to engineer) than a
> rogue
>     > packet running away in the network or micro looping 255 times on
> fast links.
>     >
>     > What I saw were replay attacks and against those RIFT is extremely
> well
>     > protected as far it's practically implementable. Plus, if one
> _really_ wants
>     > security one configures adjacency keys (and RIFT allows a full
> plethora of
>     > that) rather than rely on the 255 hack to have "kind of a little
> lick of
>     > security".
>     >
>     > So if the security folks push on it we can always go to 255 or write
> that the
>     > TLL received MUST be either 255 or 1 to satisfy both camps.
>     ...
>     > > 1500 LIEs arriving with IPv4 Time to Live (TTL) / IPv6 Hop Limit
> (HL)
>     > > 1501 larger than 1 MUST be ignored.
>     > >
>     > > [major] "MUST be ignored" The specification of the sender (§4.2.2)
> says
>     > > that "LIEs SHOULD be sent with an IPv4 Time to Live (TTL) / IPv6
> Hop Limit
>     > > (HL) of 1". This is a Normative conflict because it is only
> recommended to
>     > > the sender, but required by the receiver.
>     >
>     > aha, yes, very good. I'm reluctant to say MUST since it's not always
> possible
>     > on a platform to force TTL (this is UDP). So I make it SHOULD on
> both sides.
>     >
>     > your comment on the 255/1 stands of course.
>
>     The text in -15 says:
>
>        LIEs SHOULD be sent with an IPv4 Time to Live (TTL) / IPv6 Hop
>        Limit (HL) of either 1 or 255 to prevent RIFT information
>        reaching beyond a single L3 next-hop in the topology.
>
>        ...
>
>        LIEs arriving with IPv4 Time to Live (TTL) / IPv6 Hop Limit (HL)
>        different than 1 or 255 SHOULD be ignored.
>
>
>     The good thing is that you removed the Normative conflict: both
>     sentences now say SHOULD.  The bad thing is that, even if providing
>     more options, anything can be used (1, 2, 4, 56, 123, 254, 255, etc.)
>     because neither the sender nor the receiver is required to do
>     anything.
>
>     In the reply (above) you suggested using MUST.  That would be a good
>     start.  But then you also said you were reluctant...
>
>     Note that in my original comment I was just asking whether GTSM was
>     considered and to justify why not (if you had considered and didn't
>     want to use it), not to add the cumbersome support for both cases.
>     IOW, do it any way you want, please be clear about any requirement
>     (MUST), and justify why you're not following what other documents
>     recommend (if not using 255) and how any issues can be mitigated (or
>     if they need to be -- see the Security Considerations in rfc5082).
>
>
>
>     > > [major] "LIEs SHOULD be sent with network control precedence."
> When is it
>     > > ok to not do so? IOW, when is this behavior recommended and not
> required.
>     > > BTW, please add a reference.
>     >
>     > hard to implement often that's why it's not MUST. will add ref to
> precedence.
>
>     You didn't add a reference.
>
>
>
>     *****
>     What follows are comments on -15 covering the same sections (§4.2
>     (Specification) through §4.2.2).
>     *****
>
>     [Line numbers from idnits.]
>
>
>     ...
>     1417    4.2.  Specification
>     ...
>     1423       The FSMs, as usual, are presented as states the FSM can
> assume,
>     1424       events that it can be given and according actions performed
> when
>     1425       transitioning between states on event processing.
>
>     1427       Actions are performed before the end state is assumed.
>
>     [nit] Suggestion>
>        The FSMs are presented as states a node (?) can be in,
>        events that can take place, and resulting actions, which
>        are performed before the next state is taken.
>
>
>     ...
>     1441       Any attempt to transition from a state towards another on
> reception
>     1442       of an event where no action is specified MUST be considered
> an
>     1443       unrecoverable error, i.e. the protocol MUST reset all
> adjacencies,
>     1444       discard all the state and MAY NOT start again.
>
>     [major] "MUST be considered an unrecoverable error"
>
>     Given that the meaning of this phrase is explained, we don't need
>     Normative language:  s/MUST/must
>
>
>     [major] "MAY NOT start again"
>
>     "MAY NOT" is not an rfc2119 keyword.  I assume you mean that the
>     behavior is not optional (i.e. mandatory), right?   s/MAY NOT/MUST NOT
>
>
>
>     ...
>     1461    4.2.1.  Transport
>
>     1463       All packet formats are defined in Thrift [thrift] models in
>     1464       Appendix B.  LIE packet format is contained in the
> `LIEPacket` schema
>     1465       element.  TIE packet format is contained in `TIEPacket`,
> TIDE and
>     1466       TIRE accordingly in `TIDEPacket`, `TIREPacket` and the
> whole packet
>     1467       is a union of the above in `ProtocolPacket` while it
> contains a
>     1468       `PacketHeader` as well.
>
>     [nit] s/ and the whole packet is a union of the above in
>     `ProtocolPacket` while it contains a `PacketHeader` as well./.  The
>     whole packet is the union of a 'PacketHeader' and the above.
>
>
>
>     ...
>     1477    4.2.2.  Link (Neighbor) Discovery (LIE Exchange)
>     ...
>     1507       An implementation MAY listen and send LIEs on IPv4 and/or
> IPv6
>     1508       multicast addresses.  A node MUST NOT originate LIEs on an
> address
>     1509       family if it does not process received LIEs on that
> family.  LIEs on
>     1510       same link are considered part of the same LIE FSM
> independent of the
>     1511       address family they arrive on.  Observe further that the
> LIE source
>     1512       address may not identify the peer uniquely in unnumbered or
> link-
>     1513       local address cases so the response transmission MUST occur
> over the
>     1514       same interface the LIEs have been received on.  A node MAY
> use any of
>     1515       the adjacency's source addresses it saw in LIEs on the
> specific
>     1516       interface during adjacency formation to send TIEs (Section
> 4.2.3.3).
>     1517       That implies that an implementation MUST be ready to accept
> TIEs on
>     1518       all addresses it used as source of LIE frames.
>
>     [major] "An implementation MAY listen and send LIEs on IPv4 and/or
>     IPv6 multicast addresses."
>
>     Sorry to come back to this, but it's really bothering me.  After
>     reading this multiple times I figured out what is the problem: Yes,
>     it's optional to listen/send on IPv4 and/or IPv6 -- I too like the
>     flexibility.  *But*, for rift to work there's an expectation that at
>     least one will be used.  IOW, while the selection is flexible,
>     selecting (at least) one AF is *not* optional (otherwise the protocol
>     doesn't work).
>
>     The "MAY" above is not a Normative statement because one AF (or both)
>     should be used.  The sentence is really just a statement of fact: any
>     (or both) AF can be used.
>
>     All this is to suggest this change: s/MAY/may
>
>
>     [major] "MAY use any of the adjacency's source addresses it saw in
>     LIEs...to send TIEs."  This use of "MAY" makes using the source
>     address from a LIE optional.  §4.2.3.3 says that "TIEs...using the
>     destination address on which the LIE adjacency has been formed", which
>     doesn't make the use optional.
>
>     I'm also revisiting this -- again, statement of fact, not an option:
> s/MAY/may
>
>
>
>     1520       A simplified version on platforms with limited multicast
> support MAY
>     1521       implement optional sending and reception of LIE frames on
> IPv4 subnet
>     1522       broadcast addresses and IPv6 all routers multicast address
> though
>     1523       such technique is less optimal and presents a wider attack
> surface
>     1524       from security perspective.
>
>     [major] Ahhh...mmmm...
>
>     Let's start with this: I'm not sure how to read this sentence, a few
>     commas would help.
>
>     You don't need to solve all the cases.  What is "limited multicast
>     support"?  As I interpret the sentence, IPv6 seems to support
>     multicast so the choice is simple: use IPv6.  As you explain, the node
>     doesn't have to use both.
>
>     The phrase "MAY implement optional..." is a double indication that
>     "sending and reception" is optional: you don't have to.  Which seems
>     to point to the obvious conclusion of use IPv6.
>
>     In summary, I don't understand the value of this sentence, including
>     the use of a normative MAY ... but maybe I just don't understand the
>     sentence itself. :-(
>
>
>
>     1526       A ThreeWay adjacency (as defined in the glossary) over any
> address
>     1527       family implies support for IPv4 forwarding if the
>     1528       `ipv4_forwarding_capable` flag in `LinkCapabilities` is set
> to true.
>     1529       A node, in case of absence of IPv4 addresses on such links
> and
>     1530       advertising `ipv4_forwarding_capable` as true, MUST forward
> IPv4
>     1531       packets using gateways discovered on IPv6-only links
> advertising this
>     1532       capability.  It is expected that the whole fabric supports
> the same
>     1533       type of forwarding of address families on all the links,
> any other
>     1534       combination is outside the scope of this specification.
>     1535       `ipv4_forwarding_capable` MUST be set to true when LIEs
> from a IPv4
>     1536       address are sent and MAY be set to true in LIEs on IPv6
> address if no
>     1537       LIEs are sent from a IPv4 address.  If IPv4 and IPv6 LIEs
> indicate
>     1538       contradicting information protocol behavior is unspecified.
>
>     [major] "absence of IPv4 addresses...and advertising
>     `ipv4_forwarding_capable`...MUST forward IPv4 packets using gateways
>     discovered on IPv6-only links"
>
>     Just to be clear, the choice of which AF to use when sending LIEs is
>     up to the nodes -- which then means that nodes may decide (or be
>     configured) to only use IPv6 with RIFT, even if IPv4 is configured on
>     the interface.  Right?  I assume that's true because it is the third
>     case shown in Table 1.
>
>     IOW, not having IPv4 configured ("absence of IPv4 addresses") is not
>     the only case where ipv4_forwarding_capable can be set, and also not
>     the only case where the interface will support v4overv6.  This will
>     also happen if the LIEs/TIEs are only advertised over IPv6 (regardless
>     of which AFs are supported/configured on the interface).
>
>     The sentence above is then misleading because it mandates the v4overv6
>     behavior only in the "absence of IPv4 addresses".
>
>
>     [major] "MUST forward IPv4 packets using gateways discovered on
> IPv6-only links"
>
>     I assume that this means that the IPv4 packet is encapsulated in IPv6
>     and sent to a link-local address -- is that what you mean?  Using
>     "gateways discovered" may give the impression that there's more
>     involved.  You may want to consider being explicit about the
>     operation.
>
>     Also, in Figure 1 the text (for the 3rd case: IPv4/IPv6 over IPv6)
>     simply says "IPv4 is forwarded", giving the wrong impression that the
>     IPv4 is natively (not encapsulated) forwarded.
>
>     At some point we talked about the fact that forwarding is out of scope
>     -- that's ok.  I'm calling out this text because it requires
>     forwarding ("MUST forward").  Please make the behavior non-normative
>     and declare the specific mechanism to "forward IPv4 packets...on
>     IPv6-only links" out of scope.
>
>
>     [minor] s/`ipv4_forwarding_capable` MUST be set to true when LIEs from
>     a IPv4 address are sent and MAY be set to true in LIEs on IPv6 address
>     if no LIEs are sent from a IPv4 address./If IPv4 forwarding is
>     supported on an interface, `ipv4_forwarding_capable` MUST be set to
>     true when LIEs from a IPv4 address are sent and MAY be set to true in
>     LIEs on IPv6 address if no LIEs are sent from a IPv4 address.
>
>
>     [nit] s/information protocol/information, protocol
>
>
>
>     1540       Operation of a fabric where only some of the links are
> supporting
>     1541       forwarding on an address family or have an address in a
> family and
>     1542       others do not is outside the scope of this specification.
>
>     [] I think it's ok to leave the operation of the fabric out of scope,
>     but the ability to forward IPv4 traffic without having it configured
>     means that there are some operational aspects that are left out, and
>     which are not clearly specified anywhere else (as in not even in other
>     specifications).
>
>     For example, take a look at §3/rfc9229 (IPv4 Routes with an IPv6 Next
>     Hop in the Babel Routing Protocol).  The intent is not to compare,
>     just to point out a recent example of something similar which resulted
>     in significant discussion during IESG Evaluation.
>
>     I don't expect any action, just something to keep in mind if it comes
> up later.
>
>
>
>     ...
>     1550
> +=======+=======+=============================================+
>     1551          | AF    | AF    | Behavior
>       |
>     1552
> +=======+=======+=============================================+
>     1553          | IPv4  | IPv4  | LIEs and TIEs are exchanged over IPv4,
> no   |
>     1554          |       |       | IPv6 forwarding.  TIEs are received on
> any  |
>     1555          |       |       | of the LIE sending addresses.
>      |
>     1556
> +-------+-------+---------------------------------------------+
>     1557          | IPv6  | IPv6  | LIEs and TIEs are exchanged over IPv6
> only, |
>     1558          |       |       | no IPv4 forwarding if either of the
>      |
>     1559          |       |       | `ipv4_forwarding_capable` flags is
> false.   |
>     1560          |       |       | If both `ipv4_forwarding_capable`
> flags are |
>     1561          |       |       | true IPv4 is forwarded.  TIEs are
> received  |
>     1562          |       |       | on any of the LIE sending addresses.
>       |
>     1563
> +-------+-------+---------------------------------------------+
>     1564          | IPv4, | IPv6  | LIEs and TIEs are exchanged over IPv6,
> no   |
>     1565          | IPv6  |       | IPv4 forwarding if either of the
>       |
>     1566          |       |       | `ipv4_forwarding_capable` flags is
> false.   |
>     1567          |       |       | If both `ipv4_forwarding_capable` are
> true  |
>     1568          |       |       | IPv4 is forwarded.  TIEs are received
> on    |
>     1569          |       |       | any of the IPv6 LIE sending
> addresses.      |
>     1570
> +-------+-------+---------------------------------------------+
>     1571          | IPv4, | IPv4, | LIEs and TIEs are exchanged over IPv6
> and   |
>     1572          | IPv6  | IPv6  | IPv4, unspecified behavior if either
> of the |
>     1573          |       |       | `ipv4_forwarding_capable` flags is
> false or |
>     1574          |       |       | IPv4 and IPv6 advertise different
> flags as  |
>     1575          |       |       | described previously.  IPv4 and IPv6
> are    |
>     1576          |       |       | forwarded.  TIEs are received on any
> of the |
>     1577          |       |       | IPv4 and IPv6 LIE sending addresses.
>       |
>     1578
> +-------+-------+---------------------------------------------+
>
>     [minor] Two of the columns have an "AF" header.  It is not clear (just
>     from the header) how their meaning is different.  Please clarify.
>
>     I'm assuming that the first column shows the AFs enabled on the
>     interface, and the second one shows the AF used to exchange TIEs/LIEs.
>     Or does the first column represent which AFs are intended to be
>     forwarded (to cover the "absence of IPv4 addresses")?   In either
>     case, it looks like the table may be incomplete.
>
>
>     [minor] The Behavior of the second and third cases is the same, which
>     adds to my question about what the AF columns mean.  Are the columns
>     representing only one side?
>
>
>     [nit] Case 4: "IPv4 and IPv6 are forwarded."
>
>     The placement of this sentence after the behavior of
>     ipv4_forwarding_capable is explained (again) gives the impression that
>     IPv4 and IPv6 are always forwarded.
>
>
>     [minor] Related question, so I don't forget later.  In Case 2
>     (IPv6/IPv6), for example, if only one side sets
>     ipv4_forwarding_capable, I'm assuming the TIEs won't include any IPv4
>     information.  Is that true?  Where is that specified?  It seems
>     obvious, but it would be nice if it was explicitly specified
>     somewhere.
>
>
>
>     ...
>     1608       A node MUST form a ThreeWay adjacency (or in other words
> consider the
>     1609       neighbor "valid" and hence reflecting it) if and only if the
>     1610       following first order logic conditions are satisfied on a
> LIE packet
>     1611       as specified by the `LIEPacket` schema element and received
> on a link
>
>     [] -12 (the last version I had looked at) listed a couple of
>     conditions related to the PoDs:
>
>        A node tries to form a three-way adjacency if and only if
>
>        1.  the node is in the same PoD or either the node or the neighbor
>            advertises "undefined/any" PoD membership (PoD# = 0) AND
>
>        ...
>
>        3.  the neighbor is not member of some PoD while the node has a
>            northbound adjacency already joining another PoD AND
>
>
>     Are these conditions not needed any more?  I know they were taken off
>     because I was confused with them -- but I guess that the text (in -12)
>     about how they could be "optionally disregarded" made them not
>     required.
>
>     Just checking...
>
>
>
>     ...
>     1616       2.  the neighboring node uses a valid System ID (i.e. value
> different
>     1617           from `IllegalSystemID`) in `sender` element in
> `PacketHeader`
>     1618           *and*
>
>     [nit] s/in `sender` element/in the `sender` element
>
>
>
>     1620       3.  the neighboring node uses a different System ID than
> the node
>     1621           itself
>
>     [nit] For consistency, looks like you're missing an "*and*".
>
>
>
>     1623       4.  the advertised MTUs in `LiePacket` element match on
> both sides
>     1624           *and*
>
>     [major] This clause talks about the "advertised MTUs", but advertising
>     the MTU is optional.
>
>     Suggestion>
>        the MTU on on the link (either advertised in the `LiePacket`
>        element or the 'default_mtu_size') matches on both sides *and*
>
>
>     [EoR P2a-15]
>
>
> Juniper Business Use Only
>