Refactor Wallet Password Handling, Preventing Wallet Corruption (#6933)

* wallet should no longer deal with account passwords
* ensure tests are fixed
* Merge branch 'master' into corrupted-pass
* move mnemonic logic into right place
* rem fmts
* add fileutil
* gazelle
* imports
* move seed logic to derived
* fix tests
* imports
* gaz
* Merge refs/heads/master into corrupted-pass
* merge confs
* Merge refs/heads/master into corrupted-pass
* ivan's feedback
* Merge branch 'corrupted-pass' of github.com:prysmaticlabs/prysm into corrupted-pass
* gaz
* fix shared test
* Merge refs/heads/master into corrupted-pass
* resolve conflicts
* fix test build

Author: rauljordan

shared/cmd/BUILD.bazel

@@ -5,7 +5,6 @@ go_library(
     name = "go_default_library",
     srcs = [
         "config.go",
-        "customflags.go",
         "defaults.go",
         "flags.go",
         "helpers.go",
@@ -16,6 +15,7 @@ go_library(
     importpath = "github.com/prysmaticlabs/prysm/shared/cmd",
     visibility = ["//visibility:public"],
     deps = [
+        "//shared/fileutil:go_default_library",
         "//shared/params:go_default_library",
         "@com_github_golang_mock//gomock:go_default_library",
         "@com_github_pkg_errors//:go_default_library",
@@ -31,7 +31,6 @@ go_test(
     size = "small",
     srcs = [
         "config_test.go",
-        "customflags_test.go",
         "helpers_test.go",
     ],
     embed = [":go_default_library"],

shared/cmd/customflags.go

@@ -19,13 +19,15 @@ package cmd
 import (
 	"path/filepath"
 	"runtime"
+
+	"github.com/prysmaticlabs/prysm/shared/fileutil"
 )
 
 // DefaultDataDir is the default data directory to use for the databases and other
 // persistence requirements.
 func DefaultDataDir() string {
 	// Try to place the data folder in the user's home dir
-	home := homeDir()
+	home := fileutil.HomeDir()
 	if home != "" {
 		if runtime.GOOS == "darwin" {
 			return filepath.Join(home, "Library", "Eth2")

shared/cmd/defaults.go

@@ -0,0 +1,20 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_test")
+load("@prysm//tools/go:def.bzl", "go_library")
+
+go_library(
+    name = "go_default_library",
+    srcs = ["fileutil.go"],
+    importpath = "github.com/prysmaticlabs/prysm/shared/fileutil",
+    visibility = ["//visibility:public"],
+    deps = ["@com_github_pkg_errors//:go_default_library"],
+)
+
+go_test(
+    name = "go_default_test",
+    srcs = ["fileutil_test.go"],
+    embed = [":go_default_library"],
+    deps = [
+        "//shared/testutil/assert:go_default_library",
+        "//shared/testutil/require:go_default_library",
+    ],
+)

shared/fileutil/BUILD.bazel

@@ -0,0 +1,76 @@
+package fileutil
+
+import (
+	"io/ioutil"
+	"os"
+	"os/user"
+	"path"
+	"path/filepath"
+	"strings"
+
+	"github.com/pkg/errors"
+)
+
+// ExpandPath given a string which may be a relative path.
+// 1. replace tilde with users home dir
+// 2. expands embedded environment variables
+// 3. cleans the path, e.g. /a/b/../c -> /a/c
+// Note, it has limitations, e.g. ~someuser/tmp will not be expanded
+func ExpandPath(p string) (string, error) {
+	if strings.HasPrefix(p, "~/") || strings.HasPrefix(p, "~\\") {
+		if home := HomeDir(); home != "" {
+			p = home + p[1:]
+		}
+	}
+	return filepath.Abs(path.Clean(os.ExpandEnv(p)))
+}
+
+// HomeDir for a user.
+func HomeDir() string {
+	if home := os.Getenv("HOME"); home != "" {
+		return home
+	}
+	if usr, err := user.Current(); err == nil {
+		return usr.HomeDir
+	}
+	return ""
+}
+
+// HasDir checks if a directory indeed exists at the specified path.
+func HasDir(dirPath string) (bool, error) {
+	fullPath, err := ExpandPath(dirPath)
+	if err != nil {
+		return false, err
+	}
+	info, err := os.Stat(fullPath)
+	if os.IsNotExist(err) {
+		return false, nil
+	}
+	if info == nil {
+		return false, err
+	}
+	return info.IsDir(), err
+}
+
+// FileExists returns true if a file is not a directory and exists
+// at the specified path.
+func FileExists(filename string) bool {
+	filePath, err := ExpandPath(filename)
+	if err != nil {
+		return false
+	}
+	info, err := os.Stat(filePath)
+	if os.IsNotExist(err) {
+		return false
+	}
+	return !info.IsDir()
+}
+
+// ReadFileAsBytes expands a file name's absolute path and reads it as bytes from disk.
+func ReadFileAsBytes(filename string) ([]byte, error) {
+	filePath, err := ExpandPath(filename)
+	if err != nil {
+		return nil, errors.Wrap(err, "could not determine absolute path of password file")
+	}
+	return ioutil.ReadFile(filePath)
+}

shared/fileutil/fileutil.go

@@ -13,15 +13,15 @@
 //
 // You should have received a copy of the GNU General Public License
 // along with go-ethereum. If not, see <http://www.gnu.org/licenses/>.
-
-package cmd
+package fileutil
 
 import (
 	"os"
 	"os/user"
 	"testing"
 
 	"github.com/prysmaticlabs/prysm/shared/testutil/assert"
+	"github.com/prysmaticlabs/prysm/shared/testutil/require"
 )
 
 func TestPathExpansion(t *testing.T) {
@@ -32,7 +32,6 @@ func TestPathExpansion(t *testing.T) {
 	tests := map[string]string{
 		"/home/someuser/tmp": "/home/someuser/tmp",
 		"~/tmp":              user.HomeDir + "/tmp",
-		"~thisOtherUser/b/":  "~thisOtherUser/b",
 		"$DDDXXX/a/b":        "/tmp/a/b",
 		"/a/b/":              "/a/b",
 	}
@@ -41,6 +40,8 @@ func TestPathExpansion(t *testing.T) {
 		t.Error(err)
 	}
 	for test, expected := range tests {
-		assert.Equal(t, expected, expandPath(test))
+		expanded, err := ExpandPath(test)
+		require.NoError(t, err)
+		assert.Equal(t, expected, expanded)
 	}
 }

shared/cmd/customflags_test.go shared/fileutil/fileutil_test.goshared/cmd/customflags_test.go renamed to shared/fileutil/fileutil_test.go

@@ -8,6 +8,7 @@ go_library(
     visibility = ["//visibility:private"],
     deps = [
         "//shared/bls:go_default_library",
+        "//shared/fileutil:go_default_library",
         "//shared/params:go_default_library",
         "//shared/promptutil:go_default_library",
         "//validator/keymanager/v2:go_default_library",

tools/keystores/BUILD.bazel

@@ -11,15 +11,14 @@ import (
 	"io/ioutil"
 	"log"
 	"os"
-	"os/user"
-	"path"
 	"path/filepath"
 	"strings"
 
 	"github.com/google/uuid"
 	"github.com/logrusorgru/aurora"
 	"github.com/pkg/errors"
 	"github.com/prysmaticlabs/prysm/shared/bls"
+	"github.com/prysmaticlabs/prysm/shared/fileutil"
 	"github.com/prysmaticlabs/prysm/shared/params"
 	"github.com/prysmaticlabs/prysm/shared/promptutil"
 	v2keymanager "github.com/prysmaticlabs/prysm/validator/keymanager/v2"
@@ -92,7 +91,7 @@ func decrypt(cliCtx *cli.Context) error {
 	if keystorePath == "" {
 		return errors.New("--keystore must be set")
 	}
-	fullPath, err := expandPath(keystorePath)
+	fullPath, err := fileutil.ExpandPath(keystorePath)
 	if err != nil {
 		return errors.Wrapf(err, "could not expand path: %s", keystorePath)
 	}
@@ -104,7 +103,7 @@ func decrypt(cliCtx *cli.Context) error {
 			return nil
 		})
 	}
-	isDir, err := hasDir(fullPath)
+	isDir, err := fileutil.HasDir(fullPath)
 	if err != nil {
 		return errors.Wrapf(err, "could not check if path exists: %s", fullPath)
 	}
@@ -149,15 +148,11 @@ func encrypt(cliCtx *cli.Context) error {
 	if outputPath == "" {
 		return errors.New("--output-path must be set")
 	}
-	fullPath, err := expandPath(outputPath)
+	fullPath, err := fileutil.ExpandPath(outputPath)
 	if err != nil {
 		return errors.Wrapf(err, "could not expand path: %s", outputPath)
 	}
-	exists, err := fileExists(fullPath)
-	if err != nil {
-		return errors.Wrapf(err, "could not check if file exists: %s", fullPath)
-	}
-	if exists {
+	if fileutil.FileExists(fullPath) {
 		response, err := promptutil.ValidatePrompt(
 			fmt.Sprintf("file at path %s already exists, are you sure you want to overwrite it? [y/n]", fullPath),
 			func(s string) error {
@@ -263,58 +258,3 @@ func readAndDecryptKeystore(fullPath string, password string) error {
 	fmt.Printf("Pubkey: %#x\n", au.BrightGreen(pubKeyBytes))
 	return nil
 }
-
-// Checks if the item at the specified path exists and is a directory.
-func hasDir(dirPath string) (bool, error) {
-	fullPath, err := expandPath(dirPath)
-	if err != nil {
-		return false, err
-	}
-	info, err := os.Stat(fullPath)
-	if err != nil {
-		if os.IsNotExist(err) {
-			return false, nil
-		}
-		return false, err
-	}
-	return info.IsDir(), nil
-}
-
-// Check if a file at the specified path exists.
-func fileExists(filePath string) (bool, error) {
-	fullPath, err := expandPath(filePath)
-	if err != nil {
-		return false, err
-	}
-	if _, err := os.Stat(fullPath); err != nil {
-		if os.IsNotExist(err) {
-			return false, nil
-		}
-		return false, err
-	}
-	return true, nil
-}
-
-// Expands a file path
-// 1. replace tilde with users home dir
-// 2. expands embedded environment variables
-// 3. cleans the path, e.g. /a/b/../c -> /a/c
-// Note, it has limitations, e.g. ~someuser/tmp will not be expanded
-func expandPath(p string) (string, error) {
-	if strings.HasPrefix(p, "~/") || strings.HasPrefix(p, "~\\") {
-		if home := homeDir(); home != "" {
-			p = home + p[1:]
-		}
-	}
-	return filepath.Abs(path.Clean(os.ExpandEnv(p)))
-}
-
-func homeDir() string {
-	if home := os.Getenv("HOME"); home != "" {
-		return home
-	}
-	if usr, err := user.Current(); err == nil {
-		return usr.HomeDir
-	}
-	return ""
-}

tools/keystores/main.go

@@ -16,6 +16,7 @@ go_library(
         "//contracts/deposit-contract:go_default_library",
         "//shared/cmd:go_default_library",
         "//shared/depositutil:go_default_library",
+        "//shared/fileutil:go_default_library",
         "//shared/keystore:go_default_library",
         "//shared/params:go_default_library",
         "//validator/db/kv:go_default_library",

validator/accounts/v1/BUILD.bazel

@@ -8,7 +8,6 @@ import (
 	"fmt"
 	"io"
 	"os"
-	"os/user"
 	"path/filepath"
 	"runtime"
 	"strings"
@@ -17,6 +16,7 @@ import (
 	contract "github.com/prysmaticlabs/prysm/contracts/deposit-contract"
 	"github.com/prysmaticlabs/prysm/shared/cmd"
 	"github.com/prysmaticlabs/prysm/shared/depositutil"
+	"github.com/prysmaticlabs/prysm/shared/fileutil"
 	"github.com/prysmaticlabs/prysm/shared/keystore"
 	"github.com/prysmaticlabs/prysm/shared/params"
 	"github.com/prysmaticlabs/prysm/validator/db/kv"
@@ -190,7 +190,7 @@ func PrintPublicAndPrivateKeys(path string, passphrase string) error {
 // DefaultValidatorDir returns OS-specific default keystore directory.
 func DefaultValidatorDir() string {
 	// Try to place the data folder in the user's home dir
-	home := homeDir()
+	home := fileutil.HomeDir()
 	if home != "" {
 		if runtime.GOOS == "darwin" {
 			return filepath.Join(home, "Library", "Eth2Validators")
@@ -336,17 +336,6 @@ func changePasswordForKeyType(keystorePath string, filePrefix string, oldPasswor
 	return nil
 }
 
-// homeDir returns home directory path.
-func homeDir() string {
-	if home := os.Getenv("HOME"); home != "" {
-		return home
-	}
-	if usr, err := user.Current(); err == nil {
-		return usr.HomeDir
-	}
-	return ""
-}
-
 // ExtractPublicKeysFromKeyStore extracts only the public keys from the decrypted keys from the keystore.
 func ExtractPublicKeysFromKeyStore(keystorePath string, passphrase string) ([][]byte, error) {
 	decryptedKeys, err := DecryptKeysFromKeystore(keystorePath, params.BeaconConfig().ValidatorPrivkeyFileName, passphrase)