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

[Wesley Eddy <[e-mail]>]

First, on the one DISCUSS point remaining:

On 2/25/2022 4:00 PM, Benjamin Kaduk wrote:

> In essence, I think that we require a fairly strong justification to
> publish an Internet Standard in 2022 that says it's okay to adopt a data
> model where a host has a global piece of state that it freely sends to
> anyone who asks, where that piece of state can be used to attack/disrupt
> all new connections that host makes, as opposed to just connections on the
> 5-tuple that asked.

I agree with Joe's explanation on the applicability of the concern being 
a bit more nuanced, since it's important for some things (like BGP 
sessions, which 5961 was written in response to), but less so for 
shorter-lived connections, hosts that aren't servers, etc.


> The actual scope of utility of an ISN is local to an individual 5-tuple,
> not global to a host, and false sharing of ISNs across connections adds
> risk.  My stance as SEC AD is that the IETF should produce protocols that
> are as secure as possible *subject to any constraints on them*.  Here, we
> have the constraint of retaining compatibility with the existing deployment
> base (which is a very compelling constraint!), so you do not see me
> advocating for mandatory tcpcrypt (for example).  But I want to see some
> explanation of what harm or risk there is in saying that the ISN is always
> produced with the PRF of the 5-tuple (okay, 4-tuple since the protocol is
> always TCP), to motivate a divergence from the more-secure behavior and
> thereby justify retaining the behavior currently in the draft.  What
> constraint are we subject to that prevents doing the right (security-wise)
> thing?
>
I think that the WG would have to decide on this, since it's more than 
just an editorial matter.

A fair change might be to leave SHOULD, but be more clear about why it 
isn't felt to justify a MUST presently, if that seems to make sense to you?


And now, closing the loop on a few COMMENTs:

>
> Section 3.8.4
>
>      An implementation SHOULD send a keep-alive segment with no data
>      (SHLD-12); however, it MAY be configurable to send a keep-alive
>      segment containing one garbage octet (MAY-6), for compatibility with
>      erroneous TCP implementations.
>
> Such misbehaved TCP impelementations were misbehaved even in 1989 when
> RFC 1122 was published -- do we have a sense for whether they are still
> around to any significant degree?
>> That's a great question; I don't know, but would highly doubt they are
>> around anymore (at least not in production on the Internet).
> Might be an interesting experiment, but it sounds like we don't have enough
> data to justify changing the text here (yet).

As a follow-up action, we can bring this to attention in TCPM and see if 
anyone is interested in helping to gather data on whether any 
implementations still need this.  That would then let us see if this can 
be removed in an update.  But anyways, probably shouldn't block this doc 
(which I think you agree with, since this was just a COMMENT, not 
DISCUSS topic).


>>> Section 4
>>>
>>>      Destination Address
>>>              The network layer address of the remote endpoint.
>>>      [...]
>>>      Source Address
>>>              The network layer address of the sending endpoint.
>>>
>>> These definitions don't seem to work in the context of a receiver
>>> validating the TCP checksum, where the destination address is the local
>>> endpoint's address and the source address is the remote endpoint's
>>> address.  (I note that these definitions are different from what RFC 793
>>> itself used.)
>> Interesting point on the destination (though the source one seems fine,
>> it's always the sending endpoint).  Maybe we should just change "remote"
>> to "receiving" in the destination one?
> That's an easy fix; good idea.
>
In rev -27, I changed this to:

    Destination Address
            The network layer address of the endpoint intended to receive
            a segment.



>>>      The more secure Initial Sequence Number generation algorithm from RFC
>>>      6528 was incorporated.  See RFC 6528 for discussion of the attacks
>>>      that this mitigates, as well as advice on selecting PRF algorithms
>>>      and managing secret key data.
>>>
>>> (As I mentioned up in §3.4, that guidance is no longer current.)
>> Is updating 6528 agreed to be possible future work?
> Yes.  (Is this something where I should be expecting to take the action
> item to write a draft, or do you think someone else might take up the pen?)

It might be worthwhile to succinctly describe the proposed update to 
TCPM, and see what the interest level is.  If you have bandwidth to do 
so, you'd probably be best suited, but otherwise, I'd also be happy to 
bring it up on-list and at the next meeting (per chairs approval), as 
part of closing-out this 793bis work.



>>> Section 3.8.6.3
>>>
>>>      Note that there are several current practices that further lead to a
>>>      reduced number of ACKs, including generic receive offload (GRO), ACK
>>>      compression, and ACK decimation [26].
>>>
>>> Reference [26] seems reasonable for ACK decimation and ACK compression,
>>> but doesn't seem to cover GRO at all.
>> True; do you think we should try to find a proper GRO reference? I'm not
>> aware of there being a real standard or academic type of reference.
> I wouldn't put much effort into looking.
> It's a bit outside my normal field, so I don't have any immediate insight.
> Google finds the DPDK docs, which might be stable and well-known, and
> wikipedia'shttps://en.wikipedia.org/wiki/TCP_offload_engine  entry has a
> brief mention, with links to an LWN article and
> https://books.google.com/books?id=C3wQBwAAQBAJ  .  If we know someone with
> a copy of the latter, it might be worth checking if it would be a useful
> reference.

This one looks good to me, and I can include it in the next revision, if 
you agree:

https://www.kernel.org/doc/html/latest/networking/segmentation-offloads.html



>>> Section 3.9.2.2
>>>
>>>      Soft Errors
>>>        For ICMP these include: Destination Unreachable -- codes 0, 1, 5,
>>>        Time Exceeded -- codes 0, 1, and Parameter Problem.
>>>
>>>        For ICMPv6 these include: Destination Unreachable -- codes 0 and 3,
>>>        Time Exceeded -- codes 0, 1, and Parameter Problem -- codes 0, 1,
>>>        2.
>>>
>>>        Since these Unreachable messages indicate soft error conditions,
>>>
>>> I'm not entirely sure that I'd classify "parameter problem" as an
>>> "unreachable" message per se.
>> I'm not sure I understand this comment.  This is the list from RFC 5461.
> It looks like I was misparsing on first read, since there were commas
> serving different roles.  ("Parameter Problem" is an element in the outer
> list, matched with "Destination Unreachable" and "Time Exceeded", not one
> of the codes for Time Exceeded.)  Using semicolons as delimiters for the
> "outer" list, with commas for the lists of type codes, would help
> distinguish them.
>
I see; semicolons will be in the next revision.



>>> Section 3.10.1
>>>
>>>            the parameters of the incoming SYN segment.  Verify the
>>>            security and DiffServ value requested are allowed for this
>>>            user, if not return "error: precedence not allowed" or "error:
>>>            security/compartment not allowed."  If passive enter the LISTEN
>>>
>>> It's surprising for the error string to mention "precedence" when the
>>> predicate is DiffServ value.
>> This is a good point.  When the definition of those bits was changed, it
>> doesn't seem like the error message indicated here was correspondingly
>> changed.
> Is this something we need to leave alone for backwards compatibility, or do
> we have leeway to change the error message here?  (It looks like this text
> is unchanged in the -27.)

I don't think there's much of a compatibility issue, since the socket 
API works differently than this anyways.  We could change "precedence" 
to "DSCP".


>>>               *  If this connection was initiated with a passive OPEN
>>>                  (i.e., came from the LISTEN state), then return this
>>>                  connection to LISTEN state and return.  The user need
>>>                  not be informed.  If this connection was initiated
>>>                  with an active OPEN (i.e., came from SYN-SENT state)
>>>                  then the connection was refused, signal the user
>>>                  "connection refused".  In either case, all segments on
>>>                  the retransmission queue should be removed.  And in
>>>
>>> IIUC, what's described here as "removed" is described elsewhere as
>>> "flushed"; it would be good to use consistent terminology when possible.
>> Do you have a strong preference?
> No strong preference, just a general desire to have consistent terminology
> unless there is a difference in meaning intended.
>
>
Ok, I can change this to say that the retransmission queue should be 
flushed in the next revision.



>>> Section 4
>>>
>>>      internet datagram
>>>              The unit of data exchanged between an internet module and the
>>>              higher level protocol together with the internet header.
>>>
>>> "exchanged between an internet module and the higher level protocol"
>>> sounds like a local operation; I would have expected the definition of
>>> an *internet* datagram to involve transfer over the (inter)network.
>> Ok, can you suggest an alternative definition?  I agree this one from
>> the original 793 isn't terrific.
> I'm somewhat reluctant to concoct a new definition from scratch at this
> point in the process.  I see that RFC 1983 has "IP datagram" refer to just
> "datagram", on which it says:
>
>     datagram
>        A self-contained, independent entity of data carrying sufficient
>        information to be routed from the source to the destination
>        computer without reliance on earlier exchanges between this source
>        and destination computer and the transporting network.  See also:
>        frame, packet.
>        [Source: J. Postel]
>
> If I was to overcome my reluctance and try to revise the existing text here
> into something that makes more sense, it would be:
>
> internet datagram
>    A unit of data [from some higher layer protocol] exchanged between
>    internet hosts, together with the internet header that allows the
>    datagram to be routed from source to destination.
>
> (I'm not sure whether I'd include the bit in square brackets.)
> Feel free to use mine if you like, or go with Postel's (per RFC 1983) or
> something else.
Yours looks good to me, and I'll queue it for the next revision.