Chat application + labels #515

tiziano88 · 2020-01-22T19:32:49Z

Add support for labels to Chat example

Checklist

wait for Use single app server in examples #474 to be merged
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.
- I have raised an issue to
  cover any TODOs and/or unfinished work.
Pull request includes prototype/experimental work that is under
construction.

tiziano88 · 2020-02-04T19:21:11Z

This is ready for a first round of review, even though the labels only exist in the SDK and not in the actual runtime. I would rather not implement labels in the current C++ runtime just to throw them away and re-implement them in Rust when #540 is done.

Note to reviewers: please let me know if you have any suggestions on how to improve the API around labels in particular.

conradgrobler · 2020-02-05T11:31:23Z

examples/chat/module/rust/src/lib.rs

+            .rooms
+            .entry(grpc_invocation_label.clone())
+            .or_insert_with(|| create_room_node(&grpc_invocation_label));
+        room_node_sender.send(&grpc_invocation)


This feels like it provides a very specific solution that moves the problem elsewhere (forwarding the gRPC channel to another room) rather than providing a generic mechanism that can be used for other routing use cases where the incoming channel into the node is not gRPC.

I assume the more generic solution will come later with a more complete label system implementation.

This node solves the problem of having a single inbound channel, over which additional channel handles are received, but these channels have more restrictive labels attached to them. So in this sense, a generic router node would still need to know how to unpack incoming channel handles and inspect their labels, and I am not sure how to abstract that. Perhaps add a Routable trait in the SDK, implement it for Invocation, and then make this node generic over that? I'm adding a TODO for now, but let me know if you were thinking of a different problem, or had a different solution in mind.

The more generic problem I was thinking of is routing things other than channels, such as individual messages. The current solution means that every time routing needs to change for a new message, a client needs to make a new connection. For the chat room that means creating a separate gRPC connection for every chat room in which you would like to participate. While this is OK for a small-scale chat app, this does not seems scalable for many potential applications.

But this is outside of the scope of this PR, as it is dependent on future IDL work to support more fine-grained labelling.

Fair point, I think this would be addressed (or at least facilitated) by making it easier for client to specify labels for each call, as opposed to only once per channel. WDYT?

Yes, per-call labels should help to resolve this.

conradgrobler · 2020-02-05T11:39:03Z

examples/chat/module/rust/src/lib.rs

