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

soroban-cli: uses the same identity/account/key for everybody by default #1079

Closed
leighmcculloch opened this issue Nov 13, 2023 · 6 comments · Fixed by #1086
Closed

soroban-cli: uses the same identity/account/key for everybody by default #1079

leighmcculloch opened this issue Nov 13, 2023 · 6 comments · Fixed by #1086
Assignees
Labels
cli Related to Soroban CLI security

Comments

@leighmcculloch
Copy link
Member

What version are you using?

$ soroban version
soroban 20.0.0-rc.4.2 (v20.0.0-rc.4.1-4-g4e9a48756b96e4e1a070105bad202f39901139ad-dirty)
soroban-env 20.0.0-rc2 (8c63bff68a15d79aca3a705ee6916a68db57b7e6)
soroban-env interface version 85899345977
stellar-xdr 20.0.0-rc1 (d5ce0c9e7aa83461773a6e81662067f35d39e4c1)
xdr curr (9ac02641139e6717924fdad716f6e958d0168491)

What did you do?

Used the default identity:

$ soroban config identity address

What did you expect to see?

A non-shared address. Maybe randomly generated on first use.

What did you see instead?

An address and key that everybody who is using the soroban-cli will use by default.

GDIY6AQQ75WMD4W46EYB7O6UYMHOCGQHLAQGQTKHDX4J2DYQCHVCR4W4

Why is this a security bug?

It's surprising that any key is shared.

Tooling that provides any shared key runs the risk of causing someone to think they have a unique key when they don't. Somebody may use the key on mainnet thinking it is a key unique to themselves.

Why is this a general bug?

Developers who use the default identity will be contending for access to the network. The network will only allow one tx per account in each ledger.

@leighmcculloch
Copy link
Member Author

Ideas for how we could address this issue:

  • Remove the default identity and require folks to generate identities.
  • Auto-generate the identity on first run and place the auto-generated identity in the global config.

Anyone have other ideas?

@willemneal
Copy link
Member

True, my thought was that it was useful for generating the same transactions for testing across CI and teams. But it's not that hard to generate a test account with the default seed. This was also when we thought people would use the sandbox.

I vote the first option. Better to be explicit.

@leighmcculloch
Copy link
Member Author

leighmcculloch commented Nov 13, 2023

For test predictability is still helpful so I think we'd still want to use some common key in tests.

@willemneal
Copy link
Member

Yeah that was what I was saying.

soroban config identity generate --default-seed

Is equivalent to

soroban config identity generate --seed 0000000000000000

Then you can use --hd-path for generating multiple accounts.

@mollykarcher mollykarcher moved this from Backlog to Next Sprint Proposal in Platform Scrum Nov 13, 2023
@mollykarcher mollykarcher moved this from Next Sprint Proposal to Current Sprint in Platform Scrum Nov 15, 2023
@leighmcculloch
Copy link
Member Author

I don't think we've explicitly said here, but I believe we have agreed to remove the default identity.

cc @mollykarcher if you remember otherwise.

@mollykarcher
Copy link
Contributor

Yes, that's my understanding as well

@mollykarcher mollykarcher added the cli Related to Soroban CLI label Dec 14, 2023
@mollykarcher mollykarcher moved this from Current Sprint to Needs Review in Platform Scrum Dec 14, 2023
@mollykarcher mollykarcher moved this from Needs Review to In Progress in Platform Scrum Jan 4, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in Platform Scrum Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Related to Soroban CLI security
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants