cmd: harden the security of privkey field in config show #3141
Conversation
Kubuxu
force-pushed
the
feat/cmd/config-priv
branch
from
54902d7
to
481f9b2
Compare
Kubuxu
added
need/review
|
@Kubuxu I would do this more like: https://gist.github.com/whyrusleeping/ec7e22a5f331bf9f1309f38aa366a343 Just to be a bit more generic about scrubbing this, and making sure we don't have issues in other cases (such as "Identify" having its case changed) |
| return | ||
| } | ||
|
|
||
| delete(idmap, "PrivKey") | ||
| privKeyKey := "" // make sure we both find the name of privkey and we delete it | ||
| for key, _ := range idmap { |
There was a problem hiding this comment.
- and
idmapwill be nil ifcfg["Identity"]doesnt work and there iscfg["identity"]instead. - As we discussed, this is turning out to be a mess. easier to move the key out.
There was a problem hiding this comment.
If it is nil then command fails, which fails the sharness tests.
Kubuxu
force-pushed
the
feat/cmd/config-priv
branch
from
3d57397
to
481f9b2
Compare
|
@Kubuxu wanna review this when you get a chance? |
|
LGTM, we might want to add test time assertions using reflection or selectors on default config that those selectors are up to date. And two of my commits need to be squshed. |
License: MIT Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
License: MIT Signed-off-by: Jeromy <why@ipfs.io>
License: MIT Signed-off-by: Jeromy <why@ipfs.io>
whyrusleeping
force-pushed
the
feat/cmd/config-priv
branch
from
1064c1e
to
667f8a6
Compare
|
Alright, this looks good to me. 🚢 🚄 🚅 🚜 🚆 |
| for _, k := range key[:len(key)-1] { | ||
| foundk, val, ok := find(cur, k) | ||
| if !ok { | ||
| return fmt.Errorf("failed to find specified key") |
There was a problem hiding this comment.
wont this error out all the way to the user, if the user provides a config without an Identity.PrivKey value?
also add test for lower case config replace.