Re: [tcpm] Benjamin Kaduk's Discuss on draft-ietf-tcpm-converters-17: (with DISCUSS and COMMENT)

mohamed.boucadair@orange.com Thu, 05 March 2020 13:20 UTC

Return-Path: <mohamed.boucadair@orange.com>
X-Original-To: tcpm@ietfa.amsl.com
Delivered-To: tcpm@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 6B24E3A1478; Thu, 5 Mar 2020 05:20:37 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.098
X-Spam-Level:
X-Spam-Status: No, score=-2.098 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, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, UNPARSEABLE_RELAY=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=orange.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 5njYq-ONe7Zs; Thu, 5 Mar 2020 05:20:33 -0800 (PST)
Received: from relais-inet.orange.com (relais-inet.orange.com [80.12.70.34]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 78CBB3A1477; Thu, 5 Mar 2020 05:20:33 -0800 (PST)
Received: from opfednr05.francetelecom.fr (unknown [xx.xx.xx.69]) by opfednr24.francetelecom.fr (ESMTP service) with ESMTP id 48YBFW4YZ5z1yKs; Thu, 5 Mar 2020 14:20:31 +0100 (CET)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=orange.com; s=ORANGE001; t=1583414431; bh=oUdS+GNJwPGyRFOIsbb4RCrhJjCnbQI5pDEKeUnJEgs=; h=From:To:Subject:Date:Message-ID:Content-Type: Content-Transfer-Encoding:MIME-Version; b=KEEy3E43lS+wAbzwoX12/JxQAx3Ap0u+agy6M1m2Hw2r7ZktDcH+7QxfUW/z1JdRu eBhvWFWwdIltpkx4MtqQTudOcH75lccrUtudw84mAqloBE8j7Qdl6z81QY0TvpOzdT KfulJLxyT8fV/+8KY1W1m108ufhXHE0AZ4ncyi1JkYmNvCZz1TlC3IUZm067L3W32o yL09L00N+UX8sgYWAQs3JPOComVSBgw9hEmAzgx+OvA/4wxC5EKcpoo7p3RhOioQ9t sOkD6rACnec758Wxp5knxG6NznNkB2p1mDEZFxxj6TW8EHmfVMYdwvykX+OKT4atzB kavDUN+0JPoHg==
Received: from Exchangemail-eme6.itn.ftgroup (unknown [xx.xx.13.107]) by opfednr05.francetelecom.fr (ESMTP service) with ESMTP id 48YBFW3j0CzySM; Thu, 5 Mar 2020 14:20:31 +0100 (CET)
Received: from OPEXCAUBMA2.corporate.adroot.infra.ftgroup ([fe80::e878:bd0:c89e:5b42]) by OPEXCAUBM8F.corporate.adroot.infra.ftgroup ([::1]) with mapi id 14.03.0487.000; Thu, 5 Mar 2020 14:20:31 +0100
From: mohamed.boucadair@orange.com
To: Benjamin Kaduk <kaduk@mit.edu>, The IESG <iesg@ietf.org>
CC: "draft-ietf-tcpm-converters@ietf.org" <draft-ietf-tcpm-converters@ietf.org>, "tcpm-chairs@ietf.org" <tcpm-chairs@ietf.org>, "tcpm@ietf.org" <tcpm@ietf.org>, Michael Scharf <michael.scharf@hs-esslingen.de>
Thread-Topic: Benjamin Kaduk's Discuss on draft-ietf-tcpm-converters-17: (with DISCUSS and COMMENT)
Thread-Index: AQHV8tLXCmWd7KVroU+LQlF0djSKfKg5wgsg
Date: Thu, 05 Mar 2020 13:20:30 +0000
Message-ID: <787AE7BB302AE849A7480A190F8B93303145F89D@OPEXCAUBMA2.corporate.adroot.infra.ftgroup>
References: <158340154155.14719.4463590324876056408@ietfa.amsl.com>
In-Reply-To: <158340154155.14719.4463590324876056408@ietfa.amsl.com>
Accept-Language: fr-FR, en-US
Content-Language: fr-FR
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-originating-ip: [10.114.13.245]
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: base64
MIME-Version: 1.0
Archived-At: <https://mailarchive.ietf.org/arch/msg/tcpm/dxD21WTF_jlvHM30QvVN-hAx1Fg>
Subject: Re: [tcpm] Benjamin Kaduk's Discuss on draft-ietf-tcpm-converters-17: (with DISCUSS and COMMENT)
X-BeenThere: tcpm@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: TCP Maintenance and Minor Extensions Working Group <tcpm.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/tcpm>, <mailto:tcpm-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/tcpm/>
List-Post: <mailto:tcpm@ietf.org>
List-Help: <mailto:tcpm-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/tcpm>, <mailto:tcpm-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 05 Mar 2020 13:20:38 -0000

Hi Ben, 

Thank you for the detailed review. Much appreciated!

Please see inline. 

Cheers,
Med

> -----Message d'origine-----
> De : Benjamin Kaduk via Datatracker [mailto:noreply@ietf.org]
> Envoyé : jeudi 5 mars 2020 10:46
> À : The IESG
> Cc : draft-ietf-tcpm-converters@ietf.org; tcpm-chairs@ietf.org;
> tcpm@ietf.org; Michael Scharf; michael.scharf@hs-esslingen.de
> Objet : Benjamin Kaduk's Discuss on draft-ietf-tcpm-converters-17:
> (with DISCUSS and COMMENT)
> 
> Benjamin Kaduk has entered the following ballot position for
> draft-ietf-tcpm-converters-17: Discuss
> 
> When responding, please keep the subject line intact and reply to all
> email addresses included in the To and CC lines. (Feel free to cut
> this
> introductory paragraph, however.)
> 
> 
> Please refer to https://www.ietf.org/iesg/statement/discuss-
> criteria.html
> for more information about IESG DISCUSS and COMMENT positions.
> 
> 
> The document, along with other ballot positions, can be found here:
> https://datatracker.ietf.org/doc/draft-ietf-tcpm-converters/
> 
> 
> 
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
> 
> I agree with Roman, and present a somewhat different take on some of
> the
> key aspects of the scoping discussion.
> 
> I have pretty fundamental concerns about describing this protocol as a
> "0-RTT" service when it both requires strong mutual
> authentication/authorization of the communicating endpoints and relies
> on the (local) network to provide those properties.  If IP in general
> provided the kind of mutual authentication assumed here, the internet
> would be a much more secure place!  Unfortunately, it does not and I
> think this document does its readers a disservice by transparently
> assuming that it does (in the main body of text; the security
> considerations do touch on the requisite details).  It would be better
> to discuss the proxy protocol separately from the deployment
> considerations that allow it to be used without additional set-up in
> specific deployment scenarios.
> 
> And that's only when considering the client authenticating the server!
> Mutual authentication also requires the server to authenticate the
> client and be able to make authorization decisions based upon that
> authenticated identity.  The deployment model presented here seems to
> imply a very tight coupling between the Transport Converter operator
> and
> the network service provider (in order to determine, based on client
> IP
> address, the authenticated client identity and access an authorization
> database); this seems to make it incompatible with the stated
> possibility of using a third-party Transport Converter.
> Additionally, it raises some questions along the lines of
> draft-arkko-iab-internet-consolidation and draft-iab-for-the-users.
> 
> The sketch of a solution for more open network environments in Section
> 9.2 is insecure (once a Cookie is generated and sent once, it can be
> freely replayed by an attacker during the Cookie lifetime, which
> defeats
> the authorization requirement of the convert protocol).  Either fix it
> or remove it entirely.
> 

[Med] I hope this one is resolved with the outcome of the Roman's DISCUSS. 

> Please resolve the internal inconsistency in Section 6.2.4 which says
> that TCP option Kind 0 MUST NOT appear in the list but then goes on to
> say that the list is padded with zeros to a 32-bit boundary.  (There
> is
> no listed requirement that the options are listed in any given order,
> which would be needed to assign a boundary between "listed options"
> and
> "padding".)

[Med] I see your point. Will be fixed. 

> 
> Why do we need the Cookie TLV at all if the underlying local network
> supplies all the authentication and authorization that the protocol
> needs?
> 

[Med] We had lengthy discussion about this in both MTPCP/TCPM. The WG wanted to have the same protection as TFO for inserting data in the SYN.

> Section 1.2 says that the Convert Protocol "is an application-layer
> protocol which uses a dedicated TCP port number", but the IANA request
> in Section 10.1 is for a service name without fixed port number.
> 

[Med] s/dedicated port number/specific port number.

How that specific port number is known to clients is part of the service discovery and is as such out of scope.

> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> Section 1.1
> 
>    scheme can be applied independently on clients and servers.  Other
>    improvements such as Selective Acknowledgments [RFC2018] or large
>    windows [RFC7323] require a new TCP option or to change the
> semantics
>    of some fields in the TCP header.  These modifications must be
>    deployed on both clients and servers to be actually used on the
>    Internet.  Experience with the latter TCP extensions reveals that
>    their deployment can require many years.  Fukuda reports in
> 
> Do we want to say "the latter" or "the latter class of"?

[Med] Not sure what "class of" TCP option would mean here.

> 
>    Timestamps.  [ANRW17] describes measurements showing that TCP Fast
>    Open (TFO) [RFC7413] is still not widely deployed.
> 
> Perhaps people are turned off by its privacy properties or lack
> confidence that their data can tolerate replay?

[Med] The measurements focus on the server's side. That is, the client presents the TFO but the negotiation fails. 

  This is not the most
> compelling example of a TCP feature that everyone should be using...

[Med] Those willing to optimize the connection delay may be interested. Will add pointers for the lack of support of other TCP options (e.g., MPTCP).

> 
>    More recently, 5G bonding experimentation has been conducted into
>    global range of the incumbent 4G (LTE) connectivity using newly
>    devised clients and a Multipath TCP proxy.  Even if the 5G and the
> 4G
>    bonding relying upon Multipath TCP increases the bandwidth, it is
> as
> 
> nit: the grammar is not right here; either "to increase" or "reliance
> upon", depending on the intended meaning, I think.

[Med] Changed to:

"Even if the 5G and the 4G bonding (that relies upon Multipath TCP) increases the bandwidth"

> 
>    mobile core.  In order to handle URLLC (Ultra Reliable Low Latency
>    Communication) for the next generation mobile network, Multipath
> TCP
>    and its proxy mechanism such as the one used to provide Access
>    Traffic Steering, Switching, and Splitting (ATSSS) must be
> optimized
>    to reduce latency [TS23501].
> 
> There are a lot of missing steps of reasoning here if we want to claim
> that this is a technical fact vs. marketing material.

[Med] This is not a claim. This is a requirement for the design of ATSSS: observing a delay before making use of a connection is to be avoided.   

> 
> Section 1.2
> 
>    The Convert Protocol provides 0-RTT (Zero Round-Trip Time)
> conversion
>    service since no extra delay is induced by the protocol compared to
>    connections that are not proxied.  Particularly, the Convert
> Protocol
>    does not require extra signaling setup delays before making use of
>    the conversion service.  The Convert Protocol does not require any
> 
> In tightly controlled network environments where MAC can be assumed to
> provide sufficient authentication, authorization, and access-control
> functionality to meet the security needs of the proxy protocol;

[Med] Which is the deployment model of this spec. I hope this is now more clear with the new "Applicability scope" discussed with Roman.

 this
> cannot hold when additional mutual authentication schemes are
> required,
> as envisioned in Section 9.2

[Med] I'm not sure about this because this is out of scope of this version. 

If TLS1.3 is used for example, a connection may be established at bootstrap with the Transport Converter, and then use 0-RTT early data when the user want to solicit the converter. A more serious analysis should be done to assess if the use of early data is safe but that is for future work. 


> 
>    The Transport Converter adheres to the main principles drawn in
>    [RFC1919]. 

[Med] s/principles drawn in /steps drawn in Section 3 of. 

 In particular, a Transport Converter achieves the
> 
> Er, which principles are those?  RFC 1919 is a 35-page document
> entitiled "Classical versus Transparent IP proxies" that facially
> seems
> to be a  comparison and not a declaration of guiding principles.
> 
>    The main advantage of network-assisted conversion services is that
>    they enable new TCP extensions to be used on a subset of the path
>    between endpoints, which encourages the deployment of these
> 
> If those are the extensions that the conversion-service provider is
> interested in.  Are other extensions blocked due to inactivity on the
> part of the provider?

[Med] A host can always bypass the converter so blocking an option at the converter does not prevent a host to make use of that option is supported by the server. 

> 
> Section 2
> 
>    destination.  In today's Internet, latency is a important metric
> and
>    various protocols have ben tuned to reduce their latency
> 
> nit: s/ben/been/  "Please don't tune me!"
> 

[Med] :-) Fixed. 


