Re: [Int-dir] Intdir telechat review of draft-ietf-masque-connect-ip-10

[David Schinazi <[e-mail]>]

Hi Joe, thank you for your review. Responses inline.
David

On Fri, Apr 14, 2023 at 8:29 PM Joseph Touch via Datatracker <
[e-mail]> wrote:

> Reviewer: Joseph Touch
> Review result: Not Ready
>
> This review focuses on the behavior of the tunnel from an IP perspective.
> The
> HTTP aspects are not considered.
>
> In summary, the document presents a method for tunneling IP packets over an
> HTTP connection. Its terminology and discussion is confusing, being
> presented
> largely from the perspective of the HTTP mechanism and not sufficiently
> from
> the perspective of the resulting IP tunnel that is provided. Details are
> provided below.
>
> --
>
> The terminology is confusing. Because HTTP is a client/server protocol, it
> makes sense to refer to a web (or HTTP) proxy, a single entity that acts as
> both server and client, relaying requests received on the server side and
> initiating them on the client side. An IP proxy would relay datagrams
> received
> on one IP address and issue them with at least some new addressing. What
> appears to be described here is way to create an IP tunnel over a web
> connection, which is not a proxy. It is (and should be called) just an IP
> tunnel. The web client and server are not co-located as they are in a
> proxy;
> the term proxy is nonsensical in this context.
>

The target audience for this document is HTTP implementers, since you need
an HTTP stack in order to implement this. Because of this, it is more
helpful
to use HTTP terminology instead of tunneling terminology.

Sec 4.6 indicates that ipproto can be any number in [IANA-PN]; this is not
> consistent with the document title or remainder of the document
> description.
> That list is of protocols that can be payloads of IP packets, not of IP
> protocols - e.g., IPv6 is not 6 but 41, and when it is 41 it describes
> IPv6 as
> a payload of an outer IP packet, not an IPv6 packet. There does not appear
> to
> be a way to indicate the outermost IP packet protocol version, e.g.,
> https://www.iana.org/assignments/version-numbers/version-numbers.xhtml.
> The
> claim that implementations MUST walk the chain of extensions to find the
> protocol is unboundeded as stated; it needs to be limited, e.g., it needs
> to
> stop at the first non-IP protocol (e.g., it should not “find” TCP in IP in
> UDP
> in IP, if limited to protocol=6).
>

I don't understand what you mean here. IPv6 extension headers all contain
the
Next Header field, but IP protocols like TCP and UDP do not. So the scenario
that you describe of "UDP in TCP" isn't possible given the wire format.

Note that this also raises a further concern, that of the utility of an IP
> tunnel. The Internet is composed of links and tunnels emulate links. This
> document appears to describe tunnels that can be limited to specific types
> of
> IP packets, but it overlooks the outermost IP packet version. It also
> overlooks
> support for non-IP protocols that are critical to IP operation, e.g., for
> IPv4,
> these include ICMP and ARP. If an “IP proxy” were limited to protocol 6
> (TCP),
> it would block ICMP (which protocol 1) and ARP (which isn’t an IP packet at
> all).


ARP is not required to use an IP tunnel. The tunnel endpoints do not have
MAC
addresses, so there is no need for ARP's translation of IP to MAC.


> Note that this is also inconsistent with the ROUTE_ADVERTISEMENT capsule
> as described in Sec 4.7.3, which asserts that ICMPs are exceptions to the
> indicated protocol (at a minimum, this needs to be noted here as well).
>

Good catch! This is somewhat mentioned in security considerations but I
added a
sentence to the Scoping section:
https://github.com/ietf-wg-masque/draft-ietf-masque-connect-ip/commit/d8a95d53cfb49914e6c1e99a45deece1330cda12

Sec 4.7, although defining new capsules, is really about tunnel
> configuration.
> It should be moved to a top-level section and presented as such.
>
> Except for Sec 4.7, which explains how IP addresses and routing can be
> obtained
> over the HTTP connection, the document views the necessary behavior from
> only
> the perspective of the tunnel ingress as an HTTP client and the tunnel
> egress
> as an HTTP server.


That's not right, most of the document is written in terms of "IP proxying
endpoints"
because most of the mechanisms are symmetric. Do you have a specific
example?


> It is missing the way in which these ingress/egress
> components are viewed a their endpoints, e.g., to be useful as an IP
> tunnel,
> these need to appear as attached to (possibly virtual) network interfaces,
> i.e., to appear as a link, which allows them to then be used for local
> processes (via sockets), packet forwarding, etc.


