Introduce config map format and parsing logic #1004

tiziano88 · 2020-05-19T16:03:34Z

This will be used to pass configuration files and secrets to Oak
Applications, by specifying them via command line flags to the
oak_loader binary.

Also switch the top-level oak_loader error handling to the anyhow
crate.

Ref #689

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

ipetr0v · 2020-05-19T16:44:41Z

oak/server/rust/oak_loader/Cargo.toml

@@ -13,9 +13,13 @@ oak_debug = []
 default = ["oak_debug"]

 [dependencies]
+anyhow = "*"


I think this should also be added to oak/server/rust/oak_loader/BUILD

conradgrobler · 2020-05-19T17:04:25Z

oak/server/rust/oak_loader/src/main.rs

+impl FromStr for ConfigEntry {
+    type Err = anyhow::Error;
+    fn from_str(v: &str) -> Result<Self, Self::Err> {
+        let parts = v.split("=").collect::<Vec<_>>();


This seems quite brittle. It might be nice in future to have some better defined parsing.

I think this is fine for the foreseeable future, we fully control the file names anyways, so there is no risk that we end up with = in the file name itself. But sure, if it does happen we can extend it.

Seeing that this is also intended for sensitive data, is there a plan to add labels or some other mechanism to protect the data from untrusted nodes?

conradgrobler · 2020-05-19T17:08:58Z

oak/server/rust/oak_loader/src/main.rs

+    // We only log the keys here, since the values may be secret.
+    debug!("parsed config map entries: {:?}", config_map.items.keys());
+    // TODO(#689): Pass the `config_map` object to the Runtime instance, and make it available to
+    // the running Oak Application.


I am not clear on how this will be used. Is the intention to make this available to wasm nodes? Or to dynamically replace tokens in the application configuration?

The intention is to make it available to the first node of an Oak application, which can then take it apart and distribute it to further nodes if it needs to. It will be easier after #917 is merged, so I'll wait for that before doing the rest of the work.

conradgrobler · 2020-05-19T17:09:51Z

oak/server/rust/oak_loader/src/main.rs

+    let mut file_map = HashMap::new();
+    for config_entry in config_entries {
+        let file_content = read_file(&config_entry.filename)?;
+        file_map.insert(config_entry.key.to_string(), file_content);


Perhaps log a warning if the key already exists and the value is being overwritten?

Good point, done and added a test.

tiziano88

Thanks!

tiziano88 · 2020-05-19T17:16:48Z

oak/server/rust/oak_loader/src/main.rs

+impl FromStr for ConfigEntry {
+    type Err = anyhow::Error;
+    fn from_str(v: &str) -> Result<Self, Self::Err> {
+        let parts = v.split("=").collect::<Vec<_>>();


I think this is fine for the foreseeable future, we fully control the file names anyways, so there is no risk that we end up with = in the file name itself. But sure, if it does happen we can extend it.

tiziano88 · 2020-05-19T17:16:58Z

oak/server/rust/oak_loader/src/main.rs

+    let mut file_map = HashMap::new();
+    for config_entry in config_entries {
+        let file_content = read_file(&config_entry.filename)?;
+        file_map.insert(config_entry.key.to_string(), file_content);


Good point, done and added a test.

tiziano88 · 2020-05-19T17:18:33Z

oak/server/rust/oak_loader/src/main.rs

+    // We only log the keys here, since the values may be secret.
+    debug!("parsed config map entries: {:?}", config_map.items.keys());
+    // TODO(#689): Pass the `config_map` object to the Runtime instance, and make it available to
+    // the running Oak Application.


The intention is to make it available to the first node of an Oak application, which can then take it apart and distribute it to further nodes if it needs to. It will be easier after #917 is merged, so I'll wait for that before doing the rest of the work.

This will be used to pass configuration files and secrets to Oak Applications, by specifying them via command line flags to the `oak_loader` binary. Also switch the top-level `oak_loader` error handling to the `anyhow` crate. Ref project-oak#689

github-actions · 2020-05-19T19:02:25Z

Reproducibility index:

e98c02d369c44567feb42386d949cd85313fdf25f907fa95705ddc02e1af4a09  ./target/wasm32-unknown-unknown/release/abitest_0_frontend.wasm
6b237a388a32f724f4e6d8a5b0468f641d8cb2506fc578dc7c77b496ff62369e  ./target/wasm32-unknown-unknown/release/abitest_1_backend.wasm
8751bebd9fe5b6ad73a168a067e73a0e3d6a2de73e3e7f9d334895a1ccd59fec  ./target/wasm32-unknown-unknown/release/aggregator.wasm
eda11289a9e128ba20b4920aa2ed8b17d12d707464d56ea2aaae73c7343f25e3  ./target/wasm32-unknown-unknown/release/chat.wasm
1995ff75e523dda9165a1e3749011d05d293c8d8ca05c9f6075d6321471c4ced  ./target/wasm32-unknown-unknown/release/hello_world.wasm
296d669f7d657ade8e8db4a3f2545b4875194babb3b5f88610ee298a4ba8cd62  ./target/wasm32-unknown-unknown/release/machine_learning.wasm
4e71f536b6ea366c1595a96e0c6f3eacd86855cdd2a9c4ea7239843a5af14d1a  ./target/wasm32-unknown-unknown/release/private_set_intersection.wasm
c41676f7c5336807c1fea5616b0eae4705c716abc9e0b056443df671565bbf2b  ./target/wasm32-unknown-unknown/release/running_average.wasm
91f12039c5f7141690ede22a3ae1c6f90dc665e4c6ec867f41238429e9428875  ./target/wasm32-unknown-unknown/release/translator.wasm

tiziano88 requested review from ipetr0v and conradgrobler May 19, 2020 16:03

googlebot added the cla: yes label May 19, 2020

tiziano88 force-pushed the tzn_config_map_parse branch from b2a1e20 to 1078a29 Compare May 19, 2020 16:10

ipetr0v approved these changes May 19, 2020

View reviewed changes

tiziano88 force-pushed the tzn_config_map_parse branch from 1078a29 to ebf9e88 Compare May 19, 2020 16:53

conradgrobler approved these changes May 19, 2020

View reviewed changes

tiziano88 force-pushed the tzn_config_map_parse branch from ebf9e88 to a47ca7d Compare May 19, 2020 17:18

tiziano88 commented May 19, 2020

View reviewed changes

tiziano88 force-pushed the tzn_config_map_parse branch from a47ca7d to 8dcf10d Compare May 19, 2020 17:43

tiziano88 force-pushed the tzn_config_map_parse branch from 8dcf10d to ba4affb Compare May 19, 2020 18:12

tiziano88 merged commit f19bebb into project-oak:master May 19, 2020

This was referenced May 26, 2020

defense-in-depth configuration of oak_loader #1029

Closed

Add Trusted Information Retrieval example #1024

Closed

daviddrysdale mentioned this pull request Jun 5, 2020

SDK support for config map #1101

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce config map format and parsing logic #1004

Introduce config map format and parsing logic #1004

tiziano88 commented May 19, 2020

ipetr0v May 19, 2020

tiziano88 May 19, 2020

conradgrobler May 19, 2020

tiziano88 May 19, 2020

conradgrobler May 20, 2020

conradgrobler May 19, 2020

tiziano88 May 19, 2020

conradgrobler May 19, 2020

tiziano88 May 19, 2020

tiziano88 left a comment

tiziano88 May 19, 2020

tiziano88 May 19, 2020

tiziano88 May 19, 2020

github-actions bot commented May 19, 2020

Introduce config map format and parsing logic #1004

Introduce config map format and parsing logic #1004

Conversation

tiziano88 commented May 19, 2020

Checklist

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

github-actions bot commented May 19, 2020