> Section 4.1
> 
>    A connection can be initiated from both sides of the Transport
>    Converter (Internet-facing interface, customer-facing interface).
> 
> How does the internet-facing-initiated case work?  Does the client
> have
> to register?

[Med] The client has to create a state using PCP (RFC6887) for example that it communicates to the remote peer. Once a SYN matching that state is received by the converter, the converter will use Convert to assist the connection.  These details are discussed in Section 5.2.

> 
>    Transport Converters can be operated by network operators or third
>    parties.  Nevertheless, this document focuses on the single
> 
> I'm not convinced that the present document actually describes how to
> use a third-party-run transport converter in a secure fashion.

[Med] This text is now removed. Please refer to the discussion with Roman. 

> 
>    A Client is configured, through means that are outside the scope of
>    this document, with the names and/or the addresses of one or more
>    Transport Converters and the TCP extensions that they support.  The
> 
> Using names rather than addresses adds a "secure DNS" requirement to
> the
> existing assumptions of local-network security.

[Med] Sure, but service discovery is out of scope. 

> 
>    connections.  This encourages the deployment of new TCP extensions
>    until they are widely supported by servers, in particular.
> 
> This document has not yet shown me much to indicate that transparent
> converter implementors are more incentivized to implement new TCP
> extensions than third-party server operators are.

