WIP: split veracruz-server and runtime-manager into platform-specific crates #112

gbryant-arm · 2021-03-24T14:06:11Z

This PR aims at splitting platform-specific code (backend) from the runtime and the rest of the codebase (#81).
The goal is to make the codebase more modular and in particular to simplify the process of adding support for another platform.

Todo:

Create veracruz-server-sgx crate
Reusing structures and functions from the parent crate (veracruz-server)

WIP: Reuse structures and functions from the parent crate (veracruz-server)

dreemkiller · 2021-03-24T14:29:20Z

veracruz-server-sgx/src/server.rs

@@ -0,0 +1,133 @@
+//! The Veracruz server


There is very little in this file that is SGX-specific, and this file is likely to appear in all platform-specific servers.
Is there a way that we can use this code across all platforms instead of duplicating it?

dreemkiller · 2021-03-24T14:30:09Z

veracruz-server-sgx/src/veracruz_server.rs

@@ -0,0 +1,35 @@
+//! Veracruz server


Same comment in this one. As far as I can tell, there is no SGX-specific code in this one.

dominic-mulligan-arm · 2021-03-24T14:31:08Z

veracruz-server-sgx/Makefile

+# See the `LICENSE.markdown` file in the Veracruz root director for licensing
+# and copyright information.
+
+.PHONY: all policy-files sgx trustzone deprecated


The trustzone build target is now redundant here, I think.

dominic-mulligan-arm · 2021-03-24T14:32:45Z

veracruz-server-sgx/fuzz/Cargo.toml

+webpki-roots = "0.16"
+
+
+[dependencies.veracruz-server-sgx]


Why use [dependencies.veracruz-server-sgx] here?

dominic-mulligan-arm · 2021-03-24T14:33:44Z

veracruz-server-sgx/fuzz/fuzz_targets/provision_data.rs

+    ));
+    rustls::ClientSession::new(&std::sync::Arc::new(client_config), dns_name)
+}
+


Superfluous newlines, here.

dominic-mulligan-arm · 2021-03-24T14:34:10Z

veracruz-server-sgx/fuzz/fuzz_targets/provision_data.rs

+        let rst = client_tls_send(&client_tls_tx, &client_tls_rx, &mut client_session, &request).unwrap();
+        let rst = protobuf::parse_from_bytes::<transport_protocol::RuntimeManagerResponse>(&rst);
+        assert!(rst.is_ok());
+        //let response = rst.unwrap();


Commented code should be deleted.

dominic-mulligan-arm · 2021-03-24T14:34:59Z

veracruz-server-sgx/fuzz/fuzz_targets/provision_data.rs

