diff options
| -rw-r--r-- | init_dir.go | 1 | ||||
| -rw-r--r-- | internal/readpassword/read.go | 32 | ||||
| -rw-r--r-- | main.go | 1 | ||||
| -rw-r--r-- | mount.go | 2 | ||||
| -rw-r--r-- | tests/cli/cli_test.go | 44 |
5 files changed, 80 insertions, 0 deletions
diff --git a/init_dir.go b/init_dir.go index 89af495..bf6740e 100644 --- a/init_dir.go +++ b/init_dir.go @@ -36,6 +36,7 @@ func initDir(args *argContainer) { tlog.Info.Printf("Choose a password for protecting your files.") } password := readpassword.Twice(args.extpass) + readpassword.CheckTrailingGarbage() creator := tlog.ProgramName + " " + GitVersion err = configfile.CreateConfFile(args.config, password, args.plaintextnames, args.scryptn, creator, args.aessiv, args.raw64) if err != nil { diff --git a/internal/readpassword/read.go b/internal/readpassword/read.go index 49cf8ef..fe9be45 100644 --- a/internal/readpassword/read.go +++ b/internal/readpassword/read.go @@ -7,6 +7,8 @@ import ( "os" "os/exec" "strings" + "sync" + "time" "golang.org/x/crypto/ssh/terminal" @@ -141,3 +143,33 @@ func readLineUnbuffered(r io.Reader) (l string) { l = l + string(b) } } + +// CheckTrailingGarbage tries to read one byte from stdin and exits with a +// fatal error if the read returns any data. +// This is meant to be called after reading the password, when there is no more +// data expected. This helps to catch problems with third-party tools that +// interface with gocryptfs. +// +// This is tested via TestInitTrailingGarbage() in tests/cli/cli_test.go. +func CheckTrailingGarbage() { + if terminal.IsTerminal(int(os.Stdin.Fd())) { + // Be lenient when interacting with a human. + return + } + var wg sync.WaitGroup + wg.Add(1) + go func() { + b := make([]byte, 1) + wg.Done() + n, _ := os.Stdin.Read(b) + if n > 0 { + tlog.Fatal.Printf("Received trailing garbage after the password") + os.Exit(exitCode) + } + }() + // Wait for the goroutine to start up plus one millisecond for the read to + // return. If there is data available, this SHOULD be plenty of time to + // read one byte. However, I don't see a way to be sure. + wg.Wait() + time.Sleep(1 * time.Millisecond) +} @@ -87,6 +87,7 @@ func changePassword(args *argContainer) { } tlog.Info.Println("Please enter your new password.") newPw := readpassword.Twice(args.extpass) + readpassword.CheckTrailingGarbage() confFile.EncryptKey(masterkey, newPw, confFile.ScryptObject.LogN()) if args.masterkey != "" { bak := args.config + ".bak" @@ -22,6 +22,7 @@ import ( "github.com/rfjakob/gocryptfs/internal/ctlsock" "github.com/rfjakob/gocryptfs/internal/fusefrontend" "github.com/rfjakob/gocryptfs/internal/fusefrontend_reverse" + "github.com/rfjakob/gocryptfs/internal/readpassword" "github.com/rfjakob/gocryptfs/internal/tlog" ) @@ -96,6 +97,7 @@ func doMount(args *argContainer) int { } os.Exit(ErrExitLoadConf) } + readpassword.CheckTrailingGarbage() printMasterKey(masterkey) } // We cannot use JSON for pretty-printing as the fields are unexported diff --git a/tests/cli/cli_test.go b/tests/cli/cli_test.go index b39c816..cce7fa0 100644 --- a/tests/cli/cli_test.go +++ b/tests/cli/cli_test.go @@ -272,3 +272,47 @@ func TestShadows(t *testing.T) { t.Errorf("Should have failed") } } + +// TestInitTrailingGarbage verfies that gocryptfs exits with an error if we +// pass additional data after the password. +func TestInitTrailingGarbage(t *testing.T) { + table := []struct { + pw string + closeStdin bool + expectSuccess bool + }{ + {"foo\n", false, true}, + {"foo", true, true}, + {"foo\n", true, true}, + {"foo\n\n", false, false}, + {"foo\nbar", false, false}, + {"foo\n\n", true, false}, + {"foo\nbar", true, false}, + } + for _, row := range table { + dir, err := ioutil.TempDir(test_helpers.TmpDir, "") + if err != nil { + t.Fatal(err) + } + cmd := exec.Command(test_helpers.GocryptfsBinary, "-q", "-init", "-scryptn=10", dir) + childStdin, err := cmd.StdinPipe() + if err != nil { + t.Fatal(err) + } + err = cmd.Start() + if err != nil { + t.Fatal(err) + } + childStdin.Write([]byte(row.pw)) + if row.closeStdin { + childStdin.Close() + } + err = cmd.Wait() + success := (err == nil) + if success == true && row.expectSuccess == false { + t.Errorf("pw=%q should have failed, but succeeded", row.pw) + } else if success == false && row.expectSuccess == true { + t.Errorf("pw=%q should have succeeded, but failed", row.pw) + } + } +} |