[release-branch.go1.19] crypto/x509: respect GODEBUG changes for allowing SHA1 certif...

This allows programs that want SHA1 support to call os.Setenv at startup
instead of insisting that users set the environment variable themselves.

For #41682.
Fixes #56436.
Fixes #56438.

Change-Id: Idcb96212a1d8c560e1dd8eaf7c80b6266f16431e
Reviewed-on: https://go-review.googlesource.com/c/go/+/445496
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/445655

diff --git a/src/crypto/x509/verify_test.go b/src/crypto/x509/verify_test.go
index 7bc58d4754e5c440dcd55ba946cb3261c9906f4c..4c21e78ccc925fb015bb2ff6be94e0a8cfe6cd59 100644 (file)
--- a/src/crypto/x509/verify_test.go
+++ b/src/crypto/x509/verify_test.go
@@ -543,8 +543,8 @@ func testVerify(t *testing.T, test verifyTest, useSystemRoots bool) {
 func TestGoVerify(t *testing.T) {
        // Temporarily enable SHA-1 verification since a number of test chains
        // require it. TODO(filippo): regenerate test chains.
-       defer func(old bool) { debugAllowSHA1 = old }(debugAllowSHA1)
-       debugAllowSHA1 = true
+       t.Setenv("GODEBUG", "x509sha1=1")
+
        for _, test := range verifyTests {
                t.Run(test.name, func(t *testing.T) {
                        testVerify(t, test, false)

diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go
index 7c64761bd7603b11b68cb5a4536fd5265a6a98e3..23c514bd78dda143be07c1e081fdffb0c99d57b2 100644 (file)
--- a/src/crypto/x509/x509.go
+++ b/src/crypto/x509/x509.go
@@ -728,9 +728,6 @@ type Certificate struct {
 // involves algorithms that are not currently implemented.
 var ErrUnsupportedAlgorithm = errors.New("x509: cannot verify signature: algorithm unimplemented")
 
-// debugAllowSHA1 allows SHA-1 signatures. See issue 41682.
-var debugAllowSHA1 = godebug.Get("x509sha1") == "1"
-
 // An InsecureAlgorithmError indicates that the SignatureAlgorithm used to
 // generate the signature is not secure, and the signature has been rejected.
 //
@@ -790,7 +787,7 @@ func (c *Certificate) CheckSignatureFrom(parent *Certificate) error {
 
        // TODO(agl): don't ignore the path length constraint.
 
-       return checkSignature(c.SignatureAlgorithm, c.RawTBSCertificate, c.Signature, parent.PublicKey, debugAllowSHA1)
+       return checkSignature(c.SignatureAlgorithm, c.RawTBSCertificate, c.Signature, parent.PublicKey, false)
 }
 
 // CheckSignature verifies that signature is a valid signature over signed from
@@ -837,7 +834,8 @@ func checkSignature(algo SignatureAlgorithm, signed, signature []byte, publicKey
        case crypto.MD5:
                return InsecureAlgorithmError(algo)
        case crypto.SHA1:
-               if !allowSHA1 {
+               // SHA-1 signatures are mostly disabled. See go.dev/issue/41682.
+               if !allowSHA1 && godebug.Get("x509sha1") != "1" {
                        return InsecureAlgorithmError(algo)
                }
                fallthrough

diff --git a/src/crypto/x509/x509_test.go b/src/crypto/x509/x509_test.go
index b1cdabba283066f3e047b21b95ccf1df0fa8f9ac..167ddb7fd04d9e358cdce3dc3f10c57da37fbebd 100644 (file)
--- a/src/crypto/x509/x509_test.go
+++ b/src/crypto/x509/x509_test.go
@@ -1876,9 +1876,7 @@ func TestSHA1(t *testing.T) {
                t.Fatalf("certificate verification returned %v (%T), wanted InsecureAlgorithmError", err, err)
        }
 
-       defer func(old bool) { debugAllowSHA1 = old }(debugAllowSHA1)
-       debugAllowSHA1 = true
-
+       t.Setenv("GODEBUG", "x509sha1=1")
        if err = cert.CheckSignatureFrom(cert); err != nil {
                t.Fatalf("SHA-1 certificate did not verify with GODEBUG=x509sha1=1: %v", err)
        }
@@ -3470,8 +3468,7 @@ func TestParseUniqueID(t *testing.T) {
 }
 
 func TestDisableSHA1ForCertOnly(t *testing.T) {
-       defer func(old bool) { debugAllowSHA1 = old }(debugAllowSHA1)
-       debugAllowSHA1 = false
+       t.Setenv("GODEBUG", "")
 
        tmpl := &Certificate{
                SerialNumber:          big.NewInt(1),