[Med] The protocol is adopted by 3GPP for their ATSSS function, for example.  

> 
>    Similar to SOCKS, the architecture does not interfere with end-to-
> end
>    TLS connections [RFC8446] between the Client and the Server
>    (Figure 3).  In other words, end-to-end TLS is supported in the
> 
> Why is TLS specifically noteworthy?  Does this not apply to all
> higher-level protocols?

[Med] We discuss in the security section that TLS can also be used between the client and the converter. 

> 
>    It is out of scope of this document to elaborate on specific
>    considerations related to the use of TLS in the Client-Converter
>    connection leg to exchange Convert messages (in addition to the
> end-
>    to-end TLS connection).
> 
> Because that can't be done in 0-RTT in the general case?

[Med] It can with 0-RTT early data with TLS1.3, but that require more analysis. 

> 
> Section 4.2
> 
>    Server, without experiencing an extra delay.  The Transport
> Converter
>    waits until the receipt of the confirmation that the Server agrees
> to
>    establish the connection before confirming it to the Client.
> 
> nit: I suggest clarifying the antecedent of "it".
> 
>    that SYN.  The Transport Converter receives this SYN because it is,
>    for example, on the path between the remote host and the Client or
> it
>    provides address sharing service for the Client.  If the check
> fails,
> 
> Is there a reference for "address sharing service"?

