Why does crypto/rand expose Reader?

Frederick Akalin

unread,

Feb 26, 2018, 1:40:48 PM2/26/18

to golang-nuts

crypto/rand exposes an io.Reader variable Reader as "a global, shared instance of a cryptographically strong pseudo-random generator." Furthermore, crypto/rand.Read implicitly uses Reader for its crypto source.

This seems problematic to me because then any package can just overwrite crypto/rand.Reader to point to some other object, affecting the security of any packages that rely on crypto/rand.Read or crypto/rand.Reader for security, e.g. x/crypto/nacl.

One can say that a language can never ultimately defend against code running in your same process, but I think it should be possible to write something that depends on crypto/rand for security that wouldn't require auditing other packages for a single malicious variable write.[1]

I have a proof of concept here: https://github.com/akalin/randtest . The main code looks like:

package main

import (

"alice/eightball"

"crypto/rand"

"fmt"

)

func main() {

eightball.Ask()

var b [32]byte

rand.Read(b[:])

fmt.Printf("%x\n", b)

}

which prints:

Outlook not so good

deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef

and where I half-heartedly attempt to obfuscate the code that actually modifies crypto/rand.Reader. I'm sure more clever people can come up with more sneaky ways to hide the write.[2]

The main API flaw here, IMO, is that Reader is an io.Reader variable, whereas it should be a function that returns an io.Reader. A new API would look something like:

// Reader returns an io.Reader that reads from a cryptographically strong pseudo-random number generator.

func Reader() io.Reader

// Read is a helper function that calls Reader() and then passes it to io.ReadFull.

func Read(b []byte) (n int, err error)

Alas, with the Go 1 compatibility guarantee Reader would have to remain, and Read would still have to use Reader. But the above could be added as new functions, say MakeReader() and SafeRead(). And the standard library (and other important external packages like x/crypto/nacl) could be changed to use those safe functions.

Unfortunately it seems difficult to mitigate against malicious tampering with crypto/rand.Reader. My coworker suggested using reflect to examine its underlying type, and assert that it's *crypto/rand.devReader, but that's probably the best that can be done.

I'm interested to hear what everyone else thinks, and whether anyone know of any particular reason why Reader is exposed like this, before I file a bug. Thanks!

-- Fred

[1] Without this flaw, a malicious package would have to use the unsafe package to poke around in the internals of crypto/rand, or call out to the external OS to e.g. try to redirect access to the random device, which seems easier to audit for than a write to crypto/rand.Reader. Of course, I'm already assuming that a project worried about this is vendoring all of its dependencies.

[2] One nice thing about go is that it's more difficult to write underhanded code in than, say, C ( http://www.underhanded-c.org/ ), or dynamic languages like Python or Javascript ( https://www.nccgroup.trust/us/about-us/newsroom-and-events/blog/2011/august/javascript-cryptography-considered-harmful/ ) where monkeypatching is easily done. But it seems plausible to me that code that modifies crypto/rand.Reader can be snuck into some innocuous package that would pass a cursory audit.

Stefan Nilsson

unread,

Mar 15, 2018, 7:50:01 AM3/15/18

to golang-nuts

The philosophy of Go (in my interpretation) is that exported and unexported identifiers are only used to describe the public interface of a package and to guard against certain programming errors; they are not intended to be a security measure or hide or protect any information. In fact, it's possible to read private fields with reflection, and change their values through unsafe pointers.