Fixed bug in bad TLS conn close in gotun

gotun/mocked_test.go

@@ -27,20 +27,6 @@ const (
 	IOSIZE int = 4096
 )
 
-var (
-	P *pki
-)
-
-// will this actually run before everything?
-func init() {
-	var err error
-	P, err = newPKI()
-
-	if err != nil {
-		panic(fmt.Sprintf("can't setup PKI: %s", err))
-	}
-}
-
 type tcpserver struct {
 	net.Listener
 	t *testing.T
@@ -200,28 +186,27 @@ func newTcpClient(network, addr string, tcfg *tls.Config, t *testing.T) *tcpclie
 	return c
 }
 
-func (c *tcpclient) start(n int) {
+func (c *tcpclient) start(n int) error {
 	var err error
-
-	assert := newAsserter(c.t)
-
 	dial := &net.Dialer{
 		Timeout: 1 * time.Second,
 	}
 
 	c.Conn, err = dial.DialContext(c.ctx, c.network, c.addr)
-	assert(err == nil, "can't dial %s: %s", c.addr, err)
+	if err != nil {
+		return err
+	}
 
 	c.t.Logf("mock tcp client: connected to %s\n", c.addr)
-	c.loop(n)
+	return c.loop(n)
 }
 
 func (c *tcpclient) stop() {
 	c.cancel()
 	c.Close()
 }
 
-func (c *tcpclient) loop(n int) {
+func (c *tcpclient) loop(n int) error {
 	assert := newAsserter(c.t)
 	done := c.ctx.Done()
 	addr := c.RemoteAddr().String()
@@ -236,8 +221,9 @@ func (c *tcpclient) loop(n int) {
 	if c.tls != nil {
 		econn := tls.Client(c, c.tls)
 		err := econn.Handshake()
-		assert(err == nil, "%s: can't setup TLS: %s", from, err)
-
+		if err != nil {
+			return err
+		}
 		fd = econn
 		c.t.Logf("mock tcp client: Upgraded %s to TLS\n", from)
 	}
@@ -252,7 +238,7 @@ func (c *tcpclient) loop(n int) {
 		nw, err := writefull(fd, buf)
 		select {
 		case <-done:
-			return
+			return nil
 		default:
 		}
 		assert(err == nil, "%s: write err: %s", from, err)
@@ -269,13 +255,13 @@ func (c *tcpclient) loop(n int) {
 		nr, err := readfull(fd, sumr[:])
 		select {
 		case <-done:
-			return
+			return nil
 		default:
 		}
 
 		if errors.Is(err, io.EOF) || nr == 0 {
 			c.t.Logf("%s: EOF? nr %d\n", from, nr)
-			return
+			return nil
 		}
 		assert(err == nil, "%s: read err: %s", from, err)
 		assert(nr == len(sumr[:]), "%s: partial read, exp %d, saw %d", from, len(sumr[:]), nr)
@@ -285,6 +271,7 @@ func (c *tcpclient) loop(n int) {
 		c.nr += len(sumr[:])
 		//c.t.Logf("%s: TX %d, RX %d\n", addr, nw, len(sumr[:]))
 	}
+	return nil
 }
 
 func writefull(fd io.Writer, b []byte) (int, error) {

gotun/server.go

@@ -329,6 +329,8 @@ func (p *TCPServer) handleTCP(conn Conn, ctx context.Context) {
 		err := econn.Handshake()
 		if err != nil {
 			p.log.Warn("can't establish TLS with %s: %s", lhs, err)
+			conn.Close()
+			p.wg.Done()
 			return
 		}
 

gotun/tcp_test.go

@@ -34,6 +34,8 @@ func testSetup(lport, cport int) *Conf {
 	return defaults(c)
 }
 
+// Client -> gotun TCP
+// gotun -> backend TCP
 func TestTcpToTcp(t *testing.T) {
 	assert := newAsserter(t)
 
@@ -63,8 +65,11 @@ func TestTcpToTcp(t *testing.T) {
 	c.stop()
 	s.stop()
 	gt.Stop()
+	log.Close()
 }
 
+// Client -> gotun TLS
+// gotun -> backend TCP
 func TestTlsToTcp(t *testing.T) {
 	assert := newAsserter(t)
 
@@ -131,8 +136,11 @@ func TestTlsToTcp(t *testing.T) {
 	c.stop()
 	s.stop()
 	gt.Stop()
+	log.Close()
 }
 
+// Client -> gotun TLS with client auth
+// gotun -> backend TCP
 func TestClientTlsToTcp(t *testing.T) {
 	assert := newAsserter(t)
 
@@ -210,4 +218,84 @@ func TestClientTlsToTcp(t *testing.T) {
 	c.stop()
 	s.stop()
 	gt.Stop()
+	log.Close()
+}
+
+// Client -> gotun TLS with *bad* client auth
+// gotun -> backend TCP
+func TestClientBadTlsToTcp(t *testing.T) {
+	assert := newAsserter(t)
+
+	pki, err := newPKI()
+	assert(err == nil, "can't create PKI: %s", err)
+
+	pkic, err := newPKI()
+	assert(err == nil, "can't create client PKI: %s", err)
+
+	spool := x509.NewCertPool()
+	spool.AddCert(pki.ca)
+
+	cpool := x509.NewCertPool()
+	cpool.AddCert(pkic.ca)
+
+	cfg := testSetup(9010, 9011)
+	lc := cfg.Listen[0]
+
+	cert, err := pki.ServerCert("server.name", lc.Addr)
+	assert(err == nil, "can't create server cert: %s", err)
+
+	tlsCfg := &tls.Config{
+		MinVersion:             tls.VersionTLS13,
+		ServerName:             "server.name",
+		SessionTicketsDisabled: true,
+		CipherSuites: []uint16{
+			tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,   // Go 1.8 only
+			tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305, // Go 1.8 only
+			tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
+			tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
+			tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
+			tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
+
+			// Best disabled, as they don't provide Forward Secrecy,
+			// but might be necessary for some clients
+			// tls.TLS_RSA_WITH_AES_256_GCM_SHA384,
+			// tls.TLS_RSA_WITH_AES_128_GCM_SHA256,
+		},
+		RootCAs:      spool,
+		Certificates: []tls.Certificate{cert},
+		ClientAuth:   tls.RequireAndVerifyClientCert,
+		ClientCAs:    cpool,
+		CurvePreferences: []tls.CurveID{
+			tls.CurveP256,
+			tls.X25519,
+		},
+	}
+
+	lc.serverCfg = tlsCfg
+
+	// client TLS config; we need the proper root
+	ctlsCfg := *tlsCfg
+
+	// This is a _bad_ client cert (different root)
+	ctlsCfg.Certificates = []tls.Certificate{cert}
+
+	// create a server on the other end of a connector
+	s := newTcpServer("tcp", lc.Connect.Addr, nil, t)
+	assert(s != nil, "server creation failed")
+
+	log := newLogger(t)
+	gt := NewServer(lc, cfg, log)
+	gt.Start()
+
+	// Now create a mock client to send data to mock server
+	c := newTcpClient("tcp", lc.Addr, &ctlsCfg, t)
+	assert(c != nil, "client creation failed")
+
+	c.start(10)
+	assert(c.nr == 0, "client read from closed conn %d bytes", c.nr)
+
+	c.stop()
+	s.stop()
+	gt.Stop()
+	log.Close()
 }