[release-branch.go1.12-security] crypto/x509: mitigate CVE-2020-0601 verification bypass on Windows

An attacker can trick the Windows system verifier to use a poisoned set
of elliptic curve parameters for a trusted root, allowing it to generate
spoofed signatures. When this happens, the returned chain will present
the unmodified original root, so the actual signatures won't verify (as
they are invalid for the correct parameters). Simply double check them
as a safety measure and mitigation.

Windows users should still install the system security patch ASAP.

This is the same mitigation adopted by Chromium:

https://chromium-review.googlesource.com/c/chromium/src/+/1994434

Change-Id: I2c734f6fb2cb51d906c7fd77034318ffeeb3e146
Reviewed-on: https://go-review.googlesource.com/c/go/+/215905
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ryan Sleevi <sleevi@google.com>
Reviewed-by: Katie Hockman <katie@golang.org>
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/647124
Reviewed-by: Filippo Valsorda <valsorda@google.com>

1 files changed, 18 insertions, 2 deletions

diff --git a/src/crypto/x509/root_windows.go b/src/crypto/x509/root_windows.go
index 74d395df70..3da3d06e73 100644
--- a/src/crypto/x509/root_windows.go
+++ b/src/crypto/x509/root_windows.go
@@ -219,10 +219,26 @@ func (c *Certificate) systemVerify(opts *VerifyOptions) (chains [][]*Certificate
 	if err != nil {
 		return nil, err
 	}
+	if len(chain) < 1 {
+		return nil, errors.New("x509: internal error: system verifier returned an empty chain")
+	}
 
-	chains = append(chains, chain)
+	// Mitigate CVE-2020-0601, where the Windows system verifier might be
+	// tricked into using custom curve parameters for a trusted root, by
+	// double-checking all ECDSA signatures. If the system was tricked into
+	// using spoofed parameters, the signature will be invalid for the correct
+	// ones we parsed. (We don't support custom curves ourselves.)
+	for i, parent := range chain[1:] {
+		if parent.PublicKeyAlgorithm != ECDSA {
+			continue
+		}
+		if err := parent.CheckSignature(chain[i].SignatureAlgorithm,
+			chain[i].RawTBSCertificate, chain[i].Signature); err != nil {
+			return nil, err
+		}
+	}
 
-	return chains, nil
+	return [][]*Certificate{chain}, nil
 }
 
 func loadSystemRoots() (*CertPool, error) {