Bazk: milestones 1 & 2

[veigajoao]

I am adding this PR to start the review for Bazk milestones 1 and 2 - Decentralized contribution verifier and decentralized contribution generation respectivelly.

The team is currently working on improving documentation and tooling for testing, those are going to be added in the next days.

[kvinwang]

My Comments on the Code:

1. Data Structuring in Pod Validator:

   The current data structure in the Validator struct within the pod validator (located here) might be inefficient due to the usage of Vec for several fields.

     #[ink(storage)]
   pub struct Validator {
       owner: AccountId,
       allowlist: Vec<[u8; 32]>,
       ceremony_hashes: Vec<(u32, Vec<File>)>,
       ceremony_metadatas: Vec<(u32, Vec<Metadata>)>,
       ceremonies: Vec<Ceremony>
   }

   As discussed in the Discord group, these Vec containers holding elements like allowlist, ceremony_hashes, ceremony_metadatas, and ceremonies could potentially reach item size limitations. To address this, consider restructuring these fields using alternatives like ink::storage::Mapping or ink::storage::Lazy.

2. Incorrect Gramine Manifest:

   The Bazk build manifest template located here appears to have an issue with allowing arbitrary arguments to be passed to the node. This poses a security risk because it enables anyone to execute arbitrary code within the SGX enclave and obtain a signed RA report, essentially defeating the purpose of the RA report.

   Furthermore, the current configuration of allowed_files in the manifest (here) needs revision. At least the /etc/ssl/certs/ca-certificates.crt and index.js shouldn't be put there. Files that are not intended for runtime modification should be placed within the trusted_files section instead.

3. Potential Arbitrary Code Execution:

   A potential vulnerability exists in the application code here. This code section might allow an attacker to replace a file within the allowed_list with a malicious executable binary and execute it. If executed within the enclave, such a binary could potentially steal RA reports or other sensitive data.

BTW: Why use curl when Node.js itself can make HTTP requests?

[davibauer]

@kvinwang

1. Got it, we're changing the implementation to use Mapping for these fields.

2. We use argv here because we need to get some values at runtime such as command name, ceremony name, description and deadline. I'm not sure what other approach we could use here to avoid passing arguments to node since they are variables and we need them inside index.js. Regarding the current allowed_files setting in the manifest, we are fixing it.

3. Now we are validating the command name arg so that only a list of acceptable commands is allowed, all of these commands are in the trusted_files.

Regarding using curl, we tried using http request directly before, but we got a wasm error inside node, so we decided to use curl.

