Skip to content

Commit

Permalink
don't allow signing deposit messages (#60)
Browse files Browse the repository at this point in the history
* don't allow signing deposit messages

* Update README.md

---------

Co-authored-by: JasonVranek <jvranek@ucsc.edu>
  • Loading branch information
kamiyaa and JasonVranek authored Apr 23, 2024
1 parent 03a92fc commit e016932
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 33 deletions.
22 changes: 3 additions & 19 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ Secure-Signer is a remote signing tool for Ethereum PoS validators, with the fol
Validator keys are safeguarded in SGX's encrypted memory and the hardware enforces that Secure-Signer can only sign non-slashable messages. This reduces validator risk from slashing either from accidents or if their system is compromised.


> **SECURE SIGNER IS UNDER DEVELOPMENT AND SHOULD NOT BE USED FOR PRODUCTION**, unless you know what you are doing.
> **SECURE SIGNER IS UNDER DEVELOPMENT**, see [DockerHub](https://hub.docker.com/repository/docker/pufferfi/validator/general) for the latest enclave image.
---
## API
Expand All @@ -22,24 +22,8 @@ Validator keys are safeguarded in SGX's encrypted memory and the hardware enforc
## Developers
- [Developer Documentation](https://pufferfinance.github.io/secure-signer/developers/)

---

## Roadmap
- [x] Open source the alpha code
- [ ] Convert modules into their own open source crates
- [ ] Standardize the remote-signing specs
- [ ] Port to other LibOs's
- [ ] Support non-SGX TEEs

### TODO
- [ ] API endpoint to GET EIP-3076 SlashProtection database
- [ ] Code review and audit
- [ ] Support DCAP remote attestation

### Known Limitations / Issues
- Only one validator key can be imported per API call
- **footgun**: if you import an existing validator key, you expose yourself to slashing risk either via stale SlashProtection database or if you run the same key across multiple clients. We recommend [generating fresh keys within Secure-Signer](https://pufferfinance.github.io/secure-signer/running/client#generating-a-validator-key-in-secure-signer) to mitigate this.

## Protocols
- [Puffer Node Operator Documentation](https://docs.puffer.fi/nodes/requirements)

---

Expand Down
23 changes: 13 additions & 10 deletions src/bin/secure-signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,40 +27,43 @@ async fn main() {
genesis_fork_version,
};

let app = axum::Router::new()
// Endpoint to check health
.route(
"/upcheck",
axum::routing::get(puffersecuresigner::enclave::shared::handlers::health::handler),
)
let eth_v1 = axum::Router::new()
// Endpoint to securely generate and save an ETH sk
.route(
"/eth/v1/keygen/secp256k1",
"/keygen/secp256k1",
axum::routing::post(
puffersecuresigner::enclave::secure_signer::handlers::eth_keygen::handler,
),
)
// Endpoint to securely generate and save a BLS sk
.route(
"/eth/v1/keygen/bls",
"/keygen/bls",
axum::routing::post(
puffersecuresigner::enclave::secure_signer::handlers::bls_keygen::handler,
),
)
// Endpoint to list the pks of all the generated ETH keys
.route(
"/eth/v1/keygen/secp256k1",
"/keygen/secp256k1",
axum::routing::get(
puffersecuresigner::enclave::shared::handlers::list_eth_keys::handler,
),
)
// Endpoint to list all pks of saved bls keys in the enclave
.route(
"/eth/v1/keystores",
"/keystores",
axum::routing::get(
puffersecuresigner::enclave::shared::handlers::list_bls_keys::handler,
),
);

let app = axum::Router::new()
// Endpoint to check health
.route(
"/upcheck",
axum::routing::get(puffersecuresigner::enclave::shared::handlers::health::handler),
)
.nest("/eth/v1", eth_v1)
// Endpoint to sign DepositData message for registering validator on beacon chain
.route(
"/api/v1/eth2/deposit",
Expand Down
57 changes: 57 additions & 0 deletions src/client/tests/validator.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
use reqwest::StatusCode;

use crate::eth2::eth_types::{
BLSPubkey, Bytes32, DepositMessage, DepositRequest, GENESIS_FORK_VERSION,
};
use crate::strip_0x_prefix;

use crate::client::traits::ValidatorClientTrait;
Expand Down Expand Up @@ -145,3 +150,55 @@ async fn sign_voluntary_exit_message_with_success() {
.public_key()
.verify(&sig, root));
}

#[tokio::test]
async fn sign_message_deposit_message() {
let client = crate::client::ClientBuilder::new().build();
let payload = crate::enclave::types::AttestFreshBlsKeyPayload {
guardian_pubkeys: vec![
hex_to_pubkey("04fad76420abd33cfd92f51f31c47fe678922e476281b21aa8a738bcd56e37a776f678c94592d6aefd17af48b508feb1f27e82da4c0c46a253830e4d8637b3fbaf"),
hex_to_pubkey("0x04fad76420abd33cfd92f51f31c47fe678922e476281b21aa8a738bcd56e37a776f678c94592d6aefd17af48b508feb1f27e82da4c0c46a253830e4d8637b3fbaf"),
hex_to_pubkey("04fad76420abd33cfd92f51f31c47fe678922e476281b21aa8a738bcd56e37a776f678c94592d6aefd17af48b508feb1f27e82da4c0c46a253830e4d8637b3fbaf"),
hex_to_pubkey("0x04fad76420abd33cfd92f51f31c47fe678922e476281b21aa8a738bcd56e37a776f678c94592d6aefd17af48b508feb1f27e82da4c0c46a253830e4d8637b3fbaf")
],
withdrawal_credentials: [0; 32],
threshold: 3,
fork_version: crate::eth2::eth_types::GENESIS_FORK_VERSION,
do_remote_attestation: true,
};
let resp = client
.validator
.attest_fresh_bls_key(&payload)
.await
.unwrap();

let deposit_request = DepositRequest {
signingRoot: None,
deposit: DepositMessage {
pubkey: BLSPubkey::from(
hex::decode("04fad76420abd33cfd92f51f31c47fe678922e476281b21aa8a738bcd56e37a776f678c94592d6aefd17af48b508feb1f27e82da4c0c46a253830e4d8637b3fbaf")
.unwrap()
),
withdrawal_credentials: Bytes32::default(),
amount: 0,
},
genesis_fork_version: GENESIS_FORK_VERSION,
};
let req = crate::eth2::eth_signing::BLSSignMsg::DEPOSIT(deposit_request);

let resp = client
.validator
.client
.post(format!(
"{}/api/v1/eth2/sign/{}",
client.validator.url, resp.bls_pub_key
))
.json(&req)
.send()
.await
.unwrap();

let resp_status = resp.status();

assert_eq!(resp_status, StatusCode::BAD_REQUEST);
}
11 changes: 7 additions & 4 deletions src/enclave/shared/handlers/list_bls_keys_for_vc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,13 @@ pub async fn handler() -> axum::response::Response {
match key_management::list_bls_keys() {
Ok(list_res) => {
// safely prepend the response with "0x" to match the expected format
let list_res = list_res.iter().map(|x| {
let stripped: &str = strip_0x_prefix!(x);
format!("0x{}", stripped)
}).collect::<Vec<String>>();
let list_res = list_res
.iter()
.map(|x| {
let stripped: &str = strip_0x_prefix!(x);
format!("0x{}", stripped)
})
.collect::<Vec<String>>();
(axum::http::status::StatusCode::OK, Json(list_res)).into_response()
}
Err(e) => {
Expand Down
10 changes: 10 additions & 0 deletions src/enclave/shared/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ use axum::{
use log::{error, info};
use sha3::Digest;

use crate::eth2::eth_signing::BLSSignMsg;

/// Signs the specific type of request
/// Maintains compatibility with https://consensys.github.io/web3signer/web3signer-eth2.html#tag/Signing
pub fn sign_validator_message(
Expand All @@ -17,6 +19,14 @@ pub fn sign_validator_message(
) -> axum::response::Response {
info!("secure_sign_bls()");

if let BLSSignMsg::DEPOSIT(_) = req {
return (
axum::http::status::StatusCode::BAD_REQUEST,
format!("Signing deposit message not allowed"),
)
.into_response();
}

// Sanitize the input bls_pk_hex
let bls_pk_hex = match crate::crypto::bls_keys::sanitize_bls_pk_hex(&bls_pk_hex) {
Ok(pk) => pk,
Expand Down

0 comments on commit e016932

Please sign in to comment.