Re: [nvo3] RTG Dir QA review of draft-ietf-nvo3-gue

"Adrian Farrel" <adrian@olddog.co.uk> Tue, 28 June 2016 09:10 UTC

Return-Path: <adrian@olddog.co.uk>
X-Original-To: nvo3@ietfa.amsl.com
Delivered-To: nvo3@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 5102412D893; Tue, 28 Jun 2016 02:10:06 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.62
X-Spam-Level:
X-Spam-Status: No, score=-2.62 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01] autolearn=ham autolearn_force=no
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 RffJU2hMaibN; Tue, 28 Jun 2016 02:10:02 -0700 (PDT)
Received: from asmtp2.iomartmail.com (asmtp2.iomartmail.com [62.128.201.249]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id EF4F012DB14; Tue, 28 Jun 2016 02:09:59 -0700 (PDT)
Received: from asmtp2.iomartmail.com (localhost.localdomain [127.0.0.1]) by asmtp2.iomartmail.com (8.13.8/8.13.8) with ESMTP id u5S99uth025254; Tue, 28 Jun 2016 10:09:57 +0100
Received: from 950129200 ([79.141.128.249]) (authenticated bits=0) by asmtp2.iomartmail.com (8.13.8/8.13.8) with ESMTP id u5S99t18025219 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NO); Tue, 28 Jun 2016 10:09:56 +0100
From: Adrian Farrel <adrian@olddog.co.uk>
To: 'Tom Herbert' <tom@herbertland.com>
References: <0e6201d1cb03$5f2f2280$1d8d6780$@olddog.co.uk> <CALx6S379htYqZaWXqEXKFL92jrrz9e-WSS7F3RzmYC28NitRjQ@mail.gmail.com>
In-Reply-To: <CALx6S379htYqZaWXqEXKFL92jrrz9e-WSS7F3RzmYC28NitRjQ@mail.gmail.com>
Date: Tue, 28 Jun 2016 10:09:52 +0100
Message-ID: <063201d1d11c$d5322da0$7f9688e0$@olddog.co.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
X-Mailer: Microsoft Outlook 14.0
Thread-Index: AQKoQVNa99MnPTv+9c+iL7VJfcDpcAHKYBQlnkNT77A=
Content-Language: en-gb
X-TM-AS-MML: disable
X-TM-AS-Product-Ver: IMSS-7.1.0.1679-8.0.0.1202-22418.006
X-TM-AS-Result: No--23.603-10.0-31-10
X-imss-scan-details: No--23.603-10.0-31-10
X-TMASE-MatchedRID: yebcs53SkkCnykMun0J1wgRH1Nr7oERdVBDQSDMig9Ge9toQ6h6LE6vJ NkBKhOMlPlMse0NxtvYZQkec6uAurPz+fK9hbmIfnJ5tL+LbGOOSTnFzEHOIwaq9wgXVNwtgSaj 91MdURVGJeFBKjl9/e44aY32cTPHelh5qb5HiiQe2AZ59hFA16+7sedMuEdnYofPZqomcp3bRMP pR62Sh5cqLzInr2sJXQTtyAWXgJsnGUecy8W1eahNEPNwNrw/ru1b0t4ooa9vFpA1uJFd1miksj JeAae2ua6DP2Vs3kjTEgyASJMxFeUNqEUgJyw9cSDkh6bW+bcfDHSNFHFxB8yBs0OU9P6tb3Rfs kShJlmoEP40z+T9gYHaMyTlum7BqW/aHOX+rDdECNMj/7qB/gzGZtPrBBPZrfj4DHOYjBjgzJx1 oPS12vggmxBIDskgzndl2t9HLYUcoQMAacCt1z0+4wmL9kCTx8GRhP/nTHNaJYxhJaBLFLT/zGx sl8T/65M3BSxL9NtkF6UXumErjhMUC0EVMuztdw2taljzThMZ/r8x3wtvaX/gnJH5vm2+gw2Gv9 44oIB7mDNNFgM9dFB/VG9Ww0iWOr4Tjl93LJlcdxBAG5/hkWxkyt9CZ6KPlVz8J52OVy+TuvYzN wLSgH6lUJtroc1+hOU/FIO1t2ib3yGhlFWBvldWxbZgaqhS0O6X0o0bA9jpvOxpHnc6c8vBUq5J suvxSe/ufEHBerpOcMqhH3VPuICowKSWrVbTysBytgTF4TTTPsi61oQ+4giU/W8XGq5b2woOm9w 4MBMXuZZG0yT48ZplvK+1d2WtYApU/URSa2nmeAiCmPx4NwFkMvWAuahr8+gD2vYtOFhgqtq5d3 cxkNQP90fJP9eHt
Archived-At: <https://mailarchive.ietf.org/arch/msg/nvo3/LepdDa9LqQPha_rGJtpLvEDTxYY>
Cc: rtg-dir@ietf.org, nvo3@ietf.org, draft-ietf-nvo3-gue.all@ietf.org, int-area@ietf.org
Subject: Re: [nvo3] RTG Dir QA review of draft-ietf-nvo3-gue
X-BeenThere: nvo3@ietf.org
X-Mailman-Version: 2.1.17
Precedence: list
Reply-To: adrian@olddog.co.uk
List-Id: "Network Virtualization Overlays \(NVO3\) Working Group" <nvo3.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/nvo3>, <mailto:nvo3-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/nvo3/>
List-Post: <mailto:nvo3@ietf.org>
List-Help: <mailto:nvo3-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/nvo3>, <mailto:nvo3-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 28 Jun 2016 09:10:07 -0000

Hi Tom,

Thanks for engaging.

Some snipping...

> > Being forced to read this document, I'm afraid I was required to enter a
> > third-party IPR disclosure because of the IPR already disclosed against
> > draft-ietf-mpls-in-udp that became RFC 7510. This should show up on the
> > NVO3 mailing list.
> >
> We saw that. Note that this applies to several other UDP encapsulation
> protocols.

You might (or might not) want to enter third-party disclosures yourself against those other I-Ds which I am fortunate enough to have not been involved with.

> > In 3.1 when describing the contents of the Proto/ctype field it would
> > be helpful to b crystal clear of which set of IP protocol numbers you
> > are using. Maybe a reference to
> > http://www.iana.org/assignments/protocol-numbers/protocol-numbers.xhtml
> >
> Okay. Offhand, all protocol numbers except for IPv6 extension headers
> are normally allowed. Protocol #59 is allowed even with IPv4.

I think you will have this covered, but to be clear, I didn't mean what subset of protocols, I meant which set of numbers used to identify protocols (e.g., not Ethertypes).

> > I wonder whether you have possibly overdone the future-proofing since
> > you have defined no uses for flags nor any possible fields or extension
> > fields. The mechanism you have defined could actually be added later if
> > needed (although it is possible to believe this to not happen for a
> > number of years) by assigning a flag (the E flag as now), defining a new
> > field to contain extension flags, and proceeding as described.
>
> Extensions have been defined in several other drafts. We are
> consolidating a core set into one draft (fragmentation, security,
> payload transform, checksum, and remote checksum offload). An
> additional option for network virtualization is defined in
> draft-hy-gue-4-secure-transport.

OK. Maybe some text like...
This document does not define any extensions, but a number of extensions are defined in [reference].

And then add an Informative reference.

> > 3.3
> >
> >    Flags may be paired together to allow different lengths for an
> >    optional field. For example, if two flag bits are paired, a field may
> >    possibly be three different lengths. Regardless of how flag bits may
> >    be paired, the lengths and offsets of optional fields corresponding
> >    to a set of flags must be well defined.
> >
> > This works, of course, but aren't you again being too flexible and too
> > clever? For a field that might have two lengths, you are not saving any
> > bits. Why not simply allow a field with two possible lengths to simply
> > be defined as two different fields?
>
> Then we have to process it as two different fields. We currently have
> defined security flags to be two bits allowing three possible sizes
> (64, 128, or 256bits). Also this allows for creating multibit fields
> in the flag bits themselves, similar to how Recur is defined in GRE.

Hmmm, well, OK.
If you've thought this through.

> > 3.4
> >
> >    An encapsulator and decapsulator MUST agree on the meaning
> >    of private data before using it.
> >
> > How? Using an OID? Using a control message?
>
> It's implementation specific. For many deployments, like ours at
> Facebook, configuration is sufficient for such things. GUE does not
> require control messages for normal protocol operation, they could
> defined (some C-bit=0 messages) but that is out of scope for this
> document. The key point is that agreement and meaning of private data
> is matter between the two communicating endpoints. GUE does not
> preclude the development of a commonly used definitions for private
> data, say for instance a set of TLVs like Geneve might define, but we
> shouldn't need to declare that in the base protocol.

OK. Got it.
Maybe a few words like...

   An encapsulator and decapsulator MUST agree on the meaning
   of private data before using it. The mechanism to achieve this
   agreement is outside the scope of this document but could 
   include implementation-defined behavior, coordinated
   configuration, in-band communication using GUE control
   messages, and out-of-band messages.

> > I am not enthusiastic about allowing "private data" in the packet
> > header. I can see its use for specific functions that you have called
> > out (security and identifiers), but even then I am not too comfortable.
> > Actually, wouldn't security and identifiers by standard fields rather
> > than private data?
>
> Yes, security and identifiers are being defined as well known
> extensions. The reason for having a private data section is to allow
> implementations or sites to extend the protocol for their own
> purposes. They may or may not intend standardize. What we don't want
> to happen is that people randomly reserved bits for their own purposes
> so that in the future we we need to define a standard use for them we
> can't because there is some significant deployment already using them.
> This is especially a concern for an nvo3 protocol which is more of DC
> protocol then protocol used on the Internet so we anticipate more
> "customization".

Yeah, I get the use case. I just don't like it :-)
It's a hard line to draw. There's payload and there's overhead. I don't see that there is a way here to make people do the right thing - perhaps we shouldn't worry about that?

An option is to define a new payload protocol and put it all in there. It's marginal.

Anyway, I still think you might consider the use of an OID to help prevent surprises while processing what is otherwise unstructured/unknown data.

> > Recall that these UDP packets will be exchanged by many implementations
> > and the ideal is that every speaker should be able to understand every
> > packet. Also recall that things that might be used as covert channels
> > are best avoided.


> > In 5.4
> >
> >    ...otherwise malformed
> >    header, it must drop the packet and may log the event.
> >
> > That is better as MUST and MAY according to other usage in the draft.
> > You might do well to check all uses of must/should/may to check whether
> > they could/should be in upper case.
> >
> Okay.
> 
> > When saying "may log the event" it is traditional (and probably good) to
> > also give advice about thresholds and risks of log-swamping when under
> > attack.
>
> Do you know if there is an RFC to reference?

Hmm, I don't have a reference to this being required or advised.
I do have an example off the top of my head...
RFC 7274 section 3.1.1

The thinking here is that logging usually uses more resources than a data plane message, so it is possible to swamp a system through a relatively easy attack. Thus rate limiting on logs is a good precaution. Or event counts rather than logs, etc.

> > In 5.5
> >
> >    It
> >    may encapsulate a GUE packet in another GUE packet, for instance to
> >    implement a network tunnel.
> >
> > Doesn't that require a protocol number to be assigned for GUE?
> >
> Technically, no, This could look like IP|UDP|GUE|UDP|GUE. Will clarify.

Aha! Thanks.

> > 5.6.1 has
> >
> >    The source port set in the UDP
> >    header must be the destination port the peer would set for replies.
> >
> > But 3.1 has
> >
> >       o Source port (inner flow identifier): This should be set to a
> >         value that represents the encapsulated flow. The properties of
> >         the inner flow identifier are described below.
> >
> >       o Destination port: The GUE assigned port number, 6080.
> >
> > You can't achieve both and it would seem that the only way GUE can be
> > "symmetrical" is to use the same source port in both directions.
>
> 3.1 text is a "should" for this reason. Will clarify that symmetric
> port number for NAT is an exception to source port assignment.

