Fix more encryption issues on exFAT filesystem

CHANGELOG.md

@@ -6,6 +6,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)
 
 ### Breaking changes

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

@@ -967,6 +967,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);