Skip to content

Commit

Permalink
[release-branch.go1.18] 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 #41682.
Fixes #56436.
Fixes #56437.

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/+/445656
  • Loading branch information
rsc committed Nov 8, 2022
1 parent 156bf3d commit db5cb5f
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
Expand Up @@ -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)
Expand Down
8 changes: 3 additions & 5 deletions src/crypto/x509/x509.go
Expand Up @@ -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.
//
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
7 changes: 2 additions & 5 deletions src/crypto/x509/x509_test.go
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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),
Expand Down

0 comments on commit db5cb5f

Please sign in to comment.