+                Ok(_) => (),
+                Err(err) => return Err(format!("Error in tx.send() {:?}", err)),
+            };
+        } else {


This trailing else can be deleted.

veracruz-project-owner · 2021-03-24T16:01:11Z

AWS CodeBuild CI Report

CodeBuild project: Veracruz
Commit ID: d5e8a3a
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

…ponder with a type parameter

veracruz-project-owner · 2021-04-14T11:17:18Z

AWS CodeBuild CI Report

CodeBuild project: Veracruz
Commit ID: 90e9aad
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

dominic-mulligan-arm · 2021-04-14T13:00:53Z

@gbryant-arm, the changes you made to the VeracruzServerError type look correct to me. What errors are you getting, and on what lines?

dominic-mulligan-arm · 2021-04-14T13:02:21Z

BTW, you should probably also rebase sooner rather than later, as there have been some fairly extensive changes in veracruz-utils that may affect you. Also, there was recently some oddity with the Docker submodule in the repository that should now be fixed, though I notice your Codebuild CI runs are failing because of that.

ShaleXIONG · 2021-04-16T11:57:30Z

veracruz-server-sgx/fuzz/Cargo.toml

+[package]
+name = "veracruz-server-sgx-fuzz"
+version = "0.0.0"
+authors = ["Shale Xiong"]


I probably forgot to change here on authorship since the first initial commit and this fuzz test is no longer valid.

ShaleXIONG · 2021-04-16T12:04:50Z

veracruz-server-sgx/fuzz/fuzz_targets/provision_data.rs

+
+            match res {
+                Ok(_) => (),
+                Err(err) => return Err(format!("Error in tx.send() {:?}", err)),


map_err in Result type and ok_or in Option type, together with the ? operator, can be useful in term of propagating Error, e.g.

let res = tx.send(output).map_err(|err|format!("Error in tx.send() {:?}", err))?;

In fact here if you never want to use the Ok(_) case, you could try

tx.send(output).map_err(|err| format!("Error in tx.send() {:?}", err))?;

Just to say, there is no problem of use match. However, these two functions can help you to write less code, especially less indentation for readability.

dominic-mulligan-arm · 2021-04-19T08:59:56Z

Are you still making progress, @gbryant-arm, or are you stuck?

dominic-mulligan-arm · 2021-04-19T10:51:50Z

veracruz-server-sgx/Cargo.toml

@@ -0,0 +1,40 @@
+[package]


This file needs the copyright/licensing header at the top that all other Cargo.toml files have.

gbryant-arm · 2021-04-20T10:47:57Z

Thanks @dreemkiller @dominic-mulligan-arm @ShaleXIONG for your feedback. I will address the comments soon, but first I'd like to focus on splitting veracruz-server (that's my first MVP)

veracruz-project-owner · 2021-05-06T10:53:53Z

AWS CodeBuild CI Report

CodeBuild project: Veracruz
Commit ID: 2a8004b
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

Fix various errors

veracruz-project-owner · 2021-05-13T14:02:34Z

AWS CodeBuild CI Report

CodeBuild project: Veracruz
Commit ID: 2592959
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

veracruz-project-owner · 2021-06-01T11:28:04Z

AWS CodeBuild CI Report

CodeBuild project: Veracruz
Commit ID: 935ea78
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

dominic-mulligan-arm · 2021-07-05T12:26:47Z

Closing this as I have taken over the task.

Create veracruz-server-sgx crate

d5e8a3a

WIP: Reuse structures and functions from the parent crate (veracruz-server)

dominic-mulligan-arm added this to the Split Veracruz runtime from isolation/attestation code milestone Mar 24, 2021

dreemkiller reviewed Mar 24, 2021

View reviewed changes

dominic-mulligan-arm reviewed Mar 24, 2021

View reviewed changes

dominic-mulligan-arm changed the title ~~Platform splitting~~ WIP: split veracruz-server and runtime-manager into platform-specific crates Mar 31, 2021

WIP: Extend VeracruzServerError, VeracruzServer and VeracruzServerRes…

90e9aad

…ponder with a type parameter

ShaleXIONG reviewed Apr 16, 2021

View reviewed changes

dominic-mulligan-arm reviewed Apr 19, 2021

View reviewed changes

Specify type parameter

2a8004b

Add constraints to A: must implement Debug and Display trait

2592959

Fix various errors

gbryant-arm added 2 commits May 27, 2021 11:24

Revert introduction of type parameter

e2c35c7

Add extension

935ea78

dominic-mulligan-arm closed this Jul 5, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: split veracruz-server and runtime-manager into platform-specific crates #112

WIP: split veracruz-server and runtime-manager into platform-specific crates #112

gbryant-arm commented Mar 24, 2021 •

edited by dominic-mulligan-arm

Loading

dreemkiller Mar 24, 2021

dreemkiller Mar 24, 2021

dominic-mulligan-arm Mar 24, 2021

dominic-mulligan-arm Mar 24, 2021

dominic-mulligan-arm Mar 24, 2021

dominic-mulligan-arm Mar 24, 2021

dominic-mulligan-arm Mar 24, 2021

veracruz-project-owner commented Mar 24, 2021

veracruz-project-owner commented Apr 14, 2021

dominic-mulligan-arm commented Apr 14, 2021

dominic-mulligan-arm commented Apr 14, 2021

ShaleXIONG Apr 16, 2021 •

edited

Loading

ShaleXIONG Apr 16, 2021

dominic-mulligan-arm commented Apr 19, 2021

dominic-mulligan-arm Apr 19, 2021

gbryant-arm commented Apr 20, 2021 •

edited

Loading

veracruz-project-owner commented May 6, 2021

veracruz-project-owner commented May 13, 2021

veracruz-project-owner commented Jun 1, 2021

dominic-mulligan-arm commented Jul 5, 2021

		webpki-roots = "0.16"


		[dependencies.veracruz-server-sgx]

WIP: split veracruz-server and runtime-manager into platform-specific crates #112

WIP: split veracruz-server and runtime-manager into platform-specific crates #112

Conversation

gbryant-arm commented Mar 24, 2021 • edited by dominic-mulligan-arm Loading

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

veracruz-project-owner commented Mar 24, 2021

AWS CodeBuild CI Report

veracruz-project-owner commented Apr 14, 2021

AWS CodeBuild CI Report

dominic-mulligan-arm commented Apr 14, 2021

dominic-mulligan-arm commented Apr 14, 2021

ShaleXIONG Apr 16, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dominic-mulligan-arm commented Apr 19, 2021

Choose a reason for hiding this comment

gbryant-arm commented Apr 20, 2021 • edited Loading

veracruz-project-owner commented May 6, 2021

AWS CodeBuild CI Report

veracruz-project-owner commented May 13, 2021

AWS CodeBuild CI Report

veracruz-project-owner commented Jun 1, 2021

AWS CodeBuild CI Report

dominic-mulligan-arm commented Jul 5, 2021

gbryant-arm commented Mar 24, 2021 •

edited by dominic-mulligan-arm

Loading

ShaleXIONG Apr 16, 2021 •

edited

Loading

gbryant-arm commented Apr 20, 2021 •

edited

Loading