Only allow public Nodes to create entities #1167

tiziano88 · 2020-06-17T20:45:50Z

Implement Clone and PartialEq for NodeMessage to facilitate
testing.

Add termination check to channel_create host function.

Add more logging around labels in various host functions.

The tests are not as comprehensive as I would like them to be, since it
is hard to simulate the case in which a public node creates a non-public
channel and passes that to a non-public node.

Fixes #1143

Checklist

Pull request affects core Oak functionality (e.g. runtime, SDK, ABI)
- I have written tests that cover the code changes.
- I have checked that these tests are run by
  Cloudbuild
- I have updated documentation accordingly.

daviddrysdale

Notwithstanding any discussions at #1143, this feels a bit premature from an implementation perspective.

With this in place, a Node can have *_create() operations fail, but it hasn't got the tools to determine why (as there's not yet a node_label() host function). Maybe hold off until (say) #1137 is agreed and more of the base label-related API surface is implemented?

daviddrysdale · 2020-06-18T12:01:42Z

docs/concepts.md

+additional check that the label of the caller Node "flows to" the "public
+untrusted" label. This prevents the caller node from encoding non-public
+information in the label of the newly created Node or Channel, or even in the
+mere fact that such a Node or Channel exists at all.


Nice rationale.

daviddrysdale · 2020-06-18T12:06:18Z

oak/server/rust/oak_runtime/src/runtime/mod.rs

