Re: [tcpm] Comments on draft-ietf-tcpm-accurate-ecn-14

[Gorry Fairhurst <[e-mail]>]

Thanks,

These confirmed my understanding, and you appear to have resolutions to 
the issues, a few comments in-line:

On 22/03/2021 12:04, Bob Briscoe wrote:
> Gorry,
>
> Thank you for this additional review.
>
> On 12/03/2021 11:50, Gorry Fairhurst wrote:
>> Abstract:
>> /Given TCP header
>>    space is scarce, this allocates a reserved header bit, that was
>>    previously used for the ECN-Nonce which has now been declared
>>    historic. /
>> - English: /which/that/ /that/which/ perhaps??
>> - However, I wonder if this is necessary in the abstract, surely a 
>> reader of the abstract in future would only need to know:
>> /This allocates a reserved header bit, previously used for the 
>> ECN-Nonce./
>
> [BB] Done, except I've said:
> "previously assigned to the ECN-Nonce" rather than "used by", since 
> it's unclear how much actual use it got.
>
> I also thought the following would be better, now that this updates 
> RFC3168:
> s/ECN is specified for TCP in such a way that only one feedback signal 
> can be transmitted per Round-Trip Time (RTT)./
>  /ECN was originally specified for TCP in such a way that only one 
> feedback signal can be transmitted per Round-Trip Time (RTT)./
>
>
GF: Looks good to me.
>> I also do wonder if the abstract still rather complicated (aka long)?
>
> [BB] It's 202 words. I'd rather leave it as it is if your objection 
> isn't particularly strong.
>
GF: RFC-Ed or AD can decide, not me.
>
>> ——
>> Section 2.5:
>> /According to [RFC3168] the Data Sender is meant to
>>    set control packets to Not-ECT.  However, mechanisms in certain
>>    private networks (e.g. data centres) set control packets to be ECN
>>    capable because they are precisely the packets that performance
>>    depends on most./
>> - I think /set/ is rather odd.
>> - Can we talk about the /ECN IP field/ being set (as used later) ?
>
> [BB] I don't think 'set' is odd. IMO, it seems like the most natural 
> word to use. Do you have a preferred alternative?
>
GF: I was thinking more:
/set the ECN field of control packets to Not-ECT/
> But I agree it's useful to distinguish the IP-ECN field from TCP ECN 
> flags. Indeed, I've run through the whole of section 2, and clarified 
> where it is talking about the IP-ECN field.
>
GF: Sounds good.
>> ——
>> Two questions in the same sentence on p16, starting /So, without 
>> these rules/:
>> /could end up/
>> - this seems possibly one of those phrases that confuses non-native 
>> speakers?
>
> [BB] I'm not sure what you think the problem is.
> Possibly the two 'withouts'?
> I tried this:
>
> Original:
> /So, without these rules, the two peers could end up using different 
> feedback modes without knowing it./
> Option 1:
> /So, without these rules, the two peers could end up using different 
> feedback modes unknowingly./
> Option 2:
> /So, in the absence of these rules, the two peers could end up using 
> different feedback modes without knowing it./
>
> A US-English speaker might use 'absent these rules', but I think the 
> longer British English above is equally understood in the US. Can 
> someone confirm that?
>
> I tried unwittingly instead of unknowingly, but I think that is a bit 
> obscure for non-native English speakers.
>
> I've gone for Option 2, but pls suggest something else, if you'd 
> prefer it.
>
GF: I was thinking 2, but perhaps:
/could end up/could choose to use/
>
>>
>> /using difference feedback modes/
>> - is that word really difference not different?
>
> [BB] Thx - different was meant.
>
>>
>> ——
>>
>> Section 3.2
>> /3.3.2.  Requirements for TCP Normalizers/
>>
>> I like the introduction of subheadings within 3.2, because it helps 
>> people find the part of the spec they need to see - especially 
>> important when the implementer only needs to understand one aspect in 
>> detail, however, I wonder if this second part needs a separate heading:
>> /A middlebox claiming to be transparent/
>> … really only for normalisers? - I expected this to be relevant to 
>> any “transparent” middlebox”
>> - would it be better with a separate subsection for this important 
>> point?
>
> [BB] Just before your review, I had proposed to my co-authors that we 
> should change this para. Now, from your comments, I see that the scope 
> of the whole section needs clarifying. The idea was that a set of TCP 
> headers that complies with the RFCs can either
> a) remain completely unchanged (e.g. transparent performance enhancement)
> b) be 'normalized' to a narrower interpretation of the RFCs, which 
> might not always be up to date (e.g. an intrusion detection system).
>
> I'd rather draw this fine line within a single section, because it's 
> too difficult for two section headings to describe the difference well 
> enough. I've also removed some gratuitous criticism of middleboxes, 
> which won't encourage middlebox vendors to comply. Here's the whole 
> rewritten subsection (I avoided a full diff which was too hard to follow):
>
> +3.3.2.  Requirements for Transparent Middleboxes and TCP Normalizers
>
>      Another large class of middleboxes intervenes to some degree at the
>      transport layer, but attempts to be transparent (invisible) to the
>      end-to-end connection.  A subset of this class of middleboxes
>      attempts to `normalize' the TCP wire protocol by checking that all
> +   values in header fields comply with a rather narrow interpretation
> +   of the TCP specifications that is also not always up to date.
>
> +   A middlebox that is not normalizing the TCP protocol and does not
> +   itself act as a back-to-back pair of TCP endpoints (i.e. a middlebox
> +   that intends to be transparent or invisible at the transport layer) ought to
>      forward the AccECN TCP Option unaltered, whether or not the length
>      value matches one of those specified inSection 3.2.3  <https://datatracker.ietf.org/doc/html/draft-ietf-tcpm-accurate-ecn-14#section-3.2.3>, and whether or
>      not the initial values of the byte-counter fields
> +   match those inSection 3.2.1  <https://datatracker.ietf.org/doc/html/draft-ietf-tcpm-accurate-ecn-14#section-3.2.1>.  This is because blocking apparently
> +   invalid values prevents the standardized set of values being
>      extended in future (given outdated normalizers would block updated
>      hosts from using the extended AccECN standard).
> +   A TCP normalizer is likely to block or alter an AccECN TCP Option
> +   if the length value or the initial values of its byte-counter fields
> +   do not match one of those specified inSection 3.2.3  <https://datatracker.ietf.org/doc/html/draft-ietf-tcpm-accurate-ecn-14#section-3.2.3>  orSection 3.2.1  <https://datatracker.ietf.org/doc/html/draft-ietf-tcpm-accurate-ecn-14#section-3.2.1>.
> +                                                  However, to comply with
> +   the present AccECN specification, a middlebox MUST NOT change
> +   the ACE field; or those fields of the AccECN Option that are currently
> +   specified inSection 3.2.3  <https://datatracker.ietf.org/doc/html/draft-ietf-tcpm-accurate-ecn-14#section-3.2.3>; or any AccECN field covered by integrity
> +   protection (e.g. [RFC5925  <https://datatracker.ietf.org/doc/html/rfc5925>]).
>
> Note that I've changed the 'MUST' to 'ought to', because this is 
> really clarification advice (although the line between advice and 
> protocol interop is blurred with middleboxes). I can revert to MUST if 
> the WG would prefer.
>
GF: I like the new text.
>> —
>> I have re-read /3.3.3. Requirements for TCP ACK Filtering/ in -14, 
>> and although I am not yet sure of the speculation that using ECN in 
>> future is sufficient to regulate the ratio of ACKs to Data, I believe 
>> the text is correct in its interpretation of RFC3449, and that the 
>> present design is anyway robust to ACK mitigation methods.
>> ——
>> /Once DCTCP offload hardware supports AccECN/
>> … I’m not sure this para is perfected yet, because it is directed at 
>> DCTCP. I’d like to consider a reordering that is more like:
>>
>> /The above process has been designed to enable a continuing
>>  incremental deployment path - to more highly dynamic congestion
>>  control.  Once offload hardware supports AccECN, it will be
>> able to coalesce efficiently for any sequence of marks, instead of
>>  relying for efficiency on the long marking sequences from step
>>  marking.  In the next stage, marking can evolve from a step to
>> a ramp function.  That in turn will allow host congestion control
>> algorithms to respond faster to dynamics, while being backwards
>> compatible with existing host algorithms./
>> … i.e., do we really have to directly discuss DCTCP here, doesn’t it 
>> in general refer to AccECN rather?
>
> [BB] You're right, thank you. Done.
>
>> ——
>>
>> Security Considerations:
>>
>> The “downgrade attack” regarding a receiver deciding/accidentally 
>> omitting an option in the Security Considerations seems incorrect to me.
>> To me, the text is written in a way that sounds like it impacts the 
>> “security” of the connection. I do not agree with that 
>> interpretation. To me, this is a performance reduction, and the 
>> degrade does not impact the ability to communicate or security 
>> properties. I suggest some rewording here.
>
> Current:
>     No known way can yet be contrived to take advantage of
>     this downgrade attack, but it is mentioned here in case someone else
>     can contrive one.
> Proposed:
>     No known way can yet be contrived for a receiver to take advantage of
>     this behaviour, which seems to always degrade its own performance.
>     However, the concern is mentioned here for completeness.
>
GF: I don't see issues with that, thanks.
>
>> ——
>> Security Considerations:
>>
>> There’s now also a Security Considerations include a ToDo, that I do 
>> not yet understand, look forward to this!
>
> I propose to clarify and cut down the existing text, and to replace 
> the "{ToDo}" with the two new sentences below:
>
> Current:
>     InSection 3.2.3  <https://datatracker.ietf.org/doc/html/draft-ietf-tcpm-accurate-ecn-14#section-3.2.3>  a Data Sender is allowed to ignore an unrecognized
>     TCP AccECN Option length and read as many whole 3-octet fields from
>     it as possible up to a maximum of 3, treating the remainder as
>     padding.  This opens up a potential covert channel of up to 29B (40 -
>     (2+3*3))B. {ToDo...}
>
> Proposed:
>     InSection 3.2.3  <https://datatracker.ietf.org/doc/html/draft-ietf-tcpm-accurate-ecn-14#section-3.2.3>  a Data Sender is allowed to ignore an unrecognized
>     TCP AccECN Option length and treat extra octets as padding.  This is
>     noted here in case it could be used as a covert channel. The size is
>     up to 29 B (40 - (2+3*3)) per packet. However, it is really an overt
>     channel (not hidden) and it is no different to the use of unknown
>     TCP options with unknown option lengths in general. Therefore, where
>     such a channel is of concern, it can already be adequately mitigated
>     by regular TCP normalizer technology (see Section 3.3.2).
>
> This also requires the final para to be updated
>
> Current:
>     The AccECN protocol is not believed to introduce any new privacy
>     concerns, because it merely counts and feeds back signals at the
>     transport layer that had already been visible at the IP layer.
> Proposed:
>     The AccECN protocol is not believed to introduce any new privacy
>     concerns, because it merely counts and feeds back signals at the
>     transport layer that had already been visible at the IP layer. A
>     covert channel can be used to compromise privacy. However, as
>     explained above, undefined TCP options generally open up such
>     channels and common techniques are available to close them off.
>
> I shall also shift this para up one, so that is comes straight after 
> the previous discussion of covert channels.
>
>
GF: This addresses this point.
>> ——
>>
>> Best wishes,
>>
>> Gorry
>
> [BB] As always, thank you very much for your continued diligence.
>
>
>
> Bob
>
>>
>> (P.S. I will send one more specific question in a separate email).
>>
>
> -- 
> ________________________________________________________________
> Bob Briscoehttp://bobbriscoe.net/

Gorry