core/commands/config: do not show private key on local network #2957
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -58,6 +58,14 @@ Set the value of the 'datastore.path' key: | |||
| args := req.Arguments() | |||
| key := args[0] | |||
|
|
|||
| // This is a temporary fix until we move the private key out of the config file | |||
| switch key { | |||
| case "Identity", "Identity.PrivKey": | |||
There was a problem hiding this comment.
- may be lowercased.
- can just deny all of identity
if strings.HasPrefix(strings.ToLower(key), "identity") {- addressed
There was a problem hiding this comment.
@jbenet you cant look it up by the lowercase key, but we can add that to the permissions check.
There was a problem hiding this comment.
ipfs config Identity.PeerID is a valid usecase.
License: MIT Signed-off-by: Jakub Sztandera <[email protected]>
and replacing config. License: MIT Signed-off-by: Jakub Sztandera <[email protected]>
License: MIT Signed-off-by: Jakub Sztandera <[email protected]>
License: MIT Signed-off-by: Jakub Sztandera <[email protected]>
License: MIT Signed-off-by: Jakub Sztandera <[email protected]>
License: MIT Signed-off-by: Jakub Sztandera <[email protected]>
|
@lgierth if you could review and give a LGTM here that would be great |
| err := r.SetConfigKey(key, value) | ||
| keyF, err := getConfig(r, "Identity.PrivKey") | ||
| if err != nil { | ||
| return nil, fmt.Errorf("Failed to get PrivKey") |
License: MIT Signed-off-by: Jakub Sztandera <[email protected]>
License: MIT Signed-off-by: Jeromy <[email protected]>
License: MIT Signed-off-by: Jeromy <[email protected]>
| return | ||
| } | ||
|
|
||
| delete(idmap, "PrivKey") |
There was a problem hiding this comment.
- this wont work if the map has
privkey. this should be case insensitive. - i know this comes from our own structs, so it should be capitalized.
- but all someone has to do is change the capitalization to
Privkeythere. - come to think of it, this should be either:
- a constant there, in that file where the struct is
- or derived from the struct using reflection, to be very damn sure that doesn't get changed around these checks.
There was a problem hiding this comment.
I can also loop the map, compare keys to privkey case insensitive and remove that. SGTY?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
cc @lgierth @whyrusleeping @jbenet