Merge pull request #3141 from ipfs/feat/cmd/config-priv

cmd: harden the security of privkey field in config show

core/commands/config.go

@@ -15,6 +15,7 @@ import (
 	repo "github.com/ipfs/go-ipfs/repo"
 	config "github.com/ipfs/go-ipfs/repo/config"
 	fsrepo "github.com/ipfs/go-ipfs/repo/fsrepo"
+
 	u "gx/ipfs/QmZNVWh8LLjAavuQ2JXuFmuYH3C11xo988vSgp7UQrTRj1/go-ipfs-util"
 )
 
@@ -162,14 +163,12 @@ included in the output of this command.
 			return
 		}
 
-		idmap, ok := cfg["Identity"].(map[string]interface{})
-		if !ok {
-			res.SetError(fmt.Errorf("config has no identity"), cmds.ErrNormal)
+		err = scrubValue(cfg, []string{config.IdentityTag, config.PrivKeyTag})
+		if err != nil {
+			res.SetError(err, cmds.ErrNormal)
 			return
 		}
 
-		delete(idmap, "PrivKey")
-
 		output, err := config.HumanOutput(cfg)
 		if err != nil {
 			res.SetError(err, cmds.ErrNormal)
@@ -180,6 +179,47 @@ included in the output of this command.
 	},
 }
 
+func scrubValue(m map[string]interface{}, key []string) error {
+	find := func(m map[string]interface{}, k string) (string, interface{}, bool) {
+		lckey := strings.ToLower(k)
+		for mkey, val := range m {
+			lcmkey := strings.ToLower(mkey)
+			if lckey == lcmkey {
+				return mkey, val, true
+			}
+		}
+		return "", nil, false
+	}
+
+	cur := m
+	for _, k := range key[:len(key)-1] {
+		foundk, val, ok := find(cur, k)
+		if !ok {
+			return fmt.Errorf("failed to find specified key")
+		}
+
+		if foundk != k {
+			// case mismatch, calling this an error
+			return fmt.Errorf("case mismatch in config, expected %q but got %q", k, foundk)
+		}
+
+		mval, mok := val.(map[string]interface{})
+		if !mok {
+			return fmt.Errorf("%s was not a map", foundk)
+		}
+
+		cur = mval
+	}
+
+	todel, _, ok := find(cur, key[len(key)-1])
+	if !ok {
+		return fmt.Errorf("%s, not found", strings.Join(key, "."))
+	}
+
+	delete(cur, todel)
+	return nil
+}
+
 var configEditCmd = &cmds.Command{
 	Helptext: cmds.HelpText{
 		Tagline: "Opens the config file for editing in $EDITOR.",
@@ -250,19 +290,10 @@ func getConfig(r repo.Repo, key string) (*ConfigField, error) {
 }
 
 func setConfig(r repo.Repo, key string, value interface{}) (*ConfigField, error) {
-	keyF, err := getConfig(r, "Identity.PrivKey")
-	if err != nil {
-		return nil, errors.New("failed to get PrivKey")
-	}
-	privkey := keyF.Value
-	err = r.SetConfigKey(key, value)
+	err := r.SetConfigKey(key, value)
 	if err != nil {
 		return nil, fmt.Errorf("failed to set config value: %s (maybe use --json?)", err)
 	}
-	err = r.SetConfigKey("Identity.PrivKey", privkey)
-	if err != nil {
-		return nil, errors.New("failed to set PrivKey")
-	}
 	return getConfig(r, key)
 }
 
@@ -286,7 +317,7 @@ func replaceConfig(r repo.Repo, file io.Reader) error {
 		return errors.New("setting private key with API is not supported")
 	}
 
-	keyF, err := getConfig(r, "Identity.PrivKey")
+	keyF, err := getConfig(r, config.PrivKeySelector)
 	if err != nil {
 		return fmt.Errorf("Failed to get PrivKey")
 	}

repo/config/identity.go

@@ -5,6 +5,10 @@ import (
 	ic "gx/ipfs/QmUWER4r4qMvaCnX5zREcfyiWN7cXN9g3a7fkRqNz8qWPP/go-libp2p-crypto"
 )
 
+const IdentityTag = "Identity"
+const PrivKeyTag = "PrivKey"
+const PrivKeySelector = IdentityTag + "." + PrivKeyTag
+
 // Identity tracks the configuration of the local node's identity.
 type Identity struct {
 	PeerID  string

repo/fsrepo/fsrepo.go

@@ -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, config.PrivKeySelector)
+	if err != nil {
+		return err
+	}
+
 	// Get the type of the value associated with the key
 	oldValue, err := common.MapGetKV(mapconf, key)
 	ok := true
@@ -523,6 +531,11 @@ func (r *FSRepo) SetConfigKey(key string, value interface{}) error {
 		return err
 	}
 
+	// replace private key, in case it was overwritten.
+	if err := common.MapSetKV(mapconf, config.PrivKeySelector, pkval); err != nil {
+		return err
+	}
+
 	// This step doubles as to validate the map against the struct
 	// before serialization
 	conf, err := config.FromMap(mapconf)

test/sharness/t0021-config.sh

@@ -111,10 +111,10 @@ test_config_cmd() {
 
   test_expect_success "'ipfs config replace' injects privkey back" '
        ipfs config replace show_config &&
-	   grep "\"PrivKey\":" "$IPFS_PATH/config" | grep -e ": \".\+\"" >/dev/null
+       grep "\"PrivKey\":" "$IPFS_PATH/config" | grep -e ": \".\+\"" >/dev/null
   '
 
-  test_expect_success "'ipfs config replace' with privkey erors out" '
+  test_expect_success "'ipfs config replace' with privkey errors out" '
        cp "$IPFS_PATH/config" real_config &&
        test_expect_code 1 ipfs config replace - < real_config 2> replace_out
   '
@@ -124,6 +124,16 @@ test_config_cmd() {
        test_cmp replace_out replace_expected
   '
 
+  test_expect_success "'ipfs config replace' with lower case privkey errors out" '
+       cp "$IPFS_PATH/config" real_config &&
+       sed -i -e '\''s/PrivKey/privkey/'\'' real_config &&
+       test_expect_code 1 ipfs config replace - < real_config 2> replace_out
+  '
+
+  test_expect_success "output looks good" '
+       echo "Error: setting private key with API is not supported" > replace_expected
+       test_cmp replace_out replace_expected
+  '
 }
 
 test_init_ipfs