Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crypto/x509: respect GODEBUG changes during program lifetime #56436

Closed
rsc opened this issue Oct 26, 2022 · 6 comments
Closed

crypto/x509: respect GODEBUG changes during program lifetime #56436

rsc opened this issue Oct 26, 2022 · 6 comments
Assignees
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@rsc
Copy link
Contributor

rsc commented Oct 26, 2022

For #41682 we allow GODEBUG=x509sha1=1 to reenable SHA1 certificate support.
But programs cannot use os.Setenv at startup, because the SHA1 behavior is loaded
into a global variable during func init in crypto/x509, apparently to make testing easier.
We should tweak the code to check GODEBUG as needed, and then we should backport the change.

Among other things, this will help Kubernetes update older releases to newer Go versions.

@rsc rsc added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 26, 2022
@rsc rsc added this to the Go1.20 milestone Oct 26, 2022
@rsc rsc self-assigned this Oct 26, 2022
@rsc
Copy link
Contributor Author

rsc commented Oct 26, 2022

@gopherbot please backport

@gopherbot
Copy link

Backport issue(s) opened: #56437 (for 1.18), #56438 (for 1.19).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

@gopherbot
Copy link

Change https://go.dev/cl/445496 mentions this issue: crypto/x509: respect GODEBUG changes for allowing SHA1 certificates

@gopherbot
Copy link

Change https://go.dev/cl/445655 mentions this issue: crypto/x509: respect GODEBUG changes for allowing SHA1 certificates

@gopherbot
Copy link

Change https://go.dev/cl/445656 mentions this issue: crypto/x509: respect GODEBUG changes for allowing SHA1 certificates

romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Nov 3, 2022
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.

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>
gopherbot pushed a commit that referenced this issue Nov 8, 2022
…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
gopherbot pushed a commit that referenced this issue Nov 8, 2022
…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 #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
@gopherbot
Copy link

Change https://go.dev/cl/449504 mentions this issue: internal/godebug: define more efficient API

gopherbot pushed a commit that referenced this issue Nov 14, 2022
We have been expanding our use of GODEBUG for compatibility,
and the current implementation forces a tradeoff between
freshness and efficiency. It parses the environment variable
in full each time it is called, which is expensive. But if clients
cache the result, they won't respond to run-time GODEBUG
changes, as happened with x509sha1 (#56436).

This CL changes the GODEBUG API to provide efficient,
up-to-date results. Instead of a single Get function,
New returns a *godebug.Setting that itself has a Get method.
Clients can save the result of New, which is no more expensive
than errors.New, in a global variable, and then call that
variable's Get method to get the value. Get costs only two
atomic loads in the case where the variable hasn't changed
since the last call.

Unfortunately, these changes do require importing sync
from godebug, which will mean that sync itself will never
be able to use a GODEBUG setting. That doesn't seem like
such a hardship. If it was really necessary, the runtime could
pass a setting to package sync itself at startup, with the
caveat that that setting, like the ones used by runtime itself,
would not respond to run-time GODEBUG changes.

Change-Id: I99a3acfa24fb2a692610af26a5d14bbc62c966ac
Reviewed-on: https://go-review.googlesource.com/c/go/+/449504
Run-TryBot: Russ Cox <rsc@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
andrew-d pushed a commit to tailscale/go that referenced this issue Dec 7, 2022
…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
@golang golang locked and limited conversation to collaborators Nov 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

2 participants