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

Adds sys.Walltime and sys.Nanotime for security and determinism #616

Merged
merged 10 commits into from
Jun 4, 2022

Conversation

codefromthecrypt
Copy link
Contributor

@codefromthecrypt codefromthecrypt commented Jun 2, 2022

This adds two clock interfaces: sys.Walltime and sys.Nanotime to
allow implementations to override readings for purposes of security or
determinism.

The default values of both are a fake timestamp, to avoid the sandbox
break we formerly had by returning the real time. This is similar to how
we don't inherit OS Env values.

To use real clocks, users can do the following:

moduleBuilder.WithSysWalltime().WithSysNanotime()

They can also provide their own deterministic or higher performing clock like so:

moduleBuilder.
	 WithWalltime(walltime, sys.ClockResolution(time.Microsecond.Nanoseconds())).
	 WithNanotime(nanotime, sys.ClockResolution(1))

This change allowing consistent implementation of WASI, WASI 2, and wasm_exec clock imports.

See https://github.com/WebAssembly/WASI/blob/snapshot-01/phases/snapshot/docs.md#-clock_time_getid-clockid-precision-timestamp---errno-timestamp
See https://github.com/WebAssembly/wasi-clocks
See https://github.com/golang/go/blob/252324e879e32f948d885f787decf8af06f82be9/misc/wasm/wasm_exec.js#L243-L255

This adds two clock interfaces: `MonotonicClock` and `WallClock` to
allow implementations to override readings for purposes of security or
determinism.

Once the interface is accepted, These will be added to `ModuleConfig`
allowing consistent implementation of WASI, WASI 2, and wasm_exec clock
imports.

See https://github.com/WebAssembly/WASI/blob/snapshot-01/phases/snapshot/docs.md#-clock_time_getid-clockid-precision-timestamp---errno-timestamp
See https://github.com/WebAssembly/wasi-clocks
See https://github.com/golang/go/blob/252324e879e32f948d885f787decf8af06f82be9/misc/wasm/wasm_exec.js#L243-L255

Signed-off-by: Adrian Cole <adrian@tetrate.io>
@codefromthecrypt
Copy link
Contributor Author

cc @pkedy @sam-at-luther. I'll implement both of these with time.Now() for the default impl. Scream if something concerns you!

Signed-off-by: Adrian Cole <adrian@tetrate.io>
@codefromthecrypt
Copy link
Contributor Author

codefromthecrypt commented Jun 2, 2022

PS one thing I had considered is just a func instead of an interface for the two clocks. The clock res function isn't used anywhere in wasm I can find that runs outside the browser, and it is going to be very wrong in most cases. It is possible we could just stub that out independently to make the time plugins a func and just default via different config the resolution. For example resolution should be static unlike the returned value of the clock.

Ex. builder.WithMontonicTime(nanotime func() int64, uint32 resolution)

@mathetake @anuraaga wdyt?

Signed-off-by: Adrian Cole <adrian@tetrate.io>
@codefromthecrypt codefromthecrypt changed the title Adds sys.Clock interfaces for determinism Adds sys.Walltime and sys.Nanotime for security and determinism Jun 3, 2022
Signed-off-by: Adrian Cole <adrian@tetrate.io>
@codefromthecrypt
Copy link
Contributor Author

@sam-at-luther @knqyf263 can you check the PR description to see if the design seems good for you? I noticed we broke sandbox so might as well fix that while allowing determinism in system clocks.

Don't look too hard at the code yet as I have tests to backfill and editing to do.

Signed-off-by: Adrian Cole <adrian@tetrate.io>
@codefromthecrypt
Copy link
Contributor Author

TODO:

  • fix drift in rationale
  • backfill unit tests
  • add x/sys example for overriding the clock.

@knqyf263
Copy link
Contributor

knqyf263 commented Jun 3, 2022

Looks good!

Adrian Cole added 2 commits June 4, 2022 10:19
Signed-off-by: Adrian Cole <adrian@tetrate.io>
Signed-off-by: Adrian Cole <adrian@tetrate.io>
@codefromthecrypt
Copy link
Contributor Author

ok I added x/sys benchmarks (which underperform) and switched the default monotonic clock to use runtime.nanotime as it is routinely used to avoid performance overhead of time.Since(). Plus, it returns the actual nanotime ;)

next step is to backfill tests and add godoc about overriding the clock. I don't think overriding the clock needs its own example file.

Signed-off-by: Adrian Cole <adrian@tetrate.io>
@codefromthecrypt
Copy link
Contributor Author

While CI nodes are not the best examples of measuring perf. here's the results from that last commit on options to implement clock. While x/sys is a lot faster on my laptop, it is always worse. What is interesting that the speed of time.Since and runtime.nanotime are almost exactly the same in CI vs laptop 🤷

pkg: github.com/tetratelabs/wazero/internal/integration_test/vs/clock
cpu: Intel(R) Xeon(R) Platinum 8272CL CPU @ 2.60GHz
BenchmarkClock/time.Now-2         	21141316	        53.99 ns/op
BenchmarkClock/ClockGettime(CLOCK_REALTIME)-2         	 1897310	       629.2 ns/op
BenchmarkClock/time.Since-2                           	35992237	        33.47 ns/op
BenchmarkClock/runtime.nanotime-2                     	41405360	        28.48 ns/op
BenchmarkClock/ClockGettime(CLOCK_MONOTONIC)-2        	 1881158	       634.0 ns/op

Adrian Cole added 2 commits June 4, 2022 11:11
Signed-off-by: Adrian Cole <adrian@tetrate.io>
Signed-off-by: Adrian Cole <adrian@tetrate.io>
@codefromthecrypt
Copy link
Contributor Author

ok done with all the backfilling, docs and polishing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants