Description
What did you do?
Reading the documentation for tls.Conn (https://golang.org/pkg/crypto/tls/) the deadline behaviour is not clearly explained.
- It's not obvious that Read calls may block forever unless the write deadline is set to a non-zero time. (This is because handshaking is done internally, which writes.) Since this is different to net.TLSConn, it should be called out.
- It's reasonable to understand that Handshake calls need to have both read and write deadline set... but could be called out explicitly.
- But the killer: calling Close on a tls.Conn risks blocking forever, unless the write deadline has been set! (It writes out a close alert.)
This last one is the worst. It would be very easy to have a defer'ed call to Close, and assume that it will always return promptly, when in fact it may be possible for a malicious client to fill up the socket buffers with handshaking data, so that writing the close alert blocks (on an OS with tiny socket buffers?).
I have (or had) some code that does the following:
for {
conn, _ := listener.Accept() // error handling omitted for clarity
if tooManyConns() {
conn.Close() // oops, may block!
continue
}
go handleConn(conn)
}
I have now carefully gone and wrapped all calls to Close with a safeClose wrapper method that calls SetWriteDeadline first!
Perhaps Close should actually call SetWriteDeadline on the underlying connection with a sane value (eg time.Now().Add(100*time.Millisecond)) to ensure that no-one else is caught out.
Thankfully, http.conn.serve does call SetWriteDeadline as soon as possible for TLS connections, and never sets a zero write deadline, so that code at least is safe, and can't call Close with a zero deadline.
What did you expect to see?
More mention of what deadlines need to set, to prevent each method blocking.