From 2a2b163bb9125389c5f70e65aec21816e2ea09c5 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Tue, 25 Oct 2022 23:10:13 -0400 Subject: [PATCH] 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 Run-TryBot: Russ Cox Reviewed-by: Roland Shoemaker TryBot-Result: Gopher Robot Auto-Submit: Russ Cox --- src/crypto/x509/verify_test.go | 4 ++-- src/crypto/x509/x509.go | 8 +++----- src/crypto/x509/x509_test.go | 7 ++----- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/src/crypto/x509/verify_test.go b/src/crypto/x509/verify_test.go index 7bc58d4754e..4c21e78ccc9 100644 --- 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 3c8fc44232e..35394bfc33d 100644 --- 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 79bfe66d113..5690e200b27 100644 --- 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),