Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: unsafe reset all #1196

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions gno.land/cmd/gnoland/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,16 @@
$> gnoland start

Afterward, you can interact with [`gnokey`](../gnokey) or launch a [`gnoweb`](../gnoweb) interface.


## Reset `gnoland` node back to genesis state. It's only suitable for testnets.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Referring to #1201, to focus on a production-ready gnoland binary and maintain separate helper tools, we could simply add a contribs/gnoland-resetall, or a contribs/gnolandtools with an 'unsafe-reset-all' subcommand.


$> gnoland unsafe-reset-all

It removes the database and validator state but leaves the genesis.json and config.toml files unchanged.

The `unsafe-reset-all` command is labeled "unsafe" because:

1. It irreversibly deletes all node data, risking data loss.
2. It may lead to double signing or chain forks in production
3. It resets the `priv_validator_state.json`, and can cause network disruption if uncoordinated.
15 changes: 15 additions & 0 deletions gno.land/cmd/gnoland/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package main

import (
"context"
"fmt"
"os"
)

func main() {
rootCmd := newRootCmd()
if err := rootCmd.ParseAndRun(context.Background(), os.Args[1:]); err != nil {
_, _ = fmt.Fprintf(os.Stderr, "%+v\n", err)
os.Exit(1)
}

Check warning on line 14 in gno.land/cmd/gnoland/main.go

View check run for this annotation

Codecov / codecov/patch

gno.land/cmd/gnoland/main.go#L9-L14

Added lines #L9 - L14 were not covered by tests
}
116 changes: 116 additions & 0 deletions gno.land/cmd/gnoland/mockio.go
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm against modifying global variables like os.Stdin for testing purposes. I think commands.IO works well, allows us for mock tests, and doesn't change anything in terms of security, on top of everything because this code is in a main package which cannot be imported.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Retaining os.Stdin wrapped in commands.IO within the gno.land runtime presents a security vulnerability. Here are the concerns.

  • Future Misuse: If future developers, unaware of the security implications, start utilizing commands.IO's os.Stdin to pass tests or for any other reasons. This could lead to vulnerabilities. Code that was originally not designed to interact with os.Stdin might be altered to do so, creating an attack vector.

  • Malicious IPC: A malicious entity could exploit this implementation to inject data into the gno.land node runtime through IPC ( Interprocess Communication)

  • Trust Concerns: Node operators could have trust concerns in the system's integrity, fearing potential injection attacks through IPC due to the presence of os.Stdin in the codebase.

Copy link
Member

@moul moul Nov 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I agree with your argument, I believe it reinforces the original usage rather than the recent change. I see this change as a regression in terms of flexibility and security. Could you provide an example where your change enhances security?

Edit: my review covers this pull request: #1333, where the security and flexibility issues are clearer IMO. Please look at my latest comment here: https://github.com/gnolang/gno/pull/1196/files#r1385496885, where I suggest going back to the io.Commands approach but with a new read-only interface (without os.Stdin). This would help ensure readonly commands remain so by preventing accidental os.Stdin inputs.

Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
package main

import (
"bytes"
"fmt"
"io"
"log"
"os"
)

// This is for testing purposes only.
// For mocking tests, we redirect os.Stdin so that we don't need to pass commands.IO,
// which includes os.Stdin, to all the server commands. Exposing os.Stdin in a blockchain node is not safe.
// This replaces the global variable and should not be used in concurrent tests. It's intended to simulate CLI input.
// We purposely avoid using a mutex to prevent giving the wrong impression that it's suitable for parallel tests.

type MockStdin struct {
origStdout *os.File
stdoutReader *os.File

outCh chan []byte

origStdin *os.File
stdinWriter *os.File
}

