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: no cipher suite supported by both client and server when client doesn't set supportedPoints #53750

Closed
torntrousers opened this issue Jul 8, 2022 · 4 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@torntrousers
Copy link

torntrousers commented Jul 8, 2022

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
Copy link
Contributor

mengzhuo commented Jul 8, 2022

cc @golang/security

@seankhliao seankhliao changed the title affected/package: crypto/tls/handshake_server.go crypto/tls: no cipher suite supported by both client and server when client doesn't set supportedPoints Jul 8, 2022
@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 8, 2022
@ZekeLu
Copy link
Contributor

ZekeLu commented Jul 8, 2022

duplicate of #49126?

@torntrousers
Copy link
Author

duplicate of #49126?

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

@seankhliao
Copy link
Member

Duplicate of #49126

@seankhliao seankhliao marked this as a duplicate of #49126 Jul 28, 2022
@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Jul 28, 2022
@golang golang locked and limited conversation to collaborators Jul 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge 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

5 participants