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

all: remove nacl port #30439

Open
bradfitz opened this issue Feb 27, 2019 · 22 comments

Comments

Projects
None yet
@bradfitz
Copy link
Member

commented Feb 27, 2019

We propose to remove the nacl ports (for amd64p32, 386, and arm).

Nacl is deprecated in favor of WebAssembly and @rsc made the point that it's no longer even a great sandbox in light of all the speculative execution problems of late.

The only current user of nacl is the playground (https://play.golang.org/) which we'd need to move to something else. Perhaps gVisor's runsc (https://github.com/google/gvisor#gvisor).

Related: #30324 (for faking time, network)

/cc @randall77 @ianlancetaylor @rsc @aclements @davecheney @josharian @andybons @dmitshur

@gopherbot gopherbot added this to the Proposal milestone Feb 27, 2019

@gopherbot gopherbot added the Proposal label Feb 27, 2019

@dmitshur

This comment has been minimized.

Copy link
Member

commented Feb 27, 2019

Related issue is #25224.

The only current user of nacl is the playground (https://play.golang.org/) which we'd need to move to something else. Perhaps gVisor's runsc (https://github.com/google/gvisor#gvisor).

To expand on that, another option to consider is WebAssembly.

WebAssembly use for playground details

If using WebAssembly, there exist choices for where the playground snippet code is compiled, and where it's executed. It can be compiled either on the backend server, or in the frontend client (user's browser). The compiled WebAssembly module (application/wasm) can be executed either on the backend sever, or in the frontend client. Some of these options are more difficult to implement than others and have trade-offs.

@martisch

This comment has been minimized.

Copy link
Member

commented Feb 27, 2019

I think nacl is the only GOOS for GOARCH amd64p32. Can we therefore also remove all support for amd64p32 when removing nacl?

@bradfitz

This comment has been minimized.

Copy link
Member Author

commented Feb 27, 2019

@martisch, yes. That's one of the main motivations.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Feb 27, 2019

I'll note that gccgo uses GOARCH=amd64p32 for x32 mode (https://en.wikipedia.org/wiki/X32_ABI). In general I think we'll want to keep it if we ever choose to do an x32 mode port. Though I don't know how relevant x32 is for Go.

@bradfitz

This comment has been minimized.

Copy link
Member Author

commented Mar 19, 2019

@kortschak brought up golang.org/x/tools/cmd/present too.

Currently the present tool has flags to either use play.golang.org or run locally. And when run locally, it has an option for whether to use nacl or not. It defaults to non-nacl, local execution (not using play.golang.org).

Adding gvisor support in present is probably onerous and wouldn't help e.g. people presenting on Mac or Windows laptops.

So, I think present should probably use WebAssembly. Then perhaps we could remove one of the two current flags and then the only choice would be remote play.golang.org vs local WebAssembly.

/cc @neelance @adg

@neelance

This comment has been minimized.

Copy link
Member

commented Mar 19, 2019

There are two options for WebAssembly: Run the wasm binary on the client (in browser) or on the server (with Node.js or some other compatible wasm runtime).

When running it on the client the main issue is browser compatibility. I think for present it is not really a problem, because you usually can choose which browser to use for your presentation. For the playground however it might not be good to have it fail on incompatible browsers. On #28360 we said that (for now) wasm only needs to support the latest browsers. This requirement might not be suitable for the playground.

Running on the server however might be good for the playground, because we can take care of properly setting up the wasm runtime.

@bradfitz

This comment has been minimized.

Copy link
Member Author

commented Mar 19, 2019

For the playground (play.golang.org), we'll run it on the server and not use WebAssembly, because we control the 1 server and we can use gvisor (or most likely: just use Google Cloud Functions, which uses gvisor)

For present, there are many user machines, so running WebAssembly is the best bet, and running it in the client is best so users don't need to worry about having nodejs installed. I don't imagine many people are going to be presenting on Blackberry's browser: https://caniuse.com/#feat=wasm

Re #28360 and browser compatibility, I believe my most recent comment is still what we want to do: #28360 (comment) ... that is, support the current stable releases of all evergreen browsers by default and only use experimental WebAssembly features behind a compiler-side flag like GOWASM=foo. Alternatively, if we really had to, we could go the other way and make present tell the compiler to do GOWASM=super-portable-at-the-cost-of-speed.

@neelance

This comment has been minimized.

Copy link
Member

commented Mar 19, 2019

Re browser compatibility: Are we saying different things? I feel like we're saying the same but it sounds like you're disagreeing. Feel free to answer on #28360 so we don't clutter this thread.

@kortschak

This comment has been minimized.

Copy link
Contributor

commented Mar 19, 2019

When running it on the client the main issue is browser compatibility. I think for present it is not really a problem, because you usually can choose which browser to use for your presentation.

This is part of the story. I use present for lectures with runnable code examples including code from outside the standard library. The present server is accessible to the students during the lectures and after. I have close to zero control over the machines they use.

@adg

This comment has been minimized.

Copy link
Contributor

commented Mar 19, 2019

It would make sense to me to run code snippets using WebAssembly on the backend for both play.golang.org and present. One supported code path, the only external dependency would be some kind of JS runtime. gVisor/runsc means Docker, right? I investigated running short-lived processes on Cloud inside gVisor and it wasn't fast or easy to provision (weird permissions hacks starting Docker containers from inside Docker containers), but maybe I missed something.

@bradfitz

This comment has been minimized.

Copy link
Member Author

commented Mar 19, 2019

@adg, App Engine and Google Cloud Functions use gvisor. So all we have to do is deploy a Google Cloud Function that runs the compiler & resulting binary. That part's trivial.

For the present case, though, we don't want a requirement for present users to be that they have nodejs installed. Hence server-side compilation to WebAssembly & client-side execution of WebAssembly.

@adg

This comment has been minimized.

Copy link
Contributor

commented Mar 19, 2019

So all we have to do is deploy a Google Cloud Function that runs the compiler & resulting binary. That part's trivial.

Ah so you can deploy Cloud Functions that run code with restricted permissions (no network access)?

we don't want a requirement for present users to be that they have nodejs installed

That's fair. Although I'm curious to see how it goes, shipping megabytes of webasm to the browser on each 'run'.

@bradfitz

This comment has been minimized.

Copy link
Member Author

commented Mar 19, 2019

Ah so you can deploy Cloud Functions that run code with restricted permissions (no network access)?

Yeah, I think that between porting nacl's faketime support to linux/amd64 (which @aclements has in progress) and restricting outbound network access via modifications to the net package, we could pretty much just use the resulting linux/amd64 binaries unmodified under gvisor: the real network stack (restricted to localhost) and the real filesystem (I'll have to check whether there are magic auth token files that would cause a problem). Worst case we also port over the nacl fake filesystem behind a build tag.

