From 2bdf7d5172d3540606f098030e8ede7a3ad1dfdd Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sat, 8 Sep 2018 12:40:29 +0200 Subject: [PATCH] configfile: add LoadAndDecrypt wrapper Callers that do not want to decrypt the masterkey should call plain Load(). https://github.com/rfjakob/gocryptfs/issues/258 --- gocryptfs-xray/xray_main.go | 2 +- internal/configfile/config_file.go | 52 +++++++++++++-------- internal/configfile/config_test.go | 16 +++---- main.go | 6 +-- tests/cli/cli_test.go | 6 +-- tests/plaintextnames/plaintextnames_test.go | 2 +- tests/trezor/trezor_test.go | 2 +- 7 files changed, 50 insertions(+), 36 deletions(-) diff --git a/gocryptfs-xray/xray_main.go b/gocryptfs-xray/xray_main.go index 551b51b..569776d 100644 --- a/gocryptfs-xray/xray_main.go +++ b/gocryptfs-xray/xray_main.go @@ -61,7 +61,7 @@ func main() { func dumpMasterKey(fn string) { tlog.Info.Enabled = false pw := readpassword.Once("", "") - masterkey, _, err := configfile.Load(fn, pw) + masterkey, _, err := configfile.LoadAndDecrypt(fn, pw) if err != nil { fmt.Fprintln(os.Stderr, err) exitcodes.Exit(err) diff --git a/internal/configfile/config_file.go b/internal/configfile/config_file.go index b18d6a7..e589060 100644 --- a/internal/configfile/config_file.go +++ b/internal/configfile/config_file.go @@ -118,40 +118,63 @@ func Create(filename string, password []byte, plaintextNames bool, return cf.WriteFile() } -// Load - read config file from disk and decrypt the +// LoadAndDecrypt - read config file from disk and decrypt the // contained key using "password". // Returns the decrypted key and the ConfFile object // // If "password" is empty, the config file is read // but the key is not decrypted (returns nil in its place). -func Load(filename string, password []byte) ([]byte, *ConfFile, error) { +func LoadAndDecrypt(filename string, password []byte) ([]byte, *ConfFile, error) { + cf, err := Load(filename) + if err != nil { + return nil, nil, err + } + 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 + // TODO: Make this an error in gocryptfs v1.7. All code should now call + // Load() instead of calling LoadAndDecrypt() with an empty password. + } + + // Decrypt the masterkey using the password + key, err := cf.DecryptMasterKey(password) + if err != nil { + return nil, nil, err + } + + return key, cf, err +} + +// Load loads and parses the config file at "filename". +func Load(filename string) (*ConfFile, error) { var cf ConfFile cf.filename = filename // Read from disk js, err := ioutil.ReadFile(filename) if err != nil { - return nil, nil, err + return nil, err } if len(js) == 0 { - return nil, nil, fmt.Errorf("Config file is empty") + return nil, fmt.Errorf("Config file is empty") } // Unmarshal err = json.Unmarshal(js, &cf) if err != nil { tlog.Warn.Printf("Failed to unmarshal config file") - return nil, nil, err + return nil, err } if cf.Version != contentenc.CurrentVersion { - return nil, nil, fmt.Errorf("Unsupported on-disk format %d", cf.Version) + return nil, fmt.Errorf("Unsupported on-disk format %d", cf.Version) } // Check that all set feature flags are known for _, flag := range cf.FeatureFlags { if !cf.isFeatureFlagKnown(flag) { - return nil, nil, fmt.Errorf("Unsupported feature flag %q", flag) + return nil, fmt.Errorf("Unsupported feature flag %q", flag) } } @@ -181,20 +204,11 @@ func Load(filename string, password []byte) ([]byte, *ConfFile, error) { `+tlog.ColorReset) - return nil, nil, exitcodes.NewErr("Deprecated filesystem", exitcodes.DeprecatedFS) - } - 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 + return nil, exitcodes.NewErr("Deprecated filesystem", exitcodes.DeprecatedFS) } - key, err := cf.DecryptMasterKey(password) - if err != nil { - return nil, nil, err - } - - return key, &cf, err + // All good + return &cf, nil } // DecryptMasterKey decrypts the masterkey stored in cf.EncryptedKey using diff --git a/internal/configfile/config_test.go b/internal/configfile/config_test.go index b8ee150..0dd081c 100644 --- a/internal/configfile/config_test.go +++ b/internal/configfile/config_test.go @@ -11,7 +11,7 @@ import ( var testPw = []byte("test") func TestLoadV1(t *testing.T) { - _, _, err := Load("config_test/v1.conf", testPw) + _, _, err := LoadAndDecrypt("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() { @@ -24,7 +24,7 @@ func TestLoadV1(t *testing.T) { func TestLoadV2(t *testing.T) { t1 := time.Now() - _, _, err := Load("config_test/v2.conf", testPw) + _, _, err := LoadAndDecrypt("config_test/v2.conf", testPw) if err != nil { t.Errorf("Could not load v2 config file: %v", err) } @@ -39,21 +39,21 @@ func TestLoadV2PwdError(t *testing.T) { if !testing.Verbose() { tlog.Warn.Enabled = false } - _, _, err := Load("config_test/v2.conf", []byte("wrongpassword")) + _, _, err := LoadAndDecrypt("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 := Load("config_test/PlaintextNames.conf", testPw) + _, _, err := LoadAndDecrypt("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 := Load("config_test/StrangeFeature.conf", testPw) + _, _, err := LoadAndDecrypt("config_test/StrangeFeature.conf", testPw) if err == nil { t.Errorf("Loading unknown feature must fail but it didn't") } else if testing.Verbose() { @@ -66,7 +66,7 @@ func TestCreateConfDefault(t *testing.T) { if err != nil { t.Fatal(err) } - _, c, err := Load("config_test/tmp.conf", testPw) + _, c, err := LoadAndDecrypt("config_test/tmp.conf", testPw) if err != nil { t.Fatal(err) } @@ -94,7 +94,7 @@ func TestCreateConfPlaintextnames(t *testing.T) { if err != nil { t.Fatal(err) } - _, c, err := Load("config_test/tmp.conf", testPw) + _, c, err := LoadAndDecrypt("config_test/tmp.conf", testPw) if err != nil { t.Fatal(err) } @@ -115,7 +115,7 @@ func TestCreateConfFileAESSIV(t *testing.T) { if err != nil { t.Fatal(err) } - _, c, err := Load("config_test/tmp.conf", testPw) + _, c, err := LoadAndDecrypt("config_test/tmp.conf", testPw) if err != nil { t.Fatal(err) } diff --git a/main.go b/main.go index 26d09f5..ac12d2e 100644 --- a/main.go +++ b/main.go @@ -36,7 +36,7 @@ var raceDetector bool func loadConfig(args *argContainer) (masterkey []byte, confFile *configfile.ConfFile, err error) { // First check if the file can be read at all, and find out if a Trezor should // be used instead of a password. - _, cf1, err := configfile.Load(args.config, nil) + cf1, err := configfile.Load(args.config) if err != nil { tlog.Fatal.Printf("Cannot open config file: %v", err) return nil, nil, err @@ -56,7 +56,7 @@ func loadConfig(args *argContainer) (masterkey []byte, confFile *configfile.Conf pw = readpassword.Once(args.extpass, "") } tlog.Info.Println("Decrypting master key") - masterkey, confFile, err = configfile.Load(args.config, pw) + masterkey, confFile, err = configfile.LoadAndDecrypt(args.config, pw) for i := range pw { pw[i] = 0 } @@ -73,7 +73,7 @@ func loadConfig(args *argContainer) (masterkey []byte, confFile *configfile.Conf func changePassword(args *argContainer) { // Parse the config file, but do not unlock the master key. We only want to // know if the Trezor flag is set. - _, cf1, err := configfile.Load(args.config, nil) + cf1, err := configfile.Load(args.config) if err != nil { tlog.Fatal.Printf("Cannot open config file: %v", err) os.Exit(exitcodes.LoadConf) diff --git a/tests/cli/cli_test.go b/tests/cli/cli_test.go index 3b53181..eaa92b6 100644 --- a/tests/cli/cli_test.go +++ b/tests/cli/cli_test.go @@ -34,7 +34,7 @@ func TestMain(m *testing.M) { // Test -init flag func TestInit(t *testing.T) { dir := test_helpers.InitFS(t) - _, c, err := configfile.Load(dir+"/"+configfile.ConfDefaultName, testPw) + _, c, err := configfile.LoadAndDecrypt(dir+"/"+configfile.ConfDefaultName, testPw) if err != nil { t.Fatal(err) } @@ -51,7 +51,7 @@ func TestInitDevRandom(t *testing.T) { // Test -init with -aessiv func TestInitAessiv(t *testing.T) { dir := test_helpers.InitFS(t, "-aessiv") - _, c, err := configfile.Load(dir+"/"+configfile.ConfDefaultName, testPw) + _, c, err := configfile.LoadAndDecrypt(dir+"/"+configfile.ConfDefaultName, testPw) if err != nil { t.Fatal(err) } @@ -63,7 +63,7 @@ func TestInitAessiv(t *testing.T) { // Test -init with -reverse func TestInitReverse(t *testing.T) { dir := test_helpers.InitFS(t, "-reverse") - _, c, err := configfile.Load(dir+"/"+configfile.ConfReverseName, testPw) + _, c, err := configfile.LoadAndDecrypt(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 121a4a0..cce25b6 100644 --- a/tests/plaintextnames/plaintextnames_test.go +++ b/tests/plaintextnames/plaintextnames_test.go @@ -29,7 +29,7 @@ func TestMain(m *testing.M) { // Only the PlaintextNames feature flag should be set func TestFlags(t *testing.T) { - _, cf, err := configfile.Load(cDir+"/gocryptfs.conf", testPw) + _, cf, err := configfile.LoadAndDecrypt(cDir+"/gocryptfs.conf", testPw) if err != nil { t.Fatal(err) } diff --git a/tests/trezor/trezor_test.go b/tests/trezor/trezor_test.go index 59e10cb..5e071fc 100644 --- a/tests/trezor/trezor_test.go +++ b/tests/trezor/trezor_test.go @@ -35,7 +35,7 @@ func TestInitTrezor(t *testing.T) { // vvvvvvvvvvvvv disable -extpass dir := test_helpers.InitFS(t, "-trezor", "-extpass", "") // The freshly created config file should have the Trezor feature flag set. - _, c, err := configfile.Load(dir+"/"+configfile.ConfDefaultName, nil) + c, err := configfile.Load(dir + "/" + configfile.ConfDefaultName) if err != nil { t.Fatal(err) }