Merging existing code into new enarx-wasmldr repo #3

MikeCamel · 2020-09-04T16:47:26Z

Was in enarx/enarx/keep-runtime.
Fixes #4
Fixes #5

npmccallum · 2020-09-10T13:05:25Z

Cargo.lock

- "filetime",
- "libc",
- "tracing",
-]


Don't delete Cargo.lock. We check in the lockfile for binaries but not for libraries.

OK. My bad - could you explain why?

npmccallum · 2020-09-10T13:05:49Z

.gitignore

@@ -1,2 +1,4 @@
 /target
 Cargo.lock
+
+


Don't add extra lines.

I thought there was a failure due to a lack of carriage return in another Cargo.lock file. I'll trim to just one

Shouldn't the Cargo.lock be removed from .gitignore ??

@haraldh Yes, probably.

npmccallum · 2020-09-10T13:08:38Z

Cargo.toml

+serde_yaml = "0.8"
+serde_json = "1.0"
+serde_derive = "1.0"
+openssl = { version = "0.10", features = ["vendored"] }


This appears to only be used for certificate generation. However, warp (above) pulls in the rustls crypto chain. I'd prefer not to have two dependencies that do the same thing. So let's avoid using openssl for now.

I thought we wanted to be using openssl for FIPS futures? The main reason, however, is that I struggled to find instructions for how to generate certificates - the only way I managed to make it work was with openssl. Happy to be given pointers...

@MikeCamel warp only currently supports the rustls stack. So while FIPS is definitely a future concern, my more immediate concern is the size bloat of having two separate crypto stacks.

As far as certificate generation... there doesn't seem to be a lot of options. The rustls stack uses webpki which only provides validation. The x509-parser project only appears to deserialize the structures.

I see a couple options:

Accept multiple crypto stacks. I don't love this option.

Use an http framework that supports openssl. There don't seem to be any. One potential route would be to use hyper directly via hyper-tls. However, hyper-tls currently only supports the client side. So we'd have to patch it to support the server side.

Thoughts? @ueno Your input is valued here.

We could create an X.509 template as bytes that we serialize just key data into. This is lightweight and avoids another crypto library.

npmccallum · 2020-09-10T13:09:40Z

Cargo.toml

+serde_json = "1.0"
+serde_derive = "1.0"
+openssl = { version = "0.10", features = ["vendored"] }
+reqwest = { version = "0.10.7", features = ["blocking", "json", "native-tls"] }


reqwest appears only to be used bin a test binary. The test binary should be built as a test and then this can become a dev-dependency rather than forcing it to be used for all builds.

It is, yes. Where can I find information on how to do all of this? Agree that it should be changed: are there docs on best practices anywhere?

https://doc.rust-lang.org/rust-by-example/testing.html

You probably want integration tests. But unit tests are also valuable.

npmccallum · 2020-09-10T13:10:00Z

Cargo.toml

+[[bin]]
+name = "enarx-wasmldr-tester"
+path = "src/enarx-wasmldr-tester.rs"
+


This should be a test rather than a separate binary.

Question: are tests supposed to be stand-alone (to be run by humans) or integrated (to be auto-run)? I've been doing only the former, so that we can do PoCs - if creating tests copes with both of these cases, I'm cool.

We always want automated tests. Developers can run them with a simple cargo test. But they also get automatically integrated into our CI.

npmccallum · 2020-09-10T13:20:49Z

src/main.rs

+    Ok(warp::reply::with_status(
+        "Payload received",
+        warp::http::StatusCode::OK,
+    ))


I think the flow of this function is wrong. If I'm reading it correctly, it won't respond to the client until the wasm code has finished running.

I was designing for the single-threaded case, where this is appropriate, I think (though the Result string isn't helpful). That said, I'm not sure that the main code will work single-threaded. This may be beyond my ken - I'll have a look and see if I can work it out.

It is still wrong even in the single threaded case.

We basically want one set of APIs for adding wasm modules to the Keep and another API to start execution. Adding a module to the keep should parse the modules and make sure they are valid before returning OK. OTOH, the go() API should immediately return OK, tear down all further (external) communication and then start wasm execution.

npmccallum · 2020-09-10T13:21:21Z

src/main.rs

+fn get_credentials_bytes(listen_addr: &str) -> (Vec<u8>, Vec<u8>) {
+    let (key, cert) = match KEY_SOURCE {
+        "file-system" => (get_key_bytes_fs(), get_cert_bytes_fs()),
+        "generate" => (generate_credentials(&listen_addr)),


Only generate.

Excellent! I'll remove all of these. Didn't like them, but thought I'd leave them in for safety.

npmccallum · 2020-09-10T13:21:41Z

src/main.rs

+        }
+    };
+    in_contents
+}


Drop this function.

npmccallum · 2020-09-10T13:21:50Z

src/main.rs

+        }
+    };
+    in_contents
+}


Drop this function.

npmccallum · 2020-09-10T13:22:12Z

src/main.rs

+        key.private_key_to_pem().unwrap(),
+        certificate.to_pem().unwrap(),
+    )
+}


Use something other than openssl in this function.

(We already have a crypto stack via warp.)

See comment above.

…x-wasmldr

Signed-off-by: MikeCamel <[email protected]>

Note that this is not correctly implemented in terms of certificates: FIXME needs addressing.

Signed-off-by: MikeCamel <[email protected]>

Note that this is not correctly implemented in terms of certificates: FIXME needs addressing.

As it's now not in .gitignore

Signed-off-by: MikeCamel <[email protected]>

