[prev in list] [next in list] [prev in thread] [next in thread] 

List:       ipng
Subject:    Re: Benjamin Kaduk's Discuss on draft-ietf-6man-segment-routing-header-22: (with DISCUSS and COMMENT
From:       Benjamin Kaduk <kaduk () mit ! edu>
Date:       2019-10-04 18:46:27
Message-ID: 20191004184627.GF6722 () kduck ! mit ! edu
[Download RAW message or body]

On Tue, Oct 01, 2019 at 01:12:20PM +0000, Darren Dukes (ddukes) wrote:
> Hi Ben, this thread is for closure of your comments (Discuss items handled \
> separately).

Comments only when I have something other than "looks good" or "I agree" to
say...

> 
> > On Sep 3, 2019, at 10:52 PM, Benjamin Kaduk via Datatracker <noreply@ietf.org> \
> > wrote: 
> > Benjamin Kaduk has entered the following ballot position for
> > draft-ietf-6man-segment-routing-header-22: Discuss
> > 
> > ----------------------------------------------------------------------
> > COMMENT:
> > ----------------------------------------------------------------------
> > 
> > As something of a general note, this document (specifically, the HMAC
> > portions) was initially challenging for me to read, since in the
> > security community we tend to have HMAC (or MACs in general) apply to
> > specific messages and protect the content from end to end.  The usage
> > here seems rather different, in that the HMAC is protecting not the
> > packet payload, but (essentially) just the SRH, which is to a large
> > extent a fixed (e.g., per-flow) artifact that reflects routing policy.  So the
> > MAC is serving to authenticate policy, more than messaging, and there
> > was very little in the document that tried to make that distinction.
> > I've tried to indicate some locations/changes that would improve readability
> > in this regard.
> > 
> > Section 1
> > 
> > The Segment Routing Architecture [RFC8402] describes Segment Routing
> > and its instantiation in two data planes MPLS and IPv6.
> > 
> > nit: we need some form of punctuation after "data planes".
> 
> Will do
> 
> > 
> > Section 2
> > 
> > o  Tag: tag a packet as part of a class or group of packets, e.g.,
> > packets sharing the same set of properties.  When tag is not used
> > at source it MUST be set to zero on transmission.  When tag is not
> > used during SRH Processing it SHOULD be ignored.  Tag is not used
> > when processing the SID defined in Section 4.3.1.  It may be used
> > when processing other SIDs which are not defined in this document.
> > The allocation and use of tag is outside the scope of this
> > document.
> > 
> > At what scope does its use need to be defined?  Can it be defined on a
> > per-segment basis or must it be deployment-global?
> 
> That is not defined in this document, and as such a segment may define its use of \
> Tag.  A source may set it for use by the segments it places in the SRH. 
> > 
> > The mutability of the TLV value is defined by the most significant
> > bit in the type, as specified in Section 2.1.
> > 
> > It seems like this presents the design as one where we rely on the
> > participants to be trusted to properly respect the (im)mutability, and
> > there are not external mechanisms (e.g., cryptography) to enforce them.
> > So, we have SR nodes that are in a hybrid state of not being trusted
> > with HMAC keys (and thus to generate routing policy) but are trusted to
> > "be implemented correctly" and not mutate fields that are supposed to be
> > immutable.  This multi-tier trust system should probably be discussed in
> > the security considerations.
> 
> I believe section 7.5 "Applicability of AH" covers this.

I think I wrote this while I still misunderstood how the setup worked, so I
agree.

> > 
> > Section 2.1
> > 
> > An implementation MAY limit the number and/or length of TLVs it
> > processes based on local configuration.  It MAY:
> > [...]
> > 
> > Is this list an exhaustive enumeration of the granted permissions or are
> > other limitations possible?
> 
> Yes exhaustive

