[go] crypto/x509: respect GODEBUG changes for allowing SHA1 certificates

[Gopher Robot (Gerrit)]

Gopher Robot submitted this change.

View Change

1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.

Approvals: Russ Cox: Run TryBots; Automatically submit change David Chase: Looks good to me, approved Roland Shoemaker: Looks good to me, approved Gopher Robot: TryBots succeeded

crypto/x509: respect GODEBUG changes for allowing SHA1 certificates

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.

Change-Id: Idcb96212a1d8c560e1dd8eaf7c80b6266f16431e
Reviewed-on: https://go-review.googlesource.com/c/go/+/445496
Reviewed-by: David Chase <drc...@google.com>
Run-TryBot: Russ Cox <r...@golang.org>
Reviewed-by: Roland Shoemaker <rol...@golang.org>
TryBot-Result: Gopher Robot <go...@golang.org>
Auto-Submit: Russ Cox <r...@golang.org>
---
M src/crypto/x509/verify_test.go
M src/crypto/x509/x509.go
M src/crypto/x509/x509_test.go
3 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/src/crypto/x509/verify_test.go b/src/crypto/x509/verify_test.go
index 7bc58d4..4c21e78 100644
--- a/src/crypto/x509/verify_test.go
+++ b/src/crypto/x509/verify_test.go
@@ -543,8 +543,8 @@
 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 3c8fc44..35394bf 100644
--- a/src/crypto/x509/x509.go
+++ b/src/crypto/x509/x509.go
@@ -728,9 +728,6 @@
 // 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 @@
 
 	// 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 @@
 	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 79bfe66..5690e20 100644
--- a/src/crypto/x509/x509_test.go
+++ b/src/crypto/x509/x509_test.go
@@ -1876,9 +1876,7 @@
 		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 TestDisableSHA1ForCertOnly(t *testing.T) {
-	defer func(old bool) { debugAllowSHA1 = old }(debugAllowSHA1)
-	debugAllowSHA1 = false
+	t.Setenv("GODEBUG", "")
 
 	tmpl := &Certificate{
 		SerialNumber:          big.NewInt(1),

To view, visit change 445496. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: Idcb96212a1d8c560e1dd8eaf7c80b6266f16431e

Gerrit-Change-Number: 445496

Gerrit-PatchSet: 3

Gerrit-Owner: Russ Cox <r...@golang.org>

Gerrit-Reviewer: David Chase <drc...@google.com>

Gerrit-Reviewer: Filippo Valsorda <fil...@golang.org>

Gerrit-Reviewer: Gopher Robot <go...@golang.org>

Gerrit-Reviewer: Roland Shoemaker <rol...@golang.org>

Gerrit-Reviewer: Russ Cox <r...@golang.org>

Gerrit-MessageType: merged