Do not send unsolicited SCTs in TLS 1.3.

The draft 18 implementation did not compute scts_requested correctly. As
a result, it always believed SCTs were requested. Fix this and add tests
for unsolicited OCSP responses and SCTs at all versions.

Thanks to Daniel Hirche for the report.

Change-Id: Ifc59c5c4d7edba5703fa485c6c7a4055b15954b4
Reviewed-on: https://boringssl-review.googlesource.com/12305
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>

ssl/t1_lib.c

diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index c318a9b..08c5db0 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1397,8 +1397,16 @@
 
 static int ext_sct_parse_clienthello(SSL *ssl, uint8_t *out_alert,
                                      CBS *contents) {
+  if (contents == NULL) {
+    return 1;
+  }
+
+  if (CBS_len(contents) != 0) {
+    return 0;
+  }
+
   ssl->s3->hs->scts_requested = 1;
-  return contents == NULL || CBS_len(contents) == 0;
+  return 1;
 }
 
 static int ext_sct_add_serverhello(SSL *ssl, CBB *out) {

ssl/test/runner/common.go

diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index c3b6b04..a6496fd 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -1201,6 +1201,14 @@
 	// PSKBinderFirst, if true, causes the client to send the PSK Binder
 	// extension as the first extension instead of the last extension.
 	PSKBinderFirst bool
+
+	// NoOCSPStapling, if true, causes the client to not request OCSP
+	// stapling.
+	NoOCSPStapling bool
+
+	// NoSignedCertificateTimestamps, if true, causes the client to not
+	// request signed certificate timestamps.
+	NoSignedCertificateTimestamps bool
 }
 
 func (c *Config) serverInit() {

ssl/test/runner/handshake_client.go

diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go
index 36dc1e0..208dcca 100644
--- a/ssl/test/runner/handshake_client.go
+++ b/ssl/test/runner/handshake_client.go
@@ -64,8 +64,8 @@
 		vers:                    versionToWire(maxVersion, c.isDTLS),
 		compressionMethods:      []uint8{compressionNone},
 		random:                  make([]byte, 32),
-		ocspStapling:            true,
-		sctListSupported:        true,
+		ocspStapling:            !c.config.Bugs.NoOCSPStapling,
+		sctListSupported:        !c.config.Bugs.NoSignedCertificateTimestamps,
 		serverName:              c.config.ServerName,
 		supportedCurves:         c.config.curvePreferences(),
 		pskKEModes:              []byte{pskDHEKEMode},
@@ -729,6 +729,22 @@
 		}
 		hs.writeServerHash(certMsg.marshal())
 
+		// Check for unsolicited extensions.
+		for i, cert := range certMsg.certificates {
+			if c.config.Bugs.NoOCSPStapling && cert.ocspResponse != nil {
+				c.sendAlert(alertUnsupportedExtension)
+				return errors.New("tls: unexpected OCSP response in the server certificate")
+			}
+			if c.config.Bugs.NoSignedCertificateTimestamps && cert.sctList != nil {
+				c.sendAlert(alertUnsupportedExtension)
+				return errors.New("tls: unexpected SCT list in the server certificate")
+			}
+			if i > 0 && c.config.Bugs.ExpectNoExtensionsOnIntermediate && (cert.ocspResponse != nil || cert.sctList != nil) {
+				c.sendAlert(alertUnsupportedExtension)
+				return errors.New("tls: unexpected extensions in the server certificate")
+			}
+		}
+
 		if err := hs.verifyCertificates(certMsg); err != nil {
 			return err
 		}
@@ -736,15 +752,6 @@
 		c.ocspResponse = certMsg.certificates[0].ocspResponse
 		c.sctList = certMsg.certificates[0].sctList
 
-		if c.config.Bugs.ExpectNoExtensionsOnIntermediate {
-			for _, cert := range certMsg.certificates[1:] {
-				if cert.ocspResponse != nil || cert.sctList != nil {
-					c.sendAlert(alertUnsupportedExtension)
-					return errors.New("tls: unexpected extensions in the client certificate")
-				}
-			}
-		}
-
 		msg, err = c.readHandshake()
 		if err != nil {
 			return err
@@ -1212,10 +1219,18 @@
 		return errors.New("tls: server advertised OCSP in ServerHello over TLS 1.3")
 	}
 
+	if serverExtensions.ocspStapling && c.config.Bugs.NoOCSPStapling {
+		return errors.New("tls: server advertised unrequested OCSP extension")
+	}
+
 	if len(serverExtensions.sctList) > 0 && c.vers >= VersionTLS13 {
 		return errors.New("tls: server advertised SCTs in ServerHello over TLS 1.3")
 	}
 
+	if len(serverExtensions.sctList) > 0 && c.config.Bugs.NoSignedCertificateTimestamps {
+		return errors.New("tls: server advertised unrequested SCTs")
+	}
+
 	if serverExtensions.srtpProtectionProfile != 0 {
 		if serverExtensions.srtpMasterKeyIdentifier != "" {
 			return errors.New("tls: server selected SRTP MKI value")

ssl/test/runner/runner.go

diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 717c420..918618d 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -5197,6 +5197,25 @@
 			expectedSCTList: testSCTList,
 			resumeSession:   true,
 		})
+
+		// Test that certificate-related extensions are not sent unsolicited.
+		testCases = append(testCases, testCase{
+			testType: serverTest,
+			name:     "UnsolicitedCertificateExtensions-" + ver.name,
+			config: Config{
+				MaxVersion: ver.version,
+				Bugs: ProtocolBugs{
+					NoOCSPStapling:                true,
+					NoSignedCertificateTimestamps: true,
+				},
+			},
+			flags: []string{
+				"-ocsp-response",
+				base64.StdEncoding.EncodeToString(testOCSPResponse),
+				"-signed-cert-timestamps",
+				base64.StdEncoding.EncodeToString(testSCTList),
+			},
+		})
 	}
 
 	testCases = append(testCases, testCase{