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

Jump to bottom

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

Closed
alokmenghrajani opened this issue May 19, 2021 · 7 comments
Closed

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

alokmenghrajani opened this issue May 19, 2021 · 7 comments
Labels
arch-wasm WebAssembly issues FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@alokmenghrajani
Copy link
Contributor

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

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

@alokmenghrajani
Copy link
Contributor Author

alokmenghrajani commented May 19, 2021 •
edited

@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 commented May 19, 2021 •
edited

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
Contributor Author

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

@BenLubar BenLubar mentioned this issue May 19, 2021
Closed
@gopherbot
Copy link

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
@BenLubar
crypto/rand: limit getRandomValues buffer size
3d8d044 
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 WebAssembly issues label May 21, 2021
@dmitshur
Copy link
Contributor

CC @neelance, @FiloSottile via owners.

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. 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
@BenLubar
crypto/rand: limit read size on js/wasm
dc52bcc 
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.
@dagood dagood mentioned this issue Mar 21, 2022
Merged
@tmthrgd
Copy link
Contributor

tmthrgd commented Feb 2, 2023

This was duplicated by #58145 which has now been fixed. This can be closed.

@golang golang locked and limited conversation to collaborators Feb 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
arch-wasm WebAssembly issues FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

Successfully merging a pull request may close this issue.

crypto/rand: limit getRandomValues buffer size BenLubar-PR/golang-go
7 participants
@josharian @tmthrgd @alokmenghrajani @dmitshur @BenLubar @gopherbot @seankhliao