From 5c88744155f925b3f06cc1a32f8f678d04b5489d Mon Sep 17 00:00:00 2001
From: Sean Young <sean.young@monax.io>
Date: Mon, 4 Jun 2018 12:06:31 +0100
Subject: [PATCH] keys: check permissions of key file fail if it readable by
 non-user

Allow this to be overridden on the command line and config.

Signed-off-by: Sean Young <sean.young@monax.io>
---
 cmd/burrow/commands/configure.go |  2 +-
 cmd/burrow/commands/keys.go      | 30 ++++++++++++++++++++++++++++--
 config/config.go                 |  2 +-
 core/kernel.go                   |  2 +-
 core/kernel_test.go              |  2 +-
 keys/config.go                   | 14 ++++++++------
 keys/core.go                     |  6 ------
 keys/key_store_file.go           | 32 +++++++++++++++++++++++++++-----
 keys/server.go                   |  5 +++--
 keys/server_test.go              |  5 ++++-
 keys/test.sh                     |  6 ++++--
 11 files changed, 78 insertions(+), 28 deletions(-)

diff --git a/cmd/burrow/commands/configure.go b/cmd/burrow/commands/configure.go
index a964e2f9..b06f9a9a 100644
--- a/cmd/burrow/commands/configure.go
+++ b/cmd/burrow/commands/configure.go
@@ -94,7 +94,7 @@ func Configure(output Output) func(cmd *cli.Cmd) {
 				if err != nil {
 					output.Fatalf("Could not read GenesisSpec: %v", err)
 				}
-				keyStore := keys.NewKeyStore(conf.Keys.KeysDirectory)
+				keyStore := keys.NewKeyStore(conf.Keys.KeysDirectory, conf.Keys.AllowBadFilePermissions, logging.NewNoopLogger())
 				if *generateKeysOpt != "" {
 					keyClient := keys.NewLocalKeyClient(keyStore, logging.NewNoopLogger())
 					conf.GenesisDoc, err = genesisSpec.GenesisDoc(keyClient)
diff --git a/cmd/burrow/commands/keys.go b/cmd/burrow/commands/keys.go
index 10b3c4fe..0b2fd9cd 100644
--- a/cmd/burrow/commands/keys.go
+++ b/cmd/burrow/commands/keys.go
@@ -11,9 +11,11 @@ import (
 	"io/ioutil"
 
 	"github.com/howeyc/gopass"
+	"github.com/hyperledger/burrow/config"
 	"github.com/hyperledger/burrow/crypto"
 	"github.com/hyperledger/burrow/keys"
 	"github.com/hyperledger/burrow/keys/pbkeys"
+	"github.com/hyperledger/burrow/logging/lifecycle"
 	"github.com/jawher/mow.cli"
 	"google.golang.org/grpc"
 )
@@ -45,10 +47,34 @@ func Keys(output Output) func(cmd *cli.Cmd) {
 		}
 
 		cmd.Command("server", "run keys server", func(cmd *cli.Cmd) {
-			keysDir := cmd.StringOpt("dir", keys.DefaultKeysDir, "specify the location of the directory containing key files")
+			keysDir := cmd.StringOpt("dir", "", "specify the location of the directory containing key files")
+			badPerm := cmd.BoolOpt("allow-bad-perm", false, "Allow unix key file permissions to be readable other than user")
+			configOpt := cmd.StringOpt("c config", "", "Use the a specified burrow config file")
+
+			var conf *config.BurrowConfig
+
+			cmd.Before = func() {
+				var err error
+				conf, err = obtainBurrowConfig(*configOpt, "")
+				if err != nil {
+					output.Fatalf("Could not obtain config: %v", err)
+				}
+			}
 
 			cmd.Action = func() {
-				err := keys.StartStandAloneServer(*keysDir, *keysHost, *keysPort)
+				logger, err := lifecycle.NewLoggerFromLoggingConfig(conf.Logging)
+				if err != nil {
+					output.Fatalf("could not generate logger from logging config: %v", err)
+				}
+
+				if badPerm != nil {
+					conf.Keys.AllowBadFilePermissions = *badPerm
+				}
+				if keysDir != nil {
+					conf.Keys.KeysDirectory = *keysDir
+				}
+
+				err = keys.StartStandAloneServer(conf.Keys.KeysDirectory, *keysHost, *keysPort, conf.Keys.AllowBadFilePermissions, logger)
 				if err != nil {
 					output.Fatalf("Failed to start server: %v", err)
 				}
diff --git a/config/config.go b/config/config.go
index 1a73e9fa..a120986d 100644
--- a/config/config.go
+++ b/config/config.go
@@ -63,7 +63,7 @@ func (conf *BurrowConfig) Kernel(ctx context.Context) (*core.Kernel, error) {
 			return nil, err
 		}
 	} else {
-		keyStore = keys.NewKeyStore(conf.Keys.KeysDirectory)
+		keyStore = keys.NewKeyStore(conf.Keys.KeysDirectory, conf.Keys.AllowBadFilePermissions, logger)
 		keyClient = keys.NewLocalKeyClient(keyStore, logger)
 	}
 
diff --git a/core/kernel.go b/core/kernel.go
index 437c43fc..555b7888 100644
--- a/core/kernel.go
+++ b/core/kernel.go
@@ -219,7 +219,7 @@ func NewKernel(ctx context.Context, keyClient keys.KeyClient, privValidator tm_t
 
 				if keyConfig.GRPCServiceEnabled {
 					if keyStore == nil {
-						ks = keys.NewKeyStore(keyConfig.KeysDirectory)
+						ks = keys.NewKeyStore(keyConfig.KeysDirectory, keyConfig.AllowBadFilePermissions, logger)
 					}
 					pbkeys.RegisterKeysServer(grpcServer, &ks)
 				}
diff --git a/core/kernel_test.go b/core/kernel_test.go
index e02ef7cd..68cb5f9d 100644
--- a/core/kernel_test.go
+++ b/core/kernel_test.go
@@ -65,7 +65,7 @@ func bootWaitBlocksShutdown(privValidator tm_types.PrivValidator, genesisDoc *ge
 	tmConf *tm_config.Config, logger *logging.Logger,
 	blockChecker func(block *tm_types.EventDataNewBlock) (cont bool)) error {
 
-	keyStore := keys.NewKeyStore(keys.DefaultKeysDir)
+	keyStore := keys.NewKeyStore(keys.DefaultKeysDir, false, logger)
 	keyClient := keys.NewLocalKeyClient(keyStore, logging.NewNoopLogger())
 	kern, err := NewKernel(context.Background(), keyClient, privValidator, genesisDoc, tmConf,
 		rpc.DefaultRPCConfig(), keys.DefaultKeysConfig(), &keyStore, nil, logger)
diff --git a/keys/config.go b/keys/config.go
index d50d6133..c954a63e 100644
--- a/keys/config.go
+++ b/keys/config.go
@@ -1,16 +1,18 @@
 package keys
 
 type KeysConfig struct {
-	GRPCServiceEnabled bool
-	RemoteAddress      string
-	KeysDirectory      string
+	GRPCServiceEnabled      bool
+	AllowBadFilePermissions bool
+	RemoteAddress           string
+	KeysDirectory           string
 }
 
 func DefaultKeysConfig() *KeysConfig {
 	return &KeysConfig{
 		// Default Monax keys port
-		GRPCServiceEnabled: true,
-		RemoteAddress:      "",
-		KeysDirectory:      DefaultKeysDir,
+		GRPCServiceEnabled:      true,
+		AllowBadFilePermissions: false,
+		RemoteAddress:           "",
+		KeysDirectory:           DefaultKeysDir,
 	}
 }
diff --git a/keys/core.go b/keys/core.go
index 8642a820..dc003b68 100644
--- a/keys/core.go
+++ b/keys/core.go
@@ -38,12 +38,6 @@ func returnNamesDir(dir string) (string, error) {
 	return dir, checkMakeDataDir(dir)
 }
 
-//-----
-
-func NewKeyStore(dir string) KeyStore {
-	return KeyStore{keysDirPath: dir}
-}
-
 //----------------------------------------------------------------
 func writeKey(keyDir string, addr, keyJson []byte) ([]byte, error) {
 	dir, err := returnDataDir(keyDir)
diff --git a/keys/key_store_file.go b/keys/key_store_file.go
index 25c2f567..1b0ac87d 100644
--- a/keys/key_store_file.go
+++ b/keys/key_store_file.go
@@ -13,6 +13,8 @@ import (
 	"sync"
 
 	"github.com/hyperledger/burrow/crypto"
+	"github.com/hyperledger/burrow/logging"
+	"github.com/hyperledger/burrow/logging/structure"
 	"github.com/tmthrgd/go-hex"
 
 	"golang.org/x/crypto/scrypt"
@@ -99,9 +101,19 @@ func IsValidKeyJson(j []byte) []byte {
 	return nil
 }
 
+func NewKeyStore(dir string, AllowBadFilePermissions bool, logger *logging.Logger) KeyStore {
+	return KeyStore{
+		keysDirPath:             dir,
+		AllowBadFilePermissions: AllowBadFilePermissions,
+		logger:                  logger.With(structure.ComponentKey, "keys").WithScope("NewKeyStore"),
+	}
+}
+
 type KeyStore struct {
 	sync.Mutex
-	keysDirPath string
+	AllowBadFilePermissions bool
+	keysDirPath             string
+	logger                  *logging.Logger
 }
 
 func (ks KeyStore) Gen(passphrase string, curveType crypto.CurveType) (key *Key, err error) {
@@ -125,7 +137,7 @@ func (ks KeyStore) GetKey(passphrase string, keyAddr []byte) (*Key, error) {
 	if err != nil {
 		return nil, err
 	}
-	fileContent, err := GetKeyFile(dataDirPath, keyAddr)
+	fileContent, err := ks.GetKeyFile(dataDirPath, keyAddr)
 	if err != nil {
 		return nil, err
 	}
@@ -303,9 +315,19 @@ func (ks KeyStore) DeleteKey(passphrase string, keyAddr []byte) (err error) {
 	return os.Remove(keyDirPath)
 }
 
-func GetKeyFile(dataDirPath string, keyAddr []byte) (fileContent []byte, err error) {
-	fileName := strings.ToUpper(hex.EncodeToString(keyAddr))
-	return ioutil.ReadFile(path.Join(dataDirPath, fileName+".json"))
+func (ks *KeyStore) GetKeyFile(dataDirPath string, keyAddr []byte) (fileContent []byte, err error) {
+	filename := path.Join(dataDirPath, strings.ToUpper(hex.EncodeToString(keyAddr))+".json")
+	fileInfo, err := os.Stat(filename)
+	if err != nil {
+		return nil, err
+	}
+	if (uint32(fileInfo.Mode()) & 0077) != 0 {
+		ks.logger.InfoMsg("file should be accessible by user only", filename)
+		if !ks.AllowBadFilePermissions {
+			return nil, fmt.Errorf("file %s should be accessible by user only", filename)
+		}
+	}
+	return ioutil.ReadFile(filename)
 }
 
 func WriteKeyFile(addr []byte, dataDirPath string, content []byte) (err error) {
diff --git a/keys/server.go b/keys/server.go
index 3ffa0e43..563bf5b5 100644
--- a/keys/server.go
+++ b/keys/server.go
@@ -10,6 +10,7 @@ import (
 
 	"github.com/hyperledger/burrow/crypto"
 	"github.com/hyperledger/burrow/keys/pbkeys"
+	"github.com/hyperledger/burrow/logging"
 	"github.com/tmthrgd/go-hex"
 	"golang.org/x/crypto/ripemd160"
 	"google.golang.org/grpc"
@@ -19,13 +20,13 @@ import (
 // all cli commands pass through the http KeyStore
 // the KeyStore process also maintains the unlocked accounts
 
-func StartStandAloneServer(keysDir, host, port string) error {
+func StartStandAloneServer(keysDir, host, port string, AllowBadFilePermissions bool, logger *logging.Logger) error {
 	listen, err := net.Listen("tcp", host+":"+port)
 	if err != nil {
 		return err
 	}
 
-	ks := NewKeyStore(keysDir)
+	ks := NewKeyStore(keysDir, AllowBadFilePermissions, logger)
 
 	grpcServer := grpc.NewServer()
 	pbkeys.RegisterKeysServer(grpcServer, &ks)
diff --git a/keys/server_test.go b/keys/server_test.go
index 614daaec..5ee1e204 100644
--- a/keys/server_test.go
+++ b/keys/server_test.go
@@ -14,6 +14,7 @@ import (
 	"github.com/hyperledger/burrow/crypto"
 	"github.com/hyperledger/burrow/execution/evm/sha3"
 	"github.com/hyperledger/burrow/keys/pbkeys"
+	"github.com/hyperledger/burrow/logging"
 	tm_crypto "github.com/tendermint/go-crypto"
 	"google.golang.org/grpc"
 )
@@ -36,8 +37,10 @@ var (
 // start the server
 func init() {
 	failedCh := make(chan error)
+	testDir := "test_scratch/" + DefaultKeysDir
+	os.RemoveAll(testDir)
 	go func() {
-		err := StartStandAloneServer(DefaultKeysDir, DefaultHost, TestPort)
+		err := StartStandAloneServer(testDir, DefaultHost, TestPort, false, logging.NewNoopLogger())
 		failedCh <- err
 	}()
 	tick := time.NewTicker(time.Second)
diff --git a/keys/test.sh b/keys/test.sh
index 7bb16ed1..76824253 100755
--- a/keys/test.sh
+++ b/keys/test.sh
@@ -8,9 +8,11 @@
 
 burrow_bin=${burrow_bin:-burrow}
 
+keys_dir=./keys/test_scratch/.keys
+
 echo "-----------------------------"
 echo "starting the server"
-$burrow_bin keys server &
+$burrow_bin keys server --dir $keys_dir &
 keys_pid=$!
 sleep 1
 echo "-----------------------------"
@@ -88,7 +90,7 @@ do
 	echo "... $CURVETYPE"
 	# create a key, get its address and priv, backup the json, delete the key
 	ADDR=`$burrow_bin keys gen --curvetype $CURVETYPE --no-password`
-	DIR=.keys/data
+	DIR=$keys_dir/data
 	FILE=$DIR/$ADDR.json
 	PRIV=`cat $FILE |  jq -r .PrivateKey.Plain`
 	HEXPRIV=`echo -n "$PRIV" | base64 -d | xxd -p -u -c 256`
-- 
GitLab