@@ -560,7 +560,19 @@ impl Runtime {
        node_id: NodeId,
        label: &Label,
    ) -> Result<(oak_abi::Handle, oak_abi::Handle), OakStatus> {
+        info!("creating Channel with label: {:?}", label);


There's already a debug! log in RuntimeProxy that includes the label information – is this needed? (info level is also higher than the convention in proxy.rs, which logs all API calls with a pair of debug logs with a standard format).

Good catch, removed for now, though I think we should remove those logs from RuntimeProxy and only keep them in Runtime itself? I'll create a separate PR for that anyways.

daviddrysdale · 2020-06-18T12:06:49Z

oak/server/rust/oak_runtime/src/runtime/mod.rs

+        info!(
+            "creating Node with config: {:?} and label: {:?}",
+            config, label
+        );


As above, already debug-logged in RuntimeProxy

daviddrysdale · 2020-06-18T12:08:29Z

oak/server/rust/oak_runtime/src/runtime/mod.rs

+        // the label of the Channel to be created.
+        self.validate_can_write_to_label(node_id, &Label::public_untrusted())?;
+        // We also additionally make sure that the label of the newly created Channel can be written
+        // to by the current Node, since in general this may be lower than "public untrusted".


I don't understand this – what can be lower than public untrusted? (Or does this anticipate future support for integrity labels?)

The bottom of the information flow lattice is not public_untrusted, it is public_trusted.

Yes, it anticipates support for integrity labels (in which case trusted is lower than untrusted, as @aferr points out), and also probably will be influenced by #1213 .

daviddrysdale · 2020-06-18T12:42:23Z

docs/concepts.md

+mere fact that such a Node or Channel exists at all.
+
+This restricts the ability to create new Node and Channels to Nodes that are
+themselves already "public".


docs/abi.md could do with an update to mention this too.

aferr · 2020-06-18T16:49:17Z

docs/concepts.md

@@ -211,7 +211,7 @@ Intuitively, data can only flow from `a` to `b` if:
 - `b` is **at least as secret** as `a`
 - `a` is **at least as trusted** as `b`

-The least privileged label is usually referred to as "public trusted" (and
+The least privileged label is usually referred to as "public untrusted" (and
 represented as `⊥`, pronounced "bottom"), which corresponds to a Node or Channel


Just FYI this is very FLAM-specific. The more conventional thing in information flow would be for public_trusted to be the bottom of the lattice. Even in FLAM public_trusted is the bottom of the information flow lattice. public_untrusted is the bottom of the authority lattice which is a different thing.

aferr · 2020-06-18T16:51:25Z

oak/server/rust/oak_runtime/src/runtime/mod.rs

+        // The label (and mere presence) of the newly created Channel is effectively public, so we
+        // must ensure that the label of the calling Node flows to both "public untrusted" and to
+        // the label of the Channel to be created.
+        self.validate_can_write_to_label(node_id, &Label::public_untrusted())?;


This should be public_trusted - the bottom of the information flow lattice.

I suspect you are missing a way to represent public_trusted ?

(This is the only requested change. The other thing is more of a comment.)

I am still confused whenever this comes up, which makes me feel I am probably missing something basic.

The current model is more similar to Flume in terms of how labels are composed by set of tags, so there is no way of even representing "fully trusted", right? Or would that be the set of all possible integrity tags (which anyways is not practically representable)?

In Flume, fully trusted would be the set of all tags, which is difficult to represent.

What you could do instead is:
Have a special label which is not a set of tags, but is just called trusted. Then change meet/join/order for the integrity lattice so that:

meet(x, trusted) = trusted, meet(trusted, x) = trusted

join(x, trusted) = x, join(trusted, x) = x

trusted orderedBefore X is always true

x orderedBefore trusted is never true (unless x is trusted)
For the cases in which "trusted" is not involved you just do the normal set theory thing.

You probably want to do the same trick for secret (the TOP) of the confidentiality lattice (with the direction of everything flipped). So maybe you really want to just call this special symbol "universe".

In case it is helpful:
JoinLabelComp / MeetLabelComp here sort of do something like this:
https://github.com/apl-cornell/sirrtl/blob/master/src/main/scala/firrtl/ir/LabelComp.scala

How about this:

the Oak Runtime "public trusted" (as a side node, we don't even need to explicitly represent its label in this model)

the main WebAssembly node of an Oak Application is "public untrusted"

the (untrusted) WebAssembly node can request the (trusted) Runtime to create any additional nodes and channels

the Runtime actually checks something about each such request, and then eventually decides whether to create the requested node or not, and at which label. For instance, if the calling node is itself untrusted, the Runtime will refuse to create a new node with a more trusted label.

Is this still problematic?

What you proposed is fine if "The Oak Runtime" really is a node and really is labeled public trusted. This real node should be the one making the create_node and create_channel calls, then. In this case, what you described would be permitted by either of the following two options:

checking that L flowsTo public_trusted

checking that L flowsTo L'

If instead, the public_untrusted WebAssembly node is the one really doing the channel_create and node_create calls, then this is problematic because then we are in the 4th bullet point above.

Let's take a step back.

If we only had confidentiality labels, then I think we agree that the check simplifies to L flows_to public (or, better, L can_declassify_to public ?). This represents the fact that labels are really public entities (let's stick with this assumption for now, even though we considered alternatives), and creating a node or channel really exposes that information to the public.

IIUC, the introduction of integrity labels complicates things, because under IFC rules, a node with high integrity would not be able to read a low integrity value, and therefore if labels were untrusted, no trusted node would be able to actually read them. Is this a correct intuition @aferr ? Is there a better way of explaining this?

But now this implies that the creator of the node / channel needs to have label L flows_to trusted (for the integrity component), which is true only for a node that is already fully trusted. Though requiring this seems to me excessive, because IIUC for a node to be fully trusted it means that it may affect nodes / channels with arbitrary integrity (which seems a very powerful privilege in itself).

In our case, I think the assumption of any one application node being fully trusted seems too strong. In particular, I don't see why an untrusted node should not be allowed to create other nodes; I trust your point that there is a reason, but my intuition about integrity labels is not strong enough for me to see it.

@aferr do you have a simple example of something that would go wrong if, say, the Oak Runtime transparently endorsed labels to fully trusted, even if the operation is performed by an untrusted node?

Also AFAICT Flume does not have this problem, is it just skipping over it, or is it solved via some other mechanism?

Apologies to keep dragging this, I am actually learning a lot from the discussion, and even repeating similar points in slightly different ways helps me (and I hope others) build a more solid and intuitive understanding of such concepts, so I am really grateful for the time and effort you put into this @aferr .

The explanation you gave as to why integrity has to go from more trusted to less or equal integrity is right.

One more way of looking at it:
If node A has integrity X, then the code in A is trusted about X. If node B is creating node A, then B is really supplying the code in A. So B should also be trusted about X.

So you are right, if we want to make the label constant, the check would have to be L(creator) flowsTo public_trusted.

You are also right that "trusted" is quite powerful. So I understand why you want to limit its use. I think in practice, many applications will probably have some components that do need to be universally trusted, but we will want to keep it a goal to limit this as much as possible. As a starting point, if the initial node is public_trusted, we can stick to the convention above of trying to just use this node to create other nodes/channels. (We can also think of this node as giving up privilege.)

I think the check L(creator) flowsTo public_trusted is safe, but is actually overly restrictive if we remove introspection. Introspection seems like it will limit what we can do a lot because it makes the control flow graph public. This means no implicit flows are prevented, and in general implicit flows can contain anything. (It's very easy to create a program that leaks an arbitrary secret through control flow). The check L(creator) flowsTo L(created) should give the same security, even when downgrades are permitted. Do you buy this, or should I expand?

Re transparent endorsement:
If we use robust declassification / transparent endorsement / non-malleable IFC,
public_untrusted actually cannot downgrade anything. It can't declassify because it's untrusted and it can't endorse because it's public.

Re Flume:
It sounds like Flume does have the same restriction when creating objects (L(creator) flowsTo L(created))
Page 4, Section 3.3 under the header Objects
"When a process p creates an object o, p specifies o’s labels, subject to the restriction that p must be able to write to o"
Checking that p can write to o is the flowsTo check, given in Definition 3 (it checks that p is less secret and more trusted than o).

I have created #1213 to keep discussing this, but I'd like to merge this PR in the meanwhile. Happy to go back and change things in the code when we do arrive to a conclusion, I just think an issue is probably better than a long thread on this PR.

Implement `Clone` and `PartialEq` for `NodeMessage` to facilitate testing. Add termination check to `channel_create` host function. Add more logging around labels in various host functions. The tests are not as comprehensive as I would like them to be, since it is hard to simulate the case in which a public node creates a non-public channel and passes that to a non-public node. Fixes project-oak#1143

github-actions · 2020-06-30T12:30:19Z

Reproducibility Index:

562715d812c1ed5397402d3f4b02b03addf416ccb4a8f071eac00a8015082b42  ./examples/target/wasm32-unknown-unknown/release/abitest_0_frontend.wasm
e89bb8621965e356156c939997a61fa1af30352159ae9e7a59f50a8eb101648d  ./examples/target/wasm32-unknown-unknown/release/abitest_1_backend.wasm
b256b1c1402a8f25381352047ee48cc9a07314d4227a64bd786369a17c61d1df  ./examples/target/wasm32-unknown-unknown/release/aggregator.wasm
44eff10ce70a232c84cc32c225e75eca695634abc9ecfbf543022aa0a5682cd7  ./examples/target/wasm32-unknown-unknown/release/chat.wasm
50c1455386c91fc6fc1d5adb31d2194d6ab183eac3ad54eb3cb388eb4aafad72  ./examples/target/wasm32-unknown-unknown/release/database_proxy.wasm
b28dc802af588ea6512974d2d9ec17557dd454f760b379ab80b11efbe444ecfd  ./examples/target/wasm32-unknown-unknown/release/hello_world.wasm
1123ddd109dd00155b0f59ccc2f3025974bacc82e15a4d1fe2a83ff6ceb2b3cd  ./examples/target/wasm32-unknown-unknown/release/machine_learning.wasm
66ae9e10c2d72cab7e629b2fea2ae98939649c391480ab255d589f5da8c7ae0f  ./examples/target/wasm32-unknown-unknown/release/private_set_intersection.wasm
0cd8cb0baae0c59852e775a697ca5408f975c1c1fdc53cc8f3a0e7d07e23f5af  ./examples/target/wasm32-unknown-unknown/release/running_average.wasm
15da62b4701e02ae731276dbe8bdfd29805871e7f863e8e15b1fe11983023159  ./examples/target/wasm32-unknown-unknown/release/translator.wasm
c2b22d73cbeee8a7945043cc1ffaa415b92cb25d6de91e4e46523435390bad9f  ./examples/target/wasm32-unknown-unknown/release/trusted_information_retrieval.wasm
91ca4391145ce09746c870f0c0b4d7fd9d380fa08c79aa0e929cc64d6f8aa292  ./oak/server/target/x86_64-unknown-linux-musl/release/oak_loader

Reproducibility Index diff:

diff --git a/reproducibility_index b/reproducibility_index
index e00a6f8..2461e1a 100644
--- a/reproducibility_index
+++ b/reproducibility_index
@@ -9,4 +9,4 @@ b28dc802af588ea6512974d2d9ec17557dd454f760b379ab80b11efbe444ecfd  ./examples/tar
 0cd8cb0baae0c59852e775a697ca5408f975c1c1fdc53cc8f3a0e7d07e23f5af  ./examples/target/wasm32-unknown-unknown/release/running_average.wasm
 15da62b4701e02ae731276dbe8bdfd29805871e7f863e8e15b1fe11983023159  ./examples/target/wasm32-unknown-unknown/release/translator.wasm
 c2b22d73cbeee8a7945043cc1ffaa415b92cb25d6de91e4e46523435390bad9f  ./examples/target/wasm32-unknown-unknown/release/trusted_information_retrieval.wasm
-883e931b70af8bda0c08205d19eaa16246f142fb8dbdb379a8d7e057a08a8881  ./oak/server/target/x86_64-unknown-linux-musl/release/oak_loader
+91ca4391145ce09746c870f0c0b4d7fd9d380fa08c79aa0e929cc64d6f8aa292  ./oak/server/target/x86_64-unknown-linux-musl/release/oak_loader

tiziano88 requested review from daviddrysdale and aferr June 17, 2020 20:45

googlebot added the cla: yes label Jun 17, 2020

daviddrysdale reviewed Jun 18, 2020

View reviewed changes

daviddrysdale approved these changes Jun 18, 2020

View reviewed changes

aferr suggested changes Jun 18, 2020

View reviewed changes

aferr approved these changes Jun 19, 2020

View reviewed changes

tiziano88 mentioned this pull request Jun 30, 2020

Determine IFC label for initial Application Node #1213

Closed

tiziano88 force-pushed the tzn_only_public_creation branch from 726854c to 5b7a4a8 Compare June 30, 2020 11:09

tiziano88 merged commit 08c6a60 into project-oak:main Jun 30, 2020

tiziano88 deleted the tzn_only_public_creation branch June 30, 2020 12:01

tiziano88 mentioned this pull request Sep 3, 2020

Tracking issue: IFC evolution #1431

Closed

14 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Only allow public Nodes to create entities #1167

Only allow public Nodes to create entities #1167

tiziano88 commented Jun 17, 2020

daviddrysdale left a comment

daviddrysdale Jun 18, 2020

daviddrysdale Jun 18, 2020

tiziano88 Jun 30, 2020

daviddrysdale Jun 18, 2020

tiziano88 Jun 30, 2020

daviddrysdale Jun 18, 2020

aferr Jun 18, 2020

tiziano88 Jun 30, 2020

daviddrysdale Jun 18, 2020

tiziano88 Jun 30, 2020

aferr Jun 18, 2020

aferr Jun 18, 2020

aferr Jun 18, 2020

aferr Jun 18, 2020

tiziano88 Jun 19, 2020

aferr Jun 19, 2020

tiziano88 Jun 22, 2020

aferr Jun 22, 2020

tiziano88 Jun 23, 2020

aferr Jun 25, 2020

tiziano88 Jun 30, 2020

github-actions bot commented Jun 30, 2020

Only allow public Nodes to create entities #1167

Only allow public Nodes to create entities #1167

Conversation

tiziano88 commented Jun 17, 2020

Checklist

daviddrysdale left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jun 30, 2020