Skip to content

Commit

Permalink
Remove forbidden key check (#1236)
Browse files Browse the repository at this point in the history
* Rm forbidden key check

* Rm forbidden key error variants
  • Loading branch information
ameba23 authored Jan 7, 2025
1 parent da8f3f8 commit 458a89d
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 64 deletions.
51 changes: 16 additions & 35 deletions crates/threshold-signature-server/src/helpers/launch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ use entropy_kvdb::{
encrypted_sled::PasswordMethod,
kv_manager::{error::KvError, KvManager},
};
use entropy_shared::NETWORK_PARENT_KEY;
use serde::Deserialize;
use serde_json::json;
use subxt::ext::sp_core::{
Expand Down Expand Up @@ -53,16 +52,9 @@ pub const LATEST_BLOCK_NUMBER_PROACTIVE_REFRESH: &str = "LATEST_BLOCK_NUMBER_PRO
#[cfg(any(test, feature = "test_helpers"))]
pub const DEFAULT_ENDPOINT: &str = "ws://localhost:9944";

pub const FORBIDDEN_KEYS: [&str; 4] = [
FORBIDDEN_KEY_MNEMONIC,
FORBIDDEN_KEY_SHARED_SECRET,
FORBIDDEN_KEY_DIFFIE_HELLMAN_PUBLIC,
NETWORK_PARENT_KEY,
];

pub const FORBIDDEN_KEY_MNEMONIC: &str = "MNEMONIC";
pub const FORBIDDEN_KEY_SHARED_SECRET: &str = "SHARED_SECRET";
pub const FORBIDDEN_KEY_DIFFIE_HELLMAN_PUBLIC: &str = "DH_PUBLIC";
pub const KEY_MNEMONIC: &str = "MNEMONIC";
pub const KEY_SHARED_SECRET: &str = "SHARED_SECRET";
pub const KEY_DIFFIE_HELLMAN_PUBLIC: &str = "DH_PUBLIC";

// Deafult name for TSS server
// Will set mnemonic and db path
Expand Down Expand Up @@ -210,7 +202,7 @@ pub struct StartupArgs {
}

pub async fn has_mnemonic(kv: &KvManager) -> bool {
let exists = kv.kv().exists(FORBIDDEN_KEY_MNEMONIC).await.expect("issue querying DB");
let exists = kv.kv().exists(KEY_MNEMONIC).await.expect("issue querying DB");

if exists {
tracing::debug!("Existing mnemonic found in keystore.");
Expand Down Expand Up @@ -240,28 +232,19 @@ pub async fn setup_mnemonic(kv: &KvManager, mnemonic: bip39::Mnemonic) {
if has_mnemonic(kv).await {
tracing::warn!("Deleting account related keys from KVDB.");

kv.kv().delete(KEY_MNEMONIC).await.expect("Error deleting existing mnemonic from KVDB.");
kv.kv().delete(KEY_SHARED_SECRET).await.expect("Error deleting shared secret from KVDB.");
kv.kv()
.delete(FORBIDDEN_KEY_MNEMONIC)
.await
.expect("Error deleting existing mnemonic from KVDB.");
kv.kv()
.delete(FORBIDDEN_KEY_SHARED_SECRET)
.await
.expect("Error deleting shared secret from KVDB.");
kv.kv()
.delete(FORBIDDEN_KEY_DIFFIE_HELLMAN_PUBLIC)
.delete(KEY_DIFFIE_HELLMAN_PUBLIC)
.await
.expect("Error deleting X25519 public key from KVDB.");
}

tracing::info!("Writing new mnemonic to KVDB.");

// Write our new mnemonic to the KVDB.
let reservation = kv
.kv()
.reserve_key(FORBIDDEN_KEY_MNEMONIC.to_string())
.await
.expect("Issue reserving mnemonic");
let reservation =
kv.kv().reserve_key(KEY_MNEMONIC.to_string()).await.expect("Issue reserving mnemonic");
kv.kv()
.put(reservation, mnemonic.to_string().as_bytes().to_vec())
.await
Expand All @@ -272,11 +255,8 @@ pub async fn setup_mnemonic(kv: &KvManager, mnemonic: bip39::Mnemonic) {
let x25519_public_key = x25519_dalek::PublicKey::from(&static_secret).to_bytes();

// Write the shared secret in the KVDB
let shared_secret_reservation = kv
.kv()
.reserve_key(FORBIDDEN_KEY_SHARED_SECRET.to_string())
.await
.expect("Issue reserving ss key");
let shared_secret_reservation =
kv.kv().reserve_key(KEY_SHARED_SECRET.to_string()).await.expect("Issue reserving ss key");
kv.kv()
.put(shared_secret_reservation, static_secret.to_bytes().to_vec())
.await
Expand All @@ -285,7 +265,7 @@ pub async fn setup_mnemonic(kv: &KvManager, mnemonic: bip39::Mnemonic) {
// Write the Diffie-Hellman key in the KVDB
let diffie_hellman_reservation = kv
.kv()
.reserve_key(FORBIDDEN_KEY_DIFFIE_HELLMAN_PUBLIC.to_string())
.reserve_key(KEY_DIFFIE_HELLMAN_PUBLIC.to_string())
.await
.expect("Issue reserving DH key");

Expand All @@ -305,7 +285,7 @@ pub async fn setup_mnemonic(kv: &KvManager, mnemonic: bip39::Mnemonic) {
}

pub async fn threshold_account_id(kv: &KvManager) -> String {
let mnemonic = kv.kv().get(FORBIDDEN_KEY_MNEMONIC).await.expect("Issue getting mnemonic");
let mnemonic = kv.kv().get(KEY_MNEMONIC).await.expect("Issue getting mnemonic");
let pair = <sr25519::Pair as Pair>::from_phrase(
&String::from_utf8(mnemonic).expect("Issue converting mnemonic to string"),
None,
Expand Down Expand Up @@ -371,15 +351,16 @@ pub async fn setup_latest_block_number(kv: &KvManager) -> Result<(), KvError> {
}

pub async fn setup_only(kv: &KvManager) {
let mnemonic = kv.kv().get(FORBIDDEN_KEYS[0]).await.expect("Issue getting mnemonic");
let mnemonic = kv.kv().get(KEY_MNEMONIC).await.expect("Issue getting mnemonic");
let pair = <sr25519::Pair as Pair>::from_phrase(
&String::from_utf8(mnemonic).expect("Issue converting mnemonic to string"),
None,
)
.expect("Issue converting mnemonic to pair");
let account_id = AccountId32::new(pair.0.public().into()).to_ss58check();

let dh_public_key = kv.kv().get(FORBIDDEN_KEYS[2]).await.expect("Issue getting dh public key");
let dh_public_key =
kv.kv().get(KEY_DIFFIE_HELLMAN_PUBLIC).await.expect("Issue getting dh public key");
let dh_public_key = format!("{dh_public_key:?}").replace('"', "");
let output = json!({
"account_id": account_id,
Expand Down
2 changes: 0 additions & 2 deletions crates/threshold-signature-server/src/user/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,6 @@ pub enum UserErr {
ChainFetch(&'static str),
#[error("Too many requests - wait a block")]
TooManyRequests,
#[error("The given key is forbidden")]
ForbiddenKey,
#[error("No existing keyshare found for this user")]
UserDoesNotExist,
#[error("The remote TSS server rejected the keyshare: {0}")]
Expand Down
10 changes: 1 addition & 9 deletions crates/threshold-signature-server/src/validator/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use crate::{
},
get_signer_and_x25519_secret,
helpers::{
launch::{FORBIDDEN_KEYS, LATEST_BLOCK_NUMBER_RESHARE},
launch::LATEST_BLOCK_NUMBER_RESHARE,
substrate::{get_stash_address, get_validators_info, query_chain, submit_transaction},
},
signing_client::{api::get_channels, ProtocolErr},
Expand Down Expand Up @@ -375,14 +375,6 @@ pub async fn check_balance_for_fees(
Ok(is_min_balance)
}

pub fn check_forbidden_key(key: &str) -> Result<(), ValidatorErr> {
let forbidden = FORBIDDEN_KEYS.contains(&key);
if forbidden {
return Err(ValidatorErr::ForbiddenKey);
}
Ok(())
}

/// Filters out new signer from next signers to get old holders
pub async fn prune_old_holders(
api: &OnlineClient<EntropyConfig>,
Expand Down
2 changes: 0 additions & 2 deletions crates/threshold-signature-server/src/validator/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@ pub enum ValidatorErr {
Kv(#[from] entropy_kvdb::kv_manager::error::KvError),
#[error("User Error: {0}")]
UserErr(#[from] crate::user::UserErr),
#[error("Forbidden Key")]
ForbiddenKey,
#[error("Validation Error: {0}")]
ValidationErr(#[from] crate::validation::errors::ValidationErr),
#[error("anyhow error: {0}")]
Expand Down
19 changes: 3 additions & 16 deletions crates/threshold-signature-server/src/validator/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,16 @@
//
// You should have received a copy of the GNU Affero General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.
use super::api::{check_balance_for_fees, check_forbidden_key};
use super::api::check_balance_for_fees;
use crate::{
helpers::{
launch::{FORBIDDEN_KEYS, LATEST_BLOCK_NUMBER_RESHARE},
launch::LATEST_BLOCK_NUMBER_RESHARE,
tests::{
call_set_storage, get_port, initialize_test_logger, run_to_block, setup_client,
spawn_testing_validators, unsafe_get,
},
},
validator::{
api::{is_signer_or_delete_parent_key, prune_old_holders, validate_new_reshare},
errors::ValidatorErr,
},
validator::api::{is_signer_or_delete_parent_key, prune_old_holders, validate_new_reshare},
};
use entropy_client::{self as test_client};
use entropy_client::{
Expand Down Expand Up @@ -387,16 +384,6 @@ async fn test_check_balance_for_fees() {
.unwrap();
}

#[tokio::test]
async fn test_forbidden_keys() {
initialize_test_logger().await;
let should_fail = check_forbidden_key(FORBIDDEN_KEYS[0]);
assert_eq!(should_fail.unwrap_err().to_string(), ValidatorErr::ForbiddenKey.to_string());

let should_pass = check_forbidden_key("test");
assert_eq!(should_pass.unwrap(), ());
}

#[tokio::test]
#[serial]
async fn test_deletes_key() {
Expand Down

0 comments on commit 458a89d

Please sign in to comment.