Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

Keychain and 'self' key #1138

Closed
3 of 5 tasks
richardschneider opened this issue Dec 9, 2017 · 8 comments
Closed
3 of 5 tasks

Keychain and 'self' key #1138

richardschneider opened this issue Dec 9, 2017 · 8 comments
Assignees
Labels
exp/wizard Extensive knowledge (implications, ramifications) required kind/wontfix-migration-available P1 High: Likely tackled by core team if no one steps up status/ready Ready to be worked

Comments

@richardschneider
Copy link
Contributor

richardschneider commented Dec 9, 2017

The self key is the local peer's identity, commonly referred to as PrivKey. This key is currently stored in .config and is naked. It should be placed onto the keychain, see ipfs/kubo#4240 (comment).

  • ipfs init requires --pass to create the keychain
  • ipfs init should store peer identity as self in the keychain
  • Gracefully upgrade, at "boot/startup" ipfs should move the PrivKey in .config into the keychain
  • ipfs init should not store PrivKey in .config
  • any access to config PrivKey should error
@richardschneider
Copy link
Contributor Author

richardschneider commented Dec 9, 2017

Here'a quick ipfs search and libp2p search for PrivKey.

richardschneider added a commit that referenced this issue Dec 22, 2017
richardschneider added a commit that referenced this issue Dec 24, 2017
Older repos are automatically upgraded to a keychain when the '--pass' is present.

Helps #1138
@daviddias daviddias added the status/ready Ready to be worked label Jan 25, 2018
@daviddias
Copy link
Member

@richardschneider anything I can do to empower you to finish this work?

@daviddias daviddias added exp/wizard Extensive knowledge (implications, ramifications) required P1 High: Likely tackled by core team if no one steps up labels Jan 26, 2018
richardschneider added a commit that referenced this issue Jan 27, 2018
richardschneider added a commit that referenced this issue Jan 27, 2018
Older repos are automatically upgraded to a keychain when the '--pass' is present.

Helps #1138
@richardschneider
Copy link
Contributor Author

@diasdavid RE: removing PrivKey from .config

This will introduce a major breaking change. The --pass option will be required, otherwise the peer's private key cannot be fetched from the keychain.

@daviddias
Copy link
Member

@richardschneider doesn't have to need a breaking change. The node can check if there is a key on the config and move it to the keychain on first run.

The --pass option will be required

Why? That should be just if we want to store the private key encrypted. "" should be a valid passphrase for the users that want to store it in plaintext

@richardschneider
Copy link
Contributor Author

doesn't have to need a breaking change. The node can check if there is a key on the config and move it to the keychain on first run.

This is what is does now!

Why? That should be just if we want to store the private key encrypted. "" should be a valid passphrase for the users that want to store it in plaintext

This is all about SECURITY. Keys should never be in plain text. NIST SP 800-132 requires a minimum of 20 chars for a passphrase to a key.

Basically, I want to mandate that the user knows the passphrase. Otherwise, the private key is not private (as is the current case with it being stored in .config) and there is no security.

This is the same approach that @ianopolous took in peergos.

@daviddias
Copy link
Member

daviddias commented Feb 6, 2018

@richardschneider I believe we are having a communication issue. First, my suggestion for a migration of the key from the config.json to keychain was based on your comment above. If that is already implemented, great. Let's try to communicate all the features and notes in advance so that there are no misunderstandings.

Second, yes I do understand the security concerns of storing a key in plaintext. However, the ipfs daemon has historically enabled users to do so and changing that behaviour would be breaking the interface. We need to enable both storage of plaintext + encrypted and then provide security recommendations for our users. If we decide as a project to stop supporting plaintext keys, then we will make sure that users have a good transition process.

So here is the path to move forward:

  • Support both plaintext and encrypted storage of the keys
  • Open an issue on IPFS Specs proposing that we should drop plaintext storage
  • Work on getting documentation ready for that transition

@richardschneider
Copy link
Contributor Author

richardschneider commented Aug 1, 2018

@fiatjaf commented

Why can't the private keys be stored as plaintext and within reach from the local user without this password-PBKDF2-whatever stuff? go-ipfs does that.

The keychain was designed to protect private keys by encrypting them with a pass phrase, hence the password-PBKDF2 stuff.

As @diasdavid points out, perhaps we should use "" as the pass phrase when not specified by the user.

@whizzzkid
Copy link

js-ipfs is being deprecated in favor of Helia. You can #4336 and read the migration guide.

Please feel to reopen with any comments before 2023-06-05. We will do a final pass on reopened issues afterward (see #4336).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
exp/wizard Extensive knowledge (implications, ramifications) required kind/wontfix-migration-available P1 High: Likely tackled by core team if no one steps up status/ready Ready to be worked
Projects
No open projects
Status: Done
Development

No branches or pull requests

3 participants