Thanks

> > The text in 5.6.2
> >    This method
> >    is problematic since ports numbers do not have global meaning
> >    ([RFC7605]) and a packet which is not GUE but destined to the same
> >    port number could be misinterpreted.
> > ...sent me scurrying to 7605. I think the point is not that the port
> >    number does not have global meaning, but that "It is important to
> >    recognize that any interpretation of port numbers -- except at the
> >    endpoints -- may be incorrect, because port numbers are meaningful
> >    only at the endpoints," and "Ultimately, port numbers indicate
> >    services only to the endpoints, and any intermediate device that
> >    assigns meaning to a value can be incorrect."
> >
> > Maybe similar enough, but I think that the intent of 7605 is to say
> > that a service may be run over many different port numbers so you can't
> > guarantee to find all instances of the service by looking for the port
> > number. I don't think the intent of 7605 is to say that something in the
> > network seen using port 6080 might not be GUE.
>
> Actually, I think that is exactly what it means.
> 
> > However, if you proceed with this you'll need to:
> > - resurrect draft-herbert-udp-magic-numbers
> > - make it  normative reference
> > - explain where the GUE magic number comes from
>
> Yes, there has been more discussion related to the UDP service
> identification problem on the SPUD lists. We borrowed the idea of the
> magic-number from draft-hildebrand-spud-prototype, but I have become
> less enthralled with it over time. I'm inclined to remove the concept
> of it from GUE draft.

Wfm.

> > 6.2
> >       o An encapsulator may occasionally change the inner flow
> >         identifier used for an inner flow per its discretion (for
> >         security, route selection, etc). Changing the value should
> >         happen no more than once every thirty seconds.
> >
> > I assume the limitation is because statistical load balancing will not
> > work if there is too much variance in hashable fields. However, 30
> > seconds may be a very large number of packets. Is there any science
> > behind that value?
>
> No :-) Just that thrashing does not seem to be a great idea. The Linux
> implementation already does this, after N retransmissions of a TCP
> connection we choose new flow identifiers (src port for UDP, IPv6 flow
> label, etc.) in hopes of finding a better path.

OK, maybe say "desirable to avoid thrashing or flapping the value".

Possibly say "should be configurable" and "a default value of 30 secs is recommended"

> > Section 7 says
> >       o GUE permits encapsulation of arbitrary IP protocols, which
> >         includes layer 2 3, and 4 protocols. This potentially allows
> >         nearly all traffic within a data center to be normalized to be
> >         either TCP or UDP on the wire.
> > How so "normalized to TCP"? That doesn't seem to be mentioned anywhere
> > in this document, and so either needs a reference or to be dropped.
>
> It's a reference the fact that network HW tend to optimize for UDP and
> TCP, so the consequence is that networks would only use UDP or TCP.

Ah, OK.
So GUE allows normalization to UDP and that, in turn, allows a DC hardware to be normalized to TCP or UDP.

> > I'm not convinced that punting security to a separate document is the
> > best idea, and it will make progressing this document hard unless you
> > can get that other document adopted and well advanced.
>
> The separate item is an option for stronger security. It is not
> required for deployment. We can elaborate on security issues for the
> base GUE draft (like use IPsec if you're paranoid, etc).

Thanks, that would be good.
That means this document would describe "base security" (which should be "good enough") with a pointer out to security document for "advanced security".

> Awesome review and very helpful!

You're welcome.
I expect I am now your "designated RTG reviewer" for this document for the rest of its life, so we will probably run into each other again during the lifecycle. I will try to retain cache :-)

Best,
Adrian
--
Support an author and your imagination.
Tales from the Wood - Eighteen new fairy tales.
http://www.feedaread.com/books/Tales-from-the-Wood-9781786100924.aspx
http://www.amazon.co.uk/Tales-Wood-Adrian-Farrel/dp/1786100924
Or buy from me direct.