Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

manage proxied vault account per shard #1467

Merged
merged 27 commits into from
Oct 26, 2023

Conversation

brenzi
Copy link
Collaborator

@brenzi brenzi commented Oct 11, 2023

closes #1253
closes #1466

Register a shard vault account on Integritee parentchain (no multichain option yet for this) and register every new shard worker as a proxy

Also added draft doc diagrams on the way that helped me find my way

  • derive vault account from enclave seed and shard identifier
  • endow vault account as primary worker
  • add self as proxy
  • persist vault AccountId in state (hardcoded key)
  • dummy unshield proxy(balance.transfer_keep_alive) extrinsic (no matter if fails, but must decode correctly)
  • register new validateers as proxy upon provisioning
  • let stf make use of proxied vault account (example for later unshielding refactoring)

testing

run node:

./target/release/integritee-node --dev --unsafe-ws-external --rpc-cors all

build worker as sidechain
SGX_MODE=SW WORKER_MODE=sidechain WORKER_FEATURES=dcap make

run first worker

export RUST_LOG=info,substrate_api_client=warn,ws=warn,mio=warn,its_consensus_common=info,sidechain=info,integritee_service=trace,enclave_runtime=trace,ac_node_api=warn,sp_io=warn,itc_parentchain_indirect_calls_executor=trace,itp_stf_executor=trace,itc_parentchain_light_client=trace,itc_parentchain_block_importer=trace,itp_stf_state_handler=trace,ita_stf=trace,itp-attestation-handler=debug
./integritee-service -u ws://172.17.0.1 -c -d /tmp/worker1 run --skip-ra --dev &> worker1.log 

verify the following events are issued:

  1. teerex.AddedSgxEnclave
  2. balances.Endowed creating the vault account
  3. proxy.ProxyAdded registering enclave signer as a proxy of vault

You'll also see plenty of enclaveBridge.ProcessedParentchainBlock and sidechain.FinalizedSidechainBlock. don't care!

now, run second worker provisioning

./integritee-service -u ws://172.17.0.1 -r 3444 -P 2100 -h 2110 -w 2101 -i 8788 -c -d /tmp/worker2 request-state --skip-ra &> worker2.log

verify the following events are issued:

  1. proxy.ProxyAdded adding the second worker enclave account as a proxy for vault

then, test unshielding (replace mrenclave with yours)

export MRENCLAVE=G51nYibsmo8Gj4VJiEsLxoKttZF6xMbQGCRc3GqzTPHf
./integritee-cli -u ws://172.17.0.1 shield-funds //Alice //Alice 20000000000000 $MRENCLAVE

verify the following events are issued:

  1. enclaveBridge.ShieldFunds

then unshield:
./integritee-cli -u ws://172.17.0.1 trusted --mrenclave $MRENCLAVE --direct unshield-funds //Alice //Alice 1000000000000

verify the following events are issued:

  1. proxy.ProxyExecuted don't care this fails because we're unshielding funds which never went into the vault account. to be solved later safely unshield using balances.transfer on parentchain #1257

@brenzi brenzi added A0-core Affects a core part B1-releasenotes C1-low 📌 Does not elevate a release containing this beyond "low priority" E5-publicapi PR breaks public API labels Oct 13, 2023
@@ -346,6 +371,7 @@ where
verify_attn_report(attn_report_raw, pub_k, attestation_ocall)
} else {
// TODO Refactor state provisioning to not use MURA #1385
// TODO DCAP is currently just passed through! SECURITY!!!
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't aware that our MU RA is insecure for DCAP. Increases the urgency for #1385

@@ -234,6 +234,31 @@ pub fn percent_decode(orig: String) -> EnclaveResult<String> {
Ok(ret)
}

pub fn parse_cert_issuer(cert_der: &[u8]) -> SgxResult<Vec<u8>> {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is currently not used. I wrote it because I tried to derive the MU RA client from the TLS certificate. fell back to passing it as a payload instead. Still, I think this fn might be useful on its own. webpki and rustls hide the issuer all too well behind private fields

@@ -88,6 +88,15 @@ where
) -> Self {
ExtrinsicsFactory { genesis_hash, signer, nonce_cache, node_metadata_repository }
}

pub fn with_signer(&self, signer: Signer, nonce_cache: Arc<NonceCache>) -> Self {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this allows the enclave to send extrinsics using arbitrary signers (closes #1466)

@@ -93,6 +93,7 @@ pub trait EnclaveOnChainOCallApi: Clone + Send + Sync {
&self,
extrinsics: Vec<OpaqueExtrinsic>,
parentchain_id: &ParentchainId,
await_each_inclusion: bool,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

necessary to wait for vault to be created before trying to send an extrinsic as vault

@brenzi brenzi marked this pull request as ready for review October 13, 2023 13:10
@brenzi brenzi requested a review from clangenb October 13, 2023 13:10
DESIGN.md Show resolved Hide resolved
Copy link
Contributor

@clangenb clangenb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, looks good to me in general, only minor stuff!

DESIGN.md Outdated Show resolved Hide resolved
DESIGN.md Show resolved Hide resolved
core-primitives/types/src/parentchain.rs Show resolved Hide resolved
service/src/parentchain_handler.rs Outdated Show resolved Hide resolved
enclave-runtime/src/tls_ra/tls_ra_server.rs Show resolved Hide resolved
enclave-runtime/src/tls_ra/tls_ra_client.rs Show resolved Hide resolved
enclave-runtime/src/shard_vault.rs Show resolved Hide resolved
@brenzi brenzi requested a review from clangenb October 24, 2023 09:54
Copy link
Contributor

@clangenb clangenb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@brenzi brenzi merged commit 46515c7 into master Oct 26, 2023
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A0-core Affects a core part B1-releasenotes C1-low 📌 Does not elevate a release containing this beyond "low priority" E5-publicapi PR breaks public API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

extrinsics factory should support arbitrary signer SCV/OCW can manage proxied vault account
2 participants