From 67879f7a69e2040f2266d81ba239324534e24405 Mon Sep 17 00:00:00 2001 From: "Joe Richey joerichey@google.com" Date: Wed, 31 May 2017 17:52:15 -0700 Subject: filesystem: change support detection and bug-fixes Instead of checking if the filesystem type is correct, we now detect if a filesystem supports encryption by trying to read a policy on its root directory. The error returned tells us if there is support or not. This commit also fixes a bug in the use libblkid. Throughout all of fscrypt, cannonicalizePath() is used before any path comparison or lookup. However, the canonical device path in the blkid cache may differ from our idea of a canonical path. Additional blkid functions are needed to perform the necessary translation. This is noted in the documentation of makeLink(). Finally, this commit makes a few API changes. AllSupporedFilesystems() now returns an error, and a GetProtector() method now replaces the GetLinkedProtector() and GetEitherProtector() methods. A PathSorter has also been added so Mounts can be sorted in a reliable order. Change-Id: I664f46fafd1483ebecb743c061b03d708b3233a4 --- filesystem/filesystem.go | 120 +++++++++++++++++++++++++----------------- filesystem/filesystem_test.go | 6 +-- filesystem/mountpoint.go | 96 ++++++++++++++++++--------------- filesystem/mountpoint_test.go | 13 ++--- 4 files changed, 134 insertions(+), 101 deletions(-) (limited to 'filesystem') diff --git a/filesystem/filesystem.go b/filesystem/filesystem.go index 649345f..434826b 100644 --- a/filesystem/filesystem.go +++ b/filesystem/filesystem.go @@ -110,6 +110,13 @@ type Mount struct { Device string } +// PathSorter allows mounts to be sorted by Path. +type PathSorter []*Mount + +func (p PathSorter) Len() int { return len(p) } +func (p PathSorter) Swap(i, j int) { p[i], p[j] = p[j], p[i] } +func (p PathSorter) Less(i, j int) bool { return p[i].Path < p[j].Path } + const ( // Names of the various directories used in fscrypt baseDirName = ".fscrypt" @@ -128,6 +135,13 @@ const ( filePermissions = 0644 ) +func (m *Mount) String() string { + return fmt.Sprintf(`%s + Filsystem: %s + Options: %v + Device: %s`, m.Path, m.Filesystem, m.Options, m.Device) +} + // baseDir returns the path of the base fscrypt directory on this filesystem. func (m *Mount) baseDir() string { return filepath.Join(m.Path, baseDirName) @@ -183,15 +197,18 @@ func (m *Mount) err(err error) error { } } -// IsSetup returns true if all the fscrypt metadata directories exist. Will log -// any unexpected errors, or if any permissions are incorrect. -func (m *Mount) IsSetup() bool { +// CheckSetup returns an error if all the fscrypt metadata directories exist. +// Will log any unexpected errors, or if any permissions are incorrect. +func (m *Mount) CheckSetup() error { // Run all the checks so we will always get all the warnings baseGood := isDirCheckPerm(m.baseDir(), basePermissions) policyGood := isDirCheckPerm(m.policyDir(), dirPermissions) protectorGood := isDirCheckPerm(m.protectorDir(), dirPermissions) - return baseGood && policyGood && protectorGood + if baseGood && policyGood && protectorGood { + return nil + } + return m.err(ErrNotSetup) } // makeDirectories creates the three metadata directories with the correct @@ -217,10 +234,9 @@ func (m *Mount) makeDirectories() error { // the filesystem's feature flags. This operation is atomic, it either succeeds // or no files in the baseDir are created. func (m *Mount) Setup() error { - if m.IsSetup() { + if m.CheckSetup() == nil { return m.err(ErrAlreadySetup) } - // We build the directories under a temp Mount and then move into place. temp, err := m.tempMount() if err != nil { @@ -247,10 +263,9 @@ func (m *Mount) Setup() error { // WARNING: Will cause data loss if the metadata is used to encrypt // directories (this could include directories on other filesystems). func (m *Mount) RemoveAllMetadata() error { - if !m.IsSetup() { - return m.err(ErrNotSetup) + if err := m.CheckSetup(); err != nil { + return err } - // temp will hold the old metadata temporarily temp, err := m.tempMount() if err != nil { @@ -267,32 +282,28 @@ func (m *Mount) RemoveAllMetadata() error { func (m *Mount) writeDataAtomic(path string, data []byte) error { // Write the file to a temporary file then move into place so that the // operation will be atomic. - tmpFile, err := ioutil.TempFile(filepath.Dir(path), tempPrefix) + tempPath := filepath.Join(filepath.Dir(path), tempPrefix+filepath.Base(path)) + // We use O_SYNC so the write actually gets to stable storage. + tempFile, err := os.OpenFile(tempPath, os.O_WRONLY|os.O_CREATE|os.O_SYNC, filePermissions) if err != nil { return err } - defer os.Remove(tmpFile.Name()) + defer os.Remove(tempPath) - // Make sure the write actually gets to stable storage. - if _, err = tmpFile.Write(data); err != nil { - return err - } - if err = tmpFile.Sync(); err != nil { + if _, err = tempFile.Write(data); err != nil { + tempFile.Close() return err } - if err = tmpFile.Close(); err != nil { + if err = tempFile.Close(); err != nil { return err } - return os.Rename(tmpFile.Name(), path) + return os.Rename(tempPath, path) } // addMetadata writes the metadata structure to the file with the specified // path this will overwrite any existing data. The operation is atomic. func (m *Mount) addMetadata(path string, md metadata.Metadata) error { - if !m.IsSetup() { - return ErrNotSetup - } if !md.IsValid() { return ErrInvalidMetadata } @@ -309,10 +320,6 @@ func (m *Mount) addMetadata(path string, md metadata.Metadata) error { // getMetadata reads the metadata structure from the file with the specified // path. Only reads normal metadata files, not linked metadata. func (m *Mount) getMetadata(path string, md metadata.Metadata) error { - if !m.IsSetup() { - return ErrNotSetup - } - data, err := ioutil.ReadFile(path) if err != nil { if os.IsNotExist(err) { @@ -354,6 +361,9 @@ func (m *Mount) removeMetadata(path string) error { // will fail with ErrLinkedProtector if a linked protector with this descriptor // already exists on the filesystem. func (m *Mount) AddProtector(data *metadata.ProtectorData) error { + if err := m.CheckSetup(); err != nil { + return err + } if isRegularFile(m.linkedProtectorPath(data.ProtectorDescriptor)) { return m.err(ErrLinkedProtector) } @@ -364,6 +374,9 @@ func (m *Mount) AddProtector(data *metadata.ProtectorData) error { // AddLinkedProtector adds a link in this filesystem to the protector metadata // in the dest filesystem. func (m *Mount) AddLinkedProtector(descriptor string, dest *Mount) error { + if err := m.CheckSetup(); err != nil { + return err + } // Check that the link is good (descriptor exists, filesystem has UUID). if _, err := dest.GetRegularProtector(descriptor); err != nil { return err @@ -382,19 +395,28 @@ func (m *Mount) AddLinkedProtector(descriptor string, dest *Mount) error { // GetRegularProtector looks up the protector metadata by descriptor. This will // fail with ErrNoMetadata if the descriptor is a linked protector. func (m *Mount) GetRegularProtector(descriptor string) (*metadata.ProtectorData, error) { + if err := m.CheckSetup(); err != nil { + return nil, err + } data := new(metadata.ProtectorData) path := m.protectorPath(descriptor) return data, m.err(m.getMetadata(path, data)) } -// GetLinkedProtector returns the Mount of the filesystem containing the -// information for a linked protector and that protector's data. -func (m *Mount) GetLinkedProtector(descriptor string) (*Mount, *metadata.ProtectorData, error) { +// GetProtector returns the Mount of the filesystem containing the information +// and that protector's data. If the descriptor is a regular (not linked) +// protector, the mount will return itself. +func (m *Mount) GetProtector(descriptor string) (*Mount, *metadata.ProtectorData, error) { + if err := m.CheckSetup(); err != nil { + return nil, nil, err + } // Get the link data from the link file link, err := ioutil.ReadFile(m.linkedProtectorPath(descriptor)) if err != nil { + // If the link doesn't exist, try for a regular protector. if os.IsNotExist(err) { - err = ErrNoMetadata + data, err := m.GetRegularProtector(descriptor) + return m, data, err } return nil, nil, m.err(err) } @@ -414,19 +436,12 @@ func (m *Mount) GetLinkedProtector(descriptor string) (*Mount, *metadata.Protect return nil, nil, m.err(ErrOldLink) } -// GetEitherProtector looks up the protector metadata by descriptor. It will -// return the data for a linked protector or a regular protector. -func (m *Mount) GetEitherProtector(descriptor string) (*metadata.ProtectorData, error) { - if isRegularFile(m.linkedProtectorPath(descriptor)) { - _, data, err := m.GetLinkedProtector(descriptor) - return data, err - } - return m.GetRegularProtector(descriptor) -} - // RemoveProtector deletes the protector metadata (or an link to another // filesystem's metadata) from the filesystem storage. func (m *Mount) RemoveProtector(descriptor string) error { + if err := m.CheckSetup(); err != nil { + return err + } // We first try to remove the linkedProtector. If that metadata does not // exist, we try to remove the normal protector. err := m.removeMetadata(m.linkedProtectorPath(descriptor)) @@ -439,39 +454,51 @@ func (m *Mount) RemoveProtector(descriptor string) error { // ListProtectors lists the descriptors of all protectors on this filesystem. // This does not include linked protectors. func (m *Mount) ListProtectors() ([]string, error) { + if err := m.CheckSetup(); err != nil { + return nil, err + } protectors, err := m.listDirectory(m.protectorDir()) return protectors, m.err(err) } // AddPolicy adds the policy metadata to the filesystem storage. func (m *Mount) AddPolicy(data *metadata.PolicyData) error { + if err := m.CheckSetup(); err != nil { + return err + } + return m.err(m.addMetadata(m.policyPath(data.KeyDescriptor), data)) } // GetPolicy looks up the policy metadata by descriptor. func (m *Mount) GetPolicy(descriptor string) (*metadata.PolicyData, error) { + if err := m.CheckSetup(); err != nil { + return nil, err + } data := new(metadata.PolicyData) return data, m.err(m.getMetadata(m.policyPath(descriptor), data)) } // RemovePolicy deletes the policy metadata from the filesystem storage. func (m *Mount) RemovePolicy(descriptor string) error { + if err := m.CheckSetup(); err != nil { + return err + } return m.err(m.removeMetadata(m.policyPath(descriptor))) } // ListPolicies lists the descriptors of all policies on this filesystem. func (m *Mount) ListPolicies() ([]string, error) { + if err := m.CheckSetup(); err != nil { + return nil, err + } policies, err := m.listDirectory(m.policyDir()) return policies, m.err(err) } // listDirectory returns a list of descriptors for a metadata directory, -// excluding files which are links to other filesystem's metadata. +// including files which are links to other filesystem's metadata. func (m *Mount) listDirectory(directoryPath string) ([]string, error) { - if !m.IsSetup() { - return nil, ErrNotSetup - } - log.Printf("listing descriptors in %q", directoryPath) dir, err := os.Open(directoryPath) if err != nil { @@ -486,9 +513,8 @@ func (m *Mount) listDirectory(directoryPath string) ([]string, error) { var descriptors []string for _, name := range names { - if !strings.HasSuffix(name, linkFileExtension) { - descriptors = append(descriptors, name) - } + // Be sure to include links as well + descriptors = append(descriptors, strings.TrimSuffix(name, linkFileExtension)) } log.Printf("found %d descriptor(s)", len(descriptors)) diff --git a/filesystem/filesystem_test.go b/filesystem/filesystem_test.go index 31a131a..887b41e 100644 --- a/filesystem/filesystem_test.go +++ b/filesystem/filesystem_test.go @@ -88,8 +88,8 @@ func TestSetup(t *testing.T) { t.Fatal(err) } - if !mnt.IsSetup() { - t.Error("filesystem is not setup") + if err := mnt.CheckSetup(); err != nil { + t.Error(err) } os.RemoveAll(mnt.baseDir()) @@ -283,7 +283,7 @@ func TestLinkedProtector(t *testing.T) { t.Fatal(err) } - retMnt, retProtector, err := fakeMnt.GetLinkedProtector(protector.ProtectorDescriptor) + retMnt, retProtector, err := fakeMnt.GetProtector(protector.ProtectorDescriptor) if err != nil { t.Fatal(err) } diff --git a/filesystem/mountpoint.go b/filesystem/mountpoint.go index ddcc243..1a4b10f 100644 --- a/filesystem/mountpoint.go +++ b/filesystem/mountpoint.go @@ -33,6 +33,8 @@ const char* read_mode = "r"; // Helper function for freeing strings void string_free(char* str) { free(str); } + +// Helper function to lookup tokens */ import "C" @@ -42,16 +44,11 @@ import ( "path/filepath" "strings" "sync" + + "fscrypt/metadata" ) var ( - // SupportedFilesystems is a map of the filesystems which support - // filesystem-level encryption. - SupportedFilesystems = map[string]bool{ - "ext4": true, - "f2fs": true, - "ubifs": true, - } // These maps hold data about the state of the system's mountpoints. mountsByPath map[string]*Mount mountsByDevice map[string][]*Mount @@ -121,12 +118,7 @@ func getMountInfo() error { // filesystems are listed in mount order. mountsByPath[mnt.Path] = &mnt - // Use libblkid to get the device name - cDeviceName := C.blkid_evaluate_spec(entry.mnt_fsname, &cache) - defer C.string_free(cDeviceName) - - deviceName, err := cannonicalizePath(C.GoString(cDeviceName)) - + deviceName, err := cannonicalizePath(C.GoString(entry.mnt_fsname)) // Only use real valid devices (unlike cgroups, tmpfs, ...) if err == nil && isDevice(deviceName) { mnt.Device = deviceName @@ -138,30 +130,38 @@ func getMountInfo() error { // checkSupport returns an error if the specified mount does not support // filesystem-level encryption. func checkSupport(mount *Mount) error { - if SupportedFilesystems[mount.Filesystem] { + // Getting a policy on a filesystem which supports encryption should + // either return the policy or say there isn't one. Anything else + // indicates a problem with support. + _, err := metadata.GetPolicy(mount.Path) + if err == nil || err == metadata.ErrNotEncrypted { + log.Printf("%s filesystem at %q supports encryption (got %v)", + mount.Filesystem, mount.Path, err) return nil } - log.Printf("filesystem %s does not support filesystem encryption", mount.Filesystem) - return ErrNoSupport + + log.Printf("%s filesystem at %q probably doesn't support encryption (got %v)", + mount.Filesystem, mount.Path, err) + return err } // AllSupportedFilesystems lists all the Mounts which could support filesystem // encryption. This doesn't mean they necessarily do or that they are being used // with fscrypt. -func AllSupportedFilesystems() (mounts []*Mount) { +func AllSupportedFilesystems() ([]*Mount, error) { mountMutex.Lock() defer mountMutex.Unlock() if err := getMountInfo(); err != nil { - log.Print(err) - return + return nil, err } + var supportedMounts []*Mount for _, mount := range mountsByPath { if checkSupport(mount) == nil { - mounts = append(mounts, mount) + supportedMounts = append(supportedMounts, mount) } } - return + return supportedMounts, nil } // UpdateMountInfo updates the filesystem mountpoint maps with the current state @@ -240,28 +240,34 @@ func GetMount(mountpoint string) (*Mount, error) { // filesystem has been updated since the last call to one of the mount // functions, run UpdateMountInfo to see the change. func getMountsFromLink(link string) ([]*Mount, error) { - mountMutex.Lock() - defer mountMutex.Unlock() - if err := getMountInfo(); err != nil { - return nil, err - } - - // Use blkid to get the device + // Use blkid_evaluate_spec to get the device name. cLink := C.CString(link) defer C.string_free(cLink) + cDeviceName := C.blkid_evaluate_spec(cLink, &cache) defer C.string_free(cDeviceName) + deviceName := C.GoString(cDeviceName) + + log.Printf("blkid_evaluate_spec(%q, ) = %q", link, deviceName) - deviceName, err := cannonicalizePath(C.GoString(cDeviceName)) + if deviceName == "" { + return nil, ErrNoLink + } + deviceName, err := cannonicalizePath(deviceName) if err != nil { return nil, err } + mountMutex.Lock() + defer mountMutex.Unlock() + if err := getMountInfo(); err != nil { + return nil, err + } + if mnts, ok := mountsByDevice[deviceName]; ok { return mnts, nil } - log.Printf("link %q does not refer to a device", link) return nil, ErrNoLink } @@ -269,23 +275,29 @@ func getMountsFromLink(link string) ([]*Mount, error) { // value for the Mount's device according to libblkid. An error is returned if // the device/token pair has no value. func makeLink(mnt *Mount, token string) (string, error) { - mountMutex.Lock() - defer mountMutex.Unlock() - if err := getMountInfo(); err != nil { - return "", err - } + // The blkid cache may not always hold the canonical device path. To + // solve this we first use blkid_evaluate_spec to find the right entry + // in the cache. Then that name is used to get the token value. + cDevice := C.CString(mnt.Device) + defer C.string_free(cDevice) + + cDeviceEntry := C.blkid_evaluate_spec(cDevice, &cache) + defer C.string_free(cDeviceEntry) + deviceEntry := C.GoString(cDeviceEntry) + + log.Printf("blkid_evaluate_spec(%q, ) = %q", mnt.Device, deviceEntry) cToken := C.CString(token) defer C.string_free(cToken) - cDevice := C.CString(mnt.Device) - defer C.string_free(cDevice) - cValue := C.blkid_get_tag_value(cache, cToken, cDevice) - if cValue == nil { - log.Printf("filesystem at %q has no %s", mnt.Path, token) - return "", ErrCannotLink - } + cValue := C.blkid_get_tag_value(cache, cToken, cDeviceEntry) defer C.string_free(cValue) + value := C.GoString(cValue) + + log.Printf("blkid_get_tag_value(, %s, %s) = %s", token, deviceEntry, value) + if value == "" { + return "", ErrCannotLink + } return fmt.Sprintf("%s=%s", token, C.GoString(cValue)), nil } diff --git a/filesystem/mountpoint_test.go b/filesystem/mountpoint_test.go index 2840826..5523451 100644 --- a/filesystem/mountpoint_test.go +++ b/filesystem/mountpoint_test.go @@ -27,10 +27,7 @@ import ( func printMountInfo() { fmt.Println("\nBy Mountpoint:") for _, mnt := range mountsByPath { - fmt.Println("\t" + mnt.Path) - fmt.Println("\t\tFilesystem: " + mnt.Filesystem) - fmt.Printf("\t\tOptions: %v\n", mnt.Options) - fmt.Println("\t\tDevice: " + mnt.Device) + fmt.Println(mnt) } fmt.Println("\nBy Device:") @@ -44,11 +41,9 @@ func printMountInfo() { func printSupportedMounts() { fmt.Println("\nSupported Mountpoints:") - for _, mnt := range AllSupportedFilesystems() { - fmt.Println("\t" + mnt.Path) - fmt.Println("\t\tFilesystem: " + mnt.Filesystem) - fmt.Printf("\t\tOptions: %v\n", mnt.Options) - fmt.Println("\t\tDevice: " + mnt.Device) + mnts, _ := AllSupportedFilesystems() + for _, mnt := range mnts { + fmt.Println(mnt) } } -- cgit v1.2.3