…smldr into https-wasmldr Signed-off-by: MikeCamel <[email protected]>

MikeCamel · 2021-01-25T18:02:07Z

Out of date

enarxbot requested review from npmccallum, ueno and whitebrandy September 4, 2020 17:00

enarxbot assigned npmccallum, ueno and whitebrandy Sep 4, 2020

MikeCamel force-pushed the https-wasmldr branch from 861cb07 to 7330565 Compare September 7, 2020 15:04

MikeCamel removed the request for review from whitebrandy September 7, 2020 15:07

enarxbot requested a review from connorkuehl September 7, 2020 16:00

enarxbot assigned connorkuehl and unassigned whitebrandy Sep 7, 2020

enarxbot added the enhancement New feature or request label Sep 7, 2020

npmccallum suggested changes Sep 10, 2020

View reviewed changes

enarxbot assigned MikeCamel and unassigned ueno, npmccallum and connorkuehl Sep 10, 2020

MikeCamel added 10 commits November 10, 2020 23:47

Merge existing code from old enarx/enarx/keep-runtime into enarx/enar…

702b07f

…x-wasmldr

Minor syntax changes to pass tests.

769dbc0

Fixed a rebase issue.

c6455b7

Further changes to enhappy clippy

866662a

Added enarx-wasmldr-tester and associated files

5caefea

Signed-off-by: MikeCamel <[email protected]>

Removing Cargo.lock

a71fe72

Added carriage return at end of file

eaa1141

Added parsing for address (as well as port)

33d93ee

Note that this is not correctly implemented in terms of certificates: FIXME needs addressing.

Changed args collection to support calling by execl

4201bc4

Added authorship

7d5369b

MikeCamel added 24 commits December 14, 2020 10:15

Fixed a rebase issue.

31f897a

Further changes to enhappy clippy

6b5f97d

Added enarx-wasmldr-tester and associated files

9f49917

Signed-off-by: MikeCamel <[email protected]>

Removing Cargo.lock

8506c60

Added carriage return at end of file

2b1c9f5

Added parsing for address (as well as port)

340bc5f

Note that this is not correctly implemented in terms of certificates: FIXME needs addressing.

Changed args collection to support calling by execl

f6da283

Added authorship

3cb57ee

Initial responses to PR comments.

bddd143

Removed Cargo.lock entry from .gitignore

822318b

Adding Cargo.lock

de93248

As it's now not in .gitignore

Removed yaml file to allow rebase

d473251

Signed-off-by: MikeCamel <[email protected]>

Recovering ability to load over http(s)

3ef1ba5

Signed-off-by: MikeCamel <[email protected]>

Hard-coded ip-address and port for now.

f97b583

Signed-off-by: MikeCamel <[email protected]>

Started to sort certificate issues

bd902d8

Signed-off-by: MikeCamel <[email protected]>

Certs now work with DNS name

fa2f2e8

Signed-off-by: MikeCamel <[email protected]>

Cbor over HTTPS

5f2022c

Signed-off-by: MikeCamel <[email protected]>

Exit after payload delivery

ebbd53d

Signed-off-by: MikeCamel <[email protected]>

Tidying and linting

2f3e631

Signed-off-by: MikeCamel <[email protected]>

Changed endpoint name to /workload

64d4615

Signed-off-by: MikeCamel <[email protected]>

Move to ciborium

897480e

Signed-off-by: MikeCamel <[email protected]>

Tidying

69afe4c

Signed-off-by: MikeCamel <[email protected]>

Added build requirements info

fc1a7b8

Signed-off-by: MikeCamel <[email protected]>

Merge branch 'https-wasmldr' of https://github.com/MikeCamel/enarx-wa…

7731fdd

…smldr into https-wasmldr Signed-off-by: MikeCamel <[email protected]>

MikeCamel closed this Jan 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Merging existing code into new enarx-wasmldr repo #3

Merging existing code into new enarx-wasmldr repo #3

MikeCamel commented Sep 4, 2020 •

edited

Loading

npmccallum Sep 10, 2020

MikeCamel Sep 10, 2020

npmccallum Sep 10, 2020

MikeCamel Sep 10, 2020

haraldh Sep 10, 2020

npmccallum Sep 11, 2020

npmccallum Sep 10, 2020

MikeCamel Sep 10, 2020

npmccallum Sep 11, 2020

npmccallum Sep 11, 2020

npmccallum Sep 10, 2020

MikeCamel Sep 10, 2020

npmccallum Sep 11, 2020

npmccallum Sep 10, 2020

MikeCamel Sep 10, 2020 •

edited

Loading

npmccallum Sep 11, 2020

npmccallum Sep 10, 2020

MikeCamel Sep 10, 2020

npmccallum Sep 11, 2020

npmccallum Sep 10, 2020

MikeCamel Sep 10, 2020

npmccallum Sep 10, 2020

MikeCamel Sep 10, 2020

npmccallum Sep 10, 2020

MikeCamel Sep 10, 2020

npmccallum Sep 10, 2020

npmccallum Sep 10, 2020

MikeCamel Sep 10, 2020

MikeCamel commented Jan 25, 2021

Merging existing code into new enarx-wasmldr repo #3

Merging existing code into new enarx-wasmldr repo #3

Conversation

MikeCamel commented Sep 4, 2020 • edited 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

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

MikeCamel Sep 10, 2020 • edited 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

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

MikeCamel commented Jan 25, 2021

MikeCamel commented Sep 4, 2020 •

edited

Loading

MikeCamel Sep 10, 2020 •

edited

Loading