[Med] rfc6269#section-2. I added a pointer. 

> 
>    As discussed in [RFC7413], such change to TCP semantic raises two
>    issues.  First, duplicate SYNs can cause problems for some
>    applications that rely on TCP.  Second, TCP suffers from SYN
> flooding
> 
> Is it really best to say "some applications" when the onus to prove
> replay-safety on a per-protocol basis leads to a need to assume as a
> baseline that all protocols are not compatible with TFO until proven
> otherwise?
> 

[Med] "some applications" was used in reference to this part of RFC7413:

  "on to accept old SYN packets with data, i.e., to restrict TFO use to
   a class of applications (Section 6) that are tolerant of duplicate
   SYN packets with data."

Is there a particular change that you would like to be implemented? Thanks.

>    terminate the downstream connection by using FIN packets.  If the
>    downstream connection terminates with the exchange of FIN packets,
> 
> Why do we have "RST segment" in the previous paragraph but "FIN
> packet"
> here?

[Med] Because this one is about the behavior for terminating connection with FIN. Is there any issue?

> 
> Section 4.3
> 
>         * Lifetime is the validity lifetime of the entry as assigned
>           by the Converter.
> 
> Conceptually a lifetime is a "time to survive relative to a given
> reference time".  If the reference time is "now", this lifetime would
> need to be continuously updated; if not, don't we need to also store
> the
> reference time?  (Alternately, just store the expiration time and not
> the lifetime.)

