Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crypto/tls: ClientHelloOuter should hide the actual ALPN list #71220

Open
rthellend opened this issue Jan 10, 2025 · 2 comments
Open

crypto/tls: ClientHelloOuter should hide the actual ALPN list #71220

rthellend opened this issue Jan 10, 2025 · 2 comments
Labels
BugReport Issues describing a possible bug in the Go implementation. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@rthellend
Copy link

Go version

go version 1.24rc1

Output of go env in your module/workspace:

AR='ar'
CC='gcc'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='g++'
GCCGO='gccgo'
GO111MODULE=''
GOAMD64='v1'
GOARCH='amd64'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/home/robin/.cache/go-build'
GOCACHEPROG=''
GODEBUG='http2xconnect=0'
GOENV='/home/robin/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build784986521=/tmp/go-build -gno-record-gcc-switches'
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMOD='/home/robin/src/ech-alpn/go.mod'
GOMODCACHE='/home/robin/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/robin/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/robin/src/goroot'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/home/robin/.config/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/home/robin/src/goroot/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='devel go1.24-c7c4420ae4 Thu Jan 9 12:24:58 2025 -0800'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

While experimenting with ECH in go 1.24rc1, I noticed that the alpn extension in ClientHelloOuter is the same as in ClientHelloInner.

While the draft RFC doesn't specify exactly what should be in ClientHelloOuter, it does call out the alpn list as a potentially sensitive field.

https://datatracker.ietf.org/doc/html/draft-ietf-tls-esni/#name-introduction

This document specifies a new TLS extension, called Encrypted Client Hello (ECH), that allows clients to encrypt their ClientHello to such a deployment. This protects the SNI and other potentially sensitive fields, such as the ALPN list [RFC7301].

This code shows the ALPN list and SNI in ClientHelloOuter: https://go.dev/play/p/UwIw0DLxH7U (although it doens't run in playground)

$ go run .
ALPNProtos: ["foo"]
ServerName: "public.example.com"

My suggestion would be to create the ClientHelloOuter as if the tls Config was pretty much empty, e.g.

&Config{
  ServerName: "The PublicName from the chosen ECH Config",
  MinVersion: VersionTLS13,
  NextProtos: []string{"h2", "http/1.1"},   // Replace with "h3" with quic?
}

What did you see happen?

The client's NextProtos are visible in ClientHelloOuter.

What did you expect to see?

The client's actual NextProtos should only be in ClientHelloInner when ECH is used.

@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 10, 2025
@seankhliao
Copy link
Member

cc @rolandshoemaker

@gabyhelp gabyhelp added the BugReport Issues describing a possible bug in the Go implementation. label Jan 10, 2025
rthellend added a commit to c2FmZQ/tlsproxy that referenced this issue Jan 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BugReport Issues describing a possible bug in the Go implementation. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

3 participants