aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJakob Unterwurzacher2019-04-08 20:18:45 +0200
committerJakob Unterwurzacher2019-04-08 20:18:45 +0200
commitfe06e9f45646893dc88ebe9e657e2e991f6f5fbb (patch)
treead5d8e2910ed71a5c579e40f897093bec2e19011
parent8459bb15c1a32561c250a8b688ab4a7ecda0a4aa (diff)
readpassword: delete CheckTrailingGarbage
CheckTrailingGarbage was called even when "-passfile" was used, which is stupid, and causes false positives: https://github.com/rfjakob/gocryptfs/issues/391 (false error "Received trailing garbage after the password" when using -passfile in .bash_profile) Instead of trying to improve the logic to handle that case and make everything even more complicated, delete the function. It is unclear if actually helps in some cases, and it definitely harms as shown by the above bug report.
-rw-r--r--init_dir.go1
-rw-r--r--internal/fusefrontend_reverse/reverse_longnames.go2
-rw-r--r--internal/readpassword/read.go32
-rw-r--r--main.go1
-rw-r--r--masterkey.go3
-rw-r--r--tests/cli/cli_test.go44
6 files changed, 1 insertions, 82 deletions
diff --git a/init_dir.go b/init_dir.go
index c3aa4b5..a9c66e3 100644
--- a/init_dir.go
+++ b/init_dir.go
@@ -81,7 +81,6 @@ func initDir(args *argContainer) {
} else {
// Normal password entry
password = readpassword.Twice([]string(args.extpass), args.passfile)
- readpassword.CheckTrailingGarbage()
}
creator := tlog.ProgramName + " " + GitVersion
err = configfile.Create(args.config, password, args.plaintextnames,
diff --git a/internal/fusefrontend_reverse/reverse_longnames.go b/internal/fusefrontend_reverse/reverse_longnames.go
index 9f044e8..f07e413 100644
--- a/internal/fusefrontend_reverse/reverse_longnames.go
+++ b/internal/fusefrontend_reverse/reverse_longnames.go
@@ -105,7 +105,7 @@ func (rfs *ReverseFS) findLongnameParent(dir string, dirIV []byte, longname stri
}
func (rfs *ReverseFS) newNameFile(relPath string) (nodefs.File, fuse.Status) {
- dotName := filepath.Base(relPath) // gocryptfs.longname.XYZ.name
+ dotName := filepath.Base(relPath) // gocryptfs.longname.XYZ.name
longname := nametransform.RemoveLongNameSuffix(dotName) // gocryptfs.longname.XYZ
// cipher directory
cDir := nametransform.Dir(relPath)
diff --git a/internal/readpassword/read.go b/internal/readpassword/read.go
index 060100b..92a0886 100644
--- a/internal/readpassword/read.go
+++ b/internal/readpassword/read.go
@@ -8,8 +8,6 @@ import (
"os"
"os/exec"
"strings"
- "sync"
- "time"
"golang.org/x/crypto/ssh/terminal"
@@ -159,33 +157,3 @@ func readLineUnbuffered(r io.Reader) (l []byte) {
l = append(l, 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(exitcodes.ReadPassword)
- }
- }()
- // 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)
-}
diff --git a/main.go b/main.go
index 09c8ed7..02cc4ad 100644
--- a/main.go
+++ b/main.go
@@ -94,7 +94,6 @@ func changePassword(args *argContainer) {
}
tlog.Info.Println("Please enter your new password.")
newPw := readpassword.Twice([]string(args.extpass), args.passfile)
- readpassword.CheckTrailingGarbage()
confFile.EncryptKey(masterkey, newPw, confFile.ScryptObject.LogN())
for i := range newPw {
newPw[i] = 0
diff --git a/masterkey.go b/masterkey.go
index 8392bc6..706eb68 100644
--- a/masterkey.go
+++ b/masterkey.go
@@ -69,8 +69,5 @@ func getMasterKey(args *argContainer) (masterkey []byte, confFile *configfile.Co
}
exitcodes.Exit(err)
}
- if !args.trezor {
- readpassword.CheckTrailingGarbage()
- }
return masterkey, confFile
}
diff --git a/tests/cli/cli_test.go b/tests/cli/cli_test.go
index 58ffe44..7b6736a 100644
--- a/tests/cli/cli_test.go
+++ b/tests/cli/cli_test.go
@@ -317,50 +317,6 @@ func TestShadows(t *testing.T) {
}
}
-// 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)
- }
- }
-}
-
// TestMountPasswordIncorrect makes sure the correct exit code is used when the password
// was incorrect while mounting
func TestMountPasswordIncorrect(t *testing.T) {