[Med] I updated the text as follows: 

     * Lifetime is a timer that tracks the remaining lifetime of 
       the entry as assigned by the Converter. When the timer 
       expires, the entry is deleted.

How this is tracked is implementation-specific. 

> 
> Section 5.1
> 
> Do we want to say anything about (end-to-end?) MPTCP operation with
> the
> Transport Converter in the middle?  Does it need to do anything other
> than just pass packets (and maybe swap out addresses)?
> 

[Med] IMO, we don't have much to say here. 


> Section 5.2
> 
>    address, external port number).  The external IP address and
> external
>    port number will be then advertised using an out-of-band mechanism
> so
>    that remote hosts can initiate TCP connections to the Client via
> the
>    Converter.  Note that the external and internal information may be
> 
> Advertised by the Transport Converter or the Client? 

[Med] s/then advertised using/then advertised by the client (or the user) using

 (We leave it
> implicit that the allocated external address/port will be communicated
> back to the client by PCP.)

[Med] I added this NEW sentence: 

The external IP address, external port number, and assigned lifetime are returned back the Client in the PCP response.  

> 
>    is found, the Converter silently ignores the message.  If an entry
> is
>    found, the Converter inserts an MP_CAPABLE option and Connect TLV
> in
>    the SYN packet, rewrites the source IP address to one of its IP
>    addresses and, eventually, the destination IP address and port
> number
>    in accordance with the information stored in the mapping.  SYN+ACK
> 
> Why "eventually"?

[Med] because the converter may operate in an address preservation mode. In such case, the same address will be used.

> 
>    It is out of scope of this document to define specific Convert TLVs
>    to manage incoming connections.  These TLVs can be defined in a
>    separate document.
> 
> It seems a little bit of a bait-and-switch to do so, given that the
> discussion of the example in Figure 6 explicitly says "the Tranport
> Convert [sic] inserts a TLV message that indicates the source address
> and port number of the remote host".
> 

[Med] The discussion related to Figure 6 is correct. This note is about defining Convert TLVs to accommodate deployment where PCP is not supported. These TLVs (if defined) will basically mimic PCP requests. 

> Section 6
> 
>    Convert messages may appear only in a SYN, SYN+ACK, or in an ACK
> that
>    is sent shortly after the SYN+ACK.
> 
> Can we be more precise about "shortly after the SYN+ACK" (e.g.,
> "acknowledging the SYN")?

[Med] Will think about this one more.  