That's an implementation detail that doesn't belong in this document. Most
implementations will indeed use virtual TUN interfaces, but it's not a
requirement.
There is a known implementation with transport protocols in userspace that
doesn't
do what you describe.


> E.g., the mechanism in Sec 4.7
> is only one way that endpoints may be configured; others should be
> discussed –
> including using DHCP or IPv6 RAs over the tunnel itself.
>

We support assigning addresses out of band (e.g., in a config file) but
don't see
the need for DHCP or RAs through the tunnel.

Sec 7 explains many aspects of IP packet handling that are already
> sufficiently
> described in RFCs 1122 and 1812 (for IPv4) and 8200 (for IPv6).  That
> section
> unnecessarily repeats that detail and is also vague as to where particular
> behaviors are to be realized. I.e., parsing the IP header, hopcount
> processing,
> and packet forwarding. The document should just clearly state that tunnels
> behave as links (as explained in draft-ietf-intarea-tunnels) and the rest
> of
> processing happens *outside* the HTTP server as defined for a host or
> router.
>

Where the code lives is an implementation detail that we don't need to
constrain.

It seems incorrect to design these tunnels as something less than a typical
> IP
> interface, esp. for IPv6 (autoconfig, neighbor discovery). Doing so only
> serves
> to invite incompatibility and/or undermine existing mechanisms that are
> useful
> in detecting misconfiguration (e.g., duplicate address detection). If
> there is
> believed to be a technical justification for this limitation, the argument
> needs to be presented and its implications reviewed (e.g., an IPv6 tunnel
> that
> isn’t a true IPv6 interface may not be useful as an IPv6 tunnel).
>

The WG members who are implementing this don't have a need for a true IPv6
interface. IKEv2 made the same design choice and it's been working fine.
https://www.rfc-editor.org/rfc/rfc7296#section-3.15.3

The end of section 7 on link MTUs needs to address the additional
> requirement
> of IPv6 EMTU_R of 1500 bytes. Source fragmentation and reassembly need to
> be
> addressed for both IPv4 and IPv6, and path-fragmentation for IPv4 (unless
> it is
> not supported, which needs to be noted).
>

I'm not sure we need to go into these details that are well covered by IP
RFCs.


> Sec 8 should be clear on what entity is responsible for ICMP packet
> generation
> and receipt. This presumably should be the HTTP endpoint device, not the
> HTTP
> client or server. I.e., the tunnel transits ICMP packets but tunnel
> endpoints
> should neither generate nor consume them, just like any other link.
>

Similar to above, where the code lives is an implementation detail.

As other reviewers have noted, Sec 10 on nested congestion control is quite
> thin. The current statement is equivalent to “if you KNOW congestion is
> nested,
> turn it off” – it should be the opposite, i.e., “turn congestion ON only
> if you
> KNOW congestion is NOT nested”.
>

Fair enough. We're tweaking that section to be more permissive:
https://github.com/ietf-wg-masque/draft-ietf-masque-connect-ip/pull/162

Section 11.1 refers to fragmented packets; it should refer to them as not
> being
> able to be “re-fragmented”; source-generated fragments are still
> fragmented and
> can cross the tunnel subject to the tunnel MTU.
>

The use of "fragmented" in that section refers to QUIC datagram frames,
which
cannot be fragmented - this isn't about IP fragmentation.

Section 11.1 should require that the IP tunnel be created with a given MTU
> and
> to indicate that to the endpoint where the client/servers reside; issuing
> ICMP
> PTBs is the responsibility of those endpoints when they are seeking to use
> those tunnel endpoints, NOT of the tunnel endpoint itself. Tunnels are
> links;
> links do not issue ICMPs.
>

This use of ICMP is a pragmatic solution to ensure that the link doesn't
violate
the minimum IPv6 link MTU. Some working group members felt strongly that
we needed a solution for this.

Sec 12 should indicate how access to the HTTP proxy itself should be access
> limited, i.e., to avoid presenting an arbitrary traffic injection point.


Are you talking about things like <<don't set your root SSH password to
"password">>?


> It should also address the potential use of HTTP-level encryption (e.g.,
> TLS,
> DTLS) to protect the tunnel contents and tunnel configuration exchanges.
>

 We require the scheme to be https, which means that QUIC or TLS is
mandatory.