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/rand: panic in rand.Read() when called from wasm in a browser #46256

Open
alokmenghrajani opened this issue May 19, 2021 · 6 comments · May be fixed by #46266
Open

crypto/rand: panic in rand.Read() when called from wasm in a browser #46256

alokmenghrajani opened this issue May 19, 2021 · 6 comments · May be fixed by #46266
Labels
arch-wasm NeedsInvestigation
Milestone

Comments

@alokmenghrajani
Copy link

@alokmenghrajani alokmenghrajani commented May 19, 2021

What version of Go are you using (go version)?

$ go version
go version go1.16.4 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/amenghra/Library/Caches/go-build"
GOENV="/Users/amenghra/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/amenghra/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/amenghra/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.16.4"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/amenghra/Sites/go-wasm/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/j9/ws1b9zmn24z11lsvm78p2_g00000gn/T/go-build289091387=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

When calling rand.Read(buf) with a buffer which is too large (e.g. 65537 bytes on Chrome), the wasm code panics with: panic: JavaScript error: Failed to execute 'getRandomValues' on 'Crypto': The ArrayBufferView's byte length (65537) exceeds the number of bytes of entropy available via this API (65536).

Given that rand.Read() returns an error, it would be cleaner to return an error in this case instead of panicking.

Sample code:

	buf := make([]byte, 65537)
	n, err := rand.Read(buf)
	fmt.Printf("here: %d %+v\n", n, err)

What did you expect to see?

I was expecting err to contain the "[...]exceeds the number of bytes of entropy available via this API" message.

What did you see instead?

A panic:

panic: JavaScript error: Failed to execute 'getRandomValues' on 'Crypto': The ArrayBufferView's byte length (65537) exceeds the number of bytes of entropy available via this API (65536).
wasm_exec.js:51 
wasm_exec.js:51 goroutine 1 [running]:
wasm_exec.js:51 syscall/js.Value.Call(0x7ff800010000000d, 0x410040, 0x2c707, 0xf, 0x441e40, 0x1, 0x1, 0x10100000000001c, 0x14bb98)
wasm_exec.js:51 	/usr/local/go/src/syscall/js/js.go:400 +0x34
wasm_exec.js:51 crypto/rand.(*reader).Read(0x137960, 0x45a000, 0x10001, 0x10001, 0x14bb98, 0x0, 0x10001)
wasm_exec.js:51 	/usr/local/go/src/crypto/rand/rand_js.go:25 +0x5
wasm_exec.js:51 io.ReadAtLeast(0x46388, 0x137960, 0x45a000, 0x10001, 0x10001, 0x10001, 0x10d20, 0x14c201, 0x45a000)
wasm_exec.js:51 	/usr/local/go/src/io/io.go:328 +0x6
wasm_exec.js:51 io.ReadFull(...)
wasm_exec.js:51 	/usr/local/go/src/io/io.go:347
wasm_exec.js:51 crypto/rand.Read(0x45a000, 0x10001, 0x10001, 0x45a000, 0x42af28, 0x11ca80)
wasm_exec.js:51 	/usr/local/go/src/crypto/rand/rand.go:24 +0x4
wasm_exec.js:51 main.main()
wasm_exec.js:51 	/Users/amenghra/Sites/go-wasm/test.go:10 +0x3
wasm_exec.js:151 exit code: 2
@josharian
Copy link
Contributor

@josharian josharian commented May 19, 2021

Or it could loop and fill the buffer via multiple calls, and not return an error at all.

@alokmenghrajani
Copy link
Author

@alokmenghrajani alokmenghrajani commented May 19, 2021

@josharian agree. Except you can't hardcode a value for each call (since different browsers might have a different limit) but you can start with a given value and half each time the call fails (or something like that).

If I were to design rand.Read(), I would make it not return anything and in the rare case where there's absolutely no entropy available, panicking would be acceptable. If I were designing such an API, I would document the rare possibility of panicking.

@BenLubar
Copy link

@BenLubar BenLubar commented May 19, 2021

65536 is a specification-defined limit, so this can be handled with a loop and a hardcoded max buffer size in the js/wasm implementation of rand.Read:

https://www.w3.org/TR/WebCryptoAPI/#Crypto-method-getRandomValues

@alokmenghrajani
Copy link
Author

@alokmenghrajani alokmenghrajani commented May 19, 2021

@BenLubar my bad, you are right. Looping with 65536 seems like a reasonable solution to remove this panic.

@BenLubar BenLubar linked a pull request May 19, 2021 that will close this issue
@gopherbot
Copy link

@gopherbot gopherbot commented May 19, 2021

Change https://golang.org/cl/321189 mentions this issue: crypto/rand: limit getRandomValues buffer size

BenLubar added a commit to BenLubar-PR/golang-go that referenced this issue May 20, 2021
The crypto.getRandomValues API specifies a maximum of 65536 bytes per
call. If a larger byte slice is passed to rand.Reader.Read, only fill
the first 65536 bytes.

Fixes golang#46256.
@dmitshur dmitshur added the arch-wasm label May 21, 2021
@dmitshur
Copy link
Contributor

@dmitshur dmitshur commented May 21, 2021

CC @neelance, @FiloSottile via owners.

@dmitshur dmitshur added the NeedsInvestigation label May 21, 2021
@dmitshur dmitshur added this to the Backlog milestone May 21, 2021
BenLubar added a commit to BenLubar-PR/golang-go that referenced this issue May 24, 2021
The crypto.getRandomValues API specifies a maximum of 65536 bytes per
call. If a larger byte slice is passed to rand.Reader.Read, only fill
the first 65536 bytes.

Fixes golang#46256.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-wasm NeedsInvestigation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants