[libs/autofile & db/fsdb] Throw error if file permissions change (#2286)

* Enforce file permissions in case they've changed

* test behaviour for autofile

* use testify in tests and rename `fInf` to `fileInfo`

* return an error if file permissions have changed

- if we can't read the file, we'll still panic

* get rid of "github.com/pkg/errors" dependency

* address review comments:

- prefix instead of suffix
- add state to err and construct formatting in Error() method

* address review comments:

- move error to libs/errors

libs/autofile/autofile.go

@@ -6,6 +6,7 @@ import (
 	"time"
 
 	cmn "github.com/tendermint/tendermint/libs/common"
+	"github.com/tendermint/tendermint/libs/errors"
 )
 
 /* AutoFile usage
@@ -30,7 +31,10 @@ if err != nil {
 }
 */
 
-const autoFileOpenDuration = 1000 * time.Millisecond
+const (
+	autoFileOpenDuration = 1000 * time.Millisecond
+	autoFilePerms        = os.FileMode(0600)
+)
 
 // Automatically closes and re-opens file for writing.
 // This is useful for using a log file with the logrotate tool.
@@ -116,10 +120,17 @@ func (af *AutoFile) Sync() error {
 }
 
 func (af *AutoFile) openFile() error {
-	file, err := os.OpenFile(af.Path, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0600)
+	file, err := os.OpenFile(af.Path, os.O_RDWR|os.O_CREATE|os.O_APPEND, autoFilePerms)
 	if err != nil {
 		return err
 	}
+	fileInfo, err := file.Stat()
+	if err != nil {
+		return err
+	}
+	if fileInfo.Mode() != autoFilePerms {
+		return errors.NewErrPermissionsChanged(file.Name(), fileInfo.Mode(), autoFilePerms)
+	}
 	af.file = file
 	return nil
 }

libs/autofile/autofile_test.go

@@ -8,41 +8,33 @@ import (
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/require"
+
 	cmn "github.com/tendermint/tendermint/libs/common"
+	"github.com/tendermint/tendermint/libs/errors"
 )
 
 func TestSIGHUP(t *testing.T) {
-
 	// First, create an AutoFile writing to a tempfile dir
 	file, err := ioutil.TempFile("", "sighup_test")
-	if err != nil {
-		t.Fatalf("Error creating tempfile: %v", err)
-	}
-	if err := file.Close(); err != nil {
-		t.Fatalf("Error closing tempfile: %v", err)
-	}
+	require.NoError(t, err)
+	err = file.Close()
+	require.NoError(t, err)
 	name := file.Name()
+
 	// Here is the actual AutoFile
 	af, err := OpenAutoFile(name)
-	if err != nil {
-		t.Fatalf("Error creating autofile: %v", err)
-	}
+	require.NoError(t, err)
 
 	// Write to the file.
 	_, err = af.Write([]byte("Line 1\n"))
-	if err != nil {
-		t.Fatalf("Error writing to autofile: %v", err)
-	}
+	require.NoError(t, err)
 	_, err = af.Write([]byte("Line 2\n"))
-	if err != nil {
-		t.Fatalf("Error writing to autofile: %v", err)
-	}
+	require.NoError(t, err)
 
 	// Move the file over
 	err = os.Rename(name, name+"_old")
-	if err != nil {
-		t.Fatalf("Error moving autofile: %v", err)
-	}
+	require.NoError(t, err)
 
 	// Send SIGHUP to self.
 	oldSighupCounter := atomic.LoadInt32(&sighupCounter)
@@ -55,16 +47,11 @@ func TestSIGHUP(t *testing.T) {
 
 	// Write more to the file.
 	_, err = af.Write([]byte("Line 3\n"))
-	if err != nil {
-		t.Fatalf("Error writing to autofile: %v", err)
-	}
+	require.NoError(t, err)
 	_, err = af.Write([]byte("Line 4\n"))
-	if err != nil {
-		t.Fatalf("Error writing to autofile: %v", err)
-	}
-	if err := af.Close(); err != nil {
-		t.Fatalf("Error closing autofile")
-	}
+	require.NoError(t, err)
+	err = af.Close()
+	require.NoError(t, err)
 
 	// Both files should exist
 	if body := cmn.MustReadFile(name + "_old"); string(body) != "Line 1\nLine 2\n" {
@@ -74,3 +61,33 @@ func TestSIGHUP(t *testing.T) {
 		t.Errorf("Unexpected body %s", body)
 	}
 }
+
+// Manually modify file permissions, close, and reopen using autofile:
+// We expect the file permissions to be changed back to the intended perms.
+func TestOpenAutoFilePerms(t *testing.T) {
+	file, err := ioutil.TempFile("", "permission_test")
+	require.NoError(t, err)
+	err = file.Close()
+	require.NoError(t, err)
+	name := file.Name()
+
+	// open and change permissions
+	af, err := OpenAutoFile(name)
+	require.NoError(t, err)
+	err = af.file.Chmod(0755)
+	require.NoError(t, err)
+	err = af.Close()
+	require.NoError(t, err)
+
+	// reopen and expect an ErrPermissionsChanged as Cause
+	af, err = OpenAutoFile(name)
+	require.Error(t, err)
+	if e, ok := err.(*errors.ErrPermissionsChanged); ok {
+		t.Logf("%v", e)
+	} else {
+		t.Errorf("unexpected error %v", e)
+	}
+
+	err = af.Close()
+	require.NoError(t, err)
+}

libs/db/fsdb.go

@@ -10,7 +10,9 @@ import (
 	"sync"
 
 	"github.com/pkg/errors"
+
 	cmn "github.com/tendermint/tendermint/libs/common"
+	tmerrors "github.com/tendermint/tendermint/libs/errors"
 )
 
 const (
@@ -205,6 +207,13 @@ func write(path string, d []byte) error {
 		return err
 	}
 	defer f.Close()
+	fInfo, err := f.Stat()
+	if err != nil {
+		return err
+	}
+	if fInfo.Mode() != keyPerm {
+		return tmerrors.NewErrPermissionsChanged(f.Name(), keyPerm, fInfo.Mode())
+	}
 	_, err = f.Write(d)
 	if err != nil {
 		return err

libs/errors/errors.go

@@ -0,0 +1,26 @@
+// Package errors contains errors that are thrown across packages.
+package errors
+
+import (
+	"fmt"
+	"os"
+)
+
+// ErrPermissionsChanged occurs if the file permission have changed since the file was created.
+type ErrPermissionsChanged struct {
+	name      string
+	got, want os.FileMode
+}
+
+func NewErrPermissionsChanged(name string, got, want os.FileMode) *ErrPermissionsChanged {
+	return &ErrPermissionsChanged{name: name, got: got, want: want}
+}
+
+func (e ErrPermissionsChanged) Error() string {
+	return fmt.Sprintf(
+		"file: [%v]\nexpected file permissions: %v, got: %v",
+		e.name,
+		e.want,
+		e.got,
+	)
+}