Repository: go
Issue: x/net/nettest: TestConn cannot succeed with some valid net.Conn implementations
Labels: NeedsInvestigation

x/net/nettest.TestConn implements a test suite to verify that a net.Conn implementation conforms to the implicit contracts of net.Conn. However, some (I argue) valid implementations of net.Conn cannot pass this test, because it assumes that write timeouts are always recoverable.

As an example, https://github.com/tailscale/tailscale/pull/2261 is an implementation of a Noise IK cryptographic transport. At a high level, it functions similarly to TLS: a handshake generates some session keys, and those session keys are used to transmit encrypted data frames of the form `2b length; Nb ciphertext`.

This implementation fails 4 of the TestConn subtests:

```
    --- FAIL: TestConn/PastTimeout (60.02s)
    --- FAIL: TestConn/PresentTimeout (60.02s)
    --- FAIL: TestConn/FutureTimeout (60.01s)
    --- FAIL: TestConn/ConcurrentMethods (60.02s)
```

All of these tests set a write deadline, deliberately make it expire, then verify that the write unblocks _and_ that the connection is still usable after resetting the deadline.

However, in the Noise protocol, if a write aborts with a partially written ciphertext frame, the stream synchronization is broken and the io.Writer API is such that we cannot recover:
 - If we truthfully return less than `n` bytes written, the caller will assume that the connection is still usable and that it could retransmit the remainder of the bytes - which is not true, since the receiver would then receive a truncated ciphertext frame followed by a different full frame.
 - If we return `n` and buffer the untransmitted frame portion to retry on a later Write call, the caller may assume that the receiver has received all the bytes it sent, and switch to blocking on a Read call waiting for a response, causing a deadlock.

After thinking this through, the conclusion I reached is that in this protocol, _any_ error on Write, including timeouts, is fatal to the write portion of the net.Conn, and no further data can be transmitted from that point on. (Strictly speaking, it's possible to special-case timeouts that fire with 0 bytes sent, but that involves rollbacks of the cipher state that can lead to catastrophic cryptographic failures)

And yet, I think that this net.Conn is a working net.Conn that complies with the interface contract. Nothing in the contract says that timeouts _must_ be non-fatal (indeed, there's no way to guarantee that the connection doesn't terminate for unrelated reasons immediately after a timeout). The failing tests are trying to probe whether the implemented timeout behavior is correct _if timeouts are non-fatal_ - a valid thing to probe, but not strictly required for a net.Conn to be functional.

As another case in point, I believe tls.Conn has the same issue, since TLS data frames look and behave much the same. AFAICT, crypto/tls solves this issue by not running nettest.TestConn against tls.Conn.

(it's worth noting that tls.Conn and my noise.Conn don't have the same problem on the read side, since the Conn can gracefully buffer partial reads until the caller retries enough to get a full frame of data. So, I very much do want to run the contract tests that check for Read timeout behavior)

It would be nice to somehow partition these subtests out, to allow for net.Conns that are intolerant of any write error to still verify the rest of their behaviors.