feat(random): make get-random-bytes fallible

[ricochet]

get-random-bytes and get-insecure-random-bytes accept a u64 length with
no way to signal failure, allowing guests to request up to 2^64-1 bytes
and forcing hosts to allocate unbounded memory or hard-trap.

Add an error enum with a too-many-bytes case and change both functions
to return result<list, error>. Add max-random-bytes-length and
max-insecure-random-bytes-length query functions so guests can check
limits upfront. Hosts MUST support at least 4096 bytes.

The error type is defined in the random interface and reused by insecure
via use. get-random-u64 and get-insecure-random-u64 are unchanged.

Part of #888

[badeend]

Suggested change

	get-random-bytes: func(len: u64) -> result<list<u8>, error>;
	get-random-bytes: func(max-len: u64) -> list<u8>;

We could also allow get-random-bytes to return fewer bytes than requested.

That way we don't need the fallibility and the new max-random-bytes-length API.

[ricochet]

This is one of the most portable implementations to emulate: https://docs.rs/getrandom/0.4.2/getrandom/

So if we make get-random-bytes fallible (return result) and let callers handle errors, matching how Rust's getrandom crate works, then we remove the ceilings. Implementors may set their own limits and fail.

[badeend]

What I'm suggesting is to remove the too-many-bytes failure mode and allow hosts to simply return fewer bytes than requested. (But with a required minimum of 1?)

The guest (e.g. Rust's getrandom::fill implementation) can call the WASI API in a loop in case they don't like short reads.

And with the too-many-bytes case removed, I'm not sure we need an error variant at all here, though I'm also not fundamentally opposed to it either.

[alexcrichton]

Personally I might lean towards the max-len idea as well because it removes ambiguity about the error case. In general a failure to acquire randomness is a weird situation to be in, so by removing errors entirely it means that the guest is guaranteed it's able to acquire at least some randomness. I also would agree with @badeend that it's not a loss in expressiveness necessarily since getrandom might just end up bottoming out in mulitple calls to this function

[sunfishcode]

Making get-random-bytes and get-random-u64 infallible was a design choice. I understand the need to limit the size of the buffer for memory usage reasons, however the presence of an other arm would be a fundamental change, making these APIs generally fallible. I'm open to discussing it, but we should proceed with care.

Making randomness APIs fallible effectively asks application and library writers to find something to do when they fail. However, there is very little that an application that needs random bytes can do if random bytes aren't available.

In other systems, fallible randomness sources often tempt users into having poorly tested and sometimes insecure fallback code, including using clocks or "uninitialized" memory for seeds and implementing their own pseudo-random number generators. Also, because some wasm hosts can do snapshot+restore and don't notify wasm guests when doing so, pseudo-random number generators in wasm code are often insecure.

Making these APIs infallible, with the exception of limiting the buffer size, clearly communicates to application and library authors that no such fallback code is needed.

[ricochet]

I was hoping to present more of the design space in the last WASI SG, but we did not have time.

This is the presentation I prepped: https://link.excalidraw.com/p/readonly/NQ0G0QLqWC5W7h7o94yD

First, after exploring the design space, I agree we shouldn't make get-random-bytes fallible. For the reasons @sunfishcode calls out.

There are a few other design improvements we could make.

1. len could be u32, not u64, type-level cap at ~4 GiB.
2. len of 0 traps, this would mirror the W3C WebCrypto precedent
   where SubtleCrypto.generateKey() throws SyntaxError
   on semantically empty parameters. A zero-byte random
   request is a programming error; trapping catches it.
3. The host satisfies the full request internally. If it
   needs to chunk, it does so transparently.

Many of the issues filed try to put the work back on the guest. This is a scenario where the host should be responsible for fulfilling the request and approaches where the guest chunks or the host only partially fills the buffer will be inherently buggy. AKA if we change the parameter from len to max-len, this would allow hosts to return fewer bytes than requested, but then this will also put the onus on the guest program to check and handle this likely rare and unexpected case.

[ricochet]

The argument against len of 0 traps, is that generally in WASI we do everything we can to NOT trap. If we truly want to catch this case, then we would need to return a result with an error type, e.g. parameter-invalid. There's little practical benefit as all other implementations that I've reviewed do not handle this case.

Therefore what I'm proposing changing (but would also be in favor of changing nothing from the wit definition side) is get-random-bytes: func(len: u32) -> list<u8>;

[badeend]

The argument against len of 0 traps, is that generally in WASI we do everything we can to NOT trap.

I agree it shouldn't trap. If the guest requested 0 bytes, then they get 0 bytes. Aside from being a silly request from the guest in the first place, I don't see anything wrong with that.

The host satisfies the full request internally. If it needs to chunk, it does so transparently.

Without lazy lowering, this requires special host powers which AFAIK wasmtime doesn't expose (yet). It also prevents guest virtualization by e.g. WASI-Virt.

if we change the parameter from len to max-len, this would allow hosts to return fewer bytes than requested, but then this will also put the onus on the guest program to check and handle this likely rare and unexpected case.

Maybe I'm missing the point you're trying to get across, but I don't see the problem with this. Calling the host in a loop is exactly what Rust's getrandom does.

Note that other platforms also allow short reads. WASI won't be special in that regard.

https://man7.org/linux/man-pages/man2/getrandom.2.html

reads of up to 256 bytes will always return as many bytes as requested and will not be interrupted by signals. No such guarantees apply for larger buffer sizes. For example, if the call is interrupted by a signal handler, it may return a partially filled buffer. (...) The user of getrandom() must always check the return value, to determine whether either an error occurred or fewer bytes than requested were returned.

[pchickey]

Note that other platforms also allow short reads. WASI won't be special in that regard.

I agree with Dave that the best way to handle this is for the docs to say that the implementation may return a shorter amount than requested. Thankfully we don't need the signal-handling caveats of getrandom(2), but otherwise I think we should change the docs of both p2 and p3 to permit implementations to return less than requested.

I also agree that there's no reason to trap on zero when returning the empty list is possible. SubtleCrypto is a very qualitatively different API than this one which is trying to catch very domain-specific use errors.

[automation-wasmcloud]

I'm closing this PR in favor of short reads. @badeend will you file that?

[ricochet]

I'm closing this in favor of short reads. @badeend will you create the short read PR?

Do note that if a component wants to run on WebCrypto environments, there is a 65536 byteLength ceiling: https://w3c.github.io/webcrypto/#Crypto-method-getRandomValues

[badeend]

@ricochet Here ya go: #901