Race condition during SNI extraction when TLS ClientHello is fragmented leading to ssl-passthrough being "ignored"

[alshain]

I believe I stumbled over a problem related to tldr.fail, where SNI extraction might fail with large TLS ClientHellos and SSL-passthrough.

Due to a race condition when reading the buffer used for the SNI extraction, the extraction fails but the failure is ignored and we default to the default proxy target.

ingress-nginx/pkg/tcpproxy/tcp.go

Line 65 in 44e550e

length, err := conn.Read(data)

func (p *TCPProxy) Handle(conn net.Conn) {
	defer conn.Close()
	// See: https://www.ibm.com/docs/en/ztpf/1.1.0.15?topic=sessions-ssl-record-format
	data := make([]byte, 16384)

	// vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
        // suppose the ClientHello is fragmented over multiple packets, 
        // data might only contain a partial ClientHello where the SNI data isn't present yet
	length, err := conn.Read(data)  
	if err != nil {
		klog.V(4).ErrorS(err, "Error reading data from the connection")
		return
	}

	proxy := p.Default  
	hostname, err := parser.GetHostname(data)
	// vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
	if err == nil {  // <--- err will not be nill, so we skip this, proxy remains p.Default
		klog.V(4).InfoS("TLS Client Hello", "host", hostname)
		proxy = p.Get(hostname)
	}

	// vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
	if proxy == nil {   // because we have p.Default, we skip this
		klog.V(4).InfoS("There is no configured proxy for SSL connections.")
		return
	}
	// vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
        // now we forward traffic to NGINX instead of the proper passthrough target

	hostPort := net.JoinHostPort(proxy.IP, fmt.Sprintf("%v", proxy.Port))

[k8s-ci-robot]

This issue is currently awaiting triage.

If Ingress contributors determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[alshain]

Oh well, I had searched but couldn't find anything, already reported: #11424

Sorry about that.

[longwuyuan]

This is a complicated one so I think info coming from 2 is helpful