-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Fix config set private key protection #3147
Conversation
License: MIT Signed-off-by: Jeromy <why@ipfs.io>
@whyrusleeping -- @lgierth and i were discussing that it's likely much easier to solve this problem by just moving the key out of the config entirely for good, and at this point we keep sinking energy into making the hack work when there's likely all these complicated edge cases to cover. We could move it out in a 4-5 migration, put it in a file in |
// Load private key to guard against it being overwritten. | ||
// NOTE: this is a temporary measure to secure this field until we move | ||
// keys out of the config file. | ||
pkval, err := common.MapGetKV(mapconf, "Identity.PrivKey") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- What if the name of the fields in the structs changes and people forget to change it here? should be a const in the relevant
config.go
file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would merge a PR that cleanly moves the private key out of the config if
paired with a migration. I do not have time to work on that.
On Mon, Aug 29, 2016, 16:41 Juan Benet notifications@github.com wrote:
In repo/fsrepo/fsrepo.go
#3147 (comment):@@ -482,6 +482,14 @@ func (r *FSRepo) SetConfigKey(key string, value interface{}) error {
return err
}
- // Load private key to guard against it being overwritten.
- // NOTE: this is a temporary measure to secure this field until we move
- // keys out of the config file.
- pkval, err := common.MapGetKV(mapconf, "Identity.PrivKey")
- What if the name of the fields in the structs changes and people
forget to change it here? should be a const.—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/ipfs/go-ipfs/pull/3147/files/d290d567fc5ced87bb6dd7634d80b697272d42ff#r76708163,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABL4HGKfwgzdeG15CckDMgcf7McbFFyFks5qk24TgaJpZM4Jv9id
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jbenet as we will be moving the key out of config in the nearest future, can we accept current hack (it is hack) and release keymigrations in 0.4.4 or 0.5.
This isn't long term strategy, it is a way to keep things running for till next release.
closing, moved the changes here to #3141 |
Should address https://github.com/ipfs/go-ipfs/pull/2957/files#r76539324