From d4c954648c1d918c7b4680e1afed9872c161f201 Mon Sep 17 00:00:00 2001 From: Raul Jordan Date: Mon, 9 Nov 2020 14:27:03 -0600 Subject: [PATCH] Prevent Usage of Stdlib File/Dir Writing With Static Analysis (#7685) * write file and mkdirall analyzers * include analyzer in build bazel * comments to the single entrypoint and fix validator references * enforce 600 for files, 700 for dirs * pass validator tests * add to nogo * remove references * beaconfuzz * docker img * fix up kv issue * mkdir if not exists * radek comments * final comments * Try to fix file problem Co-authored-by: Ivan Martinez --- BUILD.bazel | 1 + beacon-chain/core/state/interop/BUILD.bazel | 1 + .../core/state/interop/write_block_to_disk.go | 4 +- .../core/state/interop/write_state_to_disk.go | 4 +- beacon-chain/db/kv/backup.go | 3 +- beacon-chain/db/kv/kv.go | 9 +- beacon-chain/p2p/BUILD.bazel | 1 + beacon-chain/p2p/utils.go | 6 +- endtoend/helpers/helpers.go | 3 - endtoend/params/params.go | 5 +- nogo_config.json | 15 ++- shared/fileutil/fileutil.go | 48 +++++++- shared/fileutil/fileutil_test.go | 72 ++++++++++++ shared/keystore/BUILD.bazel | 2 +- shared/keystore/key.go | 4 +- shared/tos/BUILD.bazel | 1 - shared/tos/tos.go | 7 +- slasher/db/kv/BUILD.bazel | 1 + slasher/db/kv/kv.go | 10 +- tools/analyzers/properpermissions/BUILD.bazel | 28 +++++ tools/analyzers/properpermissions/analyzer.go | 104 ++++++++++++++++++ .../properpermissions/analyzer_test.go | 11 ++ .../properpermissions/testdata/BUILD.bazel | 11 ++ .../testdata/custom_imports.go | 18 +++ .../testdata/regular_imports.go | 26 +++++ tools/beacon-fuzz/BUILD.bazel | 3 + tools/beacon-fuzz/main.go | 4 +- tools/benchmark-files-gen/BUILD.bazel | 1 + tools/benchmark-files-gen/main.go | 11 +- tools/enr-calculator/BUILD.bazel | 2 + tools/enr-calculator/main.go | 4 +- tools/genesis-state-gen/main.go | 6 +- tools/interop/export-genesis/BUILD.bazel | 1 + tools/interop/export-genesis/main.go | 4 +- tools/keystores/BUILD.bazel | 1 - tools/keystores/main.go | 3 +- tools/update-genesis-time/BUILD.bazel | 1 + tools/update-genesis-time/main.go | 3 +- validator/accounts/accounts_backup.go | 2 +- validator/accounts/accounts_backup_test.go | 6 +- validator/accounts/wallet/BUILD.bazel | 1 - validator/accounts/wallet/wallet.go | 31 ++++-- validator/accounts/wallet_create_test.go | 3 +- validator/db/kv/BUILD.bazel | 1 + validator/db/kv/db.go | 9 +- validator/rpc/BUILD.bazel | 1 - validator/rpc/auth.go | 7 +- 47 files changed, 432 insertions(+), 68 deletions(-) create mode 100644 tools/analyzers/properpermissions/BUILD.bazel create mode 100644 tools/analyzers/properpermissions/analyzer.go create mode 100644 tools/analyzers/properpermissions/analyzer_test.go create mode 100644 tools/analyzers/properpermissions/testdata/BUILD.bazel create mode 100644 tools/analyzers/properpermissions/testdata/custom_imports.go create mode 100644 tools/analyzers/properpermissions/testdata/regular_imports.go diff --git a/BUILD.bazel b/BUILD.bazel index eb9cc89ee..36c24763a 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -112,6 +112,7 @@ nogo( "//tools/analyzers/nop:go_tool_library", "//tools/analyzers/slicedirect:go_tool_library", "//tools/analyzers/ineffassign:go_tool_library", + "//tools/analyzers/properpermissions:go_tool_library", ] + select({ # nogo checks that fail with coverage enabled. ":coverage_enabled": [], diff --git a/beacon-chain/core/state/interop/BUILD.bazel b/beacon-chain/core/state/interop/BUILD.bazel index 635a0d473..2eb14d33c 100644 --- a/beacon-chain/core/state/interop/BUILD.bazel +++ b/beacon-chain/core/state/interop/BUILD.bazel @@ -15,6 +15,7 @@ go_library( deps = [ "//beacon-chain/state:go_default_library", "//shared/featureconfig:go_default_library", + "//shared/fileutil:go_default_library", "@com_github_prysmaticlabs_ethereumapis//eth/v1alpha1:go_default_library", "@com_github_sirupsen_logrus//:go_default_library", ], diff --git a/beacon-chain/core/state/interop/write_block_to_disk.go b/beacon-chain/core/state/interop/write_block_to_disk.go index 68dd4e6cd..63a194652 100644 --- a/beacon-chain/core/state/interop/write_block_to_disk.go +++ b/beacon-chain/core/state/interop/write_block_to_disk.go @@ -2,12 +2,12 @@ package interop import ( "fmt" - "io/ioutil" "os" "path" ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1" "github.com/prysmaticlabs/prysm/shared/featureconfig" + "github.com/prysmaticlabs/prysm/shared/fileutil" ) // WriteBlockToDisk as a block ssz. Writes to temp directory. Debug! @@ -27,7 +27,7 @@ func WriteBlockToDisk(block *ethpb.SignedBeaconBlock, failed bool) { log.WithError(err).Error("Failed to ssz encode block") return } - if err := ioutil.WriteFile(fp, enc, 0664); err != nil { + if err := fileutil.WriteFile(fp, enc); err != nil { log.WithError(err).Error("Failed to write to disk") } } diff --git a/beacon-chain/core/state/interop/write_state_to_disk.go b/beacon-chain/core/state/interop/write_state_to_disk.go index 0b9935fbb..aa7d6c5a7 100644 --- a/beacon-chain/core/state/interop/write_state_to_disk.go +++ b/beacon-chain/core/state/interop/write_state_to_disk.go @@ -2,12 +2,12 @@ package interop import ( "fmt" - "io/ioutil" "os" "path" stateTrie "github.com/prysmaticlabs/prysm/beacon-chain/state" "github.com/prysmaticlabs/prysm/shared/featureconfig" + "github.com/prysmaticlabs/prysm/shared/fileutil" ) // WriteStateToDisk as a state ssz. Writes to temp directory. Debug! @@ -22,7 +22,7 @@ func WriteStateToDisk(state *stateTrie.BeaconState) { log.WithError(err).Error("Failed to ssz encode state") return } - if err := ioutil.WriteFile(fp, enc, 0664); err != nil { + if err := fileutil.WriteFile(fp, enc); err != nil { log.WithError(err).Error("Failed to write to disk") } } diff --git a/beacon-chain/db/kv/backup.go b/beacon-chain/db/kv/backup.go index 5c2cd9f71..d7103dc18 100644 --- a/beacon-chain/db/kv/backup.go +++ b/beacon-chain/db/kv/backup.go @@ -3,7 +3,6 @@ package kv import ( "context" "fmt" - "os" "path" "github.com/pkg/errors" @@ -40,7 +39,7 @@ func (s *Store) Backup(ctx context.Context, outputDir string) error { return errors.New("no head block") } // Ensure the backups directory exists. - if err := os.MkdirAll(backupsDir, params.BeaconIoConfig().ReadWriteExecutePermissions); err != nil { + if err := fileutil.MkdirAll(backupsDir); err != nil { return err } backupPath := path.Join(backupsDir, fmt.Sprintf("prysm_beacondb_at_slot_%07d.backup", head.Block.Slot)) diff --git a/beacon-chain/db/kv/kv.go b/beacon-chain/db/kv/kv.go index 823d30285..c89a2413e 100644 --- a/beacon-chain/db/kv/kv.go +++ b/beacon-chain/db/kv/kv.go @@ -13,6 +13,7 @@ import ( prombolt "github.com/prysmaticlabs/prombbolt" "github.com/prysmaticlabs/prysm/beacon-chain/cache" "github.com/prysmaticlabs/prysm/beacon-chain/db/iface" + "github.com/prysmaticlabs/prysm/shared/fileutil" "github.com/prysmaticlabs/prysm/shared/params" bolt "go.etcd.io/bbolt" ) @@ -46,9 +47,15 @@ type Store struct { // path specified, creates the kv-buckets based on the schema, and stores // an open connection db object as a property of the Store struct. func NewKVStore(dirPath string, stateSummaryCache *cache.StateSummaryCache) (*Store, error) { - if err := os.MkdirAll(dirPath, params.BeaconIoConfig().ReadWriteExecutePermissions); err != nil { + hasDir, err := fileutil.HasDir(dirPath) + if err != nil { return nil, err } + if !hasDir { + if err := fileutil.MkdirAll(dirPath); err != nil { + return nil, err + } + } datafile := path.Join(dirPath, databaseFileName) boltDB, err := bolt.Open(datafile, params.BeaconIoConfig().ReadWritePermissions, &bolt.Options{Timeout: 1 * time.Second, InitialMmapSize: 10e6}) if err != nil { diff --git a/beacon-chain/p2p/BUILD.bazel b/beacon-chain/p2p/BUILD.bazel index 7a752ee06..f5a50ad43 100644 --- a/beacon-chain/p2p/BUILD.bazel +++ b/beacon-chain/p2p/BUILD.bazel @@ -48,6 +48,7 @@ go_library( "//beacon-chain/p2p/types:go_default_library", "//proto/beacon/p2p/v1:go_default_library", "//shared:go_default_library", + "//shared/fileutil:go_default_library", "//shared/hashutil:go_default_library", "//shared/iputils:go_default_library", "//shared/p2putils:go_default_library", diff --git a/beacon-chain/p2p/utils.go b/beacon-chain/p2p/utils.go index fc4d028ec..67924b84d 100644 --- a/beacon-chain/p2p/utils.go +++ b/beacon-chain/p2p/utils.go @@ -19,8 +19,8 @@ import ( "github.com/pkg/errors" "github.com/prysmaticlabs/go-bitfield" pbp2p "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1" + "github.com/prysmaticlabs/prysm/shared/fileutil" "github.com/prysmaticlabs/prysm/shared/iputils" - "github.com/prysmaticlabs/prysm/shared/params" "github.com/sirupsen/logrus" ) @@ -78,7 +78,7 @@ func privKey(cfg *Config) (*ecdsa.PrivateKey, error) { } dst := make([]byte, hex.EncodedLen(len(rawbytes))) hex.Encode(dst, rawbytes) - if err = ioutil.WriteFile(defaultKeyPath, dst, params.BeaconIoConfig().ReadWritePermissions); err != nil { + if err = fileutil.WriteFile(defaultKeyPath, dst); err != nil { return nil, err } convertedKey := convertFromInterfacePrivKey(priv) @@ -129,7 +129,7 @@ func metaDataFromConfig(cfg *Config) (*pbp2p.MetaData, error) { if err != nil { return nil, err } - if err = ioutil.WriteFile(defaultKeyPath, dst, params.BeaconIoConfig().ReadWritePermissions); err != nil { + if err = fileutil.WriteFile(defaultKeyPath, dst); err != nil { return nil, err } return metaData, nil diff --git a/endtoend/helpers/helpers.go b/endtoend/helpers/helpers.go index d80bf13b4..aaa0b9c21 100644 --- a/endtoend/helpers/helpers.go +++ b/endtoend/helpers/helpers.go @@ -133,9 +133,6 @@ func LogErrorOutput(t *testing.T, file io.Reader, title string, index int) { // WritePprofFiles writes the memory heap and cpu profile files to the test path. func WritePprofFiles(testDir string, index int) error { - if err := os.MkdirAll(filepath.Join(testDir), os.ModePerm); err != nil { - return err - } url := fmt.Sprintf("http://127.0.0.1:%d/debug/pprof/heap", e2e.TestParams.BeaconNodeRPCPort+50+index) filePath := filepath.Join(testDir, fmt.Sprintf(memoryHeapFileName, index)) if err := writeURLRespAtPath(url, filePath); err != nil { diff --git a/endtoend/params/params.go b/endtoend/params/params.go index d1d6d5cad..b22eedcb4 100644 --- a/endtoend/params/params.go +++ b/endtoend/params/params.go @@ -6,7 +6,7 @@ import ( "errors" "fmt" "os" - "path" + "path/filepath" "strconv" "github.com/bazelbuild/rules_go/go/tools/bazel" @@ -66,9 +66,10 @@ func Init(beaconNodeCount int) error { if err != nil { return err } + testPath = filepath.Join(testPath, fmt.Sprintf("shard-%d", testIndex)) TestParams = ¶ms{ - TestPath: path.Join(testPath, fmt.Sprintf("shard-%d", testIndex)), + TestPath: testPath, LogPath: logPath, TestShardIndex: testIndex, BeaconNodeCount: beaconNodeCount, diff --git a/nogo_config.json b/nogo_config.json index d9bbe6523..04d5f24e7 100644 --- a/nogo_config.json +++ b/nogo_config.json @@ -74,7 +74,8 @@ "shared/mock/.*\\.go": "Mocks are OK", ".*/.*mock\\.go": "Mocks are OK", ".*/testmain\\.go": "Test runner generated code", - "proto/.*": "Generated protobuf related code" + "proto/.*": "Generated protobuf related code", + "tools/analyzers/properpermissions/testdata/.*": "Analyzer breaks rules" } }, "featureconfig": { @@ -132,5 +133,17 @@ }, "exclude_files": { } + }, + "properpermissions": { + "only_files": { + "beacon-chain/.*": "", + "slasher/.*": "", + "shared/.*": "", + "validator/.*": "" + }, + "exclude_files": { + ".*_test\\.go": "Tests are ok", + "shared/fileutil/fileutil.go": "Package which defines the proper rules" + } } } diff --git a/shared/fileutil/fileutil.go b/shared/fileutil/fileutil.go index ad3e69c88..ba36f9263 100644 --- a/shared/fileutil/fileutil.go +++ b/shared/fileutil/fileutil.go @@ -8,9 +8,8 @@ import ( "path/filepath" "strings" - "github.com/prysmaticlabs/prysm/shared/params" - "github.com/pkg/errors" + "github.com/prysmaticlabs/prysm/shared/params" log "github.com/sirupsen/logrus" ) @@ -28,6 +27,51 @@ func ExpandPath(p string) (string, error) { return filepath.Abs(path.Clean(os.ExpandEnv(p))) } +// MkdirAll takes in a path, expands it if necessary, and looks through the +// permissions of every directory along the path, ensuring we are not attempting +// to overwrite any existing permissions. Finally, creates the directory accordingly +// with standardized, Prysm project permissions. This is the static-analysis enforced +// method for creating a directory programmatically in Prysm. +func MkdirAll(dirPath string) error { + expanded, err := ExpandPath(dirPath) + if err != nil { + return err + } + exists, err := HasDir(expanded) + if err != nil { + return err + } + if exists { + info, err := os.Stat(expanded) + if err != nil { + return err + } + if info.Mode().Perm() != params.BeaconIoConfig().ReadWriteExecutePermissions { + return errors.New("dir already exists without proper 0700 permissions") + } + } + return os.MkdirAll(expanded, params.BeaconIoConfig().ReadWriteExecutePermissions) +} + +// WriteFile is the static-analysis enforced method for writing binary data to a file +// in Prysm, enforcing a single entrypoint with standardized permissions. +func WriteFile(file string, data []byte) error { + expanded, err := ExpandPath(file) + if err != nil { + return err + } + if FileExists(expanded) { + info, err := os.Stat(expanded) + if err != nil { + return err + } + if info.Mode() != params.BeaconIoConfig().ReadWritePermissions { + return errors.New("file already exists without proper 0600 permissions") + } + } + return ioutil.WriteFile(expanded, data, params.BeaconIoConfig().ReadWritePermissions) +} + // HomeDir for a user. func HomeDir() string { if home := os.Getenv("HOME"); home != "" { diff --git a/shared/fileutil/fileutil_test.go b/shared/fileutil/fileutil_test.go index de7ed83d0..b5a7bfcaf 100644 --- a/shared/fileutil/fileutil_test.go +++ b/shared/fileutil/fileutil_test.go @@ -21,6 +21,7 @@ import ( "io/ioutil" "os" "os/user" + "path/filepath" "testing" "github.com/prysmaticlabs/prysm/shared/fileutil" @@ -47,6 +48,77 @@ func TestPathExpansion(t *testing.T) { } } +func TestMkdirAll_AlreadyExists_WrongPermissions(t *testing.T) { + dirName := testutil.TempDir() + "somedir" + err := os.MkdirAll(dirName, os.ModePerm) + require.NoError(t, err) + defer func() { + assert.NoError(t, os.RemoveAll(dirName)) + }() + err = fileutil.MkdirAll(dirName) + assert.ErrorContains(t, "already exists without proper 0700 permissions", err) +} + +func TestMkdirAll_AlreadyExists_OK(t *testing.T) { + dirName := testutil.TempDir() + "somedir" + err := os.MkdirAll(dirName, params.BeaconIoConfig().ReadWriteExecutePermissions) + require.NoError(t, err) + defer func() { + assert.NoError(t, os.RemoveAll(dirName)) + }() + assert.NoError(t, fileutil.MkdirAll(dirName)) +} + +func TestMkdirAll_OK(t *testing.T) { + dirName := testutil.TempDir() + "somedir" + defer func() { + assert.NoError(t, os.RemoveAll(dirName)) + }() + err := fileutil.MkdirAll(dirName) + assert.NoError(t, err) + exists, err := fileutil.HasDir(dirName) + require.NoError(t, err) + assert.Equal(t, true, exists) +} + +func TestWriteFile_AlreadyExists_WrongPermissions(t *testing.T) { + dirName := testutil.TempDir() + "somedir" + err := os.MkdirAll(dirName, os.ModePerm) + require.NoError(t, err) + defer func() { + assert.NoError(t, os.RemoveAll(dirName)) + }() + someFileName := filepath.Join(dirName, "somefile.txt") + require.NoError(t, ioutil.WriteFile(someFileName, []byte("hi"), os.ModePerm)) + err = fileutil.WriteFile(someFileName, []byte("hi")) + assert.ErrorContains(t, "already exists without proper 0600 permissions", err) +} + +func TestWriteFile_AlreadyExists_OK(t *testing.T) { + dirName := testutil.TempDir() + "somedir" + err := os.MkdirAll(dirName, os.ModePerm) + require.NoError(t, err) + defer func() { + assert.NoError(t, os.RemoveAll(dirName)) + }() + someFileName := filepath.Join(dirName, "somefile.txt") + require.NoError(t, ioutil.WriteFile(someFileName, []byte("hi"), params.BeaconIoConfig().ReadWritePermissions)) + assert.NoError(t, fileutil.WriteFile(someFileName, []byte("hi"))) +} + +func TestWriteFile_OK(t *testing.T) { + dirName := testutil.TempDir() + "somedir" + err := os.MkdirAll(dirName, os.ModePerm) + require.NoError(t, err) + defer func() { + assert.NoError(t, os.RemoveAll(dirName)) + }() + someFileName := filepath.Join(dirName, "somefile.txt") + require.NoError(t, fileutil.WriteFile(someFileName, []byte("hi"))) + exists := fileutil.FileExists(someFileName) + assert.Equal(t, true, exists) +} + func TestCopyFile(t *testing.T) { fName := testutil.TempDir() + "testfile" err := ioutil.WriteFile(fName, []byte{1, 2, 3}, params.BeaconIoConfig().ReadWritePermissions) diff --git a/shared/keystore/BUILD.bazel b/shared/keystore/BUILD.bazel index 5d8115449..8fa9999ac 100644 --- a/shared/keystore/BUILD.bazel +++ b/shared/keystore/BUILD.bazel @@ -13,7 +13,7 @@ go_library( visibility = ["//visibility:public"], deps = [ "//shared/bls:go_default_library", - "//shared/params:go_default_library", + "//shared/fileutil:go_default_library", "//shared/timeutils:go_default_library", "@com_github_minio_sha256_simd//:go_default_library", "@com_github_pborman_uuid//:go_default_library", diff --git a/shared/keystore/key.go b/shared/keystore/key.go index a0f354a73..1417607dc 100644 --- a/shared/keystore/key.go +++ b/shared/keystore/key.go @@ -27,7 +27,7 @@ import ( "github.com/pborman/uuid" "github.com/prysmaticlabs/prysm/shared/bls" - "github.com/prysmaticlabs/prysm/shared/params" + "github.com/prysmaticlabs/prysm/shared/fileutil" ) const ( @@ -170,7 +170,7 @@ func storeNewRandomKey(ks keyStore, password string) error { func writeKeyFile(file string, content []byte) error { // Create the keystore directory with appropriate permissions // in case it is not present yet. - if err := os.MkdirAll(filepath.Dir(file), params.BeaconIoConfig().ReadWriteExecutePermissions); err != nil { + if err := fileutil.MkdirAll(filepath.Dir(file)); err != nil { return err } // Atomic write: create a temporary hidden file first diff --git a/shared/tos/BUILD.bazel b/shared/tos/BUILD.bazel index 8983dfe73..876e25018 100644 --- a/shared/tos/BUILD.bazel +++ b/shared/tos/BUILD.bazel @@ -9,7 +9,6 @@ go_library( deps = [ "//shared/cmd:go_default_library", "//shared/fileutil:go_default_library", - "//shared/params:go_default_library", "//shared/promptutil:go_default_library", "@com_github_logrusorgru_aurora//:go_default_library", "@com_github_sirupsen_logrus//:go_default_library", diff --git a/shared/tos/tos.go b/shared/tos/tos.go index 348dad9c9..9bf0558d6 100644 --- a/shared/tos/tos.go +++ b/shared/tos/tos.go @@ -2,15 +2,12 @@ package tos import ( "errors" - "io/ioutil" - "os" "path/filepath" "strings" "github.com/logrusorgru/aurora" "github.com/prysmaticlabs/prysm/shared/cmd" "github.com/prysmaticlabs/prysm/shared/fileutil" - "github.com/prysmaticlabs/prysm/shared/params" "github.com/prysmaticlabs/prysm/shared/promptutil" "github.com/sirupsen/logrus" "github.com/urfave/cli/v2" @@ -66,10 +63,10 @@ func VerifyTosAcceptedOrPrompt(ctx *cli.Context) error { // saveTosAccepted creates a file when Tos accepted. func saveTosAccepted(ctx *cli.Context) { - if err := os.MkdirAll(ctx.String(cmd.DataDirFlag.Name), params.BeaconIoConfig().ReadWriteExecutePermissions); err != nil { + if err := fileutil.MkdirAll(ctx.String(cmd.DataDirFlag.Name)); err != nil { log.WithError(err).Warnf("error creating directory: %s", ctx.String(cmd.DataDirFlag.Name)) } - err := ioutil.WriteFile(filepath.Join(ctx.String(cmd.DataDirFlag.Name), acceptTosFilename), []byte(""), 0644) + err := fileutil.WriteFile(filepath.Join(ctx.String(cmd.DataDirFlag.Name), acceptTosFilename), []byte("")) if err != nil { log.WithError(err).Warnf("error writing %s to file: %s", cmd.AcceptTosFlag.Name, filepath.Join(ctx.String(cmd.DataDirFlag.Name), acceptTosFilename)) } diff --git a/slasher/db/kv/BUILD.bazel b/slasher/db/kv/BUILD.bazel index 624dbd65f..18e2578b0 100644 --- a/slasher/db/kv/BUILD.bazel +++ b/slasher/db/kv/BUILD.bazel @@ -21,6 +21,7 @@ go_library( "//beacon-chain/core/helpers:go_default_library", "//proto/slashing:go_default_library", "//shared/bytesutil:go_default_library", + "//shared/fileutil:go_default_library", "//shared/hashutil:go_default_library", "//shared/params:go_default_library", "//slasher/cache:go_default_library", diff --git a/slasher/db/kv/kv.go b/slasher/db/kv/kv.go index c092b64af..cdd3e98c2 100644 --- a/slasher/db/kv/kv.go +++ b/slasher/db/kv/kv.go @@ -8,6 +8,7 @@ import ( "path" "github.com/pkg/errors" + "github.com/prysmaticlabs/prysm/shared/fileutil" "github.com/prysmaticlabs/prysm/shared/params" "github.com/prysmaticlabs/prysm/slasher/cache" bolt "go.etcd.io/bbolt" @@ -87,9 +88,16 @@ func createBuckets(tx *bolt.Tx, buckets ...[]byte) error { // path specified, creates the kv-buckets based on the schema, and stores // an open connection db object as a property of the Store struct. func NewKVStore(dirPath string, cfg *Config) (*Store, error) { - if err := os.MkdirAll(dirPath, params.BeaconIoConfig().ReadWriteExecutePermissions); err != nil { + hasDir, err := fileutil.HasDir(dirPath) + if err != nil { return nil, err } + if !hasDir { + if err := fileutil.MkdirAll(dirPath); err != nil { + return nil, err + } + } + datafile := path.Join(dirPath, databaseFileName) boltDB, err := bolt.Open(datafile, params.BeaconIoConfig().ReadWritePermissions, &bolt.Options{Timeout: params.BeaconIoConfig().BoltTimeout}) if err != nil { diff --git a/tools/analyzers/properpermissions/BUILD.bazel b/tools/analyzers/properpermissions/BUILD.bazel new file mode 100644 index 000000000..ed2765984 --- /dev/null +++ b/tools/analyzers/properpermissions/BUILD.bazel @@ -0,0 +1,28 @@ +load("@prysm//tools/go:def.bzl", "go_library") +load("@io_bazel_rules_go//go:def.bzl", "go_tool_library") + +go_library( + name = "go_default_library", + srcs = ["analyzer.go"], + importpath = "github.com/prysmaticlabs/prysm/tools/analyzers/properpermissions", + visibility = ["//visibility:public"], + deps = [ + "@org_golang_x_tools//go/analysis:go_default_library", + "@org_golang_x_tools//go/analysis/passes/inspect:go_default_library", + "@org_golang_x_tools//go/ast/inspector:go_default_library", + ], +) + +go_tool_library( + name = "go_tool_library", + srcs = ["analyzer.go"], + importpath = "github.com/prysmaticlabs/prysm/tools/analyzers/properpermissions", + visibility = ["//visibility:public"], + deps = [ + "@org_golang_x_tools//go/analysis:go_tool_library", + "@org_golang_x_tools//go/analysis/passes/inspect:go_tool_library", + "@org_golang_x_tools//go/ast/inspector:go_tool_library", + ], +) + +# gazelle:exclude analyzer_test.go diff --git a/tools/analyzers/properpermissions/analyzer.go b/tools/analyzers/properpermissions/analyzer.go new file mode 100644 index 000000000..534f6de00 --- /dev/null +++ b/tools/analyzers/properpermissions/analyzer.go @@ -0,0 +1,104 @@ +// Package properpermissions implements a static analyzer to ensure that Prysm does not +// use ioutil.MkdirAll or os.WriteFile as they are unsafe when it comes to guaranteeing +// file permissions and not overriding existing permissions. Instead, users are warned +// to utilize shared/fileutil as the canonical solution. +package properpermissions + +import ( + "errors" + "fmt" + "go/ast" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" +) + +// Doc explaining the tool. +const Doc = "Tool to enforce usage of Prysm's internal file-writing utils instead of os.MkdirAll or ioutil.WriteFile" + +var ( + errUnsafePackage = errors.New( + "os and ioutil dir and file writing functions are not permissions-safe, use shared/fileutil", + ) + disallowedFns = []string{"MkdirAll", "WriteFile"} +) + +// Analyzer runs static analysis. +var Analyzer = &analysis.Analyzer{ + Name: "properpermissions", + Doc: Doc, + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, +} + +func run(pass *analysis.Pass) (interface{}, error) { + inspect, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + if !ok { + return nil, errors.New("analyzer is not type *inspector.Inspector") + } + + nodeFilter := []ast.Node{ + (*ast.File)(nil), + (*ast.ImportSpec)(nil), + (*ast.CallExpr)(nil), + } + + aliases := make(map[string]string) + + inspect.Preorder(nodeFilter, func(node ast.Node) { + switch stmt := node.(type) { + case *ast.File: + // Reset aliases (per file). + aliases = make(map[string]string) + case *ast.ImportSpec: + // Collect aliases. + pkg := stmt.Path.Value + if pkg == "\"os\"" { + if stmt.Name != nil { + aliases[stmt.Name.Name] = pkg + } else { + aliases["os"] = pkg + } + } + if pkg == "\"io/ioutil\"" { + if stmt.Name != nil { + aliases[stmt.Name.Name] = pkg + } else { + aliases["ioutil"] = pkg + } + } + case *ast.CallExpr: + // Check if any of disallowed functions have been used. + for alias, pkg := range aliases { + for _, fn := range disallowedFns { + if isPkgDot(stmt.Fun, alias, fn) { + pass.Reportf( + node.Pos(), + fmt.Sprintf( + "%v: %s.%s() (from %s)", + errUnsafePackage, + alias, + fn, + pkg, + ), + ) + } + } + } + } + }) + + return nil, nil +} + +func isPkgDot(expr ast.Expr, pkg, name string) bool { + sel, ok := expr.(*ast.SelectorExpr) + res := ok && isIdent(sel.X, pkg) && isIdent(sel.Sel, name) + return res +} + +func isIdent(expr ast.Expr, ident string) bool { + id, ok := expr.(*ast.Ident) + return ok && id.Name == ident +} diff --git a/tools/analyzers/properpermissions/analyzer_test.go b/tools/analyzers/properpermissions/analyzer_test.go new file mode 100644 index 000000000..fc90cb8f4 --- /dev/null +++ b/tools/analyzers/properpermissions/analyzer_test.go @@ -0,0 +1,11 @@ +package properpermissions + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" +) + +func TestAnalyzer(t *testing.T) { + analysistest.Run(t, analysistest.TestData(), Analyzer) +} diff --git a/tools/analyzers/properpermissions/testdata/BUILD.bazel b/tools/analyzers/properpermissions/testdata/BUILD.bazel new file mode 100644 index 000000000..c36798bf4 --- /dev/null +++ b/tools/analyzers/properpermissions/testdata/BUILD.bazel @@ -0,0 +1,11 @@ +load("@prysm//tools/go:def.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = [ + "custom_imports.go", + "regular_imports.go", + ], + importpath = "github.com/prysmaticlabs/prysm/tools/analyzers/properpermissions/testdata", + visibility = ["//visibility:public"], +) diff --git a/tools/analyzers/properpermissions/testdata/custom_imports.go b/tools/analyzers/properpermissions/testdata/custom_imports.go new file mode 100644 index 000000000..7d6145ed8 --- /dev/null +++ b/tools/analyzers/properpermissions/testdata/custom_imports.go @@ -0,0 +1,18 @@ +package testdata + +import ( + "crypto/rand" + "fmt" + ioAlias "io/ioutil" + "math/big" + osAlias "os" + "path/filepath" +) + +func UseAliasedPackages() { + randPath, _ := rand.Int(rand.Reader, big.NewInt(1000000)) + p := filepath.Join(tempDir(), fmt.Sprintf("/%d", randPath)) + _ = osAlias.MkdirAll(p, osAlias.ModePerm) // want "os and ioutil dir and file writing functions are not permissions-safe, use shared/fileutil" + someFile := filepath.Join(p, "some.txt") + _ = ioAlias.WriteFile(someFile, []byte("hello"), osAlias.ModePerm) // want "os and ioutil dir and file writing functions are not permissions-safe, use shared/fileutil" +} diff --git a/tools/analyzers/properpermissions/testdata/regular_imports.go b/tools/analyzers/properpermissions/testdata/regular_imports.go new file mode 100644 index 000000000..0e9cbb3f4 --- /dev/null +++ b/tools/analyzers/properpermissions/testdata/regular_imports.go @@ -0,0 +1,26 @@ +package testdata + +import ( + "crypto/rand" + "fmt" + "io/ioutil" + "math/big" + "os" + "path/filepath" +) + +func tempDir() string { + d := os.Getenv("TEST_TMPDIR") + if d == "" { + return os.TempDir() + } + return d +} + +func UseOsMkdirAllAndWriteFile() { + randPath, _ := rand.Int(rand.Reader, big.NewInt(1000000)) + p := filepath.Join(tempDir(), fmt.Sprintf("/%d", randPath)) + _ = os.MkdirAll(p, os.ModePerm) // want "os and ioutil dir and file writing functions are not permissions-safe, use shared/fileutil" + someFile := filepath.Join(p, "some.txt") + _ = ioutil.WriteFile(someFile, []byte("hello"), os.ModePerm) // want "os and ioutil dir and file writing functions are not permissions-safe, use shared/fileutil" +} diff --git a/tools/beacon-fuzz/BUILD.bazel b/tools/beacon-fuzz/BUILD.bazel index 5e111f318..0454f03f9 100644 --- a/tools/beacon-fuzz/BUILD.bazel +++ b/tools/beacon-fuzz/BUILD.bazel @@ -5,6 +5,9 @@ load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library") go_library( name = "go_default_library", srcs = ["main.go"], + deps = [ + "//shared/fileutil:go_default_library", + ], importpath = "github.com/prysmaticlabs/prysm/tools/beacon-fuzz", visibility = ["//visibility:private"], ) diff --git a/tools/beacon-fuzz/main.go b/tools/beacon-fuzz/main.go index c3e9b823f..ccfcbfcba 100644 --- a/tools/beacon-fuzz/main.go +++ b/tools/beacon-fuzz/main.go @@ -9,6 +9,8 @@ import ( "path/filepath" "strconv" "text/template" + + "github.com/prysmaticlabs/prysm/shared/fileutil" ) var ( @@ -54,7 +56,7 @@ func main() { } res := execTmpl(tpl, input{Package: "testing", MapStr: sszBytesToMapStr(m)}) - if err := ioutil.WriteFile(*output, res.Bytes(), 0644); err != nil { + if err := fileutil.WriteFile(*output, res.Bytes()); err != nil { panic(err) } } diff --git a/tools/benchmark-files-gen/BUILD.bazel b/tools/benchmark-files-gen/BUILD.bazel index d7b7ef130..573c29731 100644 --- a/tools/benchmark-files-gen/BUILD.bazel +++ b/tools/benchmark-files-gen/BUILD.bazel @@ -13,6 +13,7 @@ go_library( "//beacon-chain/state:go_default_library", "//proto/beacon/p2p/v1:go_default_library", "//shared/benchutil:go_default_library", + "//shared/fileutil:go_default_library", "//shared/interop:go_default_library", "//shared/params:go_default_library", "//shared/testutil:go_default_library", diff --git a/tools/benchmark-files-gen/main.go b/tools/benchmark-files-gen/main.go index c288b790c..f905ee8a4 100644 --- a/tools/benchmark-files-gen/main.go +++ b/tools/benchmark-files-gen/main.go @@ -9,6 +9,7 @@ import ( "path" stateTrie "github.com/prysmaticlabs/prysm/beacon-chain/state" + "github.com/prysmaticlabs/prysm/shared/fileutil" "github.com/pkg/errors" ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1" @@ -44,7 +45,7 @@ func main() { } } - if err := os.MkdirAll(*outputDir, os.ModePerm); err != nil { + if err := fileutil.MkdirAll(*outputDir); err != nil { log.Fatal(err) } @@ -77,7 +78,7 @@ func generateGenesisBeaconState() error { if err != nil { return err } - return ioutil.WriteFile(path.Join(*outputDir, benchutil.GenesisFileName), beaconBytes, 0644) + return fileutil.WriteFile(path.Join(*outputDir, benchutil.GenesisFileName), beaconBytes) } func generateMarshalledFullStateAndBlock() error { @@ -150,7 +151,7 @@ func generateMarshalledFullStateAndBlock() error { if err != nil { return err } - if err := ioutil.WriteFile(path.Join(*outputDir, benchutil.BState1EpochFileName), beaconBytes, 0644); err != nil { + if err := fileutil.WriteFile(path.Join(*outputDir, benchutil.BState1EpochFileName), beaconBytes); err != nil { return err } @@ -165,7 +166,7 @@ func generateMarshalledFullStateAndBlock() error { return err } - return ioutil.WriteFile(path.Join(*outputDir, benchutil.FullBlockFileName), blockBytes, 0644) + return fileutil.WriteFile(path.Join(*outputDir, benchutil.FullBlockFileName), blockBytes) } func generate2FullEpochState() error { @@ -200,7 +201,7 @@ func generate2FullEpochState() error { return err } - return ioutil.WriteFile(path.Join(*outputDir, benchutil.BState2EpochFileName), beaconBytes, 0644) + return fileutil.WriteFile(path.Join(*outputDir, benchutil.BState2EpochFileName), beaconBytes) } func genesisBeaconState() (*stateTrie.BeaconState, error) { diff --git a/tools/enr-calculator/BUILD.bazel b/tools/enr-calculator/BUILD.bazel index 054f9f1ee..108a59d61 100644 --- a/tools/enr-calculator/BUILD.bazel +++ b/tools/enr-calculator/BUILD.bazel @@ -10,6 +10,7 @@ go_library( importpath = "github.com/prysmaticlabs/prysm/tools/enr-calculator", visibility = ["//visibility:private"], deps = [ + "//shared/fileutil:go_default_library", "//shared/maxprocs:go_default_library", "@com_github_ethereum_go_ethereum//p2p/enode:go_default_library", "@com_github_ethereum_go_ethereum//p2p/enr:go_default_library", @@ -41,6 +42,7 @@ go_image( "@com_github_ethereum_go_ethereum//p2p/enr:go_default_library", "@com_github_libp2p_go_libp2p_core//crypto:go_default_library", "@com_github_sirupsen_logrus//:go_default_library", + "//shared/fileutil:go_default_library", "//shared/maxprocs:go_default_library", ], ) diff --git a/tools/enr-calculator/main.go b/tools/enr-calculator/main.go index 81e920e9a..6a4bfdc2f 100644 --- a/tools/enr-calculator/main.go +++ b/tools/enr-calculator/main.go @@ -6,12 +6,12 @@ import ( "crypto/ecdsa" "encoding/hex" "flag" - "io/ioutil" "net" "github.com/ethereum/go-ethereum/p2p/enode" "github.com/ethereum/go-ethereum/p2p/enr" "github.com/libp2p/go-libp2p-core/crypto" + "github.com/prysmaticlabs/prysm/shared/fileutil" _ "github.com/prysmaticlabs/prysm/shared/maxprocs" log "github.com/sirupsen/logrus" ) @@ -68,7 +68,7 @@ func main() { log.Info(localNode.Node().String()) if *outfile != "" { - err := ioutil.WriteFile(*outfile, []byte(localNode.Node().String()), 0644) + err := fileutil.WriteFile(*outfile, []byte(localNode.Node().String())) if err != nil { panic(err) } diff --git a/tools/genesis-state-gen/main.go b/tools/genesis-state-gen/main.go index 6173e0e09..54d1e4f4e 100644 --- a/tools/genesis-state-gen/main.go +++ b/tools/genesis-state-gen/main.go @@ -96,7 +96,7 @@ func main() { log.Printf("Could not ssz marshal the genesis beacon state: %v", err) return } - if err := ioutil.WriteFile(*sszOutputFile, encodedState, 0644); err != nil { + if err := fileutil.WriteFile(*sszOutputFile, encodedState); err != nil { log.Printf("Could not write encoded genesis beacon state to file: %v", err) return } @@ -108,7 +108,7 @@ func main() { log.Printf("Could not yaml marshal the genesis beacon state: %v", err) return } - if err := ioutil.WriteFile(*yamlOutputFile, encodedState, 0644); err != nil { + if err := fileutil.WriteFile(*yamlOutputFile, encodedState); err != nil { log.Printf("Could not write encoded genesis beacon state to file: %v", err) return } @@ -120,7 +120,7 @@ func main() { log.Printf("Could not json marshal the genesis beacon state: %v", err) return } - if err := ioutil.WriteFile(*jsonOutputFile, encodedState, 0644); err != nil { + if err := fileutil.WriteFile(*jsonOutputFile, encodedState); err != nil { log.Printf("Could not write encoded genesis beacon state to file: %v", err) return } diff --git a/tools/interop/export-genesis/BUILD.bazel b/tools/interop/export-genesis/BUILD.bazel index c4917a27c..86a1d42b9 100644 --- a/tools/interop/export-genesis/BUILD.bazel +++ b/tools/interop/export-genesis/BUILD.bazel @@ -9,6 +9,7 @@ go_library( deps = [ "//beacon-chain/cache:go_default_library", "//beacon-chain/db:go_default_library", + "//shared/fileutil:go_default_library", ], ) diff --git a/tools/interop/export-genesis/main.go b/tools/interop/export-genesis/main.go index a2632166a..94cf170ed 100644 --- a/tools/interop/export-genesis/main.go +++ b/tools/interop/export-genesis/main.go @@ -3,11 +3,11 @@ package main import ( "context" "fmt" - "io/ioutil" "os" "github.com/prysmaticlabs/prysm/beacon-chain/cache" "github.com/prysmaticlabs/prysm/beacon-chain/db" + "github.com/prysmaticlabs/prysm/shared/fileutil" ) // A basic tool to extract genesis.ssz from existing beaconchain.db. @@ -41,7 +41,7 @@ func main() { if err != nil { panic(err) } - if err := ioutil.WriteFile(os.Args[2], b, 0644); err != nil { + if err := fileutil.WriteFile(os.Args[2], b); err != nil { panic(err) } fmt.Println("done") diff --git a/tools/keystores/BUILD.bazel b/tools/keystores/BUILD.bazel index 99ccd1680..3dfc8499a 100644 --- a/tools/keystores/BUILD.bazel +++ b/tools/keystores/BUILD.bazel @@ -9,7 +9,6 @@ go_library( deps = [ "//shared/bls:go_default_library", "//shared/fileutil:go_default_library", - "//shared/params:go_default_library", "//shared/promptutil:go_default_library", "//validator/keymanager:go_default_library", "@com_github_google_uuid//:go_default_library", diff --git a/tools/keystores/main.go b/tools/keystores/main.go index 1766455a9..fac96b9a3 100644 --- a/tools/keystores/main.go +++ b/tools/keystores/main.go @@ -19,7 +19,6 @@ import ( "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" "github.com/prysmaticlabs/prysm/validator/keymanager" "github.com/urfave/cli/v2" @@ -209,7 +208,7 @@ func encrypt(cliCtx *cli.Context) error { if err != nil { return errors.Wrap(err, "could not json marshal keystore") } - if err := ioutil.WriteFile(fullPath, encodedFile, params.BeaconIoConfig().ReadWritePermissions); err != nil { + if err := fileutil.WriteFile(fullPath, encodedFile); err != nil { return errors.Wrapf(err, "could not write file at path: %s", fullPath) } fmt.Printf( diff --git a/tools/update-genesis-time/BUILD.bazel b/tools/update-genesis-time/BUILD.bazel index 5ae370efe..35d30dc00 100644 --- a/tools/update-genesis-time/BUILD.bazel +++ b/tools/update-genesis-time/BUILD.bazel @@ -8,6 +8,7 @@ go_library( visibility = ["//visibility:public"], deps = [ "//proto/beacon/p2p/v1:go_default_library", + "//shared/fileutil:go_default_library", "//shared/timeutils:go_default_library", "@com_github_prysmaticlabs_go_ssz//:go_default_library", ], diff --git a/tools/update-genesis-time/main.go b/tools/update-genesis-time/main.go index 75755f836..cfe2b939e 100644 --- a/tools/update-genesis-time/main.go +++ b/tools/update-genesis-time/main.go @@ -7,6 +7,7 @@ import ( "github.com/prysmaticlabs/go-ssz" pb "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1" + "github.com/prysmaticlabs/prysm/shared/fileutil" "github.com/prysmaticlabs/prysm/shared/timeutils" ) @@ -36,7 +37,7 @@ func main() { if err != nil { log.Fatalf("Could not ssz marshal the beacon state: %v", err) } - if err := ioutil.WriteFile(*inputSSZState, encodedState, 0644); err != nil { + if err := fileutil.WriteFile(*inputSSZState, encodedState); err != nil { log.Fatalf("Could not write encoded beacon state to file: %v", err) } log.Printf("Done writing to %s", *inputSSZState) diff --git a/validator/accounts/accounts_backup.go b/validator/accounts/accounts_backup.go index 90e0660ee..d14003b9e 100644 --- a/validator/accounts/accounts_backup.go +++ b/validator/accounts/accounts_backup.go @@ -204,7 +204,7 @@ func zipKeystoresToOutputDir(keystoresToBackup []*keymanager.Keystore, outputDir if len(keystoresToBackup) == 0 { return errors.New("nothing to backup") } - if err := os.MkdirAll(outputDir, os.ModePerm); err != nil { + if err := fileutil.MkdirAll(outputDir); err != nil { return errors.Wrapf(err, "could not create directory at path: %s", outputDir) } // Marshal and zip all keystore files together and write the zip file diff --git a/validator/accounts/accounts_backup_test.go b/validator/accounts/accounts_backup_test.go index 2ffa8a529..1c8059a3e 100644 --- a/validator/accounts/accounts_backup_test.go +++ b/validator/accounts/accounts_backup_test.go @@ -35,7 +35,7 @@ func TestBackupAccounts_Noninteractive_Derived(t *testing.T) { require.NoError(t, err, "Could not generate random file path") // Write a directory where we will backup accounts to. backupDir := filepath.Join(testutil.TempDir(), fmt.Sprintf("/%d", randPath), "backupDir") - require.NoError(t, os.MkdirAll(backupDir, os.ModePerm)) + require.NoError(t, os.MkdirAll(backupDir, params.BeaconIoConfig().ReadWriteExecutePermissions)) t.Cleanup(func() { require.NoError(t, os.RemoveAll(backupDir), "Failed to remove directory") }) @@ -148,11 +148,11 @@ func TestBackupAccounts_Noninteractive_Imported(t *testing.T) { require.NoError(t, err, "Could not generate random file path") // Write a directory where we will import keys from. keysDir := filepath.Join(testutil.TempDir(), fmt.Sprintf("/%d", randPath), "keysDir") - require.NoError(t, os.MkdirAll(keysDir, os.ModePerm)) + require.NoError(t, os.MkdirAll(keysDir, params.BeaconIoConfig().ReadWriteExecutePermissions)) // Write a directory where we will backup accounts to. backupDir := filepath.Join(testutil.TempDir(), fmt.Sprintf("/%d", randPath), "backupDir") - require.NoError(t, os.MkdirAll(backupDir, os.ModePerm)) + require.NoError(t, os.MkdirAll(backupDir, params.BeaconIoConfig().ReadWriteExecutePermissions)) t.Cleanup(func() { require.NoError(t, os.RemoveAll(keysDir), "Failed to remove directory") require.NoError(t, os.RemoveAll(backupDir), "Failed to remove directory") diff --git a/validator/accounts/wallet/BUILD.bazel b/validator/accounts/wallet/BUILD.bazel index 0c7d0bf4f..291e3d56b 100644 --- a/validator/accounts/wallet/BUILD.bazel +++ b/validator/accounts/wallet/BUILD.bazel @@ -8,7 +8,6 @@ go_library( visibility = ["//validator:__subpackages__"], deps = [ "//shared/fileutil:go_default_library", - "//shared/params:go_default_library", "//shared/promptutil:go_default_library", "//validator/accounts/iface:go_default_library", "//validator/accounts/prompt:go_default_library", diff --git a/validator/accounts/wallet/wallet.go b/validator/accounts/wallet/wallet.go index 38ca708b3..cb19d390f 100644 --- a/validator/accounts/wallet/wallet.go +++ b/validator/accounts/wallet/wallet.go @@ -12,7 +12,6 @@ import ( "github.com/gofrs/flock" "github.com/pkg/errors" "github.com/prysmaticlabs/prysm/shared/fileutil" - "github.com/prysmaticlabs/prysm/shared/params" "github.com/prysmaticlabs/prysm/shared/promptutil" "github.com/prysmaticlabs/prysm/validator/accounts/iface" "github.com/prysmaticlabs/prysm/validator/accounts/prompt" @@ -236,7 +235,7 @@ func OpenWallet(_ context.Context, cfg *Config) (*Wallet, error) { // SaveWallet persists the wallet's directories to disk. func (w *Wallet) SaveWallet() error { - if err := os.MkdirAll(w.accountsPath, DirectoryPermissions); err != nil { + if err := fileutil.MkdirAll(w.accountsPath); err != nil { return errors.Wrap(err, "could not create wallet directory") } return nil @@ -316,11 +315,17 @@ func (w *Wallet) InitializeKeymanager( // WriteFileAtPath within the wallet directory given the desired path, filename, and raw data. func (w *Wallet) WriteFileAtPath(_ context.Context, filePath, fileName string, data []byte) error { accountPath := filepath.Join(w.accountsPath, filePath) - if err := os.MkdirAll(accountPath, os.ModePerm); err != nil { - return errors.Wrapf(err, "could not create path: %s", accountPath) + hasDir, err := fileutil.HasDir(accountPath) + if err != nil { + return err + } + if !hasDir { + if err := fileutil.MkdirAll(accountPath); err != nil { + return errors.Wrapf(err, "could not create path: %s", accountPath) + } } fullPath := filepath.Join(accountPath, fileName) - if err := ioutil.WriteFile(fullPath, data, params.BeaconIoConfig().ReadWritePermissions); err != nil { + if err := fileutil.WriteFile(fullPath, data); err != nil { return errors.Wrapf(err, "could not write %s", filePath) } log.WithFields(logrus.Fields{ @@ -333,8 +338,14 @@ func (w *Wallet) WriteFileAtPath(_ context.Context, filePath, fileName string, d // ReadFileAtPath within the wallet directory given the desired path and filename. func (w *Wallet) ReadFileAtPath(_ context.Context, filePath, fileName string) ([]byte, error) { accountPath := filepath.Join(w.accountsPath, filePath) - if err := os.MkdirAll(accountPath, os.ModePerm); err != nil { - return nil, errors.Wrapf(err, "could not create path: %s", accountPath) + hasDir, err := fileutil.HasDir(accountPath) + if err != nil { + return nil, err + } + if !hasDir { + if err := fileutil.MkdirAll(accountPath); err != nil { + return nil, errors.Wrapf(err, "could not create path: %s", accountPath) + } } fullPath := filepath.Join(accountPath, fileName) matches, err := filepath.Glob(fullPath) @@ -355,7 +366,7 @@ func (w *Wallet) ReadFileAtPath(_ context.Context, filePath, fileName string) ([ // with a regex pattern. func (w *Wallet) FileNameAtPath(_ context.Context, filePath, fileName string) (string, error) { accountPath := filepath.Join(w.accountsPath, filePath) - if err := os.MkdirAll(accountPath, os.ModePerm); err != nil { + if err := fileutil.MkdirAll(accountPath); err != nil { return "", errors.Wrapf(err, "could not create path: %s", accountPath) } fullPath := filepath.Join(accountPath, fileName) @@ -411,7 +422,7 @@ func (w *Wallet) UnlockWalletConfigFile() error { func (w *Wallet) WriteKeymanagerConfigToDisk(_ context.Context, encoded []byte) error { configFilePath := filepath.Join(w.accountsPath, KeymanagerConfigFileName) // Write the config file to disk. - if err := ioutil.WriteFile(configFilePath, encoded, params.BeaconIoConfig().ReadWritePermissions); err != nil { + if err := fileutil.WriteFile(configFilePath, encoded); err != nil { return errors.Wrapf(err, "could not write %s", configFilePath) } log.WithField("configFilePath", configFilePath).Debug("Wrote keymanager config file to disk") @@ -433,7 +444,7 @@ func (w *Wallet) ReadEncryptedSeedFromDisk(_ context.Context) (io.ReadCloser, er func (w *Wallet) WriteEncryptedSeedToDisk(_ context.Context, encoded []byte) error { seedFilePath := filepath.Join(w.accountsPath, derived.EncryptedSeedFileName) // Write the config file to disk. - if err := ioutil.WriteFile(seedFilePath, encoded, params.BeaconIoConfig().ReadWritePermissions); err != nil { + if err := fileutil.WriteFile(seedFilePath, encoded); err != nil { return errors.Wrapf(err, "could not write %s", seedFilePath) } log.WithField("seedFilePath", seedFilePath).Debug("Wrote wallet encrypted seed file to disk") diff --git a/validator/accounts/wallet_create_test.go b/validator/accounts/wallet_create_test.go index 76658a76a..f2af243c8 100644 --- a/validator/accounts/wallet_create_test.go +++ b/validator/accounts/wallet_create_test.go @@ -14,6 +14,7 @@ import ( "testing" "github.com/pkg/errors" + "github.com/prysmaticlabs/prysm/shared/params" "github.com/prysmaticlabs/prysm/shared/testutil" "github.com/prysmaticlabs/prysm/shared/testutil/assert" "github.com/prysmaticlabs/prysm/shared/testutil/require" @@ -107,7 +108,7 @@ func setupWalletAndPasswordsDir(t testing.TB) (string, string, string) { passwordsDir := filepath.Join(testutil.TempDir(), fmt.Sprintf("/%d", randPath), "passwords") require.NoError(t, os.RemoveAll(passwordsDir), "Failed to remove directory") passwordFileDir := filepath.Join(testutil.TempDir(), fmt.Sprintf("/%d", randPath), "passwordFile") - require.NoError(t, os.MkdirAll(passwordFileDir, os.ModePerm)) + require.NoError(t, os.MkdirAll(passwordFileDir, params.BeaconIoConfig().ReadWriteExecutePermissions)) passwordFilePath := filepath.Join(passwordFileDir, passwordFileName) require.NoError(t, ioutil.WriteFile(passwordFilePath, []byte(password), os.ModePerm)) t.Cleanup(func() { diff --git a/validator/db/kv/BUILD.bazel b/validator/db/kv/BUILD.bazel index fec739f5c..ae0172aca 100644 --- a/validator/db/kv/BUILD.bazel +++ b/validator/db/kv/BUILD.bazel @@ -18,6 +18,7 @@ go_library( "//beacon-chain/core/helpers:go_default_library", "//proto/slashing:go_default_library", "//shared/bytesutil:go_default_library", + "//shared/fileutil:go_default_library", "//shared/params:go_default_library", "@com_github_gogo_protobuf//proto:go_default_library", "@com_github_pkg_errors//:go_default_library", diff --git a/validator/db/kv/db.go b/validator/db/kv/db.go index 7ad4e303e..aa8294155 100644 --- a/validator/db/kv/db.go +++ b/validator/db/kv/db.go @@ -6,6 +6,7 @@ import ( "path/filepath" "github.com/pkg/errors" + "github.com/prysmaticlabs/prysm/shared/fileutil" "github.com/prysmaticlabs/prysm/shared/params" bolt "go.etcd.io/bbolt" ) @@ -61,9 +62,15 @@ func createBuckets(tx *bolt.Tx, buckets ...[]byte) error { // path specified, creates the kv-buckets based on the schema, and stores // an open connection db object as a property of the Store struct. func NewKVStore(dirPath string, pubKeys [][48]byte) (*Store, error) { - if err := os.MkdirAll(dirPath, params.BeaconIoConfig().ReadWriteExecutePermissions); err != nil { + hasDir, err := fileutil.HasDir(dirPath) + if err != nil { return nil, err } + if !hasDir { + if err := fileutil.MkdirAll(dirPath); err != nil { + return nil, err + } + } datafile := filepath.Join(dirPath, ProtectionDbFileName) boltDB, err := bolt.Open(datafile, params.BeaconIoConfig().ReadWritePermissions, &bolt.Options{Timeout: params.BeaconIoConfig().BoltTimeout}) if err != nil { diff --git a/validator/rpc/BUILD.bazel b/validator/rpc/BUILD.bazel index 3fc11df82..44d024568 100644 --- a/validator/rpc/BUILD.bazel +++ b/validator/rpc/BUILD.bazel @@ -19,7 +19,6 @@ go_library( "//shared/event:go_default_library", "//shared/fileutil:go_default_library", "//shared/pagination:go_default_library", - "//shared/params:go_default_library", "//shared/petnames:go_default_library", "//shared/promptutil:go_default_library", "//shared/rand:go_default_library", diff --git a/validator/rpc/auth.go b/validator/rpc/auth.go index fd592a7aa..05ebe3e7b 100644 --- a/validator/rpc/auth.go +++ b/validator/rpc/auth.go @@ -2,8 +2,6 @@ package rpc import ( "context" - "io/ioutil" - "os" "path/filepath" "strings" "time" @@ -13,7 +11,6 @@ import ( "github.com/pkg/errors" pb "github.com/prysmaticlabs/prysm/proto/validator/accounts/v2" "github.com/prysmaticlabs/prysm/shared/fileutil" - "github.com/prysmaticlabs/prysm/shared/params" "github.com/prysmaticlabs/prysm/shared/promptutil" "github.com/prysmaticlabs/prysm/shared/timeutils" "golang.org/x/crypto/bcrypt" @@ -54,7 +51,7 @@ func (s *Server) Signup(ctx context.Context, req *pb.AuthRequest) (*pb.AuthRespo return nil, status.Error(codes.FailedPrecondition, "Could not check if wallet directory exists") } if !hasDir { - if err := os.MkdirAll(walletDir, os.ModePerm); err != nil { + if err := fileutil.MkdirAll(walletDir); err != nil { return nil, status.Errorf(codes.Internal, "could not write directory %s to disk: %v", walletDir, err) } } @@ -151,7 +148,7 @@ func (s *Server) SaveHashedPassword(password string) error { return errors.Wrap(err, "could not generate hashed password") } hashFilePath := filepath.Join(s.walletDir, HashedRPCPassword) - return ioutil.WriteFile(hashFilePath, hashedPassword, params.BeaconIoConfig().ReadWritePermissions) + return fileutil.WriteFile(hashFilePath, hashedPassword) } // Interval in which we should check if a user has not yet used the RPC Signup endpoint