crypto/x509: remove x509sha1 GODEBUG

Fixes #41682 Change-Id: I37760f2186e75ec7df9674db25ae466cf453d66d Reviewed-on: https://go-review.googlesource.com/c/go/+/629676 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Filippo Valsorda <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]>
golang · Nov 20, 2024 · 7c7170e · 7c7170e
1 parent c901d93
commit 7c7170e
Show file tree

Hide file tree

Showing 7 changed files with 134 additions and 352 deletions.
diff --git a/doc/godebug.md b/doc/godebug.md
@@ -188,6 +188,9 @@ calling into Go code from C will enable DIT, and disable it before returning to
 C if it was not enabled when Go code was entered.
 This currently only affects arm64 programs. For all other platforms it is a no-op.
 
+Go 1.24 removed the `x509sha1` setting.  `crypto/x509` no longer supports verifying
+signatures on certificates that use SHA-1 based signature algorithms.
+
 ### Go 1.23
 
 Go 1.23 changed the channels created by package time to be unbuffered

diff --git a/src/crypto/x509/root_unix_test.go b/src/crypto/x509/root_unix_test.go
@@ -151,7 +151,6 @@ func TestLoadSystemCertsLoadColonSeparatedDirs(t *testing.T) {
 	rootPEMs := []string{
 		gtsRoot,
 		googleLeaf,
-		startComRoot,
 	}
 
 	var certDirs []string

diff --git a/src/crypto/x509/verify_test.go b/src/crypto/x509/verify_test.go
diff --git a/src/crypto/x509/x509.go b/src/crypto/x509/x509.go
@@ -799,18 +799,10 @@ var ErrUnsupportedAlgorithm = errors.New("x509: cannot verify signature: algorit
 
 // An InsecureAlgorithmError indicates that the [SignatureAlgorithm] used to
 // generate the signature is not secure, and the signature has been rejected.
-//
-// To temporarily restore support for SHA-1 signatures, include the value
-// "x509sha1=1" in the GODEBUG environment variable. Note that this option will
-// be removed in a future release.
 type InsecureAlgorithmError SignatureAlgorithm
 
 func (e InsecureAlgorithmError) Error() string {
-	var override string
-	if SignatureAlgorithm(e) == SHA1WithRSA || SignatureAlgorithm(e) == ECDSAWithSHA1 {
-		override = " (temporarily override with GODEBUG=x509sha1=1)"
-	}
-	return fmt.Sprintf("x509: cannot verify signature: insecure algorithm %v", SignatureAlgorithm(e)) + override
+	return fmt.Sprintf("x509: cannot verify signature: insecure algorithm %v", SignatureAlgorithm(e))
 }
 
 // ConstraintViolationError results when a requested usage is not permitted by
@@ -887,8 +879,6 @@ func signaturePublicKeyAlgoMismatchError(expectedPubKeyAlgo PublicKeyAlgorithm,
 	return fmt.Errorf("x509: signature algorithm specifies an %s public key, but have public key of type %T", expectedPubKeyAlgo.String(), pubKey)
 }
 
-var x509sha1 = godebug.New("x509sha1")
-
 // checkSignature verifies that signature is a valid signature over signed from
 // a crypto.PublicKey.
 func checkSignature(algo SignatureAlgorithm, signed, signature []byte, publicKey crypto.PublicKey, allowSHA1 bool) (err error) {
@@ -911,12 +901,9 @@ func checkSignature(algo SignatureAlgorithm, signed, signature []byte, publicKey
 	case crypto.MD5:
 		return InsecureAlgorithmError(algo)
 	case crypto.SHA1:
-		// SHA-1 signatures are mostly disabled. See go.dev/issue/41682.
+		// SHA-1 signatures are only allowed for CRLs and CSRs.
 		if !allowSHA1 {
-			if x509sha1.Value() != "1" {
-				return InsecureAlgorithmError(algo)
-			}
-			x509sha1.IncNonDefault()
+			return InsecureAlgorithmError(algo)
 		}
 		fallthrough
 	default:

diff --git a/src/crypto/x509/x509_test.go b/src/crypto/x509/x509_test.go
@@ -1817,8 +1817,8 @@ func TestInsecureAlgorithmErrorString(t *testing.T) {
 		want string
 	}{
 		{MD5WithRSA, "x509: cannot verify signature: insecure algorithm MD5-RSA"},
-		{SHA1WithRSA, "x509: cannot verify signature: insecure algorithm SHA1-RSA (temporarily override with GODEBUG=x509sha1=1)"},
-		{ECDSAWithSHA1, "x509: cannot verify signature: insecure algorithm ECDSA-SHA1 (temporarily override with GODEBUG=x509sha1=1)"},
+		{SHA1WithRSA, "x509: cannot verify signature: insecure algorithm SHA1-RSA"},
+		{ECDSAWithSHA1, "x509: cannot verify signature: insecure algorithm ECDSA-SHA1"},
 		{MD2WithRSA, "x509: cannot verify signature: insecure algorithm 1"},
 		{-1, "x509: cannot verify signature: insecure algorithm -1"},
 		{0, "x509: cannot verify signature: insecure algorithm 0"},
@@ -1899,11 +1899,6 @@ func TestSHA1(t *testing.T) {
 	if _, ok := err.(InsecureAlgorithmError); !ok {
 		t.Fatalf("certificate verification returned %v (%T), wanted InsecureAlgorithmError", err, err)
 	}
-
-	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)
-	}
 }
 
 // certMissingRSANULL contains an RSA public key where the AlgorithmIdentifier

diff --git a/src/internal/godebugs/table.go b/src/internal/godebugs/table.go
@@ -61,7 +61,6 @@ var All = []Info{
 	{Name: "winsymlink", Package: "os", Changed: 22, Old: "0"},
 	{Name: "x509keypairleaf", Package: "crypto/tls", Changed: 23, Old: "0"},
 	{Name: "x509negativeserial", Package: "crypto/x509", Changed: 23, Old: "1"},
-	{Name: "x509sha1", Package: "crypto/x509"},
 	{Name: "x509usefallbackroots", Package: "crypto/x509"},
 	{Name: "x509usepolicies", Package: "crypto/x509"},
 	{Name: "zipinsecurepath", Package: "archive/zip"},

diff --git a/src/runtime/metrics/doc.go b/src/runtime/metrics/doc.go
@@ -358,10 +358,6 @@ Below is the full list of supported metrics, ordered lexicographically.
 		package due to a non-default GODEBUG=x509negativeserial=...
 		setting.
 
-	/godebug/non-default-behavior/x509sha1:events
-		The number of non-default behaviors executed by the crypto/x509
-		package due to a non-default GODEBUG=x509sha1=... setting.
-
 	/godebug/non-default-behavior/x509usefallbackroots:events
 		The number of non-default behaviors executed by the crypto/x509
 		package due to a non-default GODEBUG=x509usefallbackroots=...