I can't decide whether it's worth saying that explicitly ("MUST process all
other variations") or not.

> > 
> > o  Limit the number of consecutive Pad1 (Section 2.1.1.1) options to
> > 1, if padding of more than one byte is required then PadN
> > (Section 2.1.1.2) should be used.
> > 
> > nit: comma splice
> 
> Will fix
> 
> > 
> > Type: An 8 bit value.  Unrecognized Types MUST be ignored on receipt.
> > 
> > Length: The length of the Variable length data.
> > 
> > Do we want to give a pointer to (e.g.) the registry for these type
> > values?
> > I see we say in the next entry that Length measures bytes, though
> > perhaps it is more natural to mention it here.
> 
> Thanks, lets replace that text as follows
> <NEW>
> Type: An 8 bit value from Segment Routing Header TLVs Register TBD IANA Reference.  \
>                 Unrecognized Types MUST be ignored on receipt.
> Length: The length of the Variable length data in bytes.
> </NEW>
> 
> > 
> > Type Length Value (TLV) contain OPTIONAL information that may be used
> > by the node identified in the Destination Address (DA) of the packet.
> > 
> > nit: there seems to be a singular/plural mismatch of sorts here, in that
> > the current TLV incarnation mostly acts as a singular.  I'd suggest
> > adding another word like "entries" or "structures" after the
> > parenthetical, as a resolution.
> 
> Added the work entries
> <NEW>
> Type Length Value (TLV) entries contain OPTIONAL information that may be use

nit: s/use/used/

> by the node identified in the Destination Address (DA) of the packet.
> </NEW>
> 
> > 
> > The highest-order bit of the TLV type specifies whether or not the
> > TLV data of that type can change en route to the packet's final
> > destination:
> > 
> > For clarity: that's bit '0' in the above figure?
> 
> Added (bit0)
> <NEW>
> The highest-order bit of the TLV type (bit 0) specifies whether or not the
> TLV data of that type can change en route to the packet's final
> destination:
> </NEW>
> 
> > 
> > Section 2.1.1
> > 
> > Padding TLVs are used for meeting the alignment requirement of the
> > subsequent TLVs.
> > [...]
> > Padding TLVs are used for alignment.
> > 
> > Is there some redundancy here we could deduplicate?
> 
> <DELETE>
> Padding TLVs are used for alignment.
> </DELETE>
> 
> > 
> > Section 2.1.1.1
> > 
> > required.  If more than one byte of padding is required a Pad1 TLV
> > MUST NOT be used, the PadN TLV MUST be used.
> > 
> > nit: comma splice
> 
> Replacing the offending sentence.
> 
> <NEW>
> A Pad1 TLV MUST NOT be used if more than one consecutive byte of padding is \
> required. </NEW>
> 
> > 
> > Section 2.1.1.2
> > 
> > Padding: Length octets of padding.  Padding bits have no
> > semantics.  They MUST be set to 0 on transmission and ignored on
> > 
> > [Elsewhere in the document we've used "semantic" as a singular; it might
> > be worth a pass to check for consistency of usage]
> 
> Ack, s/semantics/semantic/
> 
> 
> > 
> > Section 2.1.2
> > 
> > I echo the sentiments already expressed about this HMAC format being
> > likely to reflect substantial wasted space in much common usage.
> > (Unless the intent is that since TLV support is optional, it's unlikely
> > to be implemented at all, in which case the wastefulness of a
> > nonexistent feature doesn't matter?)
> > In particular, having to specify the details of which bits of the 'HMAC'
> > field have significance when the HMAC algorithm in use has fewer than 32
> > bytes of output seems to be just as much effort as making the field
> > variable length.
> > Four octets of Key ID also seems like far more than will be needed,
> > though we don't really discuss the scope (in space/topology and time) in
> > which it needs to be unique -- we should probably talk about that here!
> > (I do see a bit of discussion in Section 2.1.2.2 that seems to say that
> > the key-ID is scoped by the destination IP address.)  On the other hand,
> > this much Key ID space could be used if there was a desire to have very
> > fine-grained routing policy and revocation ability (see below), but it's
> > still unclear that even in a high-churn fine-grained situation there
> > would be the will to store millions of HMAC keys on the
> > controller/verifier.
> > 
> 
> 
> The RESERVED bits are for future expansion if necessary (eg. D bit added)
> 
> I'll see if we can make this variable length... others have mentioned the same,  it \
> seems a good observation. 
> The 32bit KEY ID is based on the 32 bit SPI in ESP/AH.
> ESP SPI is destination scoped, 32 seemed like a good precedent.
> As discussed separately the KEY ID would be SR Domain scoped.
> With ESP there are seldom/ever millions of connections to a single endpoint.
> Given this is 32bit still overkill?

Given that it's domain-scoped, I no longer think 32bit is overkill.

> 
> > The HMAC TLV is used to verify the source of a packet is permitted to
> > use the current segment in the destination address of the packet, and
> > ensure the segment list is not modified in transit.
> > 
> > I'd suggest some revisions to this along the lines of:
> > 
> > %  The HMAC TLV is used to verify that the SR applied to a packet was
> > %  selected by an authorized party, and to ensure that the segment list is
> > %  not modified after generation.  This also allows for verification that
> > %  the current segment (by virtue of being in the authorized segment list)
> > %  is an authorized use.
> 
> ACK, we'll replace with 
> <NEW>
> The HMAC TLV is used to verify that the SRH applied to a packet was
> selected by an authorized party, and to ensure that the segment list is
> not modified after generation.  This also allows for verification that
> the current segment (by virtue of being in the authorized segment list)
> is authorized for use.
> </NEW>
> 
> > 
> > since the expected usage seems to not be to give the keys (and thus the
> > choice of segment lists) to the actual source of the packet, and we are
> > detecting modification of the Segment List since the HMAC generation,
> > which could span a broader period of time than just when the packet
> > carrying the HMAC is "in transit".
> > 
> > Section 2.1.2.1
> > 
> > For HMAC algorithms producing digests less than 32 octets, the digest
> > is placed in the lowest order octets of the HMAC field.  Remaining
> > octets MUST be set to zero.
> > 
> > To keep me from second-guessing myself, are these the "lowest order
> > octets" if we are treating the field as a 32-byte big-endian integer?
> > (As opposed to an array of bytes enumerated in order per the figure.)
> 
> Lowest order as in an array of bytes octet[0] is lowest order.
> 
> > 
> > If HMAC verification is successful, the packet is forwarded to the
> > next segment.
> > 
> > Does this really mean "processing proceeds as normal", to allow for the
> > possibility of local action/modification before forwarding?
> 
> replace that sentence with 
> <NEW>
> If HMAC verification is successful processing proceeds as normal.
> </NEW>
> 
> > 
> > Section 2.1.2.2
> > 
> > The HMAC Key ID field is opaque, i.e., it has neither syntax nor
> > semantic except as an identifier of the right combination of pre-
> > shared key and hash algorithm, and except that a value of 0 means
> > that there is no HMAC field.
> > 
> > "No HMAC field" or "the HMAC field is filled entirely with zeros"?  The
> > previous diagram/discussion does not suggest that the HMAC field is
> > sometimes omitted.
> 
> I see no value in this text.. Others have questioned it too. 
> It is likely a security issue – if I support key ID 0 in code I may just
> authenticate the SRH with key id 0 and continue processing.
> 
> <DELETE>
> and except that a value of 0 means that there is no HMAC field.
> </DELETE>
> 
> 
> > 
> > At the HMAC TLV verification node the Key ID uniquely identifies the
> > pre-shared key and HMAC algorithm.
> > 
> > At the HMAC TLV generating node the Key ID and destination address
> > uniquely identify the pre-shared key and HMAC algorithm.  Utilizing
> > 
> > This decision to make the key-ID space scoped to the destination address
> > seems to have some potentially substantial consequences that I don't see
> > discussed in the draft.  Specifically, since the destination address
> > will change on every hop, it would seem to preclude a model where the
> > HMAC TLV is generated by the sender and not modified subsequently (which
> > itself would probably have knock-on effects).  Is the additional scoping
> > by IP address of key-ID necessary even with the 4-octet key-ID, or would
> > scoping the key-ID to be globally unique within a SR domain suffice?
> > Unless the intent is that there is only a single verification node in a
> > given path?  The desired usage remains quite unclear to me.
> 
> Closed with global scope key ID.
> 
> 
> > 
> > Section 3.2, 3.3
> > 
> > nit(?): We seem to talk about "an IPv6 packet" when we really mean "an
> > IPv6 packet with SRH", "an SR IPv6 packet", "an SRv6" or similar.
> 
> Infact we do mean an IPv6 packet.  An SRH is not required
> 
> > 
> > Section 4.1
> > 
> > A Source node steers a packet into an SR Policy.  If the SR Policy
> > results in a segment list containing a single segment, and there is
> > no need to add information to SRH flag or TLV, the DA is set to the
> > single segment list entry and the SRH MAY be omitted.
> > 
> > nit: "to the SRH flags or to add TLVs"
> 
> ACK replacing with your text
> 
> > 
> > HMAC TLV may be set according to Section 7.
> > 
> > Do we want to say anything about other, to-be-specified, TLVs here?
> > Like "HMAC (or other) TLVs may be set" or "TLVs (including HMAC) may be
> > set according to their specifications"?
> 
> replace that text with the following:
> <NEW>
> TLVs (including HMAC) may be set according to their specifications.
> </NEW>
> 
> 
> > 
> > 4.1.1.  Reduced SRH
> > 
> > A reduced SRH does not contain the first segment of the related SR
> > Policy (the first segment is the one already in the DA of the IPv6
> > header), and the Last Entry field is set to n-2 where n is the number
> > of elements in the SR Policy.
> > 
> > This seems to imply that Segments Left is not constrained to point
> > inside the Segment List (and can point one past it); is that correct?
> 
> Correct.
> 
> > 
> > Section 4.3.1
> > 
> > This document, and section, defines a single SRv6 SID.  Future
> > documents may define additional SRv6 SIDs.  In which case, the entire
> > content of this section will be defined in that document.
> > 
> > nit: perhaps this is just me, but I tend to read "defines a single SRv6
> > SID" as "defines a single SRv6 SID value"; the intent here seems to be
> > more along the lines of "SID behavior" or "SID processing algorithm".
> > The second sentence could also be phrased similarly to "Such documents
> > will need to specify the analogous behavior as is described in this
> > section and subsections".
> 
> The term SID has been used the document to represent the behavior or the value. 
> In this context the behavior is being discussed.
> I believe the current text says what is needed quite plainly.
> 
> > 
> > Section 4.3.1.1
> > 
> > Is this pseudocode normative, or the text of the following subsections?
> > 
> 
> Both are normative.
> 
> > Section 5.1
> > 
> > Nodes outside the SR Domain are not trusted: they cannot directly use
> > the SID's of the domain.  This is enforced by two levels of access
> > 
> > nit: "SIDs" (no apostrophe)
> 
> ACK
> 
> > 
> > 1.  Any packet entering the SR Domain and destined to a SID within
> > the SR Domain is dropped.  This may be realized with the
> > following logic, other methods with equivalent outcome are
> > considered compliant:
> > 
> > nit: comma splice  (also in (2))
> 
> ACK
> 
> > 
> > *  Failure to implement this method of ingress filtering exposes
> > the SR Domain to source routing attacks as described and
> > referenced in [RFC5095]
> > 
> > nit: this doesn't seem like a bullet point in the list, but rather a
> > standalone paragraph of commentary.
> > It may also be worth reiterating that failure to do so on even a single
> > edge node puts the entire network at risk.
> 
> The sub bullet is correctly scoped.

The sub-bullet list is introduced as "with the following logic"; what
"logic" is being described here?

> 
> > 
> > Section 5.2
> > 
> > Consider a top of rack switch (T) connected to host (H) via interface
> > (I).  H receives an SRH (SRH1) with a computed HMAC via some SDN
> > method outside the scope of this document.  H classifies traffic it
> > sources and adds SRH1 to traffic requiring a specific SLA.  T is
> > 
> > I'm having a hard time parsing how the SRH1 that H receives via SDN can
> > be the same SRH1 that H adds to traffic.  The key insight seems to be
> > that the SRH received via SDN is not attached to an IPv6 packet as a
> > "live" routing header, but is rather a piece of configuration data to be
> > applied as the SRH on flows sent on a given SR.  (The formalism is
> > slightly weird, as "receiving an SRH" includes the "next header" value,
> > which is arguably artificially limiting, but this is probably minor
> > enough to ignore.)  So perhaps we want to say that "H receives, along
> > with the configuration for applying SR to a given flow, an SRH (SRH1)
> > with pre-computed HMAC that reflects that SR flow, via some SDN method
> > outside the scope fo this document."
> 
> when adding an extension header to an IPv6 header chain the next header field must \
> be set correctly This is covered by RFC8200 so I think is clear to an implementor. 
> > 
> > An operator of the SR Domain may choose to have all segments in the
> > SR Domain verify the HMAC.  This mechanism would verify that the SRH
> > segment list is not modified while traversing the SR Domain.
> > 
> > As partially covered above, this seems inconsistent with having the HMAC
> > key-ID be scoped by the destination SID when the destination SID changes
> > from segment to segment.
> > 
> 
> Closed with scope of KEY ID.
> 
> > Section 5.4
> > 
> > For the source of an invoking packet to process the ICMP error
> > message, the correct destination address must be determined.  The
> > 
> > "correct" in what sense/for what operation?
> > 
> > *  If routing header is type 4 (SRH)
> > 
> > (This is still "TBD", formally, right?)
> > 
> > +  Use the SID at Segment List[0] as the destination address of
> > the invoking packet.
> > 
> > Similarly to the first comment on this section, do we want some language
> > about how this is "use"d "for processing by the upper layer transport"?
> 
> 
> 
> "Correct" as in the destination address of the socket associated with the TCP, UDP, \
> IP tunnel session/instance. I think the following clarifications in the text will \
> help. 
> <OLD>
> For the source of an invoking packet to process the ICMP error
> message, the correct destination address must be determined.  The
> following logic is used to determine the destination address for use
> by protocol error handlers.
> 
> o  Walk all extension headers of the invoking IPv6 packet to the
> routing extension header preceding the upper layer header.
> 
> *  If routing header is type 4 (SRH)
> 
> +  Use the SID at Segment List[0] as the destination address of
> the invoking packet.
> 
> ICMP errors are then processed by upper layer transports as defined
> in [RFC4443].
> </OLD>
> <NEW>
> For the source of an invoking packet to process the ICMP error
> message, the ultimate destination address of the IPv6 header may
> be required. The
> following logic is used to determine the destination address for use
> by protocol error handlers.
> 
> o  Walk all extension headers of the invoking IPv6 packet to the
> routing extension header preceding the upper layer header.
> *  If routing header is type TBD IANA (SRH)
> +  The SID at Segment List[0] may be used as the destination address of
> the invoking packet.
> ICMP errors are then processed by upper layer transports as defined
> in [RFC4443].
> </NEW>
> 
> 
> 
> > 
> > Section 5.6
> > 
> > I'm slightly tempted to add a sentence along the lines of "the
> > mechanisms in this document are known to not be directly transferrable
> > to multi-domain deployment models without substantial impact on these
> > properties", but (1) it's an absolute statement that seems like it would
> > be hard to be 100% confident in, and (2) it's not entirely clear that it
> > adds much value to the general reader (as opposed to providing a
> > security disclaimer to make the security AD happy).  So I'm curious to
> > hear about how much thought has already gone into this, pointers to
> > ongoing documents on the topic, etc.
> 
> draft-ietf-spring-network-programming defines other SID behaviors that can be used \
> for multi-domain deployments, including the binding SID. RFC8402 describes how \
> multiple domains can share segments using global and local blocks, similar \
> strategies can be defined for SRv6.

Thanks for the pointers!

> > 
> > Section 6.1
> > 
> > I a little bit wonder if mixing zero-based (Segments Left; Last Entry)
> > and one-based (S1, S2, S3) indexing is going to cause undue cognitive
> > load for anyone.
> 
> So far it has not...
> 
> 
> > 
> > Section 6.2
> > 
> > o  The SR domain is secured as per Section 5.1 and no external packet
> > can enter the domain with a destination address equal to a segment
> > of the domain.
> > 
> > I'd suggest using "firewalled" rather than "secured", since "secured"
> > can mean many different things and we have a more-precise term
> > available.
> 
> Lets fix with..
> s/is secured/implements ingress filtering/
> 
> > 
> > Section 6.3.2
> > 
> > Is it worth calling out that P5 does not have an SRH?
> 
> 
> Adding the phrase "without SRH" below.
> <NEW>
> If the SR Policy contains only one segment (the egress router 4), the
> ingress Router 3 encapsulates P3 into an outer header (A3, S4) without SRH.  The
> packet is…
> </NEW>
> 
> > 
> > Section 6.9
> > 
> > This section describes how a function may be delegated within the SR
> > Domain to non SR source nodes.  [...]
> > 
> > I'm kind of confused about what "non SR source node" means in the
> > context of a source node that is adding an SRH for verification
> > elsewhere.
> 
> Lets remove the confusion and delete that text.  It is incorrect to call them "non \
> SR source nodes". s/to non SR source nodes//
> 
> 
> > 
> > Section 6.6.1
> > 
> > An operator may prefer to add the SRH at source 8, while 5 verifies
> > the SID list is valid.
> > 
> > I note that "add" here means "apply a fixed bitstring to the packet",
> > not "compute the value of"…
> 
> 
> Correct, it could be replaced with apply. s/add/apply/
> 
> 
> > 
> > For illustration purpose, an SDN controller provides 8 an SRH
> > terminating at node 9, with segment list <S5,S7,S6,A9>, and HMAC TLV
> > computed for the SRH.  The HMAC key is shared with 5, node 8 does not
> > know the key.  Node 5 is configured with an IACL applied to the
> > 
> > ... since the SRH value is computed by the controller and distributed to
> > 8 as almost an opaque token.  It would be good if we could reword this
> > to be more clear about how the mechanics work such that 5 does not need
> > the HMAC key.
> 
> 
> I think you mean s/The HMAC key/The HMAC key ID and key associated with the HMAC \
> TLV/.  Node 5 is in fact the verifying node so it does need the key.

I agree that I was confused about this; sorry about that.

> 
> > 
> > Node 8 originates packets with the received SRH with HMAC TLV.
> > 
> > nit: I suggest s/with HMAC TLV/including HMAC TLV/
> 
> ACK
> 
> > 
> > Section 7.1
> > 
> > Does the ability to create traffic loops or near-loops (that span the
> > same links dozens or hundreds of times) merit a specific mention outside
> > of "DOS/DDOS"?
> 
> This section says "bandwidth exhaustion" for example is mountable from within the \
> domain.  This covers that attack. 
> > 
> > Section 7.5
> > 
> > The SR Domain is a trusted domain, as defined in [RFC8402] Section 2
> > and Section 8.2.  The SR Source is trusted to add an SRH (optionally
> > verified via the HMAC TLV in this document), and segments advertised
> > 
> > I suggest s/verified/verified as having been generated by a
> > trusted configuration source/.
> 
> ACK.
> 
> > 
> > The use of SRH with AH by an SR source node, and processing at a SR
> > segment endpoint node, is not defined in this document.  Future
> > documents may define use of SRH with AH and its processing.
> > 
> > It's not entirely clear to me what this is intended to convey.  It this
> > saying "SRH MUST NOT be used in the same packet as AH" or something more
> > like "we make no statements about the use of SRH in combination with
> > AH"?
> 
> The latter, no statement.
> 
> 
> > 
> > Section 8
> > 
> > RFC will be published.  The Designated expert will post a request to
> > the 6man WG mailing list (or a successor designated by the Area
> > Director) for comment and review, including an Internet-Draft.
> > 
> > nit: this wording makes it sound like either the DE includes an I-D in
> > the body of the mail or is the author of the I-D in question; I assume
> > we just want a pointer or link to the draft requesting the
> > allocation(s).
> 
> 
> It looks like we'll change this to IETF review as per other comments, so this will \
> be removed. However a pointer is all that was needed. 
> > 
> > Section 12.2
> > 
> > I-D.filsfils-spring-srv6-interop and
> > I-D.matsushima-spring-srv6-deployment-status are only referenced from
> > the Implementation Status section (that is to be removed); should the
> > references be removed as well?
> > 
> > I think RFCs 2104, 6437, and 6438 need to be normative.
> 
> OK

Thanks!

-Ben

--------------------------------------------------------------------
IETF IPv6 working group mailing list
ipv6@ietf.org
Administrative Requests: https://www.ietf.org/mailman/listinfo/ipv6
--------------------------------------------------------------------


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic