Skip to content

Commit 5ae7da4

Browse files
committed
filesystem: store mountpoint in link files as a fallback
Currently, linked protectors use filesystem link files of the form "UUID=<uuid>". These links get broken if the filesystem's UUID changes, e.g. due to the filesystem being re-created even if the ".fscrypt" directory is backed up and restored. To prevent links from being broken (in most cases), start storing the mountpoint path in the link files too, in the form "UUID=<uuid>\nPATH=<path>\n". When following a link, try the UUID first, and if it doesn't work try the PATH. While it's possible that the path changed too, for login protectors (the usual use case of linked protectors) this won't be an issue as the path will always be "/". An alternative solution would be to fall back to scanning all filesystems for the needed protector descriptor. I decided not to do that, since relying on a global scan doesn't seem to be a good design. It wouldn't scale to large numbers of filesystems, it could cross security boundaries, and it would make it possible for adding a new filesystem to break fscrypt on existing filesystems. And if a global scan was an acceptable way to find protectors during normal use, then there would be no need for link files in the first place. Note: this change is backwards compatible (i.e., fscrypt will continue to recognize old link files) but not forwards-compatible (i.e., previous versions of fscrypt won't recognize new link files). Fixes #311
1 parent 6ec8ee0 commit 5ae7da4

5 files changed

Lines changed: 195 additions & 54 deletions

File tree

cli-tests/t_encrypt_login.out

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,3 +174,20 @@ ext4 filesystem "MNT_ROOT" has 0 protectors and 0 policies
174174

175175
[ERROR] fscrypt status: file or directory "MNT/dir" is not
176176
encrypted
177+
178+
# Test that linked protector works even if UUID link is broken
179+
180+
IMPORTANT: See "MNT/dir/fscrypt_recovery_readme.txt" for
181+
important recovery instructions. It is *strongly recommended* to
182+
record the recovery passphrase in a secure location; otherwise you
183+
will lose access to this directory if you reinstall the operating
184+
system or move this filesystem to another system.
185+
186+
ext4 filesystem "MNT" has 2 protectors and 1 policy
187+
188+
PROTECTOR LINKED DESCRIPTION
189+
desc39 No custom protector "Recovery passphrase for dir"
190+
desc40 Yes (MNT_ROOT) login protector for fscrypt-test-user
191+
192+
POLICY UNLOCKED PROTECTORS
193+
desc41 Yes desc40, desc39

cli-tests/t_encrypt_login.sh

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,3 +91,11 @@ chown "$TEST_USER" "$dir"
9191
_user_do_and_expect_failure \
9292
"echo wrong_passphrase | fscrypt encrypt --quiet --source=pam_passphrase '$dir'"
9393
show_status false
94+
95+
begin "Test that linked protector works even if UUID link is broken"
96+
echo TEST_USER_PASS | fscrypt encrypt --quiet --source=pam_passphrase --user="$TEST_USER" "$dir"
97+
protector=$(get_login_protector)
98+
link_file=$MNT/.fscrypt/protectors/$protector.link
99+
[ -e "$link_file" ] || _fail "$link_file does not exist"
100+
sed -i 's/UUID=.*/UUID=00000000-0000-0000-0000-000000000000/' "$link_file"
101+
fscrypt status "$MNT"

filesystem/filesystem.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -176,8 +176,7 @@ var SortDescriptorsByLastMtime = false
176176
//
177177
// There is also the ability to reference another filesystem's metadata. This is
178178
// used when a Policy on filesystem A is protected with Protector on filesystem
179-
// B. In this scenario, we store a "link file" in the protectors directory whose
180-
// contents look like "UUID=3a6d9a76-47f0-4f13-81bf-3332fbe984fb".
179+
// B. In this scenario, we store a "link file" in the protectors directory.
181180
//
182181
// We also allow ".fscrypt" to be a symlink which was previously created. This
183182
// 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)
588587

589588
// Right now, we only make links using UUIDs.
590589
var newLink string
591-
newLink, err = makeLink(dest, "UUID")
590+
newLink, err = makeLink(dest)
592591
if err != nil {
593592
return false, err
594593
}

filesystem/mountpoint.go

Lines changed: 103 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ var (
5151
mountsInitialized bool
5252
// Supported tokens for filesystem links
5353
uuidToken = "UUID"
54+
pathToken = "PATH"
5455
// Location to perform UUID lookup
5556
uuidDirectory = "/dev/disk/by-uuid"
5657
)
@@ -399,78 +400,132 @@ func GetMount(mountpoint string) (*Mount, error) {
399400
return mnt, nil
400401
}
401402

402-
// getMountFromLink returns the Mount object which matches the provided link.
403-
// This link is formatted as a tag (e.g. <token>=<value>) similar to how they
404-
// appear in "/etc/fstab". Currently, only "UUID" tokens are supported. An error
405-
// is returned if the link is invalid or we cannot load the required mount data.
406-
// If a mount has been updated since the last call to one of the mount
407-
// functions, run UpdateMountInfo to see the change.
408-
func getMountFromLink(link string) (*Mount, error) {
409-
// Parse the link
410-
link = strings.TrimSpace(link)
411-
linkComponents := strings.Split(link, "=")
412-
if len(linkComponents) != 2 {
413-
return nil, &ErrFollowLink{link, errors.New("invalid link format")}
414-
}
415-
token := linkComponents[0]
416-
value := linkComponents[1]
417-
if token != uuidToken {
418-
return nil, &ErrFollowLink{link, errors.Errorf("token type %q not supported", token)}
419-
}
420-
421-
// See if UUID points to an existing device
422-
searchPath := filepath.Join(uuidDirectory, value)
423-
if filepath.Base(searchPath) != value {
424-
return nil, &ErrFollowLink{link, errors.Errorf("invalid UUID format %q", value)}
425-
}
426-
deviceNumber, err := getDeviceNumber(searchPath)
427-
if err != nil {
428-
return nil, &ErrFollowLink{link, errors.Errorf("no device with UUID %s", value)}
429-
}
403+
func uuidToDeviceNumber(uuid string) (DeviceNumber, error) {
404+
uuidSymlinkPath := filepath.Join(uuidDirectory, uuid)
405+
return getDeviceNumber(uuidSymlinkPath)
406+
}
430407

431-
// Lookup mountpoints for device in global store
408+
func deviceNumberToMount(deviceNumber DeviceNumber) (*Mount, bool) {
432409
mountMutex.Lock()
433410
defer mountMutex.Unlock()
434411
if err := loadMountInfo(); err != nil {
435-
return nil, err
412+
log.Print(err)
413+
return nil, false
436414
}
437415
mnt, ok := mountsByDevice[deviceNumber]
438-
if !ok {
439-
return nil, &ErrFollowLink{link, errors.Errorf("no mounts for device %q (%v)",
440-
getDeviceName(deviceNumber), deviceNumber)}
416+
return mnt, ok
417+
}
418+
419+
// getMountFromLink returns the main Mount, if any, for the filesystem which the
420+
// given link points to. The link should contain a series of token-value pairs
421+
// (<token>=<value>), one per line. The supported tokens are "UUID" and "PATH".
422+
// If the UUID is present and it works, then it is used; otherwise, PATH is used
423+
// if it is present. (The fallback from UUID to PATH will keep the link working
424+
// if the UUID of the target filesystem changes but its mountpoint doesn't.)
425+
//
426+
// If a mount has been updated since the last call to one of the mount
427+
// functions, make sure to run UpdateMountInfo first.
428+
func getMountFromLink(link string) (*Mount, error) {
429+
// Parse the link.
430+
uuid := ""
431+
path := ""
432+
lines := strings.Split(link, "\n")
433+
for _, line := range lines {
434+
line := strings.TrimSpace(line)
435+
if line == "" {
436+
continue
437+
}
438+
pair := strings.Split(line, "=")
439+
if len(pair) != 2 {
440+
log.Printf("ignoring invalid line in filesystem link file: %q", line)
441+
continue
442+
}
443+
token := pair[0]
444+
value := pair[1]
445+
switch token {
446+
case uuidToken:
447+
uuid = value
448+
case pathToken:
449+
path = value
450+
default:
451+
log.Printf("ignoring unknown link token %q", token)
452+
}
441453
}
442-
if mnt == nil {
443-
return nil, &ErrFollowLink{link, filesystemLacksMainMountError(deviceNumber)}
454+
// At least one of UUID and PATH must be present.
455+
if uuid == "" && path == "" {
456+
return nil, &ErrFollowLink{link, errors.Errorf("invalid filesystem link file")}
444457
}
445-
return mnt, nil
446-
}
447458

448-
// makeLink returns a link of the form <token>=<value> where value is the tag
449-
// value for the Mount's device. Currently, only "UUID" tokens are supported. An
450-
// error is returned if the mount has no device, or no UUID.
451-
func makeLink(mnt *Mount, token string) (string, error) {
452-
if token != uuidToken {
453-
return "", &ErrMakeLink{mnt, errors.Errorf("token type %q not supported", token)}
459+
// Try following the UUID.
460+
errMsg := ""
461+
if uuid != "" {
462+
deviceNumber, err := uuidToDeviceNumber(uuid)
463+
if err == nil {
464+
mnt, ok := deviceNumberToMount(deviceNumber)
465+
if mnt != nil {
466+
log.Printf("resolved filesystem link using UUID %q", uuid)
467+
return mnt, nil
468+
}
469+
if ok {
470+
return nil, &ErrFollowLink{link, filesystemLacksMainMountError(deviceNumber)}
471+
}
472+
log.Printf("cannot find filesystem with UUID %q", uuid)
473+
} else {
474+
log.Printf("cannot find filesystem with UUID %q: %v", uuid, err)
475+
}
476+
errMsg += fmt.Sprintf("cannot find filesystem with UUID %q", uuid)
477+
if path != "" {
478+
log.Printf("falling back to using mountpoint path instead of UUID")
479+
}
454480
}
481+
// UUID didn't work. As a fallback, try the mountpoint path.
482+
if path != "" {
483+
mnt, err := GetMount(path)
484+
if mnt != nil {
485+
log.Printf("resolved filesystem link using mountpoint path %q", path)
486+
return mnt, nil
487+
}
488+
log.Print(err)
489+
if errMsg == "" {
490+
errMsg = fmt.Sprintf("cannot find filesystem with main mountpoint %q", path)
491+
} else {
492+
errMsg += fmt.Sprintf(" or main mountpoint %q", path)
493+
}
494+
}
495+
// No method worked; return an error.
496+
return nil, &ErrFollowLink{link, errors.New(errMsg)}
497+
}
455498

499+
func (mnt *Mount) getFilesystemUUID() (string, error) {
456500
dirContents, err := ioutil.ReadDir(uuidDirectory)
457501
if err != nil {
458-
return "", &ErrMakeLink{mnt, err}
502+
return "", err
459503
}
460504
for _, fileInfo := range dirContents {
461505
if fileInfo.Mode()&os.ModeSymlink == 0 {
462506
continue // Only interested in UUID symlinks
463507
}
464508
uuid := fileInfo.Name()
465-
deviceNumber, err := getDeviceNumber(filepath.Join(uuidDirectory, uuid))
509+
deviceNumber, err := uuidToDeviceNumber(uuid)
466510
if err != nil {
467511
log.Print(err)
468512
continue
469513
}
470514
if mnt.DeviceNumber == deviceNumber {
471-
return fmt.Sprintf("%s=%s", uuidToken, uuid), nil
515+
return uuid, nil
472516
}
473517
}
474-
return "", &ErrMakeLink{mnt, errors.Errorf("cannot determine UUID of device %q (%v)",
475-
mnt.Device, mnt.DeviceNumber)}
518+
return "", errors.Errorf("cannot determine UUID of device %q (%v)",
519+
mnt.Device, mnt.DeviceNumber)
520+
}
521+
522+
// makeLink creates the contents of a link file which will point to the given
523+
// filesystem. This will be a string of the form "UUID=<uuid>\nPATH=<path>\n".
524+
// An error is returned if the filesystem's UUID cannot be determined.
525+
func makeLink(mnt *Mount) (string, error) {
526+
uuid, err := mnt.getFilesystemUUID()
527+
if err != nil {
528+
return "", &ErrMakeLink{mnt, err}
529+
}
530+
return fmt.Sprintf("%s=%s\n%s=%s\n", uuidToken, uuid, pathToken, mnt.Path), nil
476531
}

filesystem/mountpoint_test.go

Lines changed: 65 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -373,14 +373,14 @@ func TestLoadAmbiguousMounts(t *testing.T) {
373373
}
374374
}
375375

376-
// Test making a filesystem link (i.e. "UUID=...") and following it, and test
377-
// that leading and trailing whitespace in the link is ignored.
376+
// Test making a filesystem link and following it, and test that leading and
377+
// trailing whitespace in the link is ignored.
378378
func TestGetMountFromLink(t *testing.T) {
379379
mnt, err := getTestMount(t)
380380
if err != nil {
381381
t.Skip(err)
382382
}
383-
link, err := makeLink(mnt, uuidToken)
383+
link, err := makeLink(mnt)
384384
if err != nil {
385385
t.Fatal(err)
386386
}
@@ -405,6 +405,68 @@ func TestGetMountFromLink(t *testing.T) {
405405
}
406406
}
407407

408+
// Test that old filesystem links that contain a UUID only still work.
409+
func TestGetMountFromLegacyLink(t *testing.T) {
410+
mnt, err := getTestMount(t)
411+
if err != nil {
412+
t.Skip(err)
413+
}
414+
uuid, err := mnt.getFilesystemUUID()
415+
if uuid == "" || err != nil {
416+
t.Fatal("Can't get UUID of test filesystem")
417+
}
418+
419+
link := fmt.Sprintf("UUID=%s", uuid)
420+
linkedMnt, err := getMountFromLink(link)
421+
if err != nil {
422+
t.Fatal(err)
423+
}
424+
if linkedMnt != mnt {
425+
t.Fatal("Link doesn't point to the same Mount")
426+
}
427+
}
428+
429+
// Test that if the UUID in a filesystem link doesn't work, then the PATH is
430+
// used instead, and vice versa.
431+
func TestGetMountFromLinkFallback(t *testing.T) {
432+
mnt, err := getTestMount(t)
433+
if err != nil {
434+
t.Skip(err)
435+
}
436+
badUUID := "00000000-0000-0000-0000-000000000000"
437+
badPath := "/NONEXISTENT_MOUNT"
438+
goodUUID, err := mnt.getFilesystemUUID()
439+
if goodUUID == "" || err != nil {
440+
t.Fatal("Can't get UUID of test filesystem")
441+
}
442+
443+
// only PATH valid (should succeed)
444+
link := fmt.Sprintf("UUID=%s\nPATH=%s\n", badUUID, mnt.Path)
445+
linkedMnt, err := getMountFromLink(link)
446+
if err != nil {
447+
t.Fatal(err)
448+
}
449+
if linkedMnt != mnt {
450+
t.Fatal("Link doesn't point to the same Mount")
451+
}
452+
453+
// only UUID valid (should succeed)
454+
link = fmt.Sprintf("UUID=%s\nPATH=%s\n", goodUUID, badPath)
455+
if linkedMnt, err = getMountFromLink(link); err != nil {
456+
t.Fatal(err)
457+
}
458+
if linkedMnt != mnt {
459+
t.Fatal("Link doesn't point to the same Mount")
460+
}
461+
462+
// neither valid (should fail)
463+
link = fmt.Sprintf("UUID=%s\nPATH=%s\n", badUUID, badPath)
464+
linkedMnt, err = getMountFromLink(link)
465+
if linkedMnt != nil || err == nil {
466+
t.Fatal("Following a bad link succeeded")
467+
}
468+
}
469+
408470
// Benchmarks how long it takes to update the mountpoint data
409471
func BenchmarkLoadFirst(b *testing.B) {
410472
for n := 0; n < b.N; n++ {

0 commit comments

Comments
 (0)