Add tests for the point format extension. Upstream accidentally started rejecting server-sent point formats in https://github.com/openssl/openssl/issues/2133. Our own test coverage here is also lacking, so flesh it out. Change-Id: I99059558bd28d3a540c9687649d6db7e16579d29 Reviewed-on: https://boringssl-review.googlesource.com/12979 Reviewed-by: Steven Valdez <svaldez@google.com> Reviewed-by: David Benjamin <davidben@google.com> Commit-Queue: David Benjamin <davidben@google.com> CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go index dd185ce..e527185 100644 --- a/ssl/test/runner/common.go +++ b/ssl/test/runner/common.go
@@ -127,7 +127,8 @@ // TLS Elliptic Curve Point Formats // http://www.iana.org/assignments/tls-parameters/tls-parameters.xml#tls-parameters-9 const ( - pointFormatUncompressed uint8 = 0 + pointFormatUncompressed uint8 = 0 + pointFormatCompressedPrime uint8 = 1 ) // TLS CertificateStatusType (RFC 3546) @@ -1238,6 +1239,11 @@ // ClearShortHeaderBit, if true, causes the server to send short headers // without the high bit set. ClearShortHeaderBit bool + + // SendSupportedPointFormats, if not nil, is the list of supported point + // formats to send in ClientHello or ServerHello. If set to a non-nil + // empty slice, no extension will be sent. + SendSupportedPointFormats []byte } func (c *Config) serverInit() {
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go index 9691ae6..4e715b5 100644 --- a/ssl/test/runner/handshake_client.go +++ b/ssl/test/runner/handshake_client.go
@@ -105,6 +105,10 @@ hello.compressionMethods = c.config.Bugs.SendCompressionMethods } + if c.config.Bugs.SendSupportedPointFormats != nil { + hello.supportedPoints = c.config.Bugs.SendSupportedPointFormats + } + if len(c.clientVerify) > 0 && !c.config.Bugs.EmptyRenegotiationInfo { if c.config.Bugs.BadRenegotiationInfo { hello.secureRenegotiation = append(hello.secureRenegotiation, c.clientVerify...)
diff --git a/ssl/test/runner/handshake_messages.go b/ssl/test/runner/handshake_messages.go index 1f87ad2..0d4d161 100644 --- a/ssl/test/runner/handshake_messages.go +++ b/ssl/test/runner/handshake_messages.go
@@ -316,9 +316,7 @@ extensions.addU16(extensionSupportedPoints) supportedPointsList := extensions.addU16LengthPrefixed() supportedPoints := supportedPointsList.addU8LengthPrefixed() - for _, pointFormat := range m.supportedPoints { - supportedPoints.addU8(pointFormat) - } + supportedPoints.addBytes(m.supportedPoints) } if m.hasKeyShares { extensions.addU16(extensionKeyShare) @@ -607,8 +605,7 @@ if length != l+1 { return false } - m.supportedPoints = make([]uint8, l) - copy(m.supportedPoints, data[1:]) + m.supportedPoints = data[1 : 1+l] case extensionSessionTicket: // http://tools.ietf.org/html/rfc5077#section-3.2 m.ticketSupported = true @@ -1093,6 +1090,7 @@ npnAfterAlpn bool hasKeyShare bool keyShare keyShareEntry + supportedPoints []uint8 } func (m *serverExtensions) marshal(extensions *byteBuilder) { @@ -1187,6 +1185,13 @@ keyExchange := keyShare.addU16LengthPrefixed() keyExchange.addBytes(m.keyShare.keyExchange) } + if len(m.supportedPoints) > 0 { + // http://tools.ietf.org/html/rfc4492#section-5.1.2 + extensions.addU16(extensionSupportedPoints) + supportedPointsList := extensions.addU16LengthPrefixed() + supportedPoints := supportedPointsList.addU8LengthPrefixed() + supportedPoints.addBytes(m.supportedPoints) + } } func (m *serverExtensions) unmarshal(data []byte, version uint16) bool { @@ -1287,7 +1292,15 @@ if version >= VersionTLS13 { return false } - // Ignore this extension from the server. + // http://tools.ietf.org/html/rfc4492#section-5.5.2 + if length < 1 { + return false + } + l := int(data[0]) + if length != l+1 { + return false + } + m.supportedPoints = data[1 : 1+l] case extensionSupportedCurves: // The server can only send supported_curves in TLS 1.3. if version < VersionTLS13 {
diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go index 6499806..8f4e97b 100644 --- a/ssl/test/runner/handshake_server.go +++ b/ssl/test/runner/handshake_server.go
@@ -1192,6 +1192,10 @@ serverExtensions.ticketSupported = true } + if c.config.Bugs.SendSupportedPointFormats != nil { + serverExtensions.supportedPoints = c.config.Bugs.SendSupportedPointFormats + } + if !hs.clientHello.hasGREASEExtension && config.Bugs.ExpectGREASE { return errors.New("tls: no GREASE extension found") }
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go index 502e86e..a71a9cc 100644 --- a/ssl/test/runner/runner.go +++ b/ssl/test/runner/runner.go
@@ -8121,6 +8121,102 @@ }, resumeSession: true, }) + + // Server-sent point formats are legal in TLS 1.2, but not in TLS 1.3. + testCases = append(testCases, testCase{ + name: "PointFormat-ServerHello-TLS12", + config: Config{ + MaxVersion: VersionTLS12, + Bugs: ProtocolBugs{ + SendSupportedPointFormats: []byte{pointFormatUncompressed}, + }, + }, + }) + testCases = append(testCases, testCase{ + name: "PointFormat-EncryptedExtensions-TLS13", + config: Config{ + MaxVersion: VersionTLS13, + Bugs: ProtocolBugs{ + SendSupportedPointFormats: []byte{pointFormatUncompressed}, + }, + }, + shouldFail: true, + expectedError: ":ERROR_PARSING_EXTENSION:", + }) + + // Test that we tolerate unknown point formats, as long as + // pointFormatUncompressed is present. Limit ciphers to ECDHE ciphers to + // check they are still functional. + testCases = append(testCases, testCase{ + name: "PointFormat-Client-Tolerance", + config: Config{ + MaxVersion: VersionTLS12, + Bugs: ProtocolBugs{ + SendSupportedPointFormats: []byte{42, pointFormatUncompressed, 99, pointFormatCompressedPrime}, + }, + }, + }) + testCases = append(testCases, testCase{ + testType: serverTest, + name: "PointFormat-Server-Tolerance", + config: Config{ + MaxVersion: VersionTLS12, + CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256}, + Bugs: ProtocolBugs{ + SendSupportedPointFormats: []byte{42, pointFormatUncompressed, 99, pointFormatCompressedPrime}, + }, + }, + }) + + // Test TLS 1.2 does not require the point format extension to be + // present. + testCases = append(testCases, testCase{ + name: "PointFormat-Client-Missing", + config: Config{ + MaxVersion: VersionTLS12, + CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256}, + Bugs: ProtocolBugs{ + SendSupportedPointFormats: []byte{}, + }, + }, + }) + testCases = append(testCases, testCase{ + testType: serverTest, + name: "PointFormat-Server-Missing", + config: Config{ + MaxVersion: VersionTLS12, + CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256}, + Bugs: ProtocolBugs{ + SendSupportedPointFormats: []byte{}, + }, + }, + }) + + // If the point format extension is present, uncompressed points must be + // offered. BoringSSL requires this whether or not ECDHE is used. + testCases = append(testCases, testCase{ + name: "PointFormat-Client-MissingUncompressed", + config: Config{ + MaxVersion: VersionTLS12, + Bugs: ProtocolBugs{ + SendSupportedPointFormats: []byte{pointFormatCompressedPrime}, + }, + }, + shouldFail: true, + expectedError: ":ERROR_PARSING_EXTENSION:", + }) + testCases = append(testCases, testCase{ + testType: serverTest, + name: "PointFormat-Server-MissingUncompressed", + config: Config{ + MaxVersion: VersionTLS12, + Bugs: ProtocolBugs{ + SendSupportedPointFormats: []byte{pointFormatCompressedPrime}, + }, + }, + shouldFail: true, + expectedError: ":ERROR_PARSING_EXTENSION:", + }) } func addTLS13RecordTests() {