@bradfitz

This comment has been minimized.

Copy link
Member Author

commented Mar 19, 2019

That's fair. Although I'm curious to see how it goes, shipping megabytes of webasm to the browser on each 'run'.

It's 14% smaller as of https://go-review.googlesource.com/c/go/+/167801 and 1.5% smaller with https://go-review.googlesource.com/c/go/+/167937 ... but megabytes isn't terrible terrible.

@neelance

This comment has been minimized.

Copy link
Member

commented Mar 19, 2019

Gzip compression gives a size of 650 KB for "hello world" with fmt.

@aclements

This comment has been minimized.

Copy link
Member

commented Mar 19, 2019

@aclements

This comment has been minimized.

Copy link
Member

commented Mar 19, 2019

@bradfitz

This comment has been minimized.

Copy link
Member Author

commented Mar 19, 2019

Porting nacl's fake network was the original plan so I'm still fine with that too.

I forgot to mention child processes. If we use the Google App Engine/GCF/etc existing gvisor sandbox we'll also need to block child processes. Otherwise you could just write out a binary to do network access and run that. Or we could just use runsc in a VM with nested virt and define our own gvisor sandbox, and then not worry about faking network or disk. I'll try that first.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Mar 20, 2019

To be conservative, I think we should announce removal of NaCl in the 1.13 release, and actually remove it in the 1.14 release.

@bradfitz

This comment has been minimized.

Copy link
Member Author

commented Mar 20, 2019

I tweeted about this here: https://twitter.com/bradfitz/status/1108072470320304128

One potential user from the tweet replies: tinygo-org/tinygo#211 (comment)

@andybons

This comment has been minimized.

Copy link
Member

commented Mar 27, 2019

Per discussion with @golang/proposal-review, 1.13 will be the last release to support it, with it being removed in 1.14.

@andybons andybons added Proposal-Accepted and removed Proposal labels Mar 27, 2019

@gopherbot gopherbot added the Proposal label Mar 27, 2019

@andybons andybons changed the title proposal: remove nacl port all: remove nacl port Mar 27, 2019

@aykevl

This comment has been minimized.

Copy link

commented Mar 27, 2019

One potential user from the tweet replies: tinygo-org/tinygo#211 (comment)

FYI (now also in this issue): we have decided not to use nacl after all, so from our perspective it can be removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.