From 7fed63a84963cbd790e86a0e59ff14724bcf33c4 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 14 Sep 2021 14:12:39 -0700 Subject: [PATCH] Adjust recovery passphrase generation As per the feedback at https://github.com/google/fscrypt/issues/115 where users didn't understand that the recovery passphrase is important, restore the original behavior where recovery passphrase generation happens automatically without a prompt. This applies to the case where 'fscrypt encrypt' is using a login protector on a non-root filesystem. However, leave the --no-recovery option so that the recovery passphrase can still be disabled if the user really wants to. Also, clarify the information provided about the recovery passphrase. Update https://github.com/google/fscrypt/issues/115 --- README.md | 23 ++++++++++++-------- actions/recovery.go | 22 +++++++++++++++---- actions/recovery_test.go | 3 ++- cli-tests/t_encrypt_login.out | 25 +++++++++++++++++---- cli-tests/t_encrypt_login.sh | 2 -- cmd/fscrypt/commands.go | 41 ++++++++++++++++++++--------------- cmd/fscrypt/flags.go | 2 +- cmd/fscrypt/format.go | 2 +- 8 files changed, 80 insertions(+), 40 deletions(-) diff --git a/README.md b/README.md index f581be9..d24cd03 100644 --- a/README.md +++ b/README.md @@ -474,14 +474,19 @@ via a login protector if the operating system is reinstalled or if the disk is connected to another system** -- even if the new system uses the same login passphrase for the user. -Because of this, `fscrypt encrypt` will offer to generate a recovery passphrase -when creating a login passphrase-protected directory on a non-root filesystem. -The recovery passphrase is simply a `custom_passphrase` protector with a -randomly generated high-entropy passphrase. It is strongly recommended to -accept the prompt to generate the recovery passphrase, then store the recovery -passphrase in a secure location. Then, if ever needed, you can use `fscrypt -unlock` to unlock the directory with the recovery passphrase (by choosing the -recovery protector instead of the login protector). +Because of this, `fscrypt encrypt` will automatically generate a recovery +passphrase when creating a login passphrase-protected directory on a non-root +filesystem. The recovery passphrase is simply a `custom_passphrase` protector +with a randomly generated high-entropy passphrase. Initially, the recovery +passphrase is stored in a file in the encrypted directory itself; therefore, to +use it you **must** record it in another secure location. It is strongly +recommended to do this. Then, if ever needed, you can use `fscrypt unlock` to +unlock the directory with the recovery passphrase (by choosing the recovery +protector instead of the login protector). + +If you really want to disable the generation of a recovery passphrase, use the +`--no-recovery` option. Only do this if you really know what you are doing and +are prepared for potential data loss. Alternative approaches to supporting recovery of login passphrase-protected directories include the following: @@ -493,7 +498,7 @@ directories include the following: Note that after restoring the `/.fscrypt` directory, unlocking the login protectors will require the passphrases they had at the time the backup was made **even if they were changed later**, so make sure to remember these - passphrase(s) or store them in a secure location. Also note that if the UUID + passphrase(s) or record them in a secure location. Also note that if the UUID of the root filesystem changed, you will need to manually fix the UUID in any `.fscrypt/protectors/*.link` files on other filesystems. diff --git a/actions/recovery.go b/actions/recovery.go index 458349b..f533906 100644 --- a/actions/recovery.go +++ b/actions/recovery.go @@ -86,7 +86,8 @@ func AddRecoveryPassphrase(policy *Policy, dirname string) (*crypto.Key, *Protec // file. This file should initially be located in the encrypted directory // protected by the passphrase itself. It's up to the user to store the // passphrase in a different location if they actually need it. -func WriteRecoveryInstructions(recoveryPassphrase *crypto.Key, path string) error { +func WriteRecoveryInstructions(recoveryPassphrase *crypto.Key, recoveryProtector *Protector, + policy *Policy, path string) error { file, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE, 0600) if err != nil { return err @@ -101,9 +102,22 @@ It did this because you chose to protect this directory with your login passphrase, but this directory is not on the root filesystem. Copy this passphrase to a safe place if you want to still be able to unlock this -directory if you re-install your system or connect this storage media to a -different system (which would result in your login protector being lost). -`, recoveryPassphrase.Data()) +directory if you re-install the operating system or connect this storage media +to a different system (which would result in your login protector being lost). + +To unlock this directory using this recovery passphrase, run 'fscrypt unlock' +and select the protector named %q. + +If you want to disable recovery passphrase generation (not recommended), +re-create this directory and pass the --no-recovery option to 'fscrypt encrypt'. +Alternatively, you can remove this recovery passphrase protector using: + + fscrypt metadata remove-protector-from-policy --force --protector=%s:%s --policy=%s:%s + +It is safe to keep it around though, as the recovery passphrase is high-entropy. +`, recoveryPassphrase.Data(), recoveryProtector.data.Name, + recoveryProtector.Context.Mount.Path, recoveryProtector.data.ProtectorDescriptor, + policy.Context.Mount.Path, policy.data.KeyDescriptor) if _, err = file.WriteString(str); err != nil { return err } diff --git a/actions/recovery_test.go b/actions/recovery_test.go index 4332972..5abd0a9 100644 --- a/actions/recovery_test.go +++ b/actions/recovery_test.go @@ -67,7 +67,8 @@ func TestRecoveryPassphrase(t *testing.T) { } // Test writing the recovery instructions. - if err = WriteRecoveryInstructions(passphrase, recoveryFile); err != nil { + if err = WriteRecoveryInstructions(passphrase, recoveryProtector, policy, + recoveryFile); err != nil { t.Fatal(err) } contentsBytes, err := ioutil.ReadFile(recoveryFile) diff --git a/cli-tests/t_encrypt_login.out b/cli-tests/t_encrypt_login.out index 0d77799..c531f73 100644 --- a/cli-tests/t_encrypt_login.out +++ b/cli-tests/t_encrypt_login.out @@ -1,6 +1,12 @@ # Encrypt with login protector -See "MNT/dir/fscrypt_recovery_readme.txt" for important recovery instructions! + +IMPORTANT: See "MNT/dir/fscrypt_recovery_readme.txt" for + important recovery instructions. It is *strongly recommended* to + record the recovery passphrase in a secure location; otherwise you + will lose access to this directory if you reinstall the operating + system or move this filesystem to another system. + ext4 filesystem "MNT" has 2 protectors and 1 policy PROTECTOR LINKED DESCRIPTION @@ -43,8 +49,13 @@ IMPORTANT: Before continuing, ensure you have properly set up your system for https://github.com/google/fscrypt#setting-up-for-login-protectors Enter login passphrase for fscrypt-test-user: -Protector is on a different filesystem! Generate a recovery passphrase (recommended)? [Y/n] y -See "MNT/dir/fscrypt_recovery_readme.txt" for important recovery instructions! + +IMPORTANT: See "MNT/dir/fscrypt_recovery_readme.txt" for + important recovery instructions. It is *strongly recommended* to + record the recovery passphrase in a secure location; otherwise you + will lose access to this directory if you reinstall the operating + system or move this filesystem to another system. + "MNT/dir" is now encrypted, unlocked, and ready for use. ext4 filesystem "MNT" has 2 protectors and 1 policy @@ -70,7 +81,13 @@ desc10 Yes (MNT_ROOT) login protector for fscrypt-test-user desc11 No custom protector "Recovery passphrase for dir" # Encrypt with login protector as root -See "MNT/dir/fscrypt_recovery_readme.txt" for important recovery instructions! + +IMPORTANT: See "MNT/dir/fscrypt_recovery_readme.txt" for + important recovery instructions. It is *strongly recommended* to + record the recovery passphrase in a secure location; otherwise you + will lose access to this directory if you reinstall the operating + system or move this filesystem to another system. + ext4 filesystem "MNT" has 2 protectors and 1 policy PROTECTOR LINKED DESCRIPTION diff --git a/cli-tests/t_encrypt_login.sh b/cli-tests/t_encrypt_login.sh index 11a62f1..652d860 100755 --- a/cli-tests/t_encrypt_login.sh +++ b/cli-tests/t_encrypt_login.sh @@ -50,8 +50,6 @@ expect "Enter the source number for the new protector" send "1\r" expect "Enter login passphrase" send "TEST_USER_PASS\r" -expect "Protector is on a different filesystem! Generate a recovery passphrase (recommended)?" -send "y\r" expect eof EOF show_status true diff --git a/cmd/fscrypt/commands.go b/cmd/fscrypt/commands.go index 9ebcc27..0b382a6 100644 --- a/cmd/fscrypt/commands.go +++ b/cmd/fscrypt/commands.go @@ -175,6 +175,26 @@ func validateKeyringPrereqs(ctx *actions.Context, policy *actions.Policy) error return nil } +func writeRecoveryInstructions(recoveryPassphrase *crypto.Key, recoveryProtector *actions.Protector, + policy *actions.Policy, dirPath string) error { + if recoveryPassphrase == nil { + return nil + } + recoveryFile := filepath.Join(dirPath, "fscrypt_recovery_readme.txt") + if err := actions.WriteRecoveryInstructions(recoveryPassphrase, recoveryProtector, + policy, recoveryFile); err != nil { + return err + } + msg := fmt.Sprintf(`See %q for important recovery instructions. + It is *strongly recommended* to record the recovery passphrase in a + secure location; otherwise you will lose access to this directory if you + reinstall the operating system or move this filesystem to another + system.`, recoveryFile) + hdr := "IMPORTANT: " + fmt.Print("\n" + hdr + wrapText(msg, len(hdr)) + "\n\n") + return nil +} + // encryptPath sets up encryption on path and provisions the policy to the // keyring unless --skip-unlock is used. On failure, an error is returned, any // metadata creation is reverted, and the directory is unmodified. @@ -193,6 +213,7 @@ func encryptPath(path string) (err error) { var policy *actions.Policy var recoveryPassphrase *crypto.Key + var recoveryProtector *actions.Protector if policyFlag.Value != "" { log.Printf("getting policy for %q", path) @@ -241,17 +262,8 @@ func encryptPath(path string) (err error) { } }() - // Ask to generate a recovery passphrase if the protector is on - // a different filesystem from the policy. In practice, this - // happens for login passphrase-protected directories that - // aren't on the root filesystem, since login protectors are - // always stored on the root filesystem. - var needRecovery bool + // Generate a recovery passphrase if needed. if ctx.Mount != protector.Context.Mount && !noRecoveryFlag.Value { - needRecovery, err = askQuestion("Protector is on a different filesystem! Generate a recovery passphrase (recommended)?", true) - } - if needRecovery { - var recoveryProtector *actions.Protector if recoveryPassphrase, recoveryProtector, err = actions.AddRecoveryPassphrase( policy, filepath.Base(path)); err != nil { return @@ -286,14 +298,7 @@ func encryptPath(path string) (err error) { if err = policy.Apply(path); err != nil { return } - if recoveryPassphrase != nil { - recoveryFile := filepath.Join(path, "fscrypt_recovery_readme.txt") - if err = actions.WriteRecoveryInstructions(recoveryPassphrase, recoveryFile); err != nil { - return - } - fmt.Printf("See %q for important recovery instructions!\n", recoveryFile) - } - return + return writeRecoveryInstructions(recoveryPassphrase, recoveryProtector, policy, path) } // checkEncryptable returns an error if the path cannot be encrypted. diff --git a/cmd/fscrypt/flags.go b/cmd/fscrypt/flags.go index cb2e0ac..044b71e 100644 --- a/cmd/fscrypt/flags.go +++ b/cmd/fscrypt/flags.go @@ -174,7 +174,7 @@ var ( } noRecoveryFlag = &boolFlag{ Name: "no-recovery", - Usage: `Don't ask to generate a recovery passphrase.`, + Usage: `Don't generate a recovery passphrase.`, } ) diff --git a/cmd/fscrypt/format.go b/cmd/fscrypt/format.go index fc10d31..28cc22d 100644 --- a/cmd/fscrypt/format.go +++ b/cmd/fscrypt/format.go @@ -65,7 +65,7 @@ func init() { // We use the width of the terminal unless we cannot get the width. width, _, err := term.GetSize(int(os.Stdout.Fd())) - if err != nil { + if err != nil || width <= 0 { lineLength = fallbackLineLength } else { lineLength = util.MinInt(width, maxLineLength) -- 2.39.5