golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.03k stars 17.67k forks source link

crypto/tls: no cipher suite supported by both client and server when client doesn't set supportedPoints #53750

Closed torntrousers closed 2 years ago

torntrousers commented 2 years ago

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

$  go1.18.3 linux/amd64

Does this issue reproduce with the latest release?

I think so, the master code looks like the same around this

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/aelder/.cache/go-build"
GOENV="/home/aelder/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/aelder/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/aelder/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/snap/go/9848"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/snap/go/9848/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.18.3"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build1921783961=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I've an IoT server written in Go that IoT devices connect to with TLS. It works fine with a variety of devices. Now I'm trying to connect to it with a new device using a Calypso Wifi module but it wont connect and fails with tls: no cipher suite supported by both client and server.

Debugging by littering the Go TLS code with prints it turns out that it fails because the call to supportsECDHE returns false. It's using secp256r1 ecdsa keys and both server and device are set to use TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256.

Debugging further it looks like the Calypso module is not setting any supportedPoints in its client hello (hs.clientHello.supportedPoints) so then supportsECDHE will always return false.

I'd have guessed that this is a bug with the Calypso module however there is a comment in the code just on from here saying "omitting the ec_point_formats extension is permitted", so should it actually be supported that clients don't send this?

As a quick test I tried the following and it does fix the problem and the Calyspso device is now able to connect:

diff --git a/src/crypto/tls/handshake_server.go b/src/crypto/tls/handshake_server.go
index 7606305c1d..7ae787de80 100644
--- a/src/crypto/tls/handshake_server.go
+++ b/src/crypto/tls/handshake_server.go
@@ -238,6 +238,11 @@ func (hs *serverHandshakeState) processClientHello() error {
                hs.hello.scts = hs.cert.SignedCertificateTimestamps
        }

+       if len(hs.clientHello.supportedPoints) == 0 {
+               hs.clientHello.supportedPoints = []uint8{pointFormatUncompressed}
+               fmt.Printf("****** dbg 1 fixed: clientHello.supportedPoints: %x\n", hs.clientHello.supportedPoints)
+       }
+
        hs.ecdheOk = supportsECDHE(c.config, hs.clientHello.supportedCurves, hs.clientHello.supportedPoints)

        if hs.ecdheOk {

(might be nicer to fix it in the supportsECDHE function though)

So interested in your comments? Is it really ok for a client to not specifiy supportedPoints? If so would you take a PR from me to fix it?

mengzhuo commented 2 years ago

cc @golang/security

ZekeLu commented 2 years ago

duplicate of #49126?

torntrousers commented 2 years ago

duplicate of #49126?

Yes, it does look like the fix in #49126 would fix what we are seeing.

seankhliao commented 2 years ago

Duplicate of #49126