crypto/tls: permanently broken tls.Conn should not return temporary net.Error

[jackc]

What version of Go are you using (go version)?

$ go version
go version go1.11.5 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/jack/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/jack/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build787904603=/tmp/go-build -gno-record-gcc-switches"

What did you do?

1. Set a deadline on a TLS connection.
2. Do a failing write due to that deadline. As documented the connection is permanently broken.
3. Clear deadline (doesn't really matter because connection is permanently broken - but just to mimic what would work for regular TCP connection).
4. Do another write.

Demo code: https://github.com/jackc/go-tls-deadline-temporary-error

What did you expect to see?

A net.Error where Temporary() => false or a non-net.Error.

What did you see instead?

An net.Error where Temporary() => true.

This is technically documented behavior After a Write has timed out, the TLS state is corrupt and all future writes will return the same error. But it means that code that works generically with the net.Conn interface cannot rely on temporary errors being (potentially) temporary. It must treat them as permanent.

Perhaps the original failed tls.Conn.Write could return a non-temporary error.

[FiloSottile]

Agreed, we should wrap the net.Error to make Temporary() false.

[odeke-em]

Thanks for reporting this issue @jackc and @FiloSottile for the triage and direction!

Here is a standalone test adapted from @jackc's report and source code that can be used as a regression test

package main

import (
	"crypto/tls"
	"fmt"
	"net"
	"net/http"
	"net/http/httptest"
	"net/url"
	"testing"
	"time"
)

func TestPermanentlyBrokenConnection(t *testing.T) {
	cst := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		w.Write([]byte("Hello, World!"))
	}))
	defer cst.Close()

	cstU, _ := url.Parse(cst.URL)
	conn, err := tls.Dial("tcp", cstU.Host, &tls.Config{InsecureSkipVerify: true})
	if err != nil {
		t.Fatalf("Dial err: %v", err)
	}

	if err = conn.SetDeadline(time.Now()); err != nil {
		t.Fatalf("conn.SetDeadline(time.Now()): %v", err)
	}

	if _, err = conn.Write([]byte("should fail")); err == nil {
		t.Fatal("conn.Write succeeded when it should have failed")
	}

	// Clear deadline
	err = conn.SetDeadline(time.Time{})
	if err != nil {
		t.Fatalf("final conn.SetDeadline(time.Time{}): %v", err)
	}

	_, err = conn.Write([]byte("This connection is permanently broken"))
	if err != nil {
		fmt.Println("Write err", err)
	}

	ne := err.(net.Error)
	if ne.Temporary() {
		t.Fatal("Got a net.Error that apparently is temporary")
	}
}