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/rand: rand.Read will fail on Windows if you os.ClearEnv() #8002
Labels
Comments
Can I ask two background questions: 1. why are you using os.ClearEnv(), what does clearing the environment give you? Maybe there is another way to achieve the same result. 2. Rather than not clearing %SYSTEMROOT%, maybe we can fix rand.Read on _windows to store the the environment variables it needs in another place. Maybe, i'm not sure. On unix os.ClearEnv() clears a *copy* of the environment variables, the original ones exist in their original location, but are ofcourse inaccessible to Go code. |
Comment 2 by harvey@countersoft.com: Dave, The problem came to light during testing when some code cleared the environment. Not production code. Just not sure that %SystemRoot% should ever be nuked... |
Cannot reproduce: U:\>type main.go package main import ( "crypto/rand" "fmt" "log" "os" ) func printRand() error { b := make([]byte, 5) _, err := rand.Read(b) if err != nil { return err } fmt.Printf("%v\n", b) return nil } func main() { os.Clearenv() err := printRand() if err != nil { log.Fatalln(err) } } U:\>go run main.go [51 122 78 42 65] U:\>hg id -R %GOROOT% 263c466e5491 tip U:\> Closing as TimedOut. Will reopen if more information is available. Alex Status changed to TimedOut. |
Comment 7 by saar@countersoft.com: Try this: package main import ( "crypto/rand" "fmt" "os" ) type UUID [16]byte func main() { os.Clearenv() u := new(UUID) _, err := rand.Read(u[:]) if err != nil { fmt.Println(err) return } fmt.Println(u) } |
saar@countersoft.com, Thank you for the program. Your program is similar to mine in #6. And your program succeeds on my computer (windows/386). But it does fails on windows/amd64 (I didn't think to try it there before) with "CryptAcquireContext: Invalid Signature.". So I can reproduce your issue on windows/amd64. Unfortunately, I don't see what we can do here. You suggest to leave SYSTEMROOT variable alone. But what about other variables? What else can break when some environmental variable is missing? We would have to document that behaviour (not "clearing" SYSTEMROOT in os.Clearenv on windows). We would need some "official" recommendation to leave SYSTEMROOT alone. I haven't found any. Have you? Starting new process without SYSTEMROOT listed would have similar effect. Should we prohibit that too, and silently add SYSTEMROOT variable? I think there are too many questions with your suggestion. I think most sensible thing is to leave everything as is, and rely on us programmes doing our job properly. > May 15 (4 days ago) Project Member #1 dave@cheney.net > ... > 2. Rather than not clearing %SYSTEMROOT%, maybe we can fix rand.Read on _windows to store the the environment variables it needs in another place. Maybe, i'm not sure. On unix os.ClearEnv() clears a *copy* of the environment variables, the original ones exist in their original location, but are ofcourse inaccessible to Go code. syscall.Clearenv does not clear process environment. But it has this comment "// TODO(bradfitz): pass through to C". So I think it is something that still to happen (if anyone cares about it). So I think windows version is more correct here. Mind you windows syscall.Clearenv is actually setting all vars to blank, instead of actually deleting them. Leaving issue closed until I hear better suggestions. Alex Status changed to Unfortunate. |
Thanks for the diagnosis Alex. saar, I'd like to know more about why something (it was mentioned this was coming from a library, not your code) is calling os.ClearEnv. It feels to me that there will be a better way to achieve what that code wants without having to drain the entire environment. > syscall.Clearenv does not clear process environment. But it has this comment > "// TODO(bradfitz): pass through to C". So I think it is something that > still to happen (if anyone cares about it). So I think windows version is > more correct here. Mind you windows syscall.Clearenv is actually setting all > vars to blank, instead of actually deleting them. > > Leaving issue closed until I hear better suggestions. Thanks Alex, you are correct. It's hard to describe the behaviour os.ClearEnv on unix because it is both 1. incorrect, 2. implementation specific (I have no idea what gccgo does in this case). It's even more confusing when os/exec is used as the default environment it passes down to the child *is* the in memory copy, not the original, and that would be empty if os.ClearEnv is called. |
Comment 10 by saar@countersoft.com: Alex, From my tests it is only the %SYSTEMROOT% that is causing issues. Agreed about closing it, as it is just a gotcha really. Dave, Seems like there was a confusion here about os.ClearEnv(), we explicitly called it from our code. So as above, it is only a gotcha really on Windows 64bit. |
This issue was closed.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
by harvey@countersoft.com:
The text was updated successfully, but these errors were encountered: