crypto/tls: add TestInvalidHandshakeSignature

Change-Id: I574b2443ea3695264379da28425acf716a6a6964
Reviewed-on: https://go-review.googlesource.com/c/go/+/725040
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Filippo Valsorda <filippo@golang.org>

src/crypto/tls/tls_test.go

@@ -2523,21 +2523,6 @@ func TestHandshakeMLDSA(t *testing.T) {
 	}
 }
 
-// bitFlippingSigner wraps a crypto.Signer and flips the last bit of every
-// signature it produces, used to test that peers reject invalid signatures.
-type bitFlippingSigner struct{ crypto.Signer }
-
-func (s bitFlippingSigner) Sign(rand io.Reader, msg []byte, opts crypto.SignerOpts) ([]byte, error) {
-	sig, err := s.Signer.Sign(rand, msg, opts)
-	if err != nil {
-		return nil, err
-	}
-	if len(sig) > 0 {
-		sig[len(sig)-1] ^= 1
-	}
-	return sig, nil
-}
-
 func TestX509KeyPairPopulateCertificate(t *testing.T) {
 	key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
 	if err != nil {
@@ -2858,3 +2843,116 @@ func (s messageOnlySigner) SignMessage(rand io.Reader, msg []byte, opts crypto.S
 	digest := h.Sum(nil)
 	return s.Signer.Sign(rand, digest, opts)
 }
+
+// bitFlippingSigner wraps a crypto.Signer and flips a bit in the signature,
+// producing an invalid signature.
+type bitFlippingSigner struct{ crypto.Signer }
+
+func (s bitFlippingSigner) Public() crypto.PublicKey {
+	return s.Signer.Public()
+}
+
+func (s bitFlippingSigner) Sign(rand io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte, error) {
+	sig, err := s.Signer.Sign(rand, digest, opts)
+	if err != nil {
+		return nil, err
+	}
+	sig[0] ^= 1
+	return sig, nil
+}
+
+// TestInvalidHandshakeSignature tests that invalid handshake signatures are
+// rejected for all TLS versions, for both server and client certificates,
+// even when InsecureSkipVerify or RequireAnyClientCert are used (which disable
+// certificate chain verification, but not signature verification).
+func TestInvalidHandshakeSignature(t *testing.T) {
+	t.Run("TLSv10", func(t *testing.T) {
+		skipFIPS(t)
+		testInvalidHandshakeSignature(t, VersionTLS10)
+	})
+	t.Run("TLSv12", func(t *testing.T) { testInvalidHandshakeSignature(t, VersionTLS12) })
+	t.Run("TLSv13", func(t *testing.T) { testInvalidHandshakeSignature(t, VersionTLS13) })
+}
+
+func testInvalidHandshakeSignature(t *testing.T, version uint16) {
+	serverConfig := testConfigServer.Clone()
+	serverConfig.MaxVersion = version
+	serverConfig.MinVersion = version
+	serverConfig.SessionTicketsDisabled = true
+	clientConfig := testConfigClient.Clone()
+	clientConfig.MaxVersion = version
+	clientConfig.MinVersion = version
+
+	// Test that the server rejects invalid client certificate signatures,
+	// even when RequireAnyClientCert is used.
+	t.Run("ClientSignature", func(t *testing.T) {
+		serverConfig := serverConfig.Clone()
+		serverConfig.ClientAuth = RequireAnyClientCert
+		clientConfig := clientConfig.Clone()
+		clientConfig.Certificates = []Certificate{{
+			Certificate: testClientECDSAP256Cert.Certificate,
+			PrivateKey:  bitFlippingSigner{testClientECDSAP256Cert.PrivateKey.(crypto.Signer)},
+		}}
+
+		clientErr, serverErr := testInvalidSignatureHandshake(t, clientConfig, serverConfig)
+		if serverErr == nil {
+			t.Fatalf("expected server to reject invalid client signature; client err = %v", clientErr)
+		}
+		if !strings.Contains(serverErr.Error(), "invalid signature") {
+			t.Errorf("expected 'invalid signature' error, got: %v", serverErr)
+		}
+	})
+
+	// Test that the client rejects invalid server certificate signatures.
+	t.Run("ServerSignature", func(t *testing.T) {
+		serverConfig := serverConfig.Clone()
+		serverConfig.Certificates = []Certificate{{
+			Certificate: testRSA2048Cert.Certificate,
+			PrivateKey:  bitFlippingSigner{testRSA2048Cert.PrivateKey.(crypto.Signer)},
+		}}
+
+		clientErr, serverErr := testInvalidSignatureHandshake(t, clientConfig, serverConfig)
+		if clientErr == nil {
+			t.Fatalf("expected client to reject invalid server signature; server err = %v", serverErr)
+		}
+		if !strings.Contains(clientErr.Error(), "invalid signature") {
+			t.Errorf("expected 'invalid signature' error, got: %v", clientErr)
+		}
+	})
+
+	// Test that InsecureSkipVerify doesn't disable server signature verification.
+	t.Run("ServerSignature/InsecureSkipVerify", func(t *testing.T) {
+		clientConfig := clientConfig.Clone()
+		clientConfig.InsecureSkipVerify = true
+		serverConfig := serverConfig.Clone()
+		serverConfig.Certificates = []Certificate{{
+			Certificate: testRSA2048Cert.Certificate,
+			PrivateKey:  bitFlippingSigner{testRSA2048Cert.PrivateKey.(crypto.Signer)},
+		}}
+
+		clientErr, serverErr := testInvalidSignatureHandshake(t, clientConfig, serverConfig)
+		if clientErr == nil {
+			t.Fatalf("expected client to reject invalid server signature despite InsecureSkipVerify; server err = %v", serverErr)
+		}
+		if !strings.Contains(clientErr.Error(), "invalid signature") {
+			t.Errorf("expected 'invalid signature' error, got: %v", clientErr)
+		}
+	})
+}
+
+// testInvalidSignatureHandshake performs a TLS handshake and returns the
+// errors from both client and server. Unlike testHandshake, it doesn't try
+// to exchange data after the handshake.
+func testInvalidSignatureHandshake(t *testing.T, clientConfig, serverConfig *Config) (clientErr, serverErr error) {
+	c, s := localPipe(t)
+	done := make(chan struct{})
+	go func() {
+		defer close(done)
+		clientErr = Client(c, clientConfig).Handshake()
+		c.Close()
+	}()
+	serverErr = Server(s, serverConfig).Handshake()
+	s.Close()
+	<-done
+	return
+}