-            }
-        };
+    fn handle_grpc_invocation(&mut self, grpc_invocation: Invocation) -> Result<(), oak::OakError> {
+        let grpc_invocation_label = grpc_invocation.request_receiver.label()?;


The label is now considered public, since the node is not getting tainted by reading the label.

This means in principle that anyone might get hold of the label. I know this is just a toy example, but at the very least the validity of the label should be checked somewhere (eg that the label can be generated from the bearer token). The label is used for routing to a room, which would mean anyone who has the public label could send messages to the room (or subscribe to it) without knowing the bearer token. A malicious client could manually override the gRPC metadata to use the label.

This is WAI for now. The label is the equivalent of a public key, and in principle, anyone who has a public key can encrypt data using it and send it to some storage service somewhere, so that the owner of the corresponding private key can decrypt it. This only guarantees secrecy, but no provenance. If we wanted a room to which only some people could send data, then we need to involve integrity labels, which as you said, will be based on knowing the actual bearer token. Clarified the comment above.

Agreed. The fact that someone can subscribe to a room using the public key still seems potentially problematic. They will be blocked from reading the contents once the label declassification system is implemented, but the ability to subscribe to content that you cannot access is a potential source of confustion.

daviddrysdale · 2020-02-05T09:52:55Z

sdk/rust/oak/src/io/receiver.rs

@@ -66,6 +66,11 @@ impl<T: Decodable> Receiver<T> {
        T::decode(&message)
    }

+    #[allow(clippy::trivially_copy_pass_by_ref)]
+    pub fn label(&self) -> Result<crate::label::Label, OakError> {
+        Ok(crate::label::Label(vec![1, 2, 3]))


Worth having a TODO(#NNN): text marker? (Although I guess it will be pretty obvious where the plumbing to the ABI should go).

Added TODO. I will create an issue and attach to it later in the review process if we decide to stick with this API.

Added issue # now too.

Issue number? (passim for TODOs)

sdk/rust/oak/src/label.rs

daviddrysdale · 2020-02-05T09:53:54Z

sdk/rust/oak/src/label.rs

+}
+
+pub fn get_node_label() -> Result<Label, crate::OakStatus> {
+    // TODO: Implement this via `oak_abi`.


Is there an issue number we could put here?

As for the other TODO, I am postponing creating issues until we generally agree on the shape of the API.

sdk/rust/oak/src/lib.rs

daviddrysdale · 2020-02-05T10:21:53Z

examples/chat/proto/chat.proto

@@ -64,5 +49,5 @@ service Chat {
  rpc Subscribe(SubscribeRequest) returns (stream Message);

  // Send a message to a chat room.


This proto definition is now a bit incomplete, because some of the information relevant to the API is now carried separately from the .proto message definitions (the "room ID" is effectively in metadata not data).

That obviously correlates with the need for the Router node to be restricted to looking at metadata but not data, but having a incomplete/split API definition seems awkward.

Yes that's true, and I don't have a good answer to that. The best I can think of is that this is similar to having a JSON schema for a web API, but that would not capture things like URLs or headers, so you would have to carry that context around in some other way. I think there are ways of bridging that gap, for instance by having web frameworks that convert URL fragments to JSON fields in a request, but I am not sure what would work best here.

At a more philosophical level though, the point of Oak is to minimize the trust boundary, so that a client device would only need to convince the user that some labels are applied to any outgoing message, regardless of what the message actually contains, so the distinction between trusted metadata and untrusted data is actually quite useful. For instance, a chat app Android client would only need to show that it is attaching some labels to outgoing gRPC requests, and that should be all the user cares about in terms of information flow.

That said, I'd be curious to hear others point of view on this.

/cc @project-oak/core

I agree with the philosophical point above about what a client device cares about, but the model is not great from a developer point of view. The developer now has to think about every API in terms of this split view. Validating consistency between data and metadata is more difficult as these are not tightly coupled. This is further complicated by the metadata being defined per connection and the data per message.

I have added some more comments here.

Conceptually this is similar to how in an API you may have methods that return data based on the provided authentication credentials, for instance all email messages for the authenticated user, etc. Though here the relationship is complicated by the fact that the label of the request is used to select the chat room to access. Perhaps we should go back to using some other identifier for chat rooms instead of the label itself, but then it would not be possible to route based on it because the room identifier would be protected by the label and not accessible by the router node. I think this may be solvable by an additional layer of routing nodes, but I am not sure that's the best way to go, so I'd leave that experiment and discussion for a future PR.

I also added a new method GetAllMessagesCount, which is supposed to return the number of messages from all chats that the user has access to. It does not seem practical (possible?) for it to be implemented in the current system, so I left it unimplemented, but I think it is a good discussion point to consider for future label-related discussions.

Update: I removed GetAllMessagesCount again, I'll leave that for another time, since currently it does not really add much value.

daviddrysdale · 2020-02-05T10:23:33Z

docs/programming-oak.md

-            sender: oak::io::Sender::new(wh),
-            admin_token,
-        }
+    let (wh, rh) = oak::channel_create(grpc_invocation_label).unwrap();


This leaves the doc with a bit of a puzzle in it (what's the mysterious grpc_invocation_label?) so maybe leave a TODO(#NNN) to sort out?

Clarified a bit in the previous paragraph now.

daviddrysdale · 2020-02-05T10:26:57Z

examples/chat/module/rust/src/lib.rs

-use protobuf::Message;
-use std::collections::hash_map::Entry;
+use oak::grpc::Invocation;
+use protobuf::ProtobufEnum;


Can now drop oak_derive from [dependencies]?

It's still needed by backend.rs.

examples/chat/module/rust/src/lib.rs

daviddrysdale · 2020-02-05T10:40:31Z

examples/chat/module/rust/src/lib.rs

-impl OakNode for Node {
-    fn new() -> Self {
+#[no_mangle]
+pub extern "C" fn router_oak_main(handle: u64) -> i32 {


If this is the main entrypoint for this Node, then I think it should be -> () (since cb8da90), and there probably needs to be a change somewhere in the client to look for "router_oak_main" rather than the default "oak_main"

Done. Will probably have to move things around when #462 is closed, I am happy to wait until that's done before submitting this, so we don't have to fix it twice.

tiziano88

Thanks for the comments so far

tiziano88 · 2020-02-05T12:48:09Z

examples/chat/module/rust/src/lib.rs

-use protobuf::Message;
-use std::collections::hash_map::Entry;
+use oak::grpc::Invocation;
+use protobuf::ProtobufEnum;


It's still needed by backend.rs.

examples/chat/module/rust/src/lib.rs

tiziano88 · 2020-02-05T12:56:53Z

examples/chat/module/rust/src/lib.rs

-impl OakNode for Node {
-    fn new() -> Self {
+#[no_mangle]
+pub extern "C" fn router_oak_main(handle: u64) -> i32 {


Done. Will probably have to move things around when #462 is closed, I am happy to wait until that's done before submitting this, so we don't have to fix it twice.

tiziano88 · 2020-02-05T13:02:57Z

examples/chat/module/rust/src/lib.rs

-            }
-        };
+    fn handle_grpc_invocation(&mut self, grpc_invocation: Invocation) -> Result<(), oak::OakError> {
+        let grpc_invocation_label = grpc_invocation.request_receiver.label()?;


This is WAI for now. The label is the equivalent of a public key, and in principle, anyone who has a public key can encrypt data using it and send it to some storage service somewhere, so that the owner of the corresponding private key can decrypt it. This only guarantees secrecy, but no provenance. If we wanted a room to which only some people could send data, then we need to involve integrity labels, which as you said, will be based on knowing the actual bearer token. Clarified the comment above.

tiziano88 · 2020-02-05T13:07:12Z

examples/chat/module/rust/src/lib.rs

+            .rooms
+            .entry(grpc_invocation_label.clone())
+            .or_insert_with(|| create_room_node(&grpc_invocation_label));
+        room_node_sender.send(&grpc_invocation)


This node solves the problem of having a single inbound channel, over which additional channel handles are received, but these channels have more restrictive labels attached to them. So in this sense, a generic router node would still need to know how to unpack incoming channel handles and inspect their labels, and I am not sure how to abstract that. Perhaps add a Routable trait in the SDK, implement it for Invocation, and then make this node generic over that? I'm adding a TODO for now, but let me know if you were thinking of a different problem, or had a different solution in mind.

sdk/rust/oak/src/label.rs

tiziano88 · 2020-02-05T13:16:43Z

sdk/rust/oak/src/label.rs

+}
+
+pub fn get_node_label() -> Result<Label, crate::OakStatus> {
+    // TODO: Implement this via `oak_abi`.


As for the other TODO, I am postponing creating issues until we generally agree on the shape of the API.

sdk/rust/oak/src/lib.rs

tiziano88 · 2020-02-05T17:36:39Z

docs/programming-oak.md

-            sender: oak::io::Sender::new(wh),
-            admin_token,
-        }
+    let (wh, rh) = oak::channel_create(grpc_invocation_label).unwrap();


Clarified a bit in the previous paragraph now.

conradgrobler · 2020-02-06T09:40:56Z

examples/chat/client/chat.cc

@@ -185,40 +179,37 @@ class OakApplication {
 // RAII class to handle creation/destruction of a chat room.
 class Room {
 public:
-  // Caller should ensure stub outlives this object.
+  // Caller must ensure stub outlives this object.
  Room(Chat::Stub* stub) : stub_(stub) {
    oak::NonceGenerator<64> generator;
    grpc::ClientContext context;
    CreateRoomRequest req;


nit: This seems to be unused, unless I am missing something.

conradgrobler · 2020-02-06T09:43:47Z

examples/chat/client/chat.cc

-    auto admin_token = generator.NextNonce();
-    req_.set_admin_token(std::string(admin_token.begin(), admin_token.end()));
+    admin_token_ = oak::NonceToBytes(generator.NextNonce());
+    CreateRoomRequest req_;


nit: As this is now a local variable and no longer a private field, it should probably not end with _

conradgrobler · 2020-02-06T09:47:13Z

examples/chat/module/rust/src/lib.rs

-            }
-        };
+    fn handle_grpc_invocation(&mut self, grpc_invocation: Invocation) -> Result<(), oak::OakError> {
+        let grpc_invocation_label = grpc_invocation.request_receiver.label()?;


Agreed. The fact that someone can subscribe to a room using the public key still seems potentially problematic. They will be blocked from reading the contents once the label declassification system is implemented, but the ability to subscribe to content that you cannot access is a potential source of confustion.

conradgrobler · 2020-02-06T09:59:23Z

examples/chat/module/rust/src/lib.rs

+            .rooms
+            .entry(grpc_invocation_label.clone())
+            .or_insert_with(|| create_room_node(&grpc_invocation_label));
+        room_node_sender.send(&grpc_invocation)


The more generic problem I was thinking of is routing things other than channels, such as individual messages. The current solution means that every time routing needs to change for a new message, a client needs to make a new connection. For the chat room that means creating a separate gRPC connection for every chat room in which you would like to participate. While this is OK for a small-scale chat app, this does not seems scalable for many potential applications.

But this is outside of the scope of this PR, as it is dependent on future IDL work to support more fine-grained labelling.

conradgrobler · 2020-02-06T10:29:56Z

examples/chat/proto/chat.proto

@@ -64,5 +49,5 @@ service Chat {
  rpc Subscribe(SubscribeRequest) returns (stream Message);

  // Send a message to a chat room.


I agree with the philosophical point above about what a client device cares about, but the model is not great from a developer point of view. The developer now has to think about every API in terms of this split view. Validating consistency between data and metadata is more difficult as these are not tightly coupled. This is further complicated by the metadata being defined per connection and the data per message.

tiziano88 · 2020-02-13T15:04:41Z

@anghelcovici @daviddrysdale @conradgrobler could you take another look? Especially around the proposed SDK changes (creating channels and nodes with labels, etc.)

I also updated the documentation for the chat app, let me know if you think it's reasonable.

daviddrysdale · 2020-02-13T15:09:21Z

docs/programming-oak.md

@@ -382,19 +382,18 @@ pattern for the existing Node is to:
  needed.

 For example, the [example Chat application](../examples/chat) creates a Node for
-each chat room and saves off the write handle that will be used to send messages
-to the room:
+each chat room (using the label corresponding to the incoming gRPC invocation)


Be nice if label was [label](concepts.md#labels) at some point …

Started on this now, PTAL.

daviddrysdale · 2020-02-13T15:13:15Z

examples/chat/README.md

- The **admin ID** allows destruction of the room instance at the server; the
-  client keeps hold of this ID internally.
+Additionally, an **admin token** allows destruction of the room instance at the
+server; the client keeps hold of this ID internally.


nits: be nice to match up "token" and "ID", and to mention where the token comes from, e.g.:

"When creating a room, the client also includes an admin token in the room creation request. The client keeps hold of this token and can use it to destroy the room instance at the server."

daviddrysdale · 2020-02-13T15:13:42Z

examples/chat/README.md


-This will emit a trace line that holds the information needed to:
+The application starts with a "router" node that only ever handles public data,


nit: I've generally tried to capitalize Application and Node.

daviddrysdale · 2020-02-13T15:15:17Z

examples/chat/README.md

-This will emit a trace line that holds the information needed to:
+The application starts with a "router" node that only ever handles public data,
+and is not allowed to inspect any data protected by a label (e.g. the
+aforementioned authentication bearer token).


nit: not completely clear whether the parenthetical content maps to the "data" or the "label" (that protects that data).

daviddrysdale · 2020-02-13T15:16:07Z

examples/chat/README.md

-A client can then create a **room**; which returns a pair of random IDs that act
-as bearer tokens:
+A client can then create a **room**, which is protected by an authentication
+bearer token that must be shared with any other participant to be invited to the


Maybe mention "label" or "metadata" here?

daviddrysdale · 2020-02-14T09:46:00Z

examples/chat/module/rust/src/backend.rs

+        Ok(Empty::new())
+    }
+
+    fn destroy_room(&mut self, req: DestroyRoomRequest) -> grpc::Result<Empty> {


Do room Nodes terminate, or are destroyed rooms just going to pile up?

Good question: according to the labelling rules, the Router node should not be able to learn that a room was destroyed (because such information is not public). But it feels like we should at least allow termination based on private data, otherwise no non-public node would ever be able to terminate. Not sure how / if the Router node would be able to notice though.

Created #607 to track.

daviddrysdale · 2020-02-14T09:51:26Z

examples/chat/module/rust/src/lib.rs

+/// from "public".
+#[derive(Default)]
+struct Router {
+    rooms: HashMap<oak::label::Label, oak::io::Sender<Invocation>>,


How would an entry ever get dropped from Router.rooms?

It cannot at the moment, added a comment to clarify.

daviddrysdale · 2020-02-14T09:53:13Z

examples/chat/module/rust/src/lib.rs

+                .entry(grpc_request_label.clone())
+                .or_insert_with(|| create_room_node(&grpc_request_label));
+            // Forward the incoming gRPC invocation to the appropriate room node.
+            room_node_sender.send(&grpc_invocation)


Once the Invocation has been forwarded, should the local copy be closed (i.e. both of the underlying handles closed)?

Yes but closing all these channels is getting tedious. Since they are now almost always created using higher level SDK functions, we should consider a more RAII-oriented approach (i.e. close them on drop). WDYT?

daviddrysdale · 2020-02-14T10:08:15Z

examples/chat/module/rust/src/lib.rs

-    }
+fn create_room_node(grpc_invocation_label: &oak::label::Label) -> oak::io::Sender<Invocation> {
+    let (wh, rh) = oak::channel_create_with_label(grpc_invocation_label).unwrap();
+    oak::node_create_with_label("app", "oak_main", rh, grpc_invocation_label)


Are labels intended to be static (specified up-front at channel/node creation time) or dynamic (extended when labelled data arrives) or both?

I guess I'd imagined more dynamic behaviour, which seems like it would make the explicit specification of labels here unnecessary – the new per-Room Node would start with an empty label, but would acquire the room label on its first read of a request message.

(Aside: it's also quite tricky to keep track of the different levels of indirection: the channel that the Invocation arrives on has no labels, but the Invocation itself contains two channels which do have the labels, and this Invocation is forwarded down another channel – which again can stay label-free because the labelled data is in an inner channel. Not sure what to do about that, though, other than having clear docs.)

I am trying to avoid "floating" labels that are automatically added when reading from channels, since that may introduce covert channels.

That said, it would be possible to implement this (and in fact it was one of my earlier attempts) so that a room node is created with "public" label, and then explicitly changes its label based on the first message it receives. I was not a big fan of it though, because I could not find a corresponding mechanism for channels, so I still had to let the channel creator specify the label for the channel upfront, and that should definitely not change once set. Anyways, this is not set in stone yet, happy to revisit and try out different models if this is confusing.

A more drastic approach could be to get rid of channels entirely, and give handles to nodes, effectively melding channels and nodes together, but that may have other side-effects (e.g. client streaming, as you mentioned a while ago).

daviddrysdale · 2020-02-14T10:09:09Z

examples/chat/module/rust/src/lib.rs

-        }
-    }
+fn create_room_node(grpc_invocation_label: &oak::label::Label) -> oak::io::Sender<Invocation> {
+    let (wh, rh) = oak::channel_create_with_label(grpc_invocation_label).unwrap();


Does this channel actually need the grpc_invocation_label ? If only Invocations (i.e. pairs of handles) are sent down it, then possibly not?

You are correct, changed back to plain channel_create.

conradgrobler · 2020-02-14T10:53:20Z

sdk/rust/oak/src/lib.rs

+/// Only nodes with a compatible label will be able to read and write to this channel:
+///
+/// - A node with label `L_w` may write to a channel with label `L_c` iff `L_w ⊑ L_c`.
+/// - A node with label `L_r` may read from a channel with label `L_c` iff `L_c ⊑ L_r`.


Not currently actionable, but just an obserbation to keep in mind for future design and planning: As they stand, the two rules in combination imply that bidirectional communication is only supported when both nodes have exactly the same labels. I assume these statements will change once we have implemented declassification.

The statement is still correct even in the case of declassification: declassification would allow a node to explicitly "lower" its label, if allowed by the rules, so that it matches that of the other side, at which point they would be able to have bi-directional communication. There are no other exceptions made via declassification, it's only about controlling the node own label.

In a future scenario where nodes are multi-threaded this could be tricky. One thread might try to raise the node's label to read a new message while another might lower the same node's label to write a declassified message. This would potentially require locks spanning large sections of code, effectively making it single-threaded again. A mechanism where a node could declassify a single message without changing its overall label seems easier to use.

conradgrobler · 2020-02-14T11:24:57Z

examples/chat/module/rust/src/lib.rs

+            let mut grpc_response_writer =
+                oak::grpc::ChannelResponseWriter::new(grpc_invocation.response_sender);
+            grpc_response_writer.close(Err(oak::grpc::build_status(
+                oak::grpc::Code::PERMISSION_DENIED,


Is this propagated to the client? In that case, it is possible to leak data. As a general rule, all failures should be silent unless in debug mode.

If there was a rule that needs to be manually enforced in the code in order to prevent data leak, then it would be a failure in the labelling system, right? :)

The resulting error is sent over the gRPC response channel, and respects the label on that channel; there is no special treatment for errors (intentionally).

In particular, the error will never reach the caller if it comes from a Node with a label that does not flow to the caller's label. But in this case, the error comes from a public node, and public label flows to any other label.

conradgrobler · 2020-02-14T11:31:50Z

examples/chat/README.md

-```
+In a more realistic scenario, the first client would securely share the bearer
+token with other participants that it wants to be able to join the chat room,
+using some out-of-band mechanism.


nit: The existence of a secure out of band mechanism for exchanging bearer tokens means that they probably don't need a chat room to exchange information preivately, so this does not really seem like a more realistic scenario. Perhaps add a comment that this will be improved when public-key options are implemented?

I don't think this is true. Even assuming labels representing public keys and more advanced cryptography, Oak does not (and should not, IMO) solve the key distribution problem. It only allows parties that already have each other's public keys to obtain similar guarantees as e2e encryption, but allowing the application to perform operations on the data, instead of only seeing encrypted binary blobs. Does it make sense, or are you thinking that Oak could help at a more fundamental level?

I also don't think that we should solve the key distribution problem. The main difference is between exchanging public keys versus private or symmetric keys. For public keys it is just a matter of integrity, not secrecy, so you can use a registry of public keys that you both trust. In the current implementation sharing the bearer token is equivalent to sharing a private or symmetric key. If I already had a secure out of band mechanism to share such sensitive information that provided the needed secrecy I would just send the message itself through this out of band mechanism.

tiziano88

Thanks for all the comments, apologies if I missed any, if you notice I did please reply so they are brought back to my attention.

tiziano88 · 2020-02-14T11:36:48Z

examples/chat/README.md


-This will emit a trace line that holds the information needed to:
+The application starts with a "router" node that only ever handles public data,


tiziano88 · 2020-02-14T11:37:10Z

examples/chat/README.md


- connect to the same Oak application (with `--app_address`)
- join the chat room (with `--room_id`).
+The router node receives gRPC invocations on an inbound chananel, but only looks


tiziano88 · 2020-02-14T11:40:05Z

examples/chat/README.md

+at the associated label (which is public), and based on the label only, routes
+the request to either an existing or a newly created "room worker node", which
+is created with that specific label and only ever handles data with the same
+label, for the duration of its own lifetime.


tiziano88 · 2020-02-14T11:40:57Z

examples/chat/module/rust/src/backend.rs

+            ));
+        }
+
+        info!("destroying room");


tiziano88 · 2020-02-14T11:52:41Z

examples/chat/module/rust/src/backend.rs

+        Ok(Empty::new())
+    }
+
+    fn destroy_room(&mut self, req: DestroyRoomRequest) -> grpc::Result<Empty> {


Good question: according to the labelling rules, the Router node should not be able to learn that a room was destroyed (because such information is not public). But it feels like we should at least allow termination based on private data, otherwise no non-public node would ever be able to terminate. Not sure how / if the Router node would be able to notice though.

Created #607 to track.

tiziano88 · 2020-02-14T16:19:54Z

examples/chat/README.md

-```
+In a more realistic scenario, the first client would securely share the bearer
+token with other participants that it wants to be able to join the chat room,
+using some out-of-band mechanism.


I don't think this is true. Even assuming labels representing public keys and more advanced cryptography, Oak does not (and should not, IMO) solve the key distribution problem. It only allows parties that already have each other's public keys to obtain similar guarantees as e2e encryption, but allowing the application to perform operations on the data, instead of only seeing encrypted binary blobs. Does it make sense, or are you thinking that Oak could help at a more fundamental level?

tiziano88 · 2020-02-14T18:00:24Z

examples/chat/module/rust/src/lib.rs

-    }
+fn create_room_node(grpc_invocation_label: &oak::label::Label) -> oak::io::Sender<Invocation> {
+    let (wh, rh) = oak::channel_create_with_label(grpc_invocation_label).unwrap();
+    oak::node_create_with_label("app", "oak_main", rh, grpc_invocation_label)


I am trying to avoid "floating" labels that are automatically added when reading from channels, since that may introduce covert channels.

That said, it would be possible to implement this (and in fact it was one of my earlier attempts) so that a room node is created with "public" label, and then explicitly changes its label based on the first message it receives. I was not a big fan of it though, because I could not find a corresponding mechanism for channels, so I still had to let the channel creator specify the label for the channel upfront, and that should definitely not change once set. Anyways, this is not set in stone yet, happy to revisit and try out different models if this is confusing.

A more drastic approach could be to get rid of channels entirely, and give handles to nodes, effectively melding channels and nodes together, but that may have other side-effects (e.g. client streaming, as you mentioned a while ago).

tiziano88 · 2020-02-14T18:08:41Z

examples/chat/proto/chat.proto

  rpc Subscribe(SubscribeRequest) returns (stream Message);

-  // Send a message to a chat room.
-  rpc SendMessage(SendMessageRequest) returns (google.protobuf.Empty);
+  // Send a message to a chat room identified by the provided label metadata.


tiziano88 · 2020-02-14T18:09:05Z

examples/chat/proto/chat.proto

 }

+// For all methods in this service, the request label (determined by gRPC
+// metadata) must "flow to" the response label (determined by client
+// authentication), unless otherwise specified.


Expanded in the concepts.md file and added a link here.

tiziano88 · 2020-02-14T18:16:27Z

examples/chat/module/rust/src/lib.rs

-        }
-    }
+fn create_room_node(grpc_invocation_label: &oak::label::Label) -> oak::io::Sender<Invocation> {
+    let (wh, rh) = oak::channel_create_with_label(grpc_invocation_label).unwrap();


You are correct, changed back to plain channel_create.

tiziano88

Thanks for all the comments, apologies if I missed any, if you notice I did please reply so they are brought back to my attention.

Expose methods to read channel label on Receiver and Sender objects. This change removes quite a bit of complexity from the chat app structure, and makes it actually use labels in a meaningful way, although for now only the ones related to bearer token authentication. Ref project-oak#488 project-oak#608

tiziano88 added the WIP Work in progress label Jan 22, 2020

googlebot added the cla: yes label Jan 22, 2020

tiziano88 force-pushed the tzn_chat_labels branch 2 times, most recently from dc38293 to 7936f79 Compare January 23, 2020 20:35

tiziano88 mentioned this pull request Jan 27, 2020

Encapsulate gRPC notification message into a typesafe wrapper in Rust SDK #528

Closed

tiziano88 force-pushed the tzn_chat_labels branch 5 times, most recently from 5e1ae56 to 091aafc Compare January 31, 2020 19:58

tiziano88 force-pushed the tzn_chat_labels branch 4 times, most recently from 7af274d to 7cdcc26 Compare February 4, 2020 19:17

tiziano88 requested review from daviddrysdale, anghelcovici and conradgrobler February 4, 2020 19:18

conradgrobler reviewed Feb 5, 2020

View reviewed changes

daviddrysdale reviewed Feb 5, 2020

View reviewed changes

tiziano88 force-pushed the tzn_chat_labels branch 2 times, most recently from ef3e04e to b47d2c9 Compare February 5, 2020 17:36

tiziano88 commented Feb 5, 2020

View reviewed changes

tiziano88 marked this pull request as ready for review February 5, 2020 17:37

tiziano88 requested review from conradgrobler and daviddrysdale February 5, 2020 17:37

conradgrobler reviewed Feb 6, 2020

View reviewed changes

tiziano88 force-pushed the tzn_chat_labels branch 2 times, most recently from 114b3eb to ba81e25 Compare February 11, 2020 16:46

tiziano88 force-pushed the tzn_chat_labels branch 4 times, most recently from 73bd770 to d09cd34 Compare February 12, 2020 20:13

daviddrysdale reviewed Feb 14, 2020

View reviewed changes

conradgrobler reviewed Feb 14, 2020

View reviewed changes

tiziano88 mentioned this pull request Feb 14, 2020

Implement generic Router Node #608

Closed

tiziano88 force-pushed the tzn_chat_labels branch 4 times, most recently from 08ccd50 to 8f97f6b Compare February 14, 2020 18:17

tiziano88 commented Feb 14, 2020

View reviewed changes

tiziano88 mentioned this pull request Feb 24, 2020

Implement labels and information flow control in Rust Oak Runtime #630

Closed

tiziano88 force-pushed the tzn_chat_labels branch 3 times, most recently from 65322c7 to f568ad7 Compare March 6, 2020 15:36

tiziano88 marked this pull request as draft April 22, 2020 10:51

jul-sh changed the base branch from master to main June 16, 2020 17:21

tiziano88 force-pushed the tzn_chat_labels branch from f568ad7 to f0c5b0e Compare October 13, 2020 22:15

tiziano88 force-pushed the tzn_chat_labels branch from f0c5b0e to 6e9dc5f Compare October 13, 2020 22:39

tiziano88 closed this Feb 15, 2022

		@@ -64,5 +49,5 @@ service Chat {
		rpc Subscribe(SubscribeRequest) returns (stream Message);

		// Send a message to a chat room.


		This will emit a trace line that holds the information needed to:
		The application starts with a "router" node that only ever handles public data,

Chat application + labels #515

Chat application + labels #515

Conversation

tiziano88 commented Jan 22, 2020 • edited Loading

Checklist

tiziano88 commented Feb 4, 2020

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tiziano88 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

tiziano88 commented Feb 13, 2020

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

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

tiziano88 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tiziano88 commented Jan 22, 2020 •

edited

Loading