> 
> Section 6.1
> 
> Was any consideration given to including a "magic string" in the fixed
> header, to provide additional protection against misconfiguration
> about
> whether Convert is to be used on a given connection?

[Med] Yes. This was discussed here: https://tools.ietf.org/html/draft-boucadair-mptcp-plain-mode-10#section-5.1  

   o  Magic Number: This field MUST be set to "0xFAA8 0xFAA8" to
      indicate this is an MP_CONVERT Information Element.  This field is
      meant to unambiguously distinguish any data supplied by an
      application from the one injected by an MCP.  Other magic numbers
      are considered by the authors (e.g., 64 bits that include in
      addition to "0xFAA8 0xFAA8" 32 bits to enclose the RFC number).

But we abandoned that since we decided to use a dedicated port number. 

> 
>    The Unassigned field MUST be set to zero in this version of the
>    protocol.  These bits are available for future use.
> 
> But only in a future version, the way this is defined.  (That is,
> ruling
> out using those bits as extension points within version 1.)

[Med] I have already updated that text to address one of the comment from Eric. The NEW text is as follows: 

"The Unassigned field MUST be set to zero and MUST be ignored on receipt."

> 
> Section 6.2.2
> 
>    capabilities.  Assuming the Client is authorized to invoke the
>    Transport Converter, the latter replies with the Supported TCP
>    Extensions TLV (Section 6.2.4).
> 
> Just to confirm: based on this design, the Transport Converter
> currently
> has nothing other than the Client IP address to use as input to this
> authorization decision?
> 

[Med] It can rely on the IP address, IMSI, and other inputs discussed in Section 9.5.

> Section 6.2.5
> 
>    We distinguish two types of Connect TLV based on their length: (1)
>    the Base Connect TLV has a length of 20 bytes and contains a remote
> 
> Which is to say, a Length field containing the value 5.
> 

[Med] Yes. Changed to "5 (i.e., 20 bytes)"

> 
> I'm confused about the default options semantics.  First we say that
> the
> default options are those advertised in the Supported TCP Extensions
> TLV, which would seem to imply that the converter doesn't support any
> extensions that are not listed there.  But then we go on to say that
> for
> an Extended Connect TLV, the options contained in the TLV are
> *additional* options that are *added* to the default set of options
> ...
> but only if the converter supports those options.  But if it supports
> an
> option, wouldn't it already be in the default list, since the default
> list is the list of supported options?

[Med] Ben, which part of this text is confusing: 

   Upon reception of an Extended Connect TLV, a Transport Converter
   first checks whether it supports the TCP Options listed in the 'TCP
   Options' field.  If not, it returns an error message (Section 6.2.8).
   If the above check succeeded and absent any rate limit policy or
   resource exhaustion conditions, a Transport Converter MUST attempt to
   establish a connection to the address and port that it contains.  It
   MUST include in the SYN that it sends to the Server the options
   listed in the 'TCP Options' sub-field and the TCP options that it
   would have used according to its local policies.  For the TCP options
   that are included in the TCP Options field without an optional value,
   the Transport Converter MUST generate its own value.  For the TCP
   options that are included in the 'TCP Options' field with an optional
   value, it MUST copy the entire option in the SYN sent to the remote
   server.  This procedure is designed with TFO in mind.  Particularly,
   this procedure allows to successfully exchange a TFO Cookie between
   the client and the server.


  The way it's currently worded,
> the extended connect TLV seems only useful for specifying precise
> values
> for options that take a value, but not for adding entirely new
> options.

[Med] Yes. Options that aren't supported by the converter will be rejected with "unsupported option" error. 

> 
> Section 6.2.6
> 
>    by the Server in the SYN+ACK packet.  A Transport Converter MUST
>    return this TLV if the Client sent an Extended Connect TLV and the
>    connection was accepted by the server.
> 
> Is the converter allowed to return this option if a basic connect was
> used?  Should the converter always attempt to return this option?

