Skip to content
This repository has been archived by the owner on Jun 15, 2023. It is now read-only.

feat: allow keychain interactions without a password #25

Closed
wants to merge 1 commit into from

Conversation

achingbrain
Copy link
Member

@achingbrain achingbrain commented Sep 7, 2018

Possibly contentious PR, but go-ipfs does not require a password to list or generate keys but js-ipfs does. This prevents us from having interop as the behaviour is different.

This PR sets a silly default password to enable this but I'm not 100% convinced any of this is a good idea.

Needed for the tests in ipfs/js-ipfs#1548 and ipfs/interop#35 to pass.

@vasco-santos
Copy link
Member

vasco-santos commented Sep 8, 2018

In my honest opinion, keychain interactions should be protected by a password, as we have in JS land. However, afaik there are no specs for this.

But since we have a different approach from the GO team, we should discuss together and decide if JS should abdicate from this feature, or if they should add this feature on their side.

What do you think?
@libp2p/javascript-team
@libp2p/go-team

@achingbrain
Copy link
Member Author

My feeling is that requiring the password for all interactions is a bit heavy handed - the password is not required by the code to list keys in the keychain for example.

@bigs
Copy link

bigs commented Sep 10, 2018

perhaps a flag to allow a user to set a password, similar to how SSH prompts for a password but doesn't require it?

@achingbrain achingbrain changed the title chore: allow keychain interactions without a password feat: allow keychain interactions without a password Sep 10, 2018
@@ -25,7 +25,8 @@ const defaultOptions = {
iterationCount: 10000,
salt: 'you should override this value with a crypto secure random number',
hash: 'sha2-512'
}
},
passPhrase: 'correcthorsebatterystaple'
Copy link
Member

Choose a reason for hiding this comment

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

What if the default password is just empty string?

Copy link
Member Author

Choose a reason for hiding this comment

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

@achingbrain
Copy link
Member Author

Closing because this is very old and probably not a good idea.

@achingbrain achingbrain deleted the do-not-require-passphrase branch January 27, 2023 17:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants