potential SIGPIPE if configuring tls client wrong

[hongchaodeng]

Scenario

When calling clientv3.New() with the wrong scheme -- "http://xxx", "https://xxx", program will receive SIGPIPE. Such user error should not cause program level exit.

Environment

etcd version:

• server: 3.0.17, 3.1.4, 3.1.7
• client: 3.1.0, 3.1.7

OS: linux

Reproduce

Updated with simpler reproduce steps without TLS:

...

func main() {
	c := make(chan os.Signal, 1)
	signal.Notify(c)
	go func() {
		log.Printf("received signal: %v", <-c)
		os.Exit(1)
	}()
// 2380 is peer port
	if _, err := CheckHealth("http://localhost:2380", nil); err != nil {
		panic(err)
	}

	fmt.Println("sleeping...")
	time.Sleep(5 * time.Second)
}

func CheckHealth(url string, tc *tls.Config) (bool, error) {
	cfg := clientv3.Config{
		Endpoints:   []string{url},
		DialTimeout: 5 * time.Second,
		TLS:         tc,
	}
	etcdcli, err := clientv3.New(cfg)
	if err != nil {
		return false, fmt.Errorf("failed to create etcd client for %s: %v", url, err)
	}
	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
	_, err = etcdcli.Get(ctx, "/", clientv3.WithSerializable())
	cancel()
	etcdcli.Close()
	if err != nil {
		return false, fmt.Errorf("etcd health probing failed for %s: %v", url, err)
	}
	return true, nil
}

[gyuho]

@hongchaodeng Can't reproduce. Tried the same code and etcd server/client versions, with wrong scheme. I do not get SIGPIPE, but this error, which is expected for wrong scheme.

rpc error: code = 13 desc = transport is closing

[gyuho]

@hongchaodeng This happens only in release-3.1 branch. So try with 3.2.0-rc.0 or master branch. release-3.1 passes initial endpoint when dialing, while master branch does not.
When cfg.Endpoints[0] is passed to processCreds with http scheme, we overwrite TLS fields with nil, so the client request would just hang.

release-3.1 https://github.com/coreos/etcd/blob/release-3.1/clientv3/client.go#L343-L349

client.balancer = newSimpleBalancer(cfg.Endpoints)
conn, err := client.dial(cfg.Endpoints[0], grpc.WithBalancer(client.balancer))

master https://github.com/coreos/etcd/blob/master/clientv3/client.go#L369-L370

client.balancer = newSimpleBalancer(cfg.Endpoints)
conn, err := client.dial("", grpc.WithBalancer(client.balancer))

c.f. #7733

I don't think we would backport that PR. It was for benchmarking. /cc @heyitsanthony @xiang90

[gyuho]

Was able to reproduce with @hongchaodeng

Client version (v3.1.x, master) does not matter.
Only happens when requesting against v3.1.x server.

package main

import (
	"context"
	"crypto/tls"
	"fmt"
	"log"
	"os"
	"os/signal"
	"time"

	"github.com/coreos/etcd/clientv3"
)

func main() {
	c := make(chan os.Signal, 1)
	signal.Notify(c)
	go func() {
		log.Printf("received signal: %v", <-c)
		os.Exit(1)
	}()

	if _, err := CheckHealth("http://localhost:2380", nil); err != nil {
		panic(err)
	}

	fmt.Println("sleeping...")
	time.Sleep(5 * time.Second)
}

func CheckHealth(url string, tc *tls.Config) (bool, error) {
	cfg := clientv3.Config{
		Endpoints:   []string{url},
		DialTimeout: 5 * time.Second,
		TLS:         tc,
	}
	etcdcli, err := clientv3.New(cfg)
	if err != nil {
		return false, fmt.Errorf("failed to create etcd client for %s: %v", url, err)
	}
	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
	_, err = etcdcli.Get(ctx, "/", clientv3.WithSerializable())
	cancel()
	etcdcli.Close()
	if err != nil {
		return false, fmt.Errorf("etcd health probing failed for %s: %v", url, err)
	}
	return true, nil
}

I think this is from Go runtime sending SIGPIPE https://golang.org/pkg/os/signal/#hdr-SIGPIPE
when client tries to write to wrong pipe (wrong port).

Docker has separate Notify routine to ignore this signal https://github.com/moby/moby/blob/master/pkg/signal/trap.go.

Any thoughts? @xiang90 @heyitsanthony

[gyuho]

@xiang90 @hongchaodeng I checked that grpc-go code does not trigger SIGPIPE. So it's the process getting the signals (c.f. golang/go#12931).

What other projects do (ignore SIGPIPEs and do not terminate):

• https://github.com/moby/moby/blob/master/pkg/signal/trap.go.
• x/mobile/bind: ios app crashes with SIGPIPE in background golang/go#17393

[gyuho]

Closing this for now, since it's more of Go issue, and happens only in 3.1 client.
Please reopen if anything is affected by this.