[release-branch.go1.18] 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 #56437.

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/+/445656

diff --git a/src/crypto/x509/verify_test.go b/src/crypto/x509/verify_test.go
index 100a8ff0f94ae4c134d7ba61beacbd855fa4f81c..c3ef503b27784b52fa93d196d3ff59d3db5dee44 100644 (file)
--- a/src/crypto/x509/verify_test.go
+++ b/src/crypto/x509/verify_test.go
@@ -540,8 +540,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 85720b3ccbef9d74e982fd23bec93fac406b55cf..dc470f53f2dbcb62480cd83c12bc993f752ea7b4 100644 (file)
--- a/src/crypto/x509/x509.go
+++ b/src/crypto/x509/x509.go
@@ -730,9 +730,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.
 //
@@ -792,7 +789,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
@@ -839,7 +836,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 6c1e95a2032d4438bbef936899be014b19a7e141..48a864057df486c91e45ea1977724b83bc2bef0d 100644 (file)
--- a/src/crypto/x509/x509_test.go
+++ b/src/crypto/x509/x509_test.go
@@ -1864,9 +1864,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)
        }
@@ -3335,8 +3333,7 @@ func TestLargeOID(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),