func NewMockStdin(input string) (*MockStdin, error) {
// Pipe for stdin. w ( stdinWriter ) -> r (stdin)
stdinReader, stdinWriter, err := os.Pipe()
if err != nil {
return nil, err
}

Check warning on line 32 in gno.land/cmd/gnoland/mockio.go

View check run for this annotation

Codecov / codecov/patch

gno.land/cmd/gnoland/mockio.go#L31-L32

Added lines #L31 - L32 were not covered by tests

// Pipe for stdout. w( stdout ) -> r (stdoutReader)
stdoutReader, stdoutWriter, err := os.Pipe()
if err != nil {
return nil, err
}

Check warning on line 38 in gno.land/cmd/gnoland/mockio.go

View check run for this annotation

Codecov / codecov/patch

gno.land/cmd/gnoland/mockio.go#L37-L38

Added lines #L37 - L38 were not covered by tests

origStdin := os.Stdin
os.Stdin = stdinReader

_, err = stdinWriter.Write([]byte(input))
if err != nil {
stdinWriter.Close()
os.Stdin = origStdin
return nil, err
}

Check warning on line 48 in gno.land/cmd/gnoland/mockio.go

View check run for this annotation

Codecov / codecov/patch

gno.land/cmd/gnoland/mockio.go#L45-L48

Added lines #L45 - L48 were not covered by tests

origStdout := os.Stdout
os.Stdout = stdoutWriter

outCh := make(chan []byte)

// This goroutine reads stdout into a buffer in the background.
go func() {
var b bytes.Buffer
if _, err := io.Copy(&b, stdoutReader); err != nil {
log.Println(err)
}

Check warning on line 60 in gno.land/cmd/gnoland/mockio.go

View check run for this annotation

Codecov / codecov/patch

gno.land/cmd/gnoland/mockio.go#L59-L60

Added lines #L59 - L60 were not covered by tests
outCh <- b.Bytes()
}()

return &MockStdin{
origStdout: origStdout,
stdoutReader: stdoutReader,
outCh: outCh,
origStdin: origStdin,
stdinWriter: stdinWriter,
}, nil
}

// ReadAndRestore collects all captured stdout and returns it; it also restores
// os.Stdin and os.Stdout to their original values.
func (i *MockStdin) ReadAndClose() ([]byte, error) {
if i.stdoutReader == nil {
return nil, fmt.Errorf("ReadAndRestore from closed FakeStdio")
}

Check warning on line 78 in gno.land/cmd/gnoland/mockio.go

View check run for this annotation

Codecov / codecov/patch

gno.land/cmd/gnoland/mockio.go#L77-L78

Added lines #L77 - L78 were not covered by tests

// Close the writer side of the faked stdout pipe. This signals to the
// background goroutine that it should exit.
os.Stdout.Close()
out := <-i.outCh

os.Stdout = i.origStdout
os.Stdin = i.origStdin

if i.stdoutReader != nil {
i.stdoutReader.Close()
i.stdoutReader = nil
}

if i.stdinWriter != nil {
i.stdinWriter.Close()
i.stdinWriter = nil
}

return out, nil
}

// Call this in a defer function to restore and close os.Stdout and os.Stdin.
// This acts as a safeguard.
func (i *MockStdin) Close() {
os.Stdout = i.origStdout
os.Stdin = i.origStdin

if i.stdoutReader != nil {
i.stdoutReader.Close()
i.stdoutReader = nil
}

Check warning on line 110 in gno.land/cmd/gnoland/mockio.go

View check run for this annotation

Codecov / codecov/patch

gno.land/cmd/gnoland/mockio.go#L108-L110

Added lines #L108 - L110 were not covered by tests

if i.stdinWriter != nil {
i.stdinWriter.Close()
i.stdinWriter = nil
}

Check warning on line 115 in gno.land/cmd/gnoland/mockio.go

View check run for this annotation

Codecov / codecov/patch

gno.land/cmd/gnoland/mockio.go#L113-L115

Added lines #L113 - L115 were not covered by tests
}
134 changes: 134 additions & 0 deletions gno.land/cmd/gnoland/reset.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
package main

import (
"context"
"flag"
"os"
"path/filepath"

"github.com/gnolang/gno/tm2/pkg/bft/privval"
"github.com/gnolang/gno/tm2/pkg/commands"

"github.com/gnolang/gno/tm2/pkg/log"
osm "github.com/gnolang/gno/tm2/pkg/os"
)

type resetCfg struct {
baseCfg
}

func (rc *resetCfg) RegisterFlags(fs *flag.FlagSet) {}

