Re: [Idr] Benjamin Kaduk's Discuss on draft-ietf-idr-flow-spec-v6-19: (with DISCUSS and COMMENT)

Erik Kline <ek.ietf@gmail.com> Sat, 07 November 2020 00:02 UTC

Return-Path: <ek.ietf@gmail.com>
X-Original-To: idr@ietfa.amsl.com
Delivered-To: idr@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 78F8B3A03FB; Fri, 6 Nov 2020 16:02:36 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.097
X-Spam-Level:
X-Spam-Status: No, score=-2.097 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id f4QY5w3uxFLw; Fri, 6 Nov 2020 16:02:33 -0800 (PST)
Received: from mail-ot1-x336.google.com (mail-ot1-x336.google.com [IPv6:2607:f8b0:4864:20::336]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 27DB53A03F5; Fri, 6 Nov 2020 16:02:32 -0800 (PST)
Received: by mail-ot1-x336.google.com with SMTP id h62so2931296oth.9; Fri, 06 Nov 2020 16:02:32 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=fAlZyOVfI+Ed/u7/QONWWrMcOObJZqee0iw4AVtrQpw=; b=dwXxhXK524sE2K3yTHRwFEwaGoIcsRIOxPDZsQH9Bwax43OO3oOVxtaZgy4gbkjLBC cGmYjpF5dvKBOBdmpLAVLQ33UNy0BwSjDik/wBN0hXcfChP31O8BaMz62sqhE8sl02kv QUuNfN/GzUUn/sGwPuVWZSqO+QhlM4aB5AzmvIbDw1ORJak3PiPZVucUQqQel4ggveNZ NwKbnoCavgj1OJZylV8RXdrmNdwrZUhkmsgODsJopc9xXej/Wp5vXbvWPLMiFw63gtqa QbwGEPX76O8Ke6010fiUp1ZtCrRzoxkOpOD8pN16YkaisrEE5LO2dxK8N4taF5TEkYXC tsAQ==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=fAlZyOVfI+Ed/u7/QONWWrMcOObJZqee0iw4AVtrQpw=; b=ZF/u0zHlgFrxe/KT8uWY4j1upFfMxa6DEa5Fd5OgcdyzWRZLWeeh1Pq4wkYNf+ZSP2 W77zv5+nadrT08isD+WESRpa2yD1MnLWQfAo6INU7UYhf40MVzMjkGZe61Xo1KlGAj6m Mi/f91LQkXQz726bnABvPfxX5I6Pr8O+ZOC/DuD1YbOJYxq3ZYQe1+KYhmiErWeICmKp m8ydotMyVOrlFxJK3b62vQ5OUBsR/1ztslxaV7SOvS7GVRDaJ5Cm6fURcX0OjUXFyIaD HNs8ODlF54Gz+5HZg90t2ALDJYPMc+L5QmxuVfR2Q4nEss7KsmStlue819XXvJT7PIQt 1mMg==
X-Gm-Message-State: AOAM533v3DKJbAq6SR5NCGnGWY85ADy+xymrSW1sQAJvQamgE/KdX1fQ BMJpIMse5HgLIiz+DbBZjpV4Y515vx5cLHECKWA=
X-Google-Smtp-Source: ABdhPJyKLduJz5+u84XqZlkqu2Pc0ZoXWiR9awCfPSjdX184lpup5AjE7McXGoWIVMdYECZFRAeCl8bqoDgxXWjOg14=
X-Received: by 2002:a05:6830:151a:: with SMTP id k26mr2922771otp.144.1604707352196; Fri, 06 Nov 2020 16:02:32 -0800 (PST)
MIME-Version: 1.0
References: <160442801878.21168.17412260809706411361@ietfa.amsl.com> <3C5BB93B-5E8C-42AD-B1C1-F96C359EC110@tix.at> <20201104163619.GR39170@kduck.mit.edu> <CAOj+MMENNbMSs6hDJ9SvEinKd5nVnUWB5EPWGbCvKysyBms9Ug@mail.gmail.com>
In-Reply-To: <CAOj+MMENNbMSs6hDJ9SvEinKd5nVnUWB5EPWGbCvKysyBms9Ug@mail.gmail.com>
From: Erik Kline <ek.ietf@gmail.com>
Date: Fri, 6 Nov 2020 16:02:21 -0800
Message-ID: <CAMGpriW2vQYijU_KLVqYG=dnqoH_0yvuRAOODAWib726dHNYtQ@mail.gmail.com>
To: Robert Raszuk <robert@raszuk.net>
Cc: Benjamin Kaduk <kaduk@mit.edu>, Christoph Loibl <c@tix.at>, "idr@ietf. org" <idr@ietf.org>, The IESG <iesg@ietf.org>, draft-ietf-idr-flow-spec-v6@ietf.org, idr-chairs@ietf.org
Content-Type: multipart/alternative; boundary="000000000000b23dc705b3790b91"
Archived-At: <https://mailarchive.ietf.org/arch/msg/idr/42wwbVuafG9QLefGUseYHP0RWks>
Subject: Re: [Idr] Benjamin Kaduk's Discuss on draft-ietf-idr-flow-spec-v6-19: (with DISCUSS and COMMENT)
X-BeenThere: idr@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Inter-Domain Routing <idr.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/idr>, <mailto:idr-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/idr/>
List-Post: <mailto:idr@ietf.org>
List-Help: <mailto:idr-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/idr>, <mailto:idr-request@ietf.org?subject=subscribe>
X-List-Received-Date: Sat, 07 Nov 2020 00:02:37 -0000

FWIW, I had assumed that the 1/2/4 byte formats were for encoding the
non-zero parts of a flow label, i.e. flowlabels 1-255 could fit in 1 byte,
256-65535 in 2 bytes, and everything else up to 2^20-1 in 4 bytes.

If that's true, it seemed reasonable to me.  Also reasonable, I would
think, is to just specify, say, a fixed 3 byte unsigned int.

I think it would be valuable to keep the flow label matching functionality,
not least because it can be used to group into a flow the fragments where
the upper layer header info is not present.

On Fri, Nov 6, 2020 at 3:01 PM Robert Raszuk <robert@raszuk.net> wrote:

> Hi,
>
> For the record of part of this discussion regarding size of flowspec v6
> flow label field I am perfectly ok to have it fixed size of 20 bit field.
>
> As a matter of fact if we assume that flow label allocation nature is to
> be dynamic by IETF recommendation I am also perfectly fine to remove flow
> label match from flow spec v6 all together.
>
> Kind regards,
> Robert.
>
> On Wed, Nov 4, 2020 at 5:36 PM Benjamin Kaduk <kaduk@mit.edu> wrote:
>
>> Hi Christoph,
>>
>> On Wed, Nov 04, 2020 at 12:41:13PM +0100, Christoph Loibl wrote:
>> > Hi Benjamin,
>> >
>> > Thanks for your review of the document. I have edited the document and
>> I think cleared your discuss with the changes (please see my comments
>> inline).
>>
>> Also inline, though I will not specifically comment on the (many) bits
>> that
>> look good as-is.
>>
>> > Since I have a few more changes to apply the new version is not yet
>> online (however I attached the modified version to this message). Since the
>> draft submission is closed now I will ask Alvaro to publish the changed
>> document today.
>> >
>> > Cheers  Christoph
>> >
>> >
>> > > ## DISCUSS:
>> > > A fairly minor point, but I think that allowing Type 13 (flow label)
>> > > component values to be encoded as 2-byte quantities encourages the
>> > > selection of non-random flow label values, and thus violates the
>> > > guidance from RFC 6437 that these values "should be chosen such that
>> > > their bits exhibit a high degree of variability" and that "third
>> parties
>> > > should be unlikely to be able to guess the next value that a source of
>> > > flow labels will choose." While having the short 1-byte encoding for a
>> > > flow label of 0 might be reasonable, a 2-byte label can represent at
>> > > most 16 bits of the 20-bit identifier space, discouraging the use of
>> the
>> > > high 4 bits, when such bits of unpredictability are scarce already.
>> > > Let's discuss how big an issue this is and what might be done to
>> > > mitigate it.
>> >
>> > The Type-13 component is not the flow label itself. It is only the
>> filter that allows to match on that particular field in the IPv6 header of
>> the packets. If the flow-label encoded in the packet (unlikely, but
>> possible) can be encoded in 8,16-bits one could encode the match filter in
>> a shorter form. However, this may be very unlikely. This specification does
>> not choose those flow labels, but allows to match on flow labels in packets
>> (usually generated by other systems).
>>
>> I understand that this specification is describing patterns to match flow
>> labels and not the flow labels themselves, yes.  However, this document is
>> saying that, *if* the flow labels themselves have a certain structure,
>> then
>> the flow specification (from this document) to match the flow labels has a
>> more compact encoding.  This, in turn, sets up an incentive for the party
>> assigning flow labels to conform to the specific structure of flow label
>> values, in order to take advantage of the more compact encoding.  I
>> believe
>> that using any kind of substructure when assigning flow label values is
>> contrary to the guidance of RFC 6437 (and, relately, harms the overall
>> security of the internet), and thus am supplying resistance to a mechanism
>> that incentivizes using such internal structure in flow label values.
>> The discussion that I would like to have here relates to the magnitude of
>> incentive in question (to use specific substructure when generating flow
>> label values), which in turn is related to how much benefit there is from
>> shaving two bytes off the flow specification encoding.  I do not believe
>> that your comment above is trying to make such an assessment on the
>> magnitude of the incentive in question or the value of having the slightly
>> more compact encoding available.
>>
>> > > Please also confirm that we are providing all the information
>> required of
>> > > us by RFC 5701 and 5575bis (see comments on Section 6.1); I am not
>> sure
>> > > whether I am reading the references correctly in these regards.
>> >
>> > We added the definition of the Local Administrator Field in the
>> Extended community to be aligned with the IPv4 rt community:
>> >
>> > ADDED:
>> >        The Local Administrator sub-field contains a number from a
>> numbering
>> >           space that is administered by the organization to which the IP
>> >           address carried in the Global Administrator sub-field has been
>> >           assigned by an appropriate authority.
>> >
>> > >
>> > > There seems to be an error in the sample code (flow_rule_cmp_v6()):
>> the
>> > > snippet
>> > > if comp_a.offset < comp_b.offset:
>> > > return A_HAS_PRECEDENCE
>> > > if comp_a.offset < comp_b.offset:
>> > > return B_HAS_PRECEDENCE
>> > > duplicates the condition, whereas the condition should be swapped for
>> > > correct operation.
>> >
>> > This in fact is a problem in the code that has been corrected, and a
>> test has been added.
>>
>> I am happy to hear that a test has been added in addition to fixing the
>> bug.
>>
>> > > ## COMMENT:
>> > > Thanks for this straightforward and easy-to-read document! Just a few
>> > > minor comments.
>> > >
>> > > Section 3.3
>> > >
>> > > This component uses the Numeric Operator (numeric_op) described in
>> > > [I-D.ietf-idr-rfc5575bis] Section 4.2.1.1. Type 3 component values
>> > > SHOULD be encoded as single octet (numeric_op len=00).
>> > >
>> > > Why only SHOULD? (Likewise for Section 3.4, 3.5, etc.)
>> > >
>> >
>> > This is because we want these types to be aligned with the IPv4 types
>> as much as possible in order to allow for reusable code.
>>
>> Ah, of course.  I should have checked that before commenting; sorry.
>>
>> > > Section 3.7
>> > >
>> > > Contains a list of {numeric_op, value} pairs that are used to match
>> > > the 20-bit Flow Label IPv6 header field ([RFC8200] Section 3).
>> > >
>> > > [It seems that RFC 8200 §3 just points to RFC 8200 §6, which itself
>> > > mostly points to RFC 6437. I don't know if it is useful to
>> > > short-circuit some of those references.]
>> >
>> > Flow Spec does not care about what and how this Flow Label is actually
>> encoded. It matches against this particular 20-bit field in the header. I
>> think it is better to keep this reference.
>> >
>> > > Section 3.8.1
>> > >
>> > > The following example demonstrates the prefix encoding for: "packets
>> > > from :🔢5678:9A00:0/64-104 to 2001:DB8::/32 and upper-layer-
>> > >
>> > > Where is the "/64-104" notation introduced? I did not see it in RFC
>> > > 4291. (It also appears in §3.8.2, though the latter uses "/65-104"
>> > > to demonstrate the operation of padding.)
>> >
>> > Appended this to the Type-1 compontent section:
>> >
>> >    Note: This Flow Specification component can be represented by the
>> >    notation ipv6address/length if offset is 0, or ipv6address/offset-
>> >    length.  The ipv6address in this notation is the textual IPv6
>> >    representation of the pattern shifted to the right by the number of
>> >    offset bits.  See also Section 3.8.
>>
>> Thanks!
>>
>> > >
>> > > Section 4
>> > >
>> > > The definition for the order of traffic filtering rules from
>> > > [I-D.ietf-idr-rfc5575bis] Section 5.1 is reused with new
>> > > consideration for the IPv6 prefix offset. As long as the offsets are
>> > > equal, the comparison is the same, retaining longest-prefix-match
>> > > semantics. If the offsets are not equal, the lowest offset has
>> > > precedence, as this flow matches the most significant bit.
>> > >
>> > > I will note just to confirm my understanding that this procedure seems
>> > > to give higher precedence to, e.g., 1234::/1-4 than to
>> > > :🔢5678:9a00/81-128 even though the latter is inspecting more bits
>> > > of the address/prefix. (To be clear, some choice has to be made and I
>> > > have no reason to prefer a different one over this one, I am just
>> > > exploring the consequences of this choice.)
>> >
>> > Yes this is the case. In general the lower offset generates a lower
>> number of consecutive IPv6 address blocks that would match (offset=0
>> matches IP addresses only from a single prefix, offset=1 matches 2 blocks,
>> offset=2 4, ...), this can be extended to offset=127 which matches every
>> second IPv6 address.
>> >
>> > > It is again (as per 5575bis) surprising that we allow for a
>> "not-equal"
>> > > comparison of differently encoded (e.g., flow label) values that have
>> > > the same semantic meaning, but I expect the same response (and no
>> > > document change) as when I made the comment the first time.
>> >
>> > While we add IPv6 functionality into Flow-Spec (rfc5575bis) changing
>> any of the fundamental behaviour is out of the scope of this document.
>> >
>> > > Section 5
>> > >
>> > > The validation procedure is the same as specified in
>> > > [I-D.ietf-idr-rfc5575bis] Section 6 with the exception that item a)
>> > > of the validation procedure should now read as follows:
>> > >
>> > > Are there also any AFI/SAFI differences from 5575bis to consider in
>> > > terms of "routes received over"/etc.?
>> > >
>> >
>> > Flow-Spec V6 uses a different AFI/SAFI but this is already covered in
>> the rfc5575bis text when it comes to validation:
>> >
>> >    The validation process described below validates Flow Specifications
>> >    against unicast routes received over the same AFI but the associated
>> >    unicast routing information SAFI:
>>
>> Thank you for the extra explanation; I suspected that there was something
>> like this that I was missing.
>>
>> >
>> > > Section 6.1
>> > >
>> > > This extended community uses the same encoding as the IPv6 address
>> > > specific Route Target extended community [RFC5701] Section 2 with the
>> > > high-order octet of the Type always set to 0x80 and the Sub-Type
>> > > always TBD.
>> > >
>> > > RFC 5701 suggests that since we are allocating the "TBD" sub-type
>> value,
>> > > we should also state what the semantics of the "local administrator"
>> > > 2-octet field are.
>> > >
>> >
>> > This has been edited into the document. See also the DISCUSS section.
>> >
>> > > Interferes with: All BGP Flow Specification redirect Traffic
>> > > Filtering Actions (with itself and those specified in
>> > > [I-D.ietf-idr-rfc5575bis] Section 7.4).
>> > >
>> > > I think we are supposed to also state what action to take when
>> > > encountering interfering actions.
>> >
>> > This is already in rfc5575bis Section 7.7, and also applies to this
>> document.
>> >
>> > >
>> > > Section 7
>> > >
>> > > It was a little surprising to me that the inability to match (e.g.)
>> TCP
>> > > flags or Port values on non-initial IP fragments was not reiterated in
>> > > the security considerations of 5575bis. I don't think it would make
>> > > sense to mention that just in this document's security considerations,
>> > > though -- if we are to change anything it should be in 5575bis. But I
>> > > guess we had that conversation already, at
>> > >
>> https://mailarchive.ietf.org/arch/msg/idr/zvpLPjllvKWFuyraZUVz8lB8Wz0/
>> > > and thus no change is expected.
>> >
>> > While IPv4 and IPv6 fragmentation serves the same pupose the encoding
>> is different so we needed to redefine thi section. FS is working stateless
>> on the packets and can only match on what is in the packet.
>>
>> Exactly.  I am proposing that this restriction (working stateless and
>> cannot match [application-layer headers] things not in the packet) is
>> mentioned again in the security considerations section.  I believe that
>> this limitation can be described in a way that is agnostic to IPv4 vs
>> IPv6,
>> and thus would be better placed in 5575bis than in this document.
>>
>> > I cannot access the url from above.
>>
>> Huh, interesting.  Perhaps if you log in with your datatracker
>> credentials?
>> I had specifically tried to find one in the idr archive, since the iesg
>> archive is definitely non-public.
>> The referred-to message is "Re: [Idr] Benjamin Kaduk's Discuss on
>> draft-ietf-idr-rfc5575bis-22: (with DISCUSS and COMMENT)" from  Fri, 24
>> April 2020 02:12 UTC, Message-ID: <20200424021209.GR27494@kduck.mit.edu>du>.
>>
>> Thanks again,
>>
>> Ben
>>
>> > >
>> > > Section 8.1.2
>> > >
>> > > (side note) it seems a little weird to have the IPv4 version be the
>> one
>> > > that gets the unqualified (e.g., "Destination Prefix") name.
>> >
>> > This is because we are not changing names of rfc5575bis to avoid
>> confusion.
>> >
>> >
>>
>>