sandstorm-io / sandstorm

Sandstorm is a self-hostable web productivity suite. It's implemented as a security-hardened web app package manager.
https://sandstorm.io
Other
6.72k stars 705 forks source link

Capnproto APIs: make mapping to legacy protocols more direct. #3174

Open zenhack opened 4 years ago

zenhack commented 4 years ago

This is sortof a meta issue regarding design API philosophy. I'll pick on WebSession to make the problem concrete, but similar arguments apply to other APIs that wrap existing protocols, e.g. the stuff in email.capnp.

Right now, implementing a sandstorm app that doesn't use sandstorm-http-bridge is uncompelling, even in a language with good capnproto support, because:

It seems like the current APIs are designed to capture a higher-level view of the protocols' semantics, but ironically the end result is that developers wishing to use these APIs would have to work at a lower level of abstraction than would normally be the case.

I'd propose replacing the web session interface with something simpler, which maps more directly to HTTP.

Advantages of this approach:

Spitballing a possible alternative:

interface HttpSession {
  request @0 (method :Text, headers :List(Util.KeyValue), responder :Responder)
    -> (requestBody :Util.ByteStream)
}

interface Responder {
  respond @0 (status :UInt16, headers :List(Util.KeyValue))
    -> (responseBody :Utill.ByteStream);
}

In terms of upgrade path, we could have Sandstorm try the existing WebSession methods, and if they throw Unimplemented, try calling HttpSession.request instead.

Thoughts?

kentonv commented 4 years ago

Check this out, which we use in Cloudflare Workers:

https://github.com/capnproto/capnproto/blob/master/c++/src/capnp/compat/http-over-capnp.capnp

The big reason I didn't do it that way in Sandstorm, though, is because I wanted to map out HTTP features semantically in order to decide how to treat each feature for sandboxing purposes. For one random example, it would be very bad if an app could send Public-Key-Pins.

I could possibly be convinced that it would be safe to expose a more "raw" version of HTTP provided that we limit headers to some sort of whitelist.

zenhack commented 4 years ago

To be clear, I'm not proposing we change what is accepted/filtered by Sandstorm, just that apps see an interface that looks close to normal HTTP.

Re: the schema you linked, my one concern is the WebSocket interface; it looks like it operates at the level of frames? While this is nice conceptually, when working on apps that used websockets, it was actually really useful to have Sandstorm's WebSocketStream.sendBytes, since it meant I could just use Go's existing WebSocket support. But it looks like the server has control over whether it uses that or just streams raw bytes, so as long as we implement the Sandstorm side such that it's still possible to use startResponse to handle the websocket framing in the app, I think reusing that schema as-is would be fine.

zenhack commented 4 years ago

From an implementation perspective: we could limit attack service by doing a translation between WebSession and HttpService inside the sandbox, so the privileged part of Sandstorm is still speaking the strongly-typed version.

zenhack commented 4 years ago

...for that matter, we could modify sandstorm-http-bridge to have a mode where it just proxies the grain's supervisor, and does this translation there -- so launches the child process with a socket on fd 3, and exposes all the same interfaces but wraps the app's UiSessions to translate websession calls into HttpService calls. It would probably be straightforward to adapt most of the existing code, since it's already doing the HTTP translation, so there's no semantic gap, just a marshalling change.

kentonv commented 4 years ago

But it looks like the server has control over whether it uses that or just streams raw bytes, so as long as we implement the Sandstorm side such that it's still possible to use startResponse to handle the websocket framing in the app, I think reusing that schema as-is would be fine.

Doesn't quite work. The WebSocket protocol is not a subset of standard HTTP; it diverges from HTTP as soon as the initial handshake completes. So, it needs special handling.

From an implementation perspective: we could limit attack service by doing a translation between WebSession and HttpService inside the sandbox, so the privileged part of Sandstorm is still speaking the strongly-typed version.

Hmm, but how is that better than what we have now, where we translate from WebSession to actual HTTP inside the sandbox?

zenhack commented 4 years ago

Hmm, but how is that better than what we have now, where we translate from WebSession to actual HTTP inside the sandbox?

I guess the main thing is it lets us keep things in-band on the RPC socket. The app needing to connect to "/tmp/sandstorm-api" and start a separate rpc session makes things a bit awkward, in that the bridge is forced to handle all of the expected Api, not just the http bits. This includes MainView.drop/restore, getViewInfo()...

This is part of the reason for #3027 (though it's also adding some smarts re: http apis). Fixing it involves some fiddly questions regarding how the bridge should find something to delegate to, and depending on the details questions regarding synchronization, waiting for another service to start up... I'm sure these can be solved, but it seems like it gets a lot simpler if you can just have everything on the initial RPC socket.

zenhack commented 4 years ago

Quoting Kenton Varda (2019-12-17 19:02:33)

Doesn't quite work. The WebSocket protocol is not a subset of standard HTTP; it diverges from HTTP as soon as the initial handshake completes. So, it needs special handling.

If we go with the approach of making this a wrapper over websession, we could just shell out to sendBytes() in this case, and it will still work.