STRLCPY/dnstt

Add a dial timeout to TLSPacketConn.

In my testing locally, specifying -dot with a non-responsive TCP port
would time out after about 30 seconds anyway:
$ time ./dnstt-client -dot tns.example.com:8000 -pubkey-file server.pub t.example.com 127.0.0.1:7000
dial tcp 45.79.134.119:8000: connect: connection timed out
real 0m31.398s
user 0m0.006s
sys 0m0.003s
Which is in line with the documentation for net.Dialer:
https://golang.org/pkg/net/#Dialer
With or without a timeout, the operating system may impose its
own earlier timeout. For instance, TCP timeouts are often around
3 minutes.
But may as well be explicit.

This commit has the side effect of changing the error message from
"connection timed out" to "i/o timeout".
$ time ./dnstt-client -dot tns.example.com:8000 -pubkey-file server.pub t.example.com 127.0.0.1:7000
dial tcp 45.79.134.119:8000: i/o timeout
real 0m30.007s
user 0m0.003s
sys 0m0.007s
I tried setting the dialTimeout to 40 seconds, and in that case the
system timeout take precedence after ≈31 seconds, with the "connection
timed out" error as before.

This is issue UCB-02-007 from the 2021 security audit of Turbo Tunnel by
Cure53.

The audit report additionally recommends calling SetReadDeadline before
each read operation. I have chosen not to do that. It is intended that
the TLS connection should be able to remain idle if there is nothing to
send. As DNS is a query–response protocol, one might expect a response
(and within a certain amount of time) only after sending a query;
sendLoop could refresh the ReadDeadline for recvLoop every time it sends
a query. But a malicious DoT server could keep a useless connection
alive anyway by sending Slowloris-style short responses within each
deadline, and an external adversary could capable of delaying responses
could deny service indefinitely or simply block the server. In any case,
the smux KeepAliveTimeout serves as a check that prevents stalled
connections from remaining indefinitely.

David Fifield committed 3 years ago

4de69201

1 parent 23759e20

Total 1 files

■ ■ ■ ■ ■ ■

dnstt-client/tls.go

		skipped 7 lines
8	8		"log"
9	9		"net"
10	10		"sync"
	11	+	"time"
11	12
12	13		"www.bamsoftware.com/git/dnstt.git/turbotunnel"
13	14		)
	15	+
	16	+	const dialTimeout = 30 * time.Second
14	17
15	18		// TLSPacketConn is a TLS- and TCP-based transport for DNS messages, used for
16	19		// DNS over TLS (DoT). Its WriteTo and ReadFrom methods exchange DNS messages
		skipped 24 lines
41	44		// becomes disconnected. We do the first dial here, outside the
42	45		// goroutine, so that any immediate and permanent connection errors are
43	46		// reported directly to the caller of NewTLSPacketConn.
	47	+	dialer := &net.Dialer{
	48	+	Timeout: dialTimeout,
	49	+	}
44	50		tlsConfig := &tls.Config{}
45		-	conn, err := tls.Dial("tcp", addr, tlsConfig)
	51	+	conn, err := tls.DialWithDialer(dialer, "tcp", addr, tlsConfig)
46	52		if err != nil {
47	53		return nil, err
48	54		}
		skipped 20 lines
69	75		conn.Close()
70	76
71	77		// Whenever the TLS connection dies, redial a new one.
72		-	conn, err = tls.Dial("tcp", addr, tlsConfig)
	78	+	conn, err = tls.DialWithDialer(dialer, "tcp", addr, tlsConfig)
73	79		if err != nil {
74	80		log.Printf("tls.Dial: %v", err)
75	81		break
		skipped 53 lines