// XXX: this is totally unsafe.
// it's only suitable for testnets.
// It could result in data loss and network disrutpion while running the node and without coordination
func newResetAllCmd(bc baseCfg) *commands.Command {
cfg := resetCfg{
baseCfg: bc,
}

return commands.NewCommand(
commands.Metadata{
Name: "unsafe-reset-all",
ShortUsage: "unsafe-reset-all",
ShortHelp: "(unsafe) Remove all the data and WAL, reset this node's validator to genesis state",
},
&cfg,
func(_ context.Context, args []string) error {
return execResetAll(cfg, args)
},

Check warning on line 39 in gno.land/cmd/gnoland/reset.go

View check run for this annotation

Codecov / codecov/patch

gno.land/cmd/gnoland/reset.go#L38-L39

Added lines #L38 - L39 were not covered by tests
)
}

func execResetAll(rc resetCfg, args []string) (err error) {
config := rc.tmConfig

return resetAll(
config.DBDir(),
config.PrivValidatorKeyFile(),
config.PrivValidatorStateFile(),
logger,
)

Check warning on line 51 in gno.land/cmd/gnoland/reset.go

View check run for this annotation

Codecov / codecov/patch

gno.land/cmd/gnoland/reset.go#L43-L51

Added lines #L43 - L51 were not covered by tests
}

// resetAll removes address book files plus all data, and resets the privValdiator data.
func resetAll(dbDir, privValKeyFile, privValStateFile string, logger log.Logger) error {
if err := os.RemoveAll(dbDir); err == nil {
logger.Info("Removed all blockchain history", "dir", dbDir)
} else {
logger.Error("Error removing all blockchain history", "dir", dbDir, "err", err)
}

Check warning on line 60 in gno.land/cmd/gnoland/reset.go

View check run for this annotation

Codecov / codecov/patch

gno.land/cmd/gnoland/reset.go#L59-L60

Added lines #L59 - L60 were not covered by tests

if err := osm.EnsureDir(dbDir, 0o700); err != nil {
logger.Error("unable to recreate dbDir", "err", err)
}

Check warning on line 64 in gno.land/cmd/gnoland/reset.go

View check run for this annotation

Codecov / codecov/patch

gno.land/cmd/gnoland/reset.go#L63-L64

Added lines #L63 - L64 were not covered by tests

// recreate the dbDir since the privVal state needs to live there
resetFilePV(privValKeyFile, privValStateFile, logger)
return nil
}

// resetState removes address book files plus all databases.
func resetState(dbDir string, logger log.Logger) error {
blockdb := filepath.Join(dbDir, "blockstore.db")
state := filepath.Join(dbDir, "state.db")
wal := filepath.Join(dbDir, "cs.wal")
gnolang := filepath.Join(dbDir, "gnolang.db")

if osm.FileExists(blockdb) {
if err := os.RemoveAll(blockdb); err == nil {
logger.Info("Removed all blockstore.db", "dir", blockdb)
} else {
logger.Error("error removing all blockstore.db", "dir", blockdb, "err", err)
}

Check warning on line 83 in gno.land/cmd/gnoland/reset.go

View check run for this annotation

Codecov / codecov/patch

gno.land/cmd/gnoland/reset.go#L72-L83

Added lines #L72 - L83 were not covered by tests
Comment on lines +73 to +83
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could be a for loop over an array/slice of strings instead of duplicating the same code 4 times...

also I think we can just do RemoveAll and log any error while returned by that function. 🤷

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestion.

}

if osm.FileExists(state) {
if err := os.RemoveAll(state); err == nil {
logger.Info("Removed all state.db", "dir", state)
} else {
logger.Error("error removing all state.db", "dir", state, "err", err)
}

Check warning on line 91 in gno.land/cmd/gnoland/reset.go

View check run for this annotation

Codecov / codecov/patch

gno.land/cmd/gnoland/reset.go#L86-L91

Added lines #L86 - L91 were not covered by tests
}

if osm.FileExists(wal) {
if err := os.RemoveAll(wal); err == nil {
logger.Info("Removed all cs.wal", "dir", wal)
} else {
logger.Error("error removing all cs.wal", "dir", wal, "err", err)
}

Check warning on line 99 in gno.land/cmd/gnoland/reset.go

View check run for this annotation

Codecov / codecov/patch

gno.land/cmd/gnoland/reset.go#L94-L99

Added lines #L94 - L99 were not covered by tests
}

if osm.FileExists(gnolang) {
if err := os.RemoveAll(gnolang); err == nil {
logger.Info("Removed all gnolang.db", "dir", gnolang)
} else {
logger.Error("error removing all gnolang.db", "dir", gnolang, "err", err)
}

Check warning on line 107 in gno.land/cmd/gnoland/reset.go

View check run for this annotation

Codecov / codecov/patch

gno.land/cmd/gnoland/reset.go#L102-L107

Added lines #L102 - L107 were not covered by tests
}

if err := osm.EnsureDir(dbDir, 0o700); err != nil {
logger.Error("unable to recreate dbDir", "err", err)
}
return nil

Check warning on line 113 in gno.land/cmd/gnoland/reset.go

View check run for this annotation

Codecov / codecov/patch

gno.land/cmd/gnoland/reset.go#L110-L113

Added lines #L110 - L113 were not covered by tests
}

