Fix more encryption issues on exFAT filesystem (#7162)

* refactor file operations

* fix an issue on exFAT systems where the file unique id was not correct for new Realms leading to corruption for encrypted Realms

* fix the remaining callsites where a unique id could be reused

* add changelog note

* fix an error in the tests that would cause spawned processes to not have the same fallback tmp path which could cause issues if running on a filesystem that doesn't support mkfifo such as exFAT

* return none in an interprocess race on creating a lock file and getting its unique id

* Throw an exception on unique id failure instead of

asserting in consideration of external processes.
Use prealloc to size the file instead of a separate resize operation.
Add the file path to help debug exceptions.

* better error handling and cache a File unique id to catch more errors

Author: ironage

CHANGELOG.md

@@ -7,6 +7,7 @@
 
 ### Fixed
 * <How do the end-user experience this issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)
+* Fixed several causes of "decryption failed" exceptions that could happen when opening multiple encrypted Realm files in the same process while using Apple/linux and storing the Realms on an exFAT file system. ([#7156](https://github.com/realm/realm-core/issues/7156), since the beginning) 
 * Update existing std exceptions thrown by the Sync Client to use Realm exceptions. ([#6255](https://github.com/realm/realm-core/issues/6255), since v10.2.0)
 * Having a class name of length 57 would make client reset crash as a limit of 56 was wrongly enforced (57 is the correct limit) ([#7176](https://github.com/realm/realm-core/issues/7176), since v10.0.0)
 * Automatic client reset recovery on flexible sync Realms would apply recovered changes in multiple write transactions, releasing the write lock in between. This had several observable negative effects:

src/realm/alloc_slab.cpp

@@ -827,12 +827,19 @@ ref_type SlabAlloc::attach_file(const std::string& path, Config& cfg, util::Writ
         if (REALM_UNLIKELY(cfg.read_only))
             throw InvalidDatabase("Read-only access to empty Realm file", path);
 
-        const char* data = reinterpret_cast<const char*>(&empty_file_header);
-        m_file.write(data, sizeof empty_file_header); // Throws
-
-        // Pre-alloc initial space
         size_t initial_size = page_size(); // m_initial_section_size;
+        // exFAT does not allocate a unique id for the file until it is non-empty. It must be
+        // valid at this point because File::get_unique_id() is used to distinguish
+        // mappings_for_file in the encryption layer. So the prealloc() is required before
+        // interacting with the encryption layer in File::write().
+        // Pre-alloc initial space
         m_file.prealloc(initial_size);     // Throws
+        // seek() back to the start of the file in preparation for writing the header
+        // This sequence of File operations is protected from races by
+        // DB::m_controlmutex, so we know we are the only ones operating on the file
+        m_file.seek(0);
+        const char* data = reinterpret_cast<const char*>(&empty_file_header);
+        m_file.write(data, sizeof empty_file_header); // Throws
 
         bool disable_sync = get_disable_sync_to_disk() || cfg.disable_sync;
         if (!disable_sync)

src/realm/group.cpp

@@ -966,6 +966,10 @@ void Group::write(File& file, const char* encryption_key, uint_fast64_t version_
 
     file.set_encryption_key(encryption_key);
 
+    // Force the file system to allocate a node so we get a stable unique id.
+    // See File::get_unique_id(). This is used to distinguish encrypted mappings.
+    file.resize(1);
+
     // The aim is that the buffer size should be at least 1/256 of needed size but less than 64 Mb
     constexpr size_t upper_bound = 64 * 1024 * 1024;
     size_t min_space = std::min(get_used_space() >> 8, upper_bound);

src/realm/util/file.cpp

@@ -414,6 +414,7 @@ void File::open_internal(const std::string& path, AccessMode a, CreateMode c, in
 {
     REALM_ASSERT_RELEASE(!is_attached());
     m_path = path; // for error reporting and debugging
+    m_cached_unique_id = {};
 
 #ifdef _WIN32 // Windows version
 
@@ -572,6 +573,32 @@ void File::close() noexcept
 #endif
 }
 
+void File::close_static(FileDesc fd)
+{
+#ifdef _WIN32
+    if (!fd)
+        return;
+
+    if (!CloseHandle(fd))
+        throw std::system_error(GetLastError(), std::system_category(),
+                                "CloseHandle() failed from File::close_static()");
+#else
+    if (fd < 0)
+        return;
+
+    int ret = -1;
+    do {
+        ret = ::close(fd);
+    } while (ret == -1 && errno == EINTR);
+
+    if (ret != 0) {
+        int err = errno; // Eliminate any risk of clobbering
+        if (err == EBADF || err == EIO)
+            throw SystemError(err, "File::close_static() failed");
+    }
+#endif
+}
+
 size_t File::read_static(FileDesc fd, char* data, size_t size)
 {
 #ifdef _WIN32 // Windows version
@@ -1558,22 +1585,50 @@ bool File::compare(const std::string& path_1, const std::string& path_2)
     return true;
 }
 
-bool File::is_same_file_static(FileDesc f1, FileDesc f2)
+bool File::is_same_file_static(FileDesc f1, FileDesc f2, const std::string& path1, const std::string& path2)
 {
-    return get_unique_id(f1) == get_unique_id(f2);
+    return get_unique_id(f1, path1) == get_unique_id(f2, path2);
 }
 
 bool File::is_same_file(const File& f) const
 {
     REALM_ASSERT_RELEASE(is_attached());
     REALM_ASSERT_RELEASE(f.is_attached());
-    return is_same_file_static(m_fd, f.m_fd);
+    return is_same_file_static(m_fd, f.m_fd, m_path, f.m_path);
+}
+
+FileDesc File::dup_file_desc(FileDesc fd)
+{
+    FileDesc fd_duped;
+#ifdef _WIN32
+    if (!DuplicateHandle(GetCurrentProcess(), fd, GetCurrentProcess(), &fd_duped, 0, FALSE, DUPLICATE_SAME_ACCESS))
+        throw std::system_error(GetLastError(), std::system_category(), "DuplicateHandle() failed");
+#else
+    fd_duped = dup(fd);
+
+    if (fd_duped == -1) {
+        int err = errno; // Eliminate any risk of clobbering
+        throw std::system_error(err, std::system_category(), "dup() failed");
+    }
+#endif // conditonal on _WIN32
+    return fd_duped;
 }
 
-File::UniqueID File::get_unique_id() const
+File::UniqueID File::get_unique_id()
 {
     REALM_ASSERT_RELEASE(is_attached());
-    return File::get_unique_id(m_fd);
+    File::UniqueID uid = File::get_unique_id(m_fd, m_path);
+    if (!m_cached_unique_id) {
+        m_cached_unique_id = std::make_optional(uid);
+    }
+    if (m_cached_unique_id != uid) {
+        throw FileAccessError(ErrorCodes::FileOperationFailed,
+                              util::format("The unique id of this Realm file has changed unexpectedly, this could be "
+                                           "due to modifications by an external process '%1'",
+                                           m_path),
+                              m_path);
+    }
+    return uid;
 }
 
 FileDesc File::get_descriptor() const
@@ -1597,37 +1652,59 @@ std::optional<File::UniqueID> File::get_unique_id(const std::string& path)
         throw SystemError(GetLastError(), "CreateFileW failed");
     }
 
-    return get_unique_id(fileHandle);
+    return get_unique_id(fileHandle, path);
 #else // POSIX version
     struct stat statbuf;
     if (::stat(path.c_str(), &statbuf) == 0) {
+        if (statbuf.st_size == 0) {
+            // On exFAT systems the inode and device are not populated correctly until the file
+            // has been allocated some space. The uid can also be reassigned if the file is
+            // truncated to zero. This has led to bugs where a unique id returned here was
+            // reused by different files. The following check ensures that this is not
+            // happening anywhere else in future code.
+            return none;
+        }
         return File::UniqueID(statbuf.st_dev, statbuf.st_ino);
     }
     int err = errno; // Eliminate any risk of clobbering
     // File doesn't exist
     if (err == ENOENT)
         return none;
-    throw SystemError(err, format_errno("fstat() failed: %1", err));
+    throw SystemError(err, format_errno("fstat() failed: %1 for '%2'", err, path));
 #endif
 }
 
-File::UniqueID File::get_unique_id(FileDesc file)
+File::UniqueID File::get_unique_id(FileDesc file, const std::string& debug_path)
 {
 #ifdef _WIN32 // Windows version
     REALM_ASSERT(file != nullptr);
     File::UniqueID ret;
     if (GetFileInformationByHandleEx(file, FileIdInfo, &ret.id_info, sizeof(ret.id_info)) == 0) {
-        throw std::system_error(GetLastError(), std::system_category(), "GetFileInformationByHandleEx() failed");
+        throw std::system_error(GetLastError(), std::system_category(),
+                                util::format("GetFileInformationByHandleEx() failed for '%1'", debug_path));
     }
 
     return ret;
 #else // POSIX version
     REALM_ASSERT(file >= 0);
     struct stat statbuf;
     if (::fstat(file, &statbuf) == 0) {
+        // On exFAT systems the inode and device are not populated correctly until the file
+        // has been allocated some space. The uid can also be reassigned if the file is
+        // truncated to zero. This has led to bugs where a unique id returned here was
+        // reused by different files. The following check ensures that this is not
+        // happening anywhere else in future code.
+        if (statbuf.st_size == 0) {
+            throw FileAccessError(
+                ErrorCodes::FileOperationFailed,
+                util::format("Attempt to get unique id on an empty file. This could be due to an external "
+                             "process modifying Realm files. '%1'",
+                             debug_path),
+                debug_path);
+        }
         return UniqueID(statbuf.st_dev, statbuf.st_ino);
     }
-    throw std::system_error(errno, std::system_category(), "fstat() failed");
+    throw std::system_error(errno, std::system_category(), util::format("fstat() failed for '%1'", debug_path));
 #endif
 }
 

src/realm/util/file.hpp

@@ -176,6 +176,7 @@ class File {
     /// regardless of whether this instance currently is attached to
     /// an open file.
     void close() noexcept;
+    static void close_static(FileDesc fd); // throws
 
     /// Check whether this File instance is currently attached to an
     /// open file.
@@ -529,7 +530,9 @@ class File {
     /// Both instances have to be attached to open files. If they are
     /// not, this function has undefined behavior.
     bool is_same_file(const File&) const;
-    static bool is_same_file_static(FileDesc f1, FileDesc f2);
+    static bool is_same_file_static(FileDesc f1, FileDesc f2, const std::string& path1, const std::string& path2);
+
+    static FileDesc dup_file_desc(FileDesc fd);
 
     /// Resolve the specified path against the specified base directory.
     ///
@@ -604,18 +607,20 @@ class File {
     };
     // Return the unique id for the current opened file descriptor.
     // Same UniqueID means they are the same file.
-    UniqueID get_unique_id() const;
+    UniqueID get_unique_id(); // Throws
     // Return the file descriptor for the file
     FileDesc get_descriptor() const;
     // Return the path of the open file, or an empty string if
     // this file has never been opened.
     std::string get_path() const;
 
     // Return none if the file doesn't exist. Throws on other errors.
+    // If the file does exist but has a size of zero, the file may be resized
+    // to force the file system to allocate a unique id.
     static std::optional<UniqueID> get_unique_id(const std::string& path);
 
     // Return the unique id for the file descriptor. Throws if the underlying stat operation fails.
-    static UniqueID get_unique_id(FileDesc file);
+    static UniqueID get_unique_id(FileDesc file, const std::string& debug_path);
 
     template <class>
     class Map;
@@ -641,6 +646,7 @@ class File {
 #endif
     std::unique_ptr<const char[]> m_encryption_key = nullptr;
     std::string m_path;
+    std::optional<UniqueID> m_cached_unique_id;
 
     bool lock(bool exclusive, bool non_blocking);
     bool rw_lock(bool exclusive, bool non_blocking);