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

ed25519::Pair::from_standard_components returns unexpected Err when derivation path contains a "Soft" junction #3294

Open
bddap opened this issue Aug 2, 2019 · 3 comments

Comments

@bddap
Copy link
Contributor

@bddap bddap commented Aug 2, 2019

When a the derive path passed to ed25519::Pair::from_standard_components contains a soft junction, the function returns an error. The function does not document the error case, leaving the user to discover it at runtime.

The behavior is different from sr25519::Pair::from_standard_components, which does allow Soft junctions.

Suggested fix: Either change ed25519::Pair to allow soft junctions (like sr25519::Pair does), or document the error case as recommended in the excellent rust api guidelines.

@kianenigma

This comment has been minimized.

Copy link
Contributor

@kianenigma kianenigma commented Aug 2, 2019

@burdges

This comment has been minimized.

Copy link

@burdges burdges commented Aug 4, 2019

Just fyi, there are several poor designs for soft derivations on Ed25519 around:
https://forum.web3.foundation/t/key-recovery-attack-on-bip32-ed25519/44

I implemented a reasonable design in https://github.com/w3f/hd-ed25519 but never really polished it. And Tor has another good design. I'd prefer if we do not rush the hd-ed25519 crate into production but if we actually need it anywhere then we can clean it up

@jacogr might know if we should do this sooner or if it can wait.

@jacogr

This comment has been minimized.

Copy link
Contributor

@jacogr jacogr commented Aug 4, 2019

Soft paths are not implemented for ed25519 - specifically for the reasons mentioned above, i.e. not to rush a half-baked version. It is intentional that the code returns an error here (unlike sr25519), i.e.

DeriveJunction::Soft(_cc) => return Err(DeriveError::SoftKeyInPath),

As it stands, it is up to the callers (i.e. be it subkey or a wallet or a key generator), to not allow the soft paths in ed25519, while allowing hard. If a solid solution becomes available, we should adapt, but it is not something that probably should be rushed into production tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.