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

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

Closed
daviddrysdale opened this issue Nov 3, 2020 · 9 comments · Fixed by #1688
Closed

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

daviddrysdale opened this issue Nov 3, 2020 · 9 comments · Fixed by #1688
Assignees
Labels
Milestone

Comments

@daviddrysdale
Copy link
Contributor

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.

  • The simplest example is the logging pseudo-Node: any information sent to it is made public (by logging it), so any attempt to create a logging pseudo-Node with label that's not public_untrusted should fail.
    • (Note that exploiting the lack of policing here would need a bit of indirection: only a public_untrusted node can create other Nodes, but such a node could create a logging pseudo-Node with label (say) user:X and pass the associated logging channel handle to a Wasm Node also with label user:X.)
  • Ditto for the Roughtime pseudo-Node.
  • For the client pseudo-Nodes (HTTP/gRPC), secrecy tags other than domain:* should be rejected.
  • For the server pseudo-Nodes (HTTP/gRPC), secrecy tags other than user:* or bearer token tags should be rejected.
@conradgrobler conradgrobler added this to the IFC v0 milestone Nov 3, 2020
@tiziano88
Copy link
Collaborator

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
Copy link
Collaborator

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).

oak/oak_runtime/src/lib.rs

Lines 753 to 759 in 2fbd54a

/// Returns whether the given Node is allowed to write to an entity with the provided [`Label`],
/// taking into account all the [downgrade privilege](NodeInfo::privilege) the Node possesses.
fn validate_can_write_to_label(
&self,
node_id: NodeId,
target_label: &Label,
) -> Result<(), OakStatus> {

@tiziano88
Copy link
Collaborator

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
Copy link
Collaborator

conradgrobler commented Nov 3, 2020

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 tiziano88 added the P0 label Nov 3, 2020
@tiziano88
Copy link
Collaborator

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
Copy link
Collaborator

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 conradgrobler self-assigned this Nov 4, 2020
@conradgrobler
Copy link
Collaborator

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
Copy link
Collaborator

👍 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
Copy link
Collaborator

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.

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

Successfully merging a pull request may close this issue.

3 participants