Quick question, is report.verify here still valid? this comes from ias::SignedIasReport. I noticed that in the latest version of sgx-attestation (https://github.com/Phala-Network/phala-blockchain) it doesn't exist.

[kvinwang]

I'm not sure what other approach we could use here to avoid passing arguments to node since they are variables and we need them inside index.js

Some alternatives comes in my mind:

• passing the command via env vars.
• using a custom loader as entrypoint rather than ./node, which always execute index.js.
• starting an http server in the index.js and listening for commands.

Regarding using curl, we tried using http request directly before, but we got a wasm error inside node, so we decided to use curl.

The WASM error may have been caused by a third-party npm package. Consider using an alternative HTTP client package to resolve the issue, or just use the native HTTP API provided by Node.js.

Quick question, is report.verify here still valid?

Yes, it is now under a feature flag.

[veigajoao]

In our current scenario I don't believe using .env makes sense. Each different user of the application is going to have different parameters.

Do you have any examples on doing an http server working that we can use?

[kvinwang]

In our current scenario I don't believe using .env makes sense. Each different user of the application is going to have different parameters.

Using env vars doesn't mean you have to use a .env file.
The typical way passing env vars is via command line, for example:

COMMAND=challenge1 ./gramine-sgx node

[davibauer]

@kvinwang After enabling the feature verify, I started getting this error:
Any thoughts?

   Compiling ink_ir v4.3.0
   Compiling synstructure v0.12.6
   Compiling num_enum_derive v0.6.1
   Compiling rustls-webpki v0.102.0-alpha.3 (https://github.com/rustls/webpki?rev=2ed9a4324f48c2c46ffdd7dc9d3eb315af25fce2#2ed9a432)
error[E0277]: `RingAlgorithm` doesn't implement `Debug`
   --> /root/.cargo/git/checkouts/webpki-b32ad968b7235a62/2ed9a43/src/ring_algs.rs:27:41
    |
27  | impl SignatureVerificationAlgorithm for RingAlgorithm {
    |                                         ^^^^^^^^^^^^^ `RingAlgorithm` cannot be formatted using `{:?}`
    |
    = help: the trait `Debug` is not implemented for `RingAlgorithm`
    = note: add `#[derive(Debug)]` to `RingAlgorithm` or manually `impl Debug for RingAlgorithm`
note: required by a bound in `SignatureVerificationAlgorithm`
   --> /root/.cargo/registry/src/index.crates.io-6f17d22bba15001f/rustls-pki-types-0.2.3/src/lib.rs:323:57
    |
323 | pub trait SignatureVerificationAlgorithm: Send + Sync + fmt::Debug {
    |                                                         ^^^^^^^^^^ required by this bound in `SignatureVerificationAlgorithm`
help: consider annotating `RingAlgorithm` with `#[derive(Debug)]`
    |
21  + #[derive(Debug)]
22  | struct RingAlgorithm {

[kvinwang]

@kvinwang After enabling the feature verify, I started getting this error:
Any thoughts?

Oh, this should be a breaking change in the upstream update.
The workaround could be edit the Cargo.lock and make sure the rustls-pki-types is 0.2.1 and the checksum matches below:

 [[package]]
 name = "rustls-pki-types"
 version = "0.2.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "a47003264dea418db67060fa420ad16d0d2f8f0a0360d825c00e177ac52cb5d8"

[davibauer]

@kvinwang After running cargo contract build and adding ink_storage dependencies (to allow use of Mapping in the contract):

[dependencies]
ink_storage = "5.0"

[features]
std = [
    "ink_storage/std"

I started getting errors like that:

error[E0152]: duplicate lang item in crate `core` (which `byte_slice_cast` depends on): `CStr`.
  |
  = note: the lang item is first defined in crate `core` (which `std` depends on)
  = note: first definition in `core` loaded from /root/.rustup/toolchains/1.73-x86_64-unknown-linux-gnu/lib/rustlib/wasm32-unknown-unknown/lib/libcore-27002f4433b346f4.rlib
  = note: second definition in `core` loaded from /mnt/d/Projects/HAC/Repos/bazk/packages/pod-validator/target/ink/wasm32-unknown-unknown/release/deps/libcore-8697656726ecafa8.rmeta

I'm not really sure about the root cause, I'll leave here how my Cargo.toml is configured. I appreciate any help.

[package]
name = "pod_validator"
version = "0.1.0"
authors = ["Kevin Wang <wy721@qq.com>"]
edition = "2021"

[dependencies]
ink = { version = "4.2.0", default-features = false }
hex = { version = "0.4", default-features = false, features = ["alloc"] }
hex_fmt = "0.3"
ink_storage = "5.0"

[dependencies.pink]
package = "pink-extension"
version = "0.4.6"
default-features = false
features = ["dlmalloc"]

[dependencies.scale]
package = "parity-scale-codec"
version = "3"
default-features = false
features = ["derive"]

[dependencies.scale-info]
version = "2.6"
default-features = false
features = ["derive"]
optional = true

[dependencies.sgx-attestation]
# TODO.kevin, use crates.io once it's published
git = "https://github.com/Phala-Network/phala-blockchain"
features = ["verify"]
default-features = false

[dev-dependencies]
ink_e2e = "4.2.0"

[lib]
path = "lib.rs"

[features]
default = ["std"]
std = [
    "ink/std",
    "ink_storage/std",
    "scale/std",
    "scale-info/std",
    "pink/std",
    "sgx-attestation/std",
]
ink-as-dependency = []
e2e-tests = []

[patch.crates-io]
ring = { git = "https://github.com/jasl/ring-xous", branch = "better-wasm32-support" }

[kvinwang]

[dependencies]
ink_storage = "5.0"

[features]
std = [
"ink_storage/std"

The default features should be disabled and use v4.2:

ink_storage = { version = "4.2.0", default-features = false }

Actually, adding this dependency is unnecessary because it is re-exported in the crate ink. You can directly use ink::storage in the source code.

[davibauer]

Hi @kvinwang, another help :)

I added the changes you mentioned, but now I'm stuck with this:

Also, inside mod pod_validator I added this: use ink::storage::traits::StorageLayout;

error[E0433]: failed to resolve: could not find `metadata` in `ink`
  --> /mnt/d/Projects/HAC/Repos/bazk/packages/pod-validator/lib.rs:75:37
   |
75 |     #[derive(Clone, Encode, Decode, StorageLayout)]
   |                                     ^^^^^^^^^^^^^ could not find `metadata` in `ink`
   |
   = note: this error originates in the derive macro `StorageLayout` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0404]: expected trait, found derive macro `ink::storage::traits::StorageLayout`
  --> /mnt/d/Projects/HAC/Repos/bazk/packages/pod-validator/lib.rs:75:37
   |
75 |     #[derive(Clone, Encode, Decode, StorageLayout)]
   |                                     ^^^^^^^^^^^^^ not a trait
   |
   = note: this error originates in the derive macro `StorageLayout` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0433]: failed to resolve: could not find `metadata` in `ink`
  --> /mnt/d/Projects/HAC/Repos/bazk/packages/pod-validator/lib.rs:75:37
   |
75 |     #[derive(Clone, Encode, Decode, StorageLayout)]
   |                                     ^^^^^^^^^^^^^ could not find `metadata` in `ink`

If I remove StorageLayout from the structs, it asks me to add it again.

error[E0277]: the trait bound `Ceremony: StorageLayout` is not satisfied
  --> /mnt/d/Projects/HAC/Repos/bazk/packages/pod-validator/lib.rs:65:5
   |
65 |     pub struct Validator {
   |     ^^^ the trait `StorageLayout` is not implemented for `Ceremony`
   |

[kvinwang]

#[derive(Clone, Encode, Decode, StorageLayout)]

It should be only required in std.

#[cfg_attr(feature = "std", derive(StorageLayout))]
#[derive(Clone, Encode, Decode)]

[veigajoao]

We just finalized the corrections, gonna merge them to main today

[shelvenzhou]

LGTM