Merge pull request #337 from google/broken-links

filesystem: store mountpoint in link files as a fallback

Author: ebiggers

cli-tests/t_encrypt_login.out

@@ -174,3 +174,20 @@ ext4 filesystem "MNT_ROOT" has 0 protectors and 0 policies
 
 [ERROR] fscrypt status: file or directory "MNT/dir" is not
                         encrypted
+
+# Test that linked protector works even if UUID link is broken
+
+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
+desc39  No                                  custom protector "Recovery passphrase for dir"
+desc40  Yes (MNT_ROOT)  login protector for fscrypt-test-user
+
+POLICY                            UNLOCKED  PROTECTORS
+desc41  Yes       desc40, desc39

cli-tests/t_encrypt_login.sh

@@ -86,3 +86,11 @@ chown "$TEST_USER" "$dir"
 _user_do_and_expect_failure \
 	"echo wrong_passphrase | fscrypt encrypt --quiet --source=pam_passphrase '$dir'"
 show_status false
+
+begin "Test that linked protector works even if UUID link is broken"
+echo TEST_USER_PASS | fscrypt encrypt --quiet --source=pam_passphrase --user="$TEST_USER" "$dir"
+protector=$(get_login_protector)
+link_file=$MNT/.fscrypt/protectors/$protector.link
+[ -e "$link_file" ] || _fail "$link_file does not exist"
+sed -i 's/UUID=.*/UUID=00000000-0000-0000-0000-000000000000/' "$link_file"
+fscrypt status "$MNT"

filesystem/filesystem.go

@@ -176,8 +176,7 @@ var SortDescriptorsByLastMtime = false
 //
 // There is also the ability to reference another filesystem's metadata. This is
 // used when a Policy on filesystem A is protected with Protector on filesystem
-// B. In this scenario, we store a "link file" in the protectors directory whose
-// contents look like "UUID=3a6d9a76-47f0-4f13-81bf-3332fbe984fb".
+// B. In this scenario, we store a "link file" in the protectors directory.
 //
 // We also allow ".fscrypt" to be a symlink which was previously created. This
 // allows login protectors to be created when the root filesystem is read-only,
@@ -588,7 +587,7 @@ func (m *Mount) AddLinkedProtector(descriptor string, dest *Mount) (bool, error)
 
 	// Right now, we only make links using UUIDs.
 	var newLink string
-	newLink, err = makeLink(dest, "UUID")
+	newLink, err = makeLink(dest)
 	if err != nil {
 		return false, err
 	}

filesystem/mountpoint.go

@@ -51,6 +51,7 @@ var (
 	mountsInitialized bool
 	// Supported tokens for filesystem links
 	uuidToken = "UUID"
+	pathToken = "PATH"
 	// Location to perform UUID lookup
 	uuidDirectory = "/dev/disk/by-uuid"
 )
@@ -399,78 +400,132 @@ func GetMount(mountpoint string) (*Mount, error) {
 	return mnt, nil
 }
 
