project-oak / oak

Meaningful control of data in distributed systems.
Apache License 2.0
1.32k stars 113 forks source link

Restrict the labels that can be attached to pseudo-Nodes #1669

Closed daviddrysdale closed 4 years ago

daviddrysdale commented 4 years ago

An Oak Application creates a pseudo-Node using the normal node_create ABI entrypoint, and this entrypoint includes a label parameter. However, each pseudo-Node type communicates with the world outside of Oak in some manner, so we should police the labels attached to pseudo-Node creation to ensure that information can't unexpectedly escape.

tiziano88 commented 4 years ago

More formally, I think what we want to check is that each of those nodes, once instantiated, are able to write to a public channel, at least in principle, based on their label and privilege.

For instance, the log node does not have any privilege, therefore the check boils down to whether its label is already public. If not, then we can probably fail at creation time rather than wait for the first message to arrive, since labels are static anyways. Note that the implementation of the log node does not actually write to any actual public oak channel, but the check is the same as if it were.

Similarly, an HTTP client for domain example.com would already have the privilege to declassify the domain:example.com tag; checking whether it can write to a public channel would ensure that indeed its label is at most domain:example.com, since anything higher than that would not be declassifiable based on its privilege.

tiziano88 commented 4 years ago

In fact I think this may be solved elegantly by calling validate_can_write_to_label(&Label::public_untrusted()) at node creation time (we may need to expose that through RuntimeProxy if it's not already there).

https://github.com/project-oak/oak/blob/2fbd54a89589385ec007a1011b68a49112eabdd5/oak_runtime/src/lib.rs#L753-L759

tiziano88 commented 4 years ago

Using the terminology from the Flume paper, any native side effect that a pseudo-node performs is effectively modelled as an uncontrolled channel, which is not within the realm of IFC itself, but it may be considered as a public channel.

conradgrobler commented 4 years ago

There is a slight complication with implementing this check as part of node creation. I am doing something roughly similar to this as part of #1673 to fix #814 and ran into the issue.

The label associated with a node is not passed to the new function during creation. A node is created without a label, and then the label is associated with the node in the rutnime during registration. So only once the node is running can it check whether it has the appropriate label.

tiziano88 commented 4 years ago

An alternative (perhaps better) way of doing this may be to actually let the runtime perform this check automatically for "uncontrolled" nodes. We may pass a bool / enum that determines whether the node is uncontrolled, and if so then the runtime would perform the downgrade check after instantiation, but before returning from the node_create ABI call. Would that work?

conradgrobler commented 4 years ago

It should work. It might still be a good idea for these nodes to also validate before taking actions that would violate IFC (e.g. the gRPC client node should check that it has the right label and privilege before connecting to an external service) as a defence-in-depth mechanism. This should keep the system safe even if there is a different code path that allows for bypassing the creationg checks.

conradgrobler commented 4 years ago

After some further investigation, I think we can do this in the runtime by adding fn external_facing(&self) -> bool to the Node trait. If a node is external facing, we can validate that the node has the privilege to write to public_untrusted in runtime::node_register. This should block any node from running in the runtime without having the appropriate privilege, which in turn should mean that we don't have to do additional checks in the pseudo nodes themselves.

tiziano88 commented 4 years ago

:+1: that's what I also meant when I was referring to "uncontrolled" or "native" nodes (though I don't like either of these names) and having a bool / enum to indicate that. not sure whether "external facing" is much better, though we can change the naming later on in any case.

conradgrobler commented 4 years ago

Yes, I assumed "uncontrolled" meant the same as "external facing". I also don't like any of them very much, so happy to change when we think of a better name.