Skip to content

Commit 00b6348

Browse files
committed
crypto/tls: err for unsupported point format configs
If a client or server explicitly offers point formats, and the point formats don't include the uncompressed format, then error. This matches BoringSSL and Rustls behaviour and allows enabling the PointFormat-Client-MissingUncompressed bogo test. Updates #72006 Change-Id: I27a2cd231e4b8762b0d9e2dbd3d8ddd5b87fd5c5 Reviewed-on: https://go-review.googlesource.com/c/go/+/669157 Reviewed-by: Roland Shoemaker <roland@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
1 parent 992d154 commit 00b6348

File tree

5 files changed

+31
-7
lines changed

5 files changed

+31
-7
lines changed

src/crypto/tls/bogo_config.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,6 @@
131131
"SendClientVersion-RSA": "TODO: first pass, this should be fixed",
132132
"NoCommonCurves": "TODO: first pass, this should be fixed",
133133
"PointFormat-EncryptedExtensions-TLS13": "TODO: first pass, this should be fixed",
134-
"PointFormat-Client-MissingUncompressed": "TODO: first pass, this should be fixed",
135134
"TLS13-SendNoKEMModesWithPSK-Server": "TODO: first pass, this should be fixed",
136135
"TLS13-DuplicateTicketEarlyDataSupport": "TODO: first pass, this should be fixed",
137136
"Basic-Client-NoTicket-TLS-Sync": "TODO: first pass, this should be fixed",

src/crypto/tls/common.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1372,7 +1372,11 @@ func (chi *ClientHelloInfo) SupportsCertificate(c *Certificate) error {
13721372
}
13731373

13741374
// The only signed key exchange we support is ECDHE.
1375-
if !supportsECDHE(config, vers, chi.SupportedCurves, chi.SupportedPoints) {
1375+
ecdheSupported, err := supportsECDHE(config, vers, chi.SupportedCurves, chi.SupportedPoints)
1376+
if err != nil {
1377+
return err
1378+
}
1379+
if !ecdheSupported {
13761380
return supportsRSAFallback(errors.New("client doesn't support ECDHE, can only use legacy RSA key exchange"))
13771381
}
13781382

src/crypto/tls/handshake_client.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -893,6 +893,19 @@ func (hs *clientHandshakeState) processServerHello() (bool, error) {
893893
return false, errors.New("tls: server selected unsupported compression format")
894894
}
895895

896+
supportsPointFormat := false
897+
offeredNonCompressedFormat := false
898+
for _, format := range hs.serverHello.supportedPoints {
899+
if format == pointFormatUncompressed {
900+
supportsPointFormat = true
901+
} else {
902+
offeredNonCompressedFormat = true
903+
}
904+
}
905+
if !supportsPointFormat && offeredNonCompressedFormat {
906+
return false, errors.New("tls: server offered only incompatible point formats")
907+
}
908+
896909
if c.handshakes == 0 && hs.serverHello.secureRenegotiationSupported {
897910
c.secureRenegotiation = true
898911
if len(hs.serverHello.secureRenegotiation) != 0 {

src/crypto/tls/handshake_server.go

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,11 @@ func (hs *serverHandshakeState) processClientHello() error {
272272
hs.hello.scts = hs.cert.SignedCertificateTimestamps
273273
}
274274

275-
hs.ecdheOk = supportsECDHE(c.config, c.vers, hs.clientHello.supportedCurves, hs.clientHello.supportedPoints)
275+
hs.ecdheOk, err = supportsECDHE(c.config, c.vers, hs.clientHello.supportedCurves, hs.clientHello.supportedPoints)
276+
if err != nil {
277+
c.sendAlert(alertMissingExtension)
278+
return err
279+
}
276280

277281
if hs.ecdheOk && len(hs.clientHello.supportedPoints) > 0 {
278282
// Although omitting the ec_point_formats extension is permitted, some
@@ -343,7 +347,7 @@ func negotiateALPN(serverProtos, clientProtos []string, quic bool) (string, erro
343347

344348
// supportsECDHE returns whether ECDHE key exchanges can be used with this
345349
// pre-TLS 1.3 client.
346-
func supportsECDHE(c *Config, version uint16, supportedCurves []CurveID, supportedPoints []uint8) bool {
350+
func supportsECDHE(c *Config, version uint16, supportedCurves []CurveID, supportedPoints []uint8) (bool, error) {
347351
supportsCurve := false
348352
for _, curve := range supportedCurves {
349353
if c.supportsCurve(version, curve) {
@@ -353,10 +357,12 @@ func supportsECDHE(c *Config, version uint16, supportedCurves []CurveID, support
353357
}
354358

355359
supportsPointFormat := false
360+
offeredNonCompressedFormat := false
356361
for _, pointFormat := range supportedPoints {
357362
if pointFormat == pointFormatUncompressed {
358363
supportsPointFormat = true
359-
break
364+
} else {
365+
offeredNonCompressedFormat = true
360366
}
361367
}
362368
// Per RFC 8422, Section 5.1.2, if the Supported Point Formats extension is
@@ -365,9 +371,11 @@ func supportsECDHE(c *Config, version uint16, supportedCurves []CurveID, support
365371
// the parser. See https://go.dev/issue/49126.
366372
if len(supportedPoints) == 0 {
367373
supportsPointFormat = true
374+
} else if offeredNonCompressedFormat && !supportsPointFormat {
375+
return false, errors.New("tls: client offered only incompatible point formats")
368376
}
369377

370-
return supportsCurve && supportsPointFormat
378+
return supportsCurve && supportsPointFormat, nil
371379
}
372380

373381
func (hs *serverHandshakeState) pickCipherSuite() error {

src/crypto/tls/tls_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1479,7 +1479,7 @@ func TestClientHelloInfo_SupportsCertificate(t *testing.T) {
14791479
SupportedPoints: []uint8{1},
14801480
SignatureSchemes: []SignatureScheme{ECDSAWithP256AndSHA256},
14811481
SupportedVersions: []uint16{VersionTLS12},
1482-
}, "doesn't support ECDHE"},
1482+
}, "only incompatible point formats"},
14831483
{ecdsaCert, &ClientHelloInfo{
14841484
CipherSuites: []uint16{TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256},
14851485
SupportedCurves: []CurveID{CurveP256},

0 commit comments

Comments
 (0)