summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJakob Unterwurzacher2017-02-12 15:35:50 +0100
committerJakob Unterwurzacher2017-02-12 17:59:09 +0100
commit8adfbf2dc34560df7436c89b59a9749d2dd3b78e (patch)
treeb1f7a10f654a6c2e35fdc60f53770d83bff1267c
parent2dd90ac19ce97544c412d90a24df0f68e66311db (diff)
Check for trailing garbage after the password
From the comment: // 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.
-rw-r--r--init_dir.go1
-rw-r--r--internal/readpassword/read.go32
-rw-r--r--main.go1
-rw-r--r--mount.go2
-rw-r--r--tests/cli/cli_test.go44
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)
+}
diff --git a/main.go b/main.go
index bb47e46..21f2d06 100644
--- a/main.go
+++ b/main.go
@@ -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"
diff --git a/mount.go b/mount.go
index cd14dd9..032589d 100644
--- a/mount.go
+++ b/mount.go
@@ -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)
+ }
+ }
+}