From db5cb5fceed48a487ca15c8321ddcb19652cc1b9 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Tue, 25 Oct 2022 23:10:13 -0400 Subject: [release-branch.go1.18] 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. Fixes #56437. 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 Reviewed-on: https://go-review.googlesource.com/c/go/+/445656 --- 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 100a8ff0f9..c3ef503b27 100644 --- 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 85720b3ccb..dc470f53f2 100644 --- 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 6c1e95a203..48a864057d 100644 --- 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), -- cgit v1.2.1