-// getMountFromLink returns the Mount object which matches the provided link.
-// This link is formatted as a tag (e.g. <token>=<value>) similar to how they
-// appear in "/etc/fstab". Currently, only "UUID" tokens are supported. An error
-// is returned if the link is invalid or we cannot load the required mount data.
-// If a mount has been updated since the last call to one of the mount
-// functions, run UpdateMountInfo to see the change.
-func getMountFromLink(link string) (*Mount, error) {
-	// Parse the link
-	link = strings.TrimSpace(link)
-	linkComponents := strings.Split(link, "=")
-	if len(linkComponents) != 2 {
-		return nil, &ErrFollowLink{link, errors.New("invalid link format")}
-	}
-	token := linkComponents[0]
-	value := linkComponents[1]
-	if token != uuidToken {
-		return nil, &ErrFollowLink{link, errors.Errorf("token type %q not supported", token)}
-	}
-
-	// See if UUID points to an existing device
-	searchPath := filepath.Join(uuidDirectory, value)
-	if filepath.Base(searchPath) != value {
-		return nil, &ErrFollowLink{link, errors.Errorf("invalid UUID format %q", value)}
-	}
-	deviceNumber, err := getDeviceNumber(searchPath)
-	if err != nil {
-		return nil, &ErrFollowLink{link, errors.Errorf("no device with UUID %s", value)}
-	}
+func uuidToDeviceNumber(uuid string) (DeviceNumber, error) {
+	uuidSymlinkPath := filepath.Join(uuidDirectory, uuid)
+	return getDeviceNumber(uuidSymlinkPath)
+}
 
-	// Lookup mountpoints for device in global store
+func deviceNumberToMount(deviceNumber DeviceNumber) (*Mount, bool) {
 	mountMutex.Lock()
 	defer mountMutex.Unlock()
 	if err := loadMountInfo(); err != nil {
-		return nil, err
+		log.Print(err)
+		return nil, false
 	}
 	mnt, ok := mountsByDevice[deviceNumber]
-	if !ok {
-		return nil, &ErrFollowLink{link, errors.Errorf("no mounts for device %q (%v)",
-			getDeviceName(deviceNumber), deviceNumber)}
+	return mnt, ok
+}
+
+// getMountFromLink returns the main Mount, if any, for the filesystem which the
+// given link points to.  The link should contain a series of token-value pairs
+// (<token>=<value>), one per line.  The supported tokens are "UUID" and "PATH".
+// If the UUID is present and it works, then it is used; otherwise, PATH is used
+// if it is present.  (The fallback from UUID to PATH will keep the link working
+// if the UUID of the target filesystem changes but its mountpoint doesn't.)
+//
+// If a mount has been updated since the last call to one of the mount
+// functions, make sure to run UpdateMountInfo first.
+func getMountFromLink(link string) (*Mount, error) {
+	// Parse the link.
+	uuid := ""
+	path := ""
+	lines := strings.Split(link, "\n")
+	for _, line := range lines {
+		line := strings.TrimSpace(line)
+		if line == "" {
+			continue
+		}
+		pair := strings.Split(line, "=")
+		if len(pair) != 2 {
+			log.Printf("ignoring invalid line in filesystem link file: %q", line)
+			continue
+		}
+		token := pair[0]
+		value := pair[1]
+		switch token {
+		case uuidToken:
+			uuid = value
+		case pathToken:
+			path = value
+		default:
+			log.Printf("ignoring unknown link token %q", token)
+		}
 	}
-	if mnt == nil {
-		return nil, &ErrFollowLink{link, filesystemLacksMainMountError(deviceNumber)}
+	// At least one of UUID and PATH must be present.
+	if uuid == "" && path == "" {
+		return nil, &ErrFollowLink{link, errors.Errorf("invalid filesystem link file")}
 	}
-	return mnt, nil
-}
 
-// makeLink returns a link of the form <token>=<value> where value is the tag
-// value for the Mount's device. Currently, only "UUID" tokens are supported. An
-// error is returned if the mount has no device, or no UUID.
-func makeLink(mnt *Mount, token string) (string, error) {
-	if token != uuidToken {
-		return "", &ErrMakeLink{mnt, errors.Errorf("token type %q not supported", token)}
+	// Try following the UUID.
+	errMsg := ""
+	if uuid != "" {
+		deviceNumber, err := uuidToDeviceNumber(uuid)
+		if err == nil {
+			mnt, ok := deviceNumberToMount(deviceNumber)
+			if mnt != nil {
+				log.Printf("resolved filesystem link using UUID %q", uuid)
+				return mnt, nil
+			}
+			if ok {
+				return nil, &ErrFollowLink{link, filesystemLacksMainMountError(deviceNumber)}
+			}
+			log.Printf("cannot find filesystem with UUID %q", uuid)
+		} else {
+			log.Printf("cannot find filesystem with UUID %q: %v", uuid, err)
+		}
+		errMsg += fmt.Sprintf("cannot find filesystem with UUID %q", uuid)
+		if path != "" {
+			log.Printf("falling back to using mountpoint path instead of UUID")
+		}
 	}
+	// UUID didn't work.  As a fallback, try the mountpoint path.
+	if path != "" {
+		mnt, err := GetMount(path)
+		if mnt != nil {
+			log.Printf("resolved filesystem link using mountpoint path %q", path)
+			return mnt, nil
+		}
+		log.Print(err)
+		if errMsg == "" {
+			errMsg = fmt.Sprintf("cannot find filesystem with main mountpoint %q", path)
+		} else {
+			errMsg += fmt.Sprintf(" or main mountpoint %q", path)
+		}
+	}
+	// No method worked; return an error.
+	return nil, &ErrFollowLink{link, errors.New(errMsg)}
+}
 
+func (mnt *Mount) getFilesystemUUID() (string, error) {
 	dirContents, err := ioutil.ReadDir(uuidDirectory)
 	if err != nil {
-		return "", &ErrMakeLink{mnt, err}
+		return "", err
 	}
 	for _, fileInfo := range dirContents {
 		if fileInfo.Mode()&os.ModeSymlink == 0 {
 			continue // Only interested in UUID symlinks
 		}
 		uuid := fileInfo.Name()
-		deviceNumber, err := getDeviceNumber(filepath.Join(uuidDirectory, uuid))
+		deviceNumber, err := uuidToDeviceNumber(uuid)
 		if err != nil {
 			log.Print(err)
 			continue
 		}
 		if mnt.DeviceNumber == deviceNumber {
-			return fmt.Sprintf("%s=%s", uuidToken, uuid), nil
+			return uuid, nil
 		}
 	}
-	return "", &ErrMakeLink{mnt, errors.Errorf("cannot determine UUID of device %q (%v)",
-		mnt.Device, mnt.DeviceNumber)}
+	return "", errors.Errorf("cannot determine UUID of device %q (%v)",
+		mnt.Device, mnt.DeviceNumber)
+}
+
+// makeLink creates the contents of a link file which will point to the given
+// filesystem.  This will be a string of the form "UUID=<uuid>\nPATH=<path>\n".
+// An error is returned if the filesystem's UUID cannot be determined.
+func makeLink(mnt *Mount) (string, error) {
+	uuid, err := mnt.getFilesystemUUID()
+	if err != nil {
+		return "", &ErrMakeLink{mnt, err}
+	}
+	return fmt.Sprintf("%s=%s\n%s=%s\n", uuidToken, uuid, pathToken, mnt.Path), nil
 }

filesystem/mountpoint_test.go

@@ -373,14 +373,14 @@ func TestLoadAmbiguousMounts(t *testing.T) {
 	}
 }
 
-// Test making a filesystem link (i.e. "UUID=...") and following it, and test
-// that leading and trailing whitespace in the link is ignored.
+// Test making a filesystem link and following it, and test that leading and
+// trailing whitespace in the link is ignored.
 func TestGetMountFromLink(t *testing.T) {
 	mnt, err := getTestMount(t)
 	if err != nil {
 		t.Skip(err)
 	}
-	link, err := makeLink(mnt, uuidToken)
+	link, err := makeLink(mnt)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -405,6 +405,68 @@ func TestGetMountFromLink(t *testing.T) {
 	}
 }
 
+// Test that old filesystem links that contain a UUID only still work.
+func TestGetMountFromLegacyLink(t *testing.T) {
+	mnt, err := getTestMount(t)
+	if err != nil {
+		t.Skip(err)
+	}
+	uuid, err := mnt.getFilesystemUUID()
+	if uuid == "" || err != nil {
+		t.Fatal("Can't get UUID of test filesystem")
+	}
+
+	link := fmt.Sprintf("UUID=%s", uuid)
+	linkedMnt, err := getMountFromLink(link)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if linkedMnt != mnt {
+		t.Fatal("Link doesn't point to the same Mount")
+	}
+}
+
+// Test that if the UUID in a filesystem link doesn't work, then the PATH is
+// used instead, and vice versa.
+func TestGetMountFromLinkFallback(t *testing.T) {
+	mnt, err := getTestMount(t)
+	if err != nil {
+		t.Skip(err)
+	}
+	badUUID := "00000000-0000-0000-0000-000000000000"
+	badPath := "/NONEXISTENT_MOUNT"
+	goodUUID, err := mnt.getFilesystemUUID()
+	if goodUUID == "" || err != nil {
+		t.Fatal("Can't get UUID of test filesystem")
+	}
+
+	// only PATH valid (should succeed)
+	link := fmt.Sprintf("UUID=%s\nPATH=%s\n", badUUID, mnt.Path)
+	linkedMnt, err := getMountFromLink(link)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if linkedMnt != mnt {
+		t.Fatal("Link doesn't point to the same Mount")
+	}
+
+	// only UUID valid (should succeed)
+	link = fmt.Sprintf("UUID=%s\nPATH=%s\n", goodUUID, badPath)
+	if linkedMnt, err = getMountFromLink(link); err != nil {
+		t.Fatal(err)
+	}
+	if linkedMnt != mnt {
+		t.Fatal("Link doesn't point to the same Mount")
+	}
+
+	// neither valid (should fail)
+	link = fmt.Sprintf("UUID=%s\nPATH=%s\n", badUUID, badPath)
+	linkedMnt, err = getMountFromLink(link)
+	if linkedMnt != nil || err == nil {
+		t.Fatal("Following a bad link succeeded")
+	}
+}
+
 // Benchmarks how long it takes to update the mountpoint data
 func BenchmarkLoadFirst(b *testing.B) {
 	for n := 0; n < b.N; n++ {