From 3b8f5cbb17c964224456bb36b096feafb0e24f44 Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sun, 18 Feb 2018 14:26:54 +0100 Subject: [PATCH] readpassword: convert from string to []byte This will allows us to overwrite the password with zeros once we are done with it. https://github.com/rfjakob/gocryptfs/issues/211 --- internal/configfile/config_file.go | 8 +++---- internal/configfile/config_test.go | 26 +++++++++++---------- internal/configfile/scrypt.go | 4 ++-- internal/configfile/scrypt_test.go | 2 +- internal/readpassword/extpass_test.go | 8 ++++--- internal/readpassword/read.go | 19 ++++++++------- internal/readpassword/stdin_test.go | 4 ++-- main.go | 2 +- tests/cli/cli_test.go | 8 ++++--- tests/plaintextnames/plaintextnames_test.go | 4 +++- 10 files changed, 47 insertions(+), 38 deletions(-) diff --git a/internal/configfile/config_file.go b/internal/configfile/config_file.go index 01e3b80..3fd16c7 100644 --- a/internal/configfile/config_file.go +++ b/internal/configfile/config_file.go @@ -67,7 +67,7 @@ func randBytesDevRandom(n int) []byte { // CreateConfFile - create a new config with a random key encrypted with // "password" and write it to "filename". // Uses scrypt with cost parameter logN. -func CreateConfFile(filename string, password string, plaintextNames bool, logN int, creator string, aessiv bool, devrandom bool) error { +func CreateConfFile(filename string, password []byte, plaintextNames bool, logN int, creator string, aessiv bool, devrandom bool) error { var cf ConfFile cf.filename = filename cf.Creator = creator @@ -114,7 +114,7 @@ func CreateConfFile(filename string, password string, plaintextNames bool, logN // // If "password" is empty, the config file is read // but the key is not decrypted (returns nil in its place). -func LoadConfFile(filename string, password string) ([]byte, *ConfFile, error) { +func LoadConfFile(filename string, password []byte) ([]byte, *ConfFile, error) { var cf ConfFile cf.filename = filename @@ -171,7 +171,7 @@ func LoadConfFile(filename string, password string) ([]byte, *ConfFile, error) { return nil, nil, fmt.Errorf("Deprecated filesystem") } - if password == "" { + if len(password) == 0 { // We have validated the config file, but without a password we cannot // decrypt the master key. Return only the parsed config. return nil, &cf, nil @@ -199,7 +199,7 @@ func LoadConfFile(filename string, password string) ([]byte, *ConfFile, error) { // and store it in cf.EncryptedKey. // Uses scrypt with cost parameter logN and stores the scrypt parameters in // cf.ScryptObject. -func (cf *ConfFile) EncryptKey(key []byte, password string, logN int) { +func (cf *ConfFile) EncryptKey(key []byte, password []byte, logN int) { // Generate scrypt-derived key from password cf.ScryptObject = NewScryptKDF(logN) scryptHash := cf.ScryptObject.DeriveKey(password) diff --git a/internal/configfile/config_test.go b/internal/configfile/config_test.go index b984a37..15728c6 100644 --- a/internal/configfile/config_test.go +++ b/internal/configfile/config_test.go @@ -8,8 +8,10 @@ import ( "github.com/rfjakob/gocryptfs/internal/tlog" ) +var testPw = []byte("test") + func TestLoadV1(t *testing.T) { - _, _, err := LoadConfFile("config_test/v1.conf", "test") + _, _, err := LoadConfFile("config_test/v1.conf", testPw) if err == nil { t.Errorf("Outdated v1 config file must fail to load but it didn't") } else if testing.Verbose() { @@ -22,7 +24,7 @@ func TestLoadV1(t *testing.T) { func TestLoadV2(t *testing.T) { t1 := time.Now() - _, _, err := LoadConfFile("config_test/v2.conf", "test") + _, _, err := LoadConfFile("config_test/v2.conf", testPw) if err != nil { t.Errorf("Could not load v2 config file: %v", err) } @@ -37,21 +39,21 @@ func TestLoadV2PwdError(t *testing.T) { if !testing.Verbose() { tlog.Warn.Enabled = false } - _, _, err := LoadConfFile("config_test/v2.conf", "wrongpassword") + _, _, err := LoadConfFile("config_test/v2.conf", []byte("wrongpassword")) if err == nil { t.Errorf("Loading with wrong password must fail but it didn't") } } func TestLoadV2Feature(t *testing.T) { - _, _, err := LoadConfFile("config_test/PlaintextNames.conf", "test") + _, _, err := LoadConfFile("config_test/PlaintextNames.conf", testPw) if err != nil { t.Errorf("Could not load v2 PlaintextNames config file: %v", err) } } func TestLoadV2StrangeFeature(t *testing.T) { - _, _, err := LoadConfFile("config_test/StrangeFeature.conf", "test") + _, _, err := LoadConfFile("config_test/StrangeFeature.conf", testPw) if err == nil { t.Errorf("Loading unknown feature must fail but it didn't") } else if testing.Verbose() { @@ -60,11 +62,11 @@ func TestLoadV2StrangeFeature(t *testing.T) { } func TestCreateConfDefault(t *testing.T) { - err := CreateConfFile("config_test/tmp.conf", "test", false, 10, "test", false, false) + err := CreateConfFile("config_test/tmp.conf", testPw, false, 10, "test", false, false) if err != nil { t.Fatal(err) } - _, c, err := LoadConfFile("config_test/tmp.conf", "test") + _, c, err := LoadConfFile("config_test/tmp.conf", testPw) if err != nil { t.Fatal(err) } @@ -81,18 +83,18 @@ func TestCreateConfDefault(t *testing.T) { } func TestCreateConfDevRandom(t *testing.T) { - err := CreateConfFile("config_test/tmp.conf", "test", false, 10, "test", false, true) + err := CreateConfFile("config_test/tmp.conf", testPw, false, 10, "test", false, true) if err != nil { t.Fatal(err) } } func TestCreateConfPlaintextnames(t *testing.T) { - err := CreateConfFile("config_test/tmp.conf", "test", true, 10, "test", false, false) + err := CreateConfFile("config_test/tmp.conf", testPw, true, 10, "test", false, false) if err != nil { t.Fatal(err) } - _, c, err := LoadConfFile("config_test/tmp.conf", "test") + _, c, err := LoadConfFile("config_test/tmp.conf", testPw) if err != nil { t.Fatal(err) } @@ -109,11 +111,11 @@ func TestCreateConfPlaintextnames(t *testing.T) { // Reverse mode uses AESSIV func TestCreateConfFileAESSIV(t *testing.T) { - err := CreateConfFile("config_test/tmp.conf", "test", false, 10, "test", true, false) + err := CreateConfFile("config_test/tmp.conf", testPw, false, 10, "test", true, false) if err != nil { t.Fatal(err) } - _, c, err := LoadConfFile("config_test/tmp.conf", "test") + _, c, err := LoadConfFile("config_test/tmp.conf", testPw) if err != nil { t.Fatal(err) } diff --git a/internal/configfile/scrypt.go b/internal/configfile/scrypt.go index b5a3edb..a1caf06 100644 --- a/internal/configfile/scrypt.go +++ b/internal/configfile/scrypt.go @@ -61,10 +61,10 @@ func NewScryptKDF(logN int) ScryptKDF { } // DeriveKey returns a new key from a supplied password. -func (s *ScryptKDF) DeriveKey(pw string) []byte { +func (s *ScryptKDF) DeriveKey(pw []byte) []byte { s.validateParams() - k, err := scrypt.Key([]byte(pw), s.Salt, s.N, s.R, s.P, s.KeyLen) + k, err := scrypt.Key(pw, s.Salt, s.N, s.R, s.P, s.KeyLen) if err != nil { log.Panicf("DeriveKey failed: %v", err) } diff --git a/internal/configfile/scrypt_test.go b/internal/configfile/scrypt_test.go index c1a656a..8f7a5c8 100644 --- a/internal/configfile/scrypt_test.go +++ b/internal/configfile/scrypt_test.go @@ -23,7 +23,7 @@ ok github.com/rfjakob/gocryptfs/cryptfs 18.772s func benchmarkScryptN(n int, b *testing.B) { kdf := NewScryptKDF(n) for i := 0; i < b.N; i++ { - kdf.DeriveKey("test") + kdf.DeriveKey(testPw) } } diff --git a/internal/readpassword/extpass_test.go b/internal/readpassword/extpass_test.go index 4af775a..cdfea4e 100644 --- a/internal/readpassword/extpass_test.go +++ b/internal/readpassword/extpass_test.go @@ -8,6 +8,8 @@ import ( "github.com/rfjakob/gocryptfs/internal/tlog" ) +var testPw = []byte("test") + func TestMain(m *testing.M) { // Shut up info output tlog.Info.Enabled = false @@ -16,7 +18,7 @@ func TestMain(m *testing.M) { func TestExtpass(t *testing.T) { p1 := "ads2q4tw41reg52" - p2 := readPasswordExtpass("echo " + p1) + p2 := string(readPasswordExtpass("echo " + p1)) if p1 != p2 { t.Errorf("p1=%q != p2=%q", p1, p2) } @@ -24,7 +26,7 @@ func TestExtpass(t *testing.T) { func TestOnceExtpass(t *testing.T) { p1 := "lkadsf0923rdfi48rqwhdsf" - p2 := Once("echo " + p1) + p2 := string(Once("echo " + p1)) if p1 != p2 { t.Errorf("p1=%q != p2=%q", p1, p2) } @@ -32,7 +34,7 @@ func TestOnceExtpass(t *testing.T) { func TestTwiceExtpass(t *testing.T) { p1 := "w5w44t3wfe45srz434" - p2 := Once("echo " + p1) + p2 := string(Once("echo " + p1)) if p1 != p2 { t.Errorf("p1=%q != p2=%q", p1, p2) } diff --git a/internal/readpassword/read.go b/internal/readpassword/read.go index 481911b..047eba9 100644 --- a/internal/readpassword/read.go +++ b/internal/readpassword/read.go @@ -2,6 +2,7 @@ package readpassword import ( + "bytes" "fmt" "io" "os" @@ -23,7 +24,7 @@ const ( // Once tries to get a password from the user, either from the terminal, extpass // or stdin. -func Once(extpass string) string { +func Once(extpass string) []byte { if extpass != "" { return readPasswordExtpass(extpass) } @@ -35,7 +36,7 @@ func Once(extpass string) string { // Twice is the same as Once but will prompt twice if we get the password from // the terminal. -func Twice(extpass string) string { +func Twice(extpass string) []byte { if extpass != "" { return readPasswordExtpass(extpass) } @@ -44,7 +45,7 @@ func Twice(extpass string) string { } p1 := readPasswordTerminal("Password: ") p2 := readPasswordTerminal("Repeat: ") - if p1 != p2 { + if !bytes.Equal(p1, p2) { tlog.Fatal.Println("Passwords do not match") os.Exit(exitcodes.ReadPassword) } @@ -53,7 +54,7 @@ func Twice(extpass string) string { // readPasswordTerminal reads a line from the terminal. // Exits on read error or empty result. -func readPasswordTerminal(prompt string) string { +func readPasswordTerminal(prompt string) []byte { fd := int(os.Stdin.Fd()) fmt.Fprintf(os.Stderr, prompt) // terminal.ReadPassword removes the trailing newline @@ -67,12 +68,12 @@ func readPasswordTerminal(prompt string) string { tlog.Fatal.Println("Password is empty") os.Exit(exitcodes.PasswordEmpty) } - return string(p) + return p } // readPasswordStdin reads a line from stdin. // It exits with a fatal error on read error or empty result. -func readPasswordStdin() string { +func readPasswordStdin() []byte { tlog.Info.Println("Reading password from stdin") p := readLineUnbuffered(os.Stdin) if len(p) == 0 { @@ -85,7 +86,7 @@ func readPasswordStdin() string { // readPasswordExtpass executes the "extpass" program and returns the first line // of the output. // Exits on read error or empty result. -func readPasswordExtpass(extpass string) string { +func readPasswordExtpass(extpass string) []byte { tlog.Info.Println("Reading password from extpass program") var parts []string // The option "-passfile=FILE" gets transformed to @@ -125,7 +126,7 @@ func readPasswordExtpass(extpass string) string { // readLineUnbuffered reads single bytes from "r" util it gets "\n" or EOF. // The returned string does NOT contain the trailing "\n". -func readLineUnbuffered(r io.Reader) (l string) { +func readLineUnbuffered(r io.Reader) (l []byte) { b := make([]byte, 1) for { if len(l) > maxPasswordLen { @@ -146,7 +147,7 @@ func readLineUnbuffered(r io.Reader) (l string) { if b[0] == '\n' { return l } - l = l + string(b) + l = append(l, b...) } } diff --git a/internal/readpassword/stdin_test.go b/internal/readpassword/stdin_test.go index 2d9f93f..8cf9954 100644 --- a/internal/readpassword/stdin_test.go +++ b/internal/readpassword/stdin_test.go @@ -11,7 +11,7 @@ import ( func TestStdin(t *testing.T) { p1 := "g55434t55wef" if os.Getenv("TEST_SLAVE") == "1" { - p2 := readPasswordStdin() + p2 := string(readPasswordStdin()) if p1 != p2 { fmt.Fprintf(os.Stderr, "%q != %q", p1, p2) os.Exit(1) @@ -44,7 +44,7 @@ func TestStdin(t *testing.T) { func TestStdinEof(t *testing.T) { p1 := "asd45as5f4a36" if os.Getenv("TEST_SLAVE") == "1" { - p2 := readPasswordStdin() + p2 := string(readPasswordStdin()) if p1 != p2 { fmt.Fprintf(os.Stderr, "%q != %q", p1, p2) os.Exit(1) diff --git a/main.go b/main.go index 797701a..ed5784f 100644 --- a/main.go +++ b/main.go @@ -44,7 +44,7 @@ func loadConfig(args *argContainer) (masterkey []byte, confFile *configfile.Conf // password). if args.masterkey != "" { masterkey = parseMasterKey(args.masterkey) - _, confFile, err = configfile.LoadConfFile(args.config, "") + _, confFile, err = configfile.LoadConfFile(args.config, nil) } else { pw := readpassword.Once(args.extpass) tlog.Info.Println("Decrypting master key") diff --git a/tests/cli/cli_test.go b/tests/cli/cli_test.go index c7752f2..9eebc5f 100644 --- a/tests/cli/cli_test.go +++ b/tests/cli/cli_test.go @@ -16,6 +16,8 @@ import ( "github.com/rfjakob/gocryptfs/tests/test_helpers" ) +var testPw = []byte("test") + func TestMain(m *testing.M) { test_helpers.ResetTmpDir(false) r := m.Run() @@ -25,7 +27,7 @@ func TestMain(m *testing.M) { // Test -init flag func TestInit(t *testing.T) { dir := test_helpers.InitFS(t) - _, c, err := configfile.LoadConfFile(dir+"/"+configfile.ConfDefaultName, "test") + _, c, err := configfile.LoadConfFile(dir+"/"+configfile.ConfDefaultName, testPw) if err != nil { t.Fatal(err) } @@ -42,7 +44,7 @@ func TestInitDevRandom(t *testing.T) { // Test -init with -aessiv func TestInitAessiv(t *testing.T) { dir := test_helpers.InitFS(t, "-aessiv") - _, c, err := configfile.LoadConfFile(dir+"/"+configfile.ConfDefaultName, "test") + _, c, err := configfile.LoadConfFile(dir+"/"+configfile.ConfDefaultName, testPw) if err != nil { t.Fatal(err) } @@ -54,7 +56,7 @@ func TestInitAessiv(t *testing.T) { // Test -init with -reverse func TestInitReverse(t *testing.T) { dir := test_helpers.InitFS(t, "-reverse") - _, c, err := configfile.LoadConfFile(dir+"/"+configfile.ConfReverseName, "test") + _, c, err := configfile.LoadConfFile(dir+"/"+configfile.ConfReverseName, testPw) if err != nil { t.Fatal(err) } diff --git a/tests/plaintextnames/plaintextnames_test.go b/tests/plaintextnames/plaintextnames_test.go index cda5149..6b513aa 100644 --- a/tests/plaintextnames/plaintextnames_test.go +++ b/tests/plaintextnames/plaintextnames_test.go @@ -15,6 +15,8 @@ import ( var cDir string var pDir string +var testPw = []byte("test") + // Create and mount "-plaintextnames" fs func TestMain(m *testing.M) { cDir = test_helpers.InitFS(nil, "-plaintextnames") @@ -27,7 +29,7 @@ func TestMain(m *testing.M) { // Only the PlaintextNames feature flag should be set func TestFlags(t *testing.T) { - _, cf, err := configfile.LoadConfFile(cDir+"/gocryptfs.conf", "test") + _, cf, err := configfile.LoadConfFile(cDir+"/gocryptfs.conf", testPw) if err != nil { t.Fatal(err) }