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

Disallow PubKey as PrivKey for cmd_set_default_account command - display error if user accidentally pass wrong key. #1645

Merged
merged 2 commits into from
Nov 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion simulator/src/resim/cmd_new_account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ impl NewAccount {
|| configs.default_owner_badge.is_none()
{
configs.default_account = Some(account);
configs.default_private_key = Some(hex::encode(private_key.to_bytes()));
configs.default_private_key = Some(private_key.to_hex());
configs.default_owner_badge = Some(owner_badge);
set_configs(&configs)?;

Expand Down
37 changes: 36 additions & 1 deletion simulator/src/resim/cmd_set_default_account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,47 @@ pub struct SetDefaultAccount {
impl SetDefaultAccount {
pub fn run<O: std::io::Write>(&self, out: &mut O) -> Result<(), Error> {
let mut configs = get_configs()?;
let private_key = parse_private_key_from_str(&self.private_key).map_err(|e| {
if Secp256k1PublicKey::from_str(&self.private_key).is_ok() {
Error::GotPublicKeyExpectedPrivateKey
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we fail to create a PrivateKey (32 bytes/64 hex char input) from the passed in string, we try to parse it as a PublicKey to see if the user made the very reasonable error of accidentally copy pasting the public key - like I did!

} else {
e
}
})?;
configs.default_account = Some(self.component_address.0);
configs.default_private_key = Some(self.private_key.clone());
configs.default_private_key = Some(private_key.to_hex());
configs.default_owner_badge = Some(self.owner_badge.clone().0);
set_configs(&configs)?;

writeln!(out, "Default account updated!").map_err(Error::IOError)?;
Ok(())
}
}

#[cfg(test)]
#[test]
fn test_validation() {
let mut out = std::io::stdout();
let private_key = Secp256k1PrivateKey::from_hex(
"6847c11e2d602548dbf38789e0a1f4543c1e7719e4f591d4aa6e5684f5c13d9c",
)
.unwrap();
let public_key = private_key.public_key().to_string();

let make_cmd = |key_string: String| {
return SetDefaultAccount {
component_address: SimulatorComponentAddress::from_str(
"account_sim1c9yeaya6pehau0fn7vgavuggeev64gahsh05dauae2uu25njk224xz",
)
.unwrap(),
private_key: key_string,
owner_badge: SimulatorNonFungibleGlobalId::from_str(
"resource_sim1ngvrads4uj3rgq2v9s78fzhvry05dw95wzf3p9r8skhqusf44dlvmr:#1#",
)
.unwrap(),
};
};

assert!(make_cmd(private_key.to_hex()).run(&mut out).is_ok());
assert!(make_cmd(public_key.to_string()).run(&mut out).is_err());
}
2 changes: 1 addition & 1 deletion simulator/src/resim/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ pub fn get_default_account() -> Result<ComponentAddress, Error> {
pub fn get_default_private_key() -> Result<Secp256k1PrivateKey, Error> {
get_configs()?
.default_private_key
.map(|v| Secp256k1PrivateKey::from_bytes(&hex::decode(&v).unwrap()).unwrap())
.map(|v| Secp256k1PrivateKey::from_hex(&v).unwrap())
.ok_or(Error::NoDefaultPrivateKey)
}

Expand Down
3 changes: 3 additions & 0 deletions simulator/src/resim/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ pub enum Error {

InvalidPrivateKey,

/// e.g. if you accidentally pass in a public key in `set_default_account` command.
GotPublicKeyExpectedPrivateKey,

NonFungibleGlobalIdError(ParseNonFungibleGlobalIdError),

FailedToBuildArguments(BuildCallArgumentError),
Expand Down
21 changes: 12 additions & 9 deletions simulator/src/resim/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -318,19 +318,22 @@ pub fn process_receipt(receipt: TransactionReceipt) -> Result<TransactionReceipt
}
}

pub fn parse_private_key_from_bytes(slice: &[u8]) -> Result<Secp256k1PrivateKey, Error> {
Secp256k1PrivateKey::from_bytes(slice).map_err(|_| Error::InvalidPrivateKey)
}

pub fn parse_private_key_from_str(key: &str) -> Result<Secp256k1PrivateKey, Error> {
hex::decode(key)
.map_err(|_| Error::InvalidPrivateKey)
.and_then(|bytes| parse_private_key_from_bytes(&bytes))
}

pub fn get_signing_keys(signing_keys: &Option<String>) -> Result<Vec<Secp256k1PrivateKey>, Error> {
let private_keys = if let Some(keys) = signing_keys {
keys.split(",")
.map(str::trim)
.filter(|s| !s.is_empty())
.map(|key| {
hex::decode(key)
.map_err(|_| Error::InvalidPrivateKey)
.and_then(|bytes| {
Secp256k1PrivateKey::from_bytes(&bytes)
.map_err(|_| Error::InvalidPrivateKey)
})
})
.filter(|s: &&str| !s.is_empty())
.map(parse_private_key_from_str)
.collect::<Result<Vec<Secp256k1PrivateKey>, Error>>()?
} else {
vec![get_default_private_key()?]
Expand Down
10 changes: 10 additions & 0 deletions transaction/src/signing/secp256k1/private_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,16 @@ impl Secp256k1PrivateKey {
self.0.secret_bytes().to_vec()
}

pub fn to_hex(&self) -> String {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We convert between key and hex in a few places, makes sense for the type type to have these methods IMO.

hex::encode(self.to_bytes())
}

pub fn from_hex(s: &str) -> Result<Self, ()> {
hex::decode(s)
.map_err(|_| ())
.and_then(|v| Self::from_bytes(&v))
}

pub fn from_bytes(slice: &[u8]) -> Result<Self, ()> {
if slice.len() != Secp256k1PrivateKey::LENGTH {
return Err(());
Expand Down
Loading