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.
This commit is contained in:
Jakob Unterwurzacher 2019-04-08 20:18:45 +02:00
parent 8459bb15c1
commit fe06e9f456
6 changed files with 1 additions and 82 deletions

View File

@ -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,

View File

@ -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)

View File

@ -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)
}

View File

@ -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

View File

@ -69,8 +69,5 @@ func getMasterKey(args *argContainer) (masterkey []byte, confFile *configfile.Co
}
exitcodes.Exit(err)
}
if !args.trezor {
readpassword.CheckTrailingGarbage()
}
return masterkey, confFile
}

View File

@ -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) {