[Med] No, this is only for TCP option are negotiated end-to-end (e.g., TFO). 

> 
> Section 6.2.7
> 
>    the Client.  This Cookie can be configured on the Client by means
>    that are outside of this document or provided by the Transport
>    Converter as follows.
> 
> [But not as immediately follows; there's other content in between.]

[Med] deleted "as follows".

> 
> Section 6.2.8
> 
>    An Error TLV can be included in the SYN+ACK or an ACK sent shortly
>    after the SYN+ACK.
> 
> [same comment about "shortly after"]
> 
>       encoded in 8 bits.  The list of supported versions should be
>       padded with zeros to end on a 32 bits boundary.
> 
> "should be"?  It's a protocol violation to not do something to make it
> end on a 32-bit boundary, and putting anything other than zero is
> liable
> to cause the peer to make bad assumptions, so this seems pretty
> mandatory to me.

[Med] Good catch. s/should/MUST. 

> 
>       Upon receipt of this error code, the remote peer (e.g., Client)
>       checks whether it supports one of the versions returned by the
>       peer.  The highest common supported version MUST be used by the
>       remote peer in subsequent exchanges with the peer.
> 
> There doesn't seem to be any protection against version downgrade
> (e.g.,
> by MITM) in this exchange.

[Med] Agree. I added this NEW text in the security section: 

   Attacks from within the network between a Client and a Transport
   Converter are yet another actual threat.  Means to ensure that
   illegitimate nodes cannot connect to a network should be implemented.

Should we say more?

> 
>       To ease troubleshooting, the value field MUST echo the received
>       message shifted by one byte to keep to original alignment of the
>       message.
> 
> I'm not sure what's meant by this "shifted" language is going to be
> clear to all readers.  (Occurs multiple times.)

[Med] All error message share the same format: 

                      1                   2                   3
  0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
 +---------------+---------------+----------------+--------------+
 |     Type=0x1E |     Length    |    Error Code  |  Value       |
 +---------------+---------------+----------------+--------------+
 | value...

Instead of starting echoing the message right after the "Error Code", we set the next byte to 0s and then insert the message: 

                      1                   2                   3
  0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
 +---------------+---------------+----------------+--------------+
 |     Type=0x1E |     Length    |    Error Code  |  zeros       |
 +---------------+---------------+----------------+--------------+
 | Echo'ed message



> 
>    o  Not Authorized (32): This error code indicates that the
> Transport
>       Converter refused to create a connection because of a lack of
>       authorization (e.g., administratively prohibited, authorization
>       failure, invalid Cookie TLV, etc.).  The Value field MUST be set
> 
> invalid or missing, IIUC.

[Med] if the cookie is missing, we send "Missing Cookie (3)".

> 
>    o  Unsupported TCP Option (33): A TCP option that the Client
>       requested to advertise to the final Server cannot be safely
> used.
> 
> Why the mismatch between "unsupported" and "cannot be safely used"?

[Med] Because we don't know if an option that is blindly relayed does not have an impacts on other requests conversion services (hence, safe of use). 

> 
> Section 7.6
> 
>    types of Connect TLV.  If such a Transport Converter receives a
>    Connect TLV with the TCP Fast Open cookie option that does not
>    contain a cookie, it MUST add an empty TCP Fast Open cookie option
> in
>    the SYN sent to the remote server.  If such a Transport Converter
>    receives a Connect TLV with the TCP Fast Open cookie option that
>    contains a cookie, it MUST copy the TCP Fast Open cookie option in
>    the SYN sent to the remote server.
> 
> Just to check: we don't need to specify any additional behavior for
> the
> response (e.g., to give the client the cookie from the server), since
> that falls out naturally from the requirement to send the Extended TCP
> Header TLV?
> 

[Med] The converter will relay the option received from the server. It is better to make it explicit: 

NEW:
   If the remote server supports TFO, it responds with a SYN-ACK
   according to the procedure in Section 4.1.2 of [RFC7413].  This
   SYN-ACK may contain a Fast Open option with a cookie.  Upon receipt
   of the SYN-ACK by the Converter, it relays Fast Open option with the
   cookie to the Client.


> Section 7.7
> 
>    A Transport Converter MUST NOT advertise the TCP-AO option
> (Kind=29)
>    in the Supported TCP Extensions TLV.  If a Transport Converter
>    receives a Connect TLV that contains the TCP-AO option, it MUST
>    reject the establishment of the connection with error code set to
>    "Unsupported TCP Option", except if the TCP-AO-NAT option is used.
> 
> Perhaps a reminder that, since TCP-AO-NAT is Experimental, such usage
> is
> not currently defined and must be specified by some other document
> before it can be used, would be in order?

[Med] Yes. 

> 
> Section 9
> 
> An implementation needs to check that the TLVs are properly framed
> within the boundary indicated by the Total Length in the fixed header.

[Med] Good point. Added. 

> 
> Section 9.1
> 
>    processes.  As such, it MUST be protected as a core IP router
> (e.g.,
>    [RFC1812]).
> 
> nit: I don't think the grammar is quite right here; maybe "as
> protected
> as" or "protected as if it was".

[Med] Thanks. Fixed. 

> 
>    This document assumes that all network attachments are managed by
> the
>    same administrative entity.  Therefore, enforcing anti-spoofing
>    filters at these network ensures that hosts are not sending traffic
>    with spoofed source IP addresses.
> 
> Until some network infrastructure equipment gets compromised.  (That
> is
> to say "ensures" may be stronger language than is merited.)

[Med] s/ensures/is a guard

> 
> Section 9.2
> 
>    The Convert Protocol is intended to be used in managed networks
> where
>    end hosts can be identified by their IP address.
> 
> End hosts can always be identified by their IP address in any network
> ... subject to spoofing.  Perhaps you mean to say "securely
> identified"?

[Med] Indeed. 

> Section 9.4
> 
> I'm confused why we're talking about an illegitimate converter being
> possible given that we assume the network is secure.

[Med] This is to record an attack against the discovery of the converter on the host side. It is fair to be recorded, IMO.

> 
> Section 9.5
> 
>    The operator that manages the various network attachments
> (including
>    the Transport Converters) can enforce authentication and
>    authorization policies using appropriate mechanisms.  For example,
> a
> 
> I suggest "has various options for enforcing authentication and
> authorization policies"; the current language could be misread as
> saying
> that such enforcement is optional.

[Med] Good suggestion. Updated. Thanks.

> 
>    o  The network provider may enforce a policy based upon Access
>       Control Lists (ACLs), e.g., at a Broadband Network Gateway (BNG)
>       to control the hosts that are authorized to communicate with a
>       Transport Converter.  These ACLs may be installed as a result of
>       RADIUS exchanges, e.g., [I-D.boucadair-radext-tcpm-converter].
>       This method does not require any interaction with the Transport
>       Converter for authorization matters.
> 
>    o  A device that embeds a Transport Converter may also host a
> RADIUS
>       client that will solicit an AAA server to check whether
>       connections received from a given source IP address are
> authorized
>       or not [I-D.boucadair-radext-tcpm-converter].
> 
> These both seem to require that the network has strong anti-spoofing
> protections in place.

[Med] Yes. 

> 
> Section 11.1
> 
> It's not entirely clear that RFC 6978 needs to be normative, given
> that
> we explicitly do *not* include support for that protocol in this
> document (shunting non-standards-track tcp option support to other
> documents).
> 

[Med] Fair. 

> Section 11.2
> 
> RFC 1919 might be normative if I had a better understanding of what we
> wanted from it (see above).
> 
> I'm surprised that RFC 2782 isn't normative given that SRV discovery
> seems like the only defined way to discover the port number of the
> convert service.
> 
> 

[Med] this is because service discovery is out of scope and that 2782 is cited as an example. No normative language is used.