Skip to content

Commit

Permalink
[release-branch.go1.19] crypto/x509: respect GODEBUG changes for allo…
Browse files Browse the repository at this point in the history
…wing 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 golang#41682.
Fixes golang#56436.
Fixes golang#56438.

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/+/445655
  • Loading branch information
rsc authored and andrew-d committed Dec 7, 2022
1 parent 8b2e39b commit ace82e4
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 12 deletions.
4 changes: 2 additions & 2 deletions src/crypto/x509/verify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 3 additions & 5 deletions src/crypto/x509/x509.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
7 changes: 2 additions & 5 deletions src/crypto/x509/x509_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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),
Expand Down

0 comments on commit ace82e4

Please sign in to comment.