Add with_privilege versions of node and channel function to ABI

[conradgrobler]

At the moment operations such as channel_read will automatically try to apply the privilege of the node. This means that it is not always obvious whether privilege is being used or not. The following functions should be added to the ABI to make clear that privilege is being used:

• channel_read
• channel_write
• node_create
• channel_create

The following might also need with_privilege versions:

• wait_on_channels
• channel_close

Question: should the explicity privilege to use be passed to the with_privilege versions, or should the signature just be the same as the original, but making it explicit that privilege is being used?

[tiziano88]

For now, I suggest only implementing the privilege-aware version of channel_write, since it is the one that we have a use for (and IMO the safest one to use in general anyways).

Also I think for now a single boolean of whether to use privilege or not is sufficient. There are also additional complications when dealing with a wasm node wanting to refer to its wasm hash privilege, since the wasm hash would have to contain a quoted version of itself etc.

[tiziano88]

Another thing to consider: in the ABI we already have methods for retrieving the label of a channel. In principle, it should be possible to implement the privilege-aware methods directly on higher level APIs such as Sender and Receiver, by retrieving the label of the underlying channel, and comparing it to that of the current node. In fact this may be a better way to go about it, since it means we can have the final API in the SDK, without having to make any changes to the ABI. Separately we can then decide whether it is worth pushing those changes down to the ABI.

[conradgrobler]

I thought there was a requirement to distinguish between normal reads/writes and reads/writes that use privilege at the ABI- and runtime levels.

[tiziano88]

I thought there was a requirement to distinguish between normal reads/writes and reads/writes that use privilege at the ABI- and runtime levels.

In general yes, but since all our nodes are written in Rust, and we look at the Rust code to decide whether to trust them or not, I'm saying we can start by making the changes in the high level Rust API by emulating the behaviour we want, and then later consider implementing it at the ABI level.

[conradgrobler]

I don't like the idea of only adding this in the SDK and not in the ABI (it feels similar checking security restrictions only in javascript and not on the server in a web app). Doing it in two steps seems like double work, as we will first implement the logic in the SDK and later reimplement the same logic in the ABI (and presumably remove the logic from the SDK).

[tiziano88]

My point is that we can have the final API in the SDK before making changes to the ABI. The logic will have to move, but the API will stay the same. Even if we decide to do it at the ABI level to start with, I strongly suggest making sure coming up with the high-level API for the SDK that can be used in the context of the examples. e.g. are we going to add a Sender::send_declassify method and make the existing one not declassify? Or something else?

[conradgrobler]

I agree that we need to update the SDK (whether we update the ABI or not). My main point is that I wan't to ensure we don't stop there and never update the ABI and runtime. This concern was prompted by "Separately we can then decide whether it is worth pushing those changes down to the ABI".

[tiziano88]

Got it. I note anyways that we basically never look at the generated Wasm code at all (I think?); we make judgements of whether to trust a node or not based on the Rust code itself, so having the enforcement at the ABI or SDK does not matter that much in practice for the kind of Rust-based nodes we have currently.

[conradgrobler]

After some further thinking and looking at the code, my suggestion for an incremental approach is somewhat different:

• Remove the default privilege from the runtime code for all node and channel functions and see what breaks
• Add with_privilege versions only for what is needed to get all tests passing and examples running again
• Add additional with_privilege functions to the runtime, ABI and SDK if and when needed

As far as I can tell from an initial search, we would only need to implement a runtime-internal channel_read_with_privilege to support the HTTP and gRPC server pseudo nodes and a channel_write_with_privilege that is exposed via the ABI and the SDK for the private database, aggregator, and private set intersection examples.

This would mean that pseudo-nodes will also have to explicitly use privileges, which should make the intent of the TCB code clearer as well.

[tiziano88]

Thanks @conradgrobler , I like your suggestion.

Because we currently don't have all the labels in place (pending #1066), it may be that we will have to do another pass (or rather, change some of the logic to use the newly created _with_privilege versions when we introduce the extra labels) for some of the examples.

Also, if there is any case that may be resolved by either implementing either a declassifying read or a declassifying write, we should prefer the declassifying write for the time being, all other things being equal. It may be that we don't need to implement declassifying reads after all.