func resetFilePV(privValKeyFile, privValStateFile string, logger log.Logger) {
if _, err := os.Stat(privValKeyFile); err == nil {
pv := privval.LoadFilePVEmptyState(privValKeyFile, privValStateFile)
pv.Reset()
logger.Info(
"Reset private validator file to genesis state",
"keyFile", privValKeyFile,
"stateFile", privValStateFile,
)
} else {
pv := privval.GenFilePV(privValKeyFile, privValStateFile)
pv.Save()
logger.Info(
"Generated private validator file",
"keyFile", privValKeyFile,
"stateFile", privValStateFile,
)
}

Check warning on line 133 in gno.land/cmd/gnoland/reset.go

View check run for this annotation

Codecov / codecov/patch

gno.land/cmd/gnoland/reset.go#L126-L133

Added lines #L126 - L133 were not covered by tests
}
68 changes: 68 additions & 0 deletions gno.land/cmd/gnoland/reset_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package main

import (
"path/filepath"
"testing"

bft "github.com/gnolang/gno/tm2/pkg/bft/types"
tmtime "github.com/gnolang/gno/tm2/pkg/bft/types/time"
"github.com/stretchr/testify/require"

cfg "github.com/gnolang/gno/tm2/pkg/bft/config"
"github.com/gnolang/gno/tm2/pkg/bft/privval"
"github.com/gnolang/gno/tm2/pkg/p2p"
)

func TestResetAll(t *testing.T) {
config := cfg.TestConfig()
dir := t.TempDir()
config.SetRootDir(dir)
config.EnsureDirs()

require.NoError(t, initFilesWithConfig(config))
pv := privval.LoadFilePV(config.PrivValidatorKeyFile(), config.PrivValidatorStateFile())
pv.LastSignState.Height = 10
pv.Save()

require.NoError(t, resetAll(config.DBDir(), config.PrivValidatorKeyFile(),
config.PrivValidatorStateFile(), logger))

require.DirExists(t, config.DBDir())
require.NoFileExists(t, filepath.Join(config.DBDir(), "block.db"))
require.NoFileExists(t, filepath.Join(config.DBDir(), "state.db"))
require.NoFileExists(t, filepath.Join(config.DBDir(), "gnolang.db"))
require.FileExists(t, config.PrivValidatorStateFile())
require.FileExists(t, config.GenesisFile())
pv = privval.LoadFilePV(config.PrivValidatorKeyFile(), config.PrivValidatorStateFile())
require.Equal(t, int64(0), pv.LastSignState.Height)
}

func initFilesWithConfig(config *cfg.Config) error {
// private validator
privValKeyFile := config.PrivValidatorKeyFile()
privValStateFile := config.PrivValidatorStateFile()
var pv *privval.FilePV
pv = privval.GenFilePV(privValKeyFile, privValStateFile)
pv.Save()
nodeKeyFile := config.NodeKeyFile()
if _, err := p2p.LoadOrGenNodeKey(nodeKeyFile); err != nil {
return err
}

genFile := config.GenesisFile()
genDoc := bft.GenesisDoc{
ChainID: "test-chain-%v",
GenesisTime: tmtime.Now(),
ConsensusParams: bft.DefaultConsensusParams(),
}
key := pv.GetPubKey()
genDoc.Validators = []bft.GenesisValidator{{
Address: key.Address(),
PubKey: key,
Power: 10,
}}
if err := genDoc.SaveAs(genFile); err != nil {
return err
}
return nil
}
Loading
Loading