m4sterchain commented on code in PR #150:
URL: 
https://github.com/apache/incubator-teaclave-trustzone-sdk/pull/150#discussion_r1871722725


##########
projects/web3/eth_wallet/ta/src/main.rs:
##########
@@ -0,0 +1,182 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#![no_main]
+#![feature(c_size_t)]
+
+mod hash;
+mod secure_storage;
+mod wallet;
+
+use crate::secure_storage::{
+    delete_from_secure_storage, load_from_secure_storage, 
save_in_secure_storage,
+};
+use optee_utee::{
+    ta_close_session, ta_create, ta_destroy, ta_invoke_command, 
ta_open_session, trace_println,
+};
+use optee_utee::{Error, ErrorKind, Parameters};
+use proto::Command;
+
+use anyhow::{anyhow, bail, Result};
+use std::convert::TryInto;
+use std::io::Write;
+use wallet::Wallet;
+
+#[ta_create]
+fn create() -> optee_utee::Result<()> {
+    trace_println!("[+] TA create");
+    Ok(())
+}
+
+#[ta_open_session]
+fn open_session(_params: &mut Parameters) -> optee_utee::Result<()> {
+    trace_println!("[+] TA open session");
+    Ok(())
+}
+
+#[ta_close_session]
+fn close_session() {
+    trace_println!("[+] TA close session");
+}
+
+#[ta_destroy]
+fn destroy() {
+    trace_println!("[+] TA destroy");
+}
+
+#[cfg(debug_assertions)]
+macro_rules! dbg_println {
+    ($($arg:tt)*) => (trace_println!($($arg)*));
+}
+
+#[cfg(not(debug_assertions))]
+macro_rules! dbg_println {
+    ($($arg:tt)*) => {};
+}
+
+fn create_wallet(_input: &proto::CreateWalletInput) -> 
Result<proto::CreateWalletOutput> {
+    let wallet = Wallet::new()?;
+    let wallet_id = wallet.get_id();
+    let mnemonic = wallet.get_mnemonic()?;
+    dbg_println!("[+] Wallet ID: {:?}", wallet_id);
+
+    let secure_object: Vec<u8> = wallet.try_into()?;
+    save_in_secure_storage(wallet_id.as_bytes(), &secure_object)?;
+    dbg_println!("[+] Wallet saved in secure storage");
+
+    Ok(proto::CreateWalletOutput {
+        wallet_id,
+        mnemonic,

Review Comment:
   Please highlight the security assumption and potential risks for returning 
mnemonic in this sample TA.



##########
projects/web3/eth_wallet/README.md:
##########
@@ -0,0 +1,188 @@
+# Eth-Wallet: A Sample Trusted Application for Wallet Abstraction and 
Transaction Signing
+
+This repository provides a reference implementation of an Ethereum wallet as a
+Trusted Application (TA) written in Rust. The primary goal is to ensure that
+secret credentials (such as private keys) remain securely within the Trusted
+Execution Environment (TEE) throughout their entire lifecycle, enhancing
+security and privacy for Ethereum-based operations. This reference
+implementation can be extended to support additional wallet features or adapted
+to other blockchain platforms with similar requirements for secure key
+management. The implementation provides basic wallet abstractions, including:
+
+- Key Generation: Securely generating random seeds within the TEE.
+- Key Derivation: Deriving keys from seeds within the TEE.
+- Key Persistency: Storing cryptographic keys securely in the TEE.
+- Transaction Signing: Signing Ethereum transactions without exposing private
+  keys to the normal world.
+- Key Erase: Erasing keys when they are no longer needed.
+

Review Comment:
   Could you please add
   - Security assumptions for this example if possible. 
   - Disclaimers.



##########
projects/web3/eth_wallet/ta/src/main.rs:
##########
@@ -0,0 +1,182 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#![no_main]
+#![feature(c_size_t)]
+
+mod hash;
+mod secure_storage;
+mod wallet;
+
+use crate::secure_storage::{
+    delete_from_secure_storage, load_from_secure_storage, 
save_in_secure_storage,
+};
+use optee_utee::{
+    ta_close_session, ta_create, ta_destroy, ta_invoke_command, 
ta_open_session, trace_println,
+};
+use optee_utee::{Error, ErrorKind, Parameters};
+use proto::Command;
+
+use anyhow::{anyhow, bail, Result};
+use std::convert::TryInto;
+use std::io::Write;
+use wallet::Wallet;
+
+#[ta_create]
+fn create() -> optee_utee::Result<()> {
+    trace_println!("[+] TA create");
+    Ok(())
+}
+
+#[ta_open_session]
+fn open_session(_params: &mut Parameters) -> optee_utee::Result<()> {
+    trace_println!("[+] TA open session");
+    Ok(())
+}
+
+#[ta_close_session]
+fn close_session() {
+    trace_println!("[+] TA close session");
+}
+
+#[ta_destroy]
+fn destroy() {
+    trace_println!("[+] TA destroy");
+}
+
+#[cfg(debug_assertions)]
+macro_rules! dbg_println {
+    ($($arg:tt)*) => (trace_println!($($arg)*));
+}
+
+#[cfg(not(debug_assertions))]
+macro_rules! dbg_println {
+    ($($arg:tt)*) => {};
+}
+
+fn create_wallet(_input: &proto::CreateWalletInput) -> 
Result<proto::CreateWalletOutput> {
+    let wallet = Wallet::new()?;
+    let wallet_id = wallet.get_id();
+    let mnemonic = wallet.get_mnemonic()?;
+    dbg_println!("[+] Wallet ID: {:?}", wallet_id);
+
+    let secure_object: Vec<u8> = wallet.try_into()?;
+    save_in_secure_storage(wallet_id.as_bytes(), &secure_object)?;
+    dbg_println!("[+] Wallet saved in secure storage");
+
+    Ok(proto::CreateWalletOutput {
+        wallet_id,
+        mnemonic,
+    })
+}
+
+fn remove_wallet(input: &proto::RemoveWalletInput) -> 
Result<proto::RemoveWalletOutput> {
+    dbg_println!("[+] Removing wallet: {:?}", input.wallet_id);
+
+    delete_from_secure_storage(input.wallet_id.as_bytes())?;
+    dbg_println!("[+] Wallet removed");
+
+    Ok(proto::RemoveWalletOutput {})
+}
+
+fn derive_address(input: &proto::DeriveAddressInput) -> 
Result<proto::DeriveAddressOutput> {
+    let secure_object = load_from_secure_storage(input.wallet_id.as_bytes())
+        .map_err(|e| anyhow!("[+] Deriving address: error: wallet not found: 
{:?}", e))?;
+    dbg_println!("[+] Deriving address: secure object loaded");
+
+    let wallet: Wallet = secure_object.try_into()?;
+    let (address, public_key) = wallet.derive_address(&input.hd_path)?;
+    dbg_println!("[+] Deriving address: address: {:?}", address);
+    dbg_println!("[+] Deriving address: public key: {:?}", public_key);
+
+    Ok(proto::DeriveAddressOutput {
+        address,
+        public_key,
+    })
+}
+
+fn sign_transaction(input: &proto::SignTransactionInput) -> 
Result<proto::SignTransactionOutput> {
+    let secure_object = load_from_secure_storage(input.wallet_id.as_bytes())
+        .map_err(|e| anyhow!("[+] Sign transaction: error: wallet not found: 
{:?}", e))?;
+    dbg_println!("[+] Sign transaction: secure object loaded");
+
+    let wallet: Wallet = secure_object.try_into()?;
+    let signature = wallet.sign_transaction(&input.hd_path, 
&input.transaction)?;
+    dbg_println!("[+] Sign transaction: signature: {:?}", signature);
+
+    Ok(proto::SignTransactionOutput { signature })
+}
+
+fn handle_invoke(command: Command, serialized_input: &[u8]) -> Result<Vec<u8>> 
{
+    fn process<T: serde::de::DeserializeOwned, U: serde::Serialize, F: Fn(&T) 
-> Result<U>>(
+        serialized_input: &[u8],
+        handler: F,
+    ) -> Result<Vec<u8>> {
+        let input: T = bincode::deserialize(serialized_input)?;
+        let output = handler(&input)?;
+        let serialized_output = bincode::serialize(&output)?;
+        Ok(serialized_output)
+    }
+
+    match command {
+        Command::CreateWallet => process(serialized_input, create_wallet),
+        Command::RemoveWallet => process(serialized_input, remove_wallet),
+        Command::DeriveAddress => process(serialized_input, derive_address),
+        Command::SignTransaction => process(serialized_input, 
sign_transaction),
+        _ => bail!("Unsupported command"),
+    }
+}
+
+#[ta_invoke_command]
+fn invoke_command(cmd_id: u32, params: &mut Parameters) -> 
optee_utee::Result<()> {
+    dbg_println!("[+] TA invoke command");
+    let mut p0 = unsafe { params.0.as_memref().unwrap() };

Review Comment:
   Please eliminate `unwrap` to demonstrate good practices for developing Rust 
TAs.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@teaclave.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@teaclave.apache.org
For additional commands, e-mail: dev-h...@teaclave.apache.org

Reply via email to