RCORE-1969 Fix bundled encrypted Realm regression

CHANGELOG.md

@@ -11,6 +11,10 @@
 * SyncUser::all_sessions() included sessions in every state *except* for waiting for access token, which was weirdly inconsistent. It now includes all sessions. ([PR #7300](https://github.com/realm/realm-core/pull/7300)).
 * App::all_users() included logged out users only if they were logged out while the App instance existed. It now always includes all logged out users. ([PR #7300](https://github.com/realm/realm-core/pull/7300)).
 * Deleting the active user left the active user unset rather than selecting another logged-in user as the active user like logging out and removing users did. ([PR #7300](https://github.com/realm/realm-core/pull/7300)).
+* Fixed several issues around encrypted file portability (copying a "bundled" encrypted Realm from one device to another):
+  * Fixed `Assertion failed: new_size % (1ULL << m_page_shift) == 0` when opening an encrypted Realm less than 64Mb that was generated on a platform with a different page size than the current platform. ([#7322](https://github.com/realm/realm-core/issues/7322), since v13.17.1)
+  * Fixed a `DecryptionFailed` exception thrown when opening a small (<4k of data) Realm generated on a device with a page size of 4k if it was bundled and opened on a device with a larger page size (since the beginning).
+  * Fixed an issue during a subsequent open of an encrypted Realm for some rare allocation patterns when the top ref was within ~50 bytes of the end of a page. This could manifest as a DecryptionFailed exception or as an assertion: `encrypted_file_mapping.hpp:183: Assertion failed: local_ndx < m_page_state.size()`. ([#7319](https://github.com/realm/realm-core/issues/7319))
 
 ### Breaking changes
 * The following things have been renamed or moved as part of moving all of the App Services functionality to the app namespace:

src/realm/alloc.hpp

@@ -154,7 +154,7 @@ class Allocator {
 
     static constexpr size_t section_size() noexcept
     {
-        return 1 << section_shift;
+        return 1UL << section_shift;
     }
 
 protected:

src/realm/alloc_slab.cpp

@@ -74,7 +74,7 @@ size_t SlabAlloc::get_total_slab_size() noexcept
 
 SlabAlloc::SlabAlloc()
 {
-    m_initial_section_size = 1UL << section_shift; // page_size();
+    m_initial_section_size = section_size();
     m_free_space_state = free_space_Clean;
     m_baseline = 0;
 }
@@ -718,16 +718,9 @@ bool SlabAlloc::align_filesize_for_mmap(ref_type top_ref, Config& cfg)
     // check if online compaction allows us to shrink the file:
     if (top_ref) {
         // Get the expected file size by looking up logical file size stored in top array
-        constexpr size_t max_top_size = (Group::s_file_size_ndx + 1) * 8 + sizeof(Header);
-        size_t top_page_base = top_ref & ~(page_size() - 1);
-        size_t top_offset = top_ref - top_page_base;
-        size_t map_size = std::min(max_top_size + top_offset, size - top_page_base);
-        File::Map<char> map_top(m_file, top_page_base, File::access_ReadOnly, map_size, 0, m_write_observer);
-        realm::util::encryption_read_barrier(map_top, top_offset, max_top_size);
-        auto top_header = map_top.get_addr() + top_offset;
-        auto top_data = NodeHeader::get_data_from_header(top_header);
-        auto w = NodeHeader::get_width_from_header(top_header);
-        auto logical_size = size_t(get_direct(top_data, w, Group::s_file_size_ndx)) >> 1;
+        Array top(*this);
+        top.init_from_ref(top_ref);
+        size_t logical_size = Group::get_logical_file_size(top);
         // make sure we're page aligned, so the code below doesn't first
         // truncate the file, then expand it again
         expected_size = round_up_to_page_size(logical_size);
@@ -738,7 +731,6 @@ bool SlabAlloc::align_filesize_for_mmap(ref_type top_ref, Config& cfg)
         detach(true); // keep m_file open
         m_file.resize(expected_size);
         m_file.close();
-        size = expected_size;
         return true;
     }
 
@@ -849,7 +841,7 @@ 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);
 
-        size_t initial_size = page_size(); // m_initial_section_size;
+        size_t initial_size = page_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
@@ -882,6 +874,12 @@ ref_type SlabAlloc::attach_file(const std::string& path, Config& cfg, util::Writ
     DetachGuard dg(*this);
 
     reset_free_space_tracking();
+
+    // the file could have been produced on a device with a different
+    // page size than our own so don't expect the size to be aligned
+    if (cfg.encryption_key && size != 0 && size != round_up_to_page_size(size)) {
+        size = round_up_to_page_size(size);
+    }
     update_reader_view(size);
     REALM_ASSERT(m_mappings.size());
     m_data = m_mappings[0].primary_mapping.get_addr();

src/realm/util/encrypted_file_mapping.cpp

@@ -30,6 +30,7 @@
 #include <cstdlib>
 #include <algorithm>
 #include <chrono>
+#include <sstream>
 #include <stdexcept>
 #include <string_view>
 #include <system_error>
@@ -81,6 +82,13 @@ SharedFileInfo::SharedFileInfo(const uint8_t* key)
 // the ciphertext, we can still determine that we should use the old IV, since
 // the ciphertext's hash will match the old ciphertext.
 
+// This produces a file on disk with the following layout:
+// 4k block of metadata   (up to 64 iv_table instances stored here)
+// 64 * 4k blocks of data (up to 262144 bytes of data are stored here)
+// 4k block of metadata
+// 64 * 4k blocks of data
+// ...
+
 struct iv_table {
     uint32_t iv1 = 0;
     std::array<uint8_t, 28> hmac1 = {};
@@ -102,6 +110,8 @@ const size_t block_size = 4096;
 
 const size_t metadata_size = sizeof(iv_table);
 const size_t blocks_per_metadata_block = block_size / metadata_size;
+static_assert(metadata_size == 64,
+              "changing the size of the metadata breaks compatibility with existing Realm files");
 
 // map an offset in the data to the actual location in the file
 template <typename Int>
@@ -152,6 +162,10 @@ size_t check_read(FileDesc fd, off_t pos, void* dst, size_t len)
 
 } // anonymous namespace
 
+// first block is iv data, second page is data
+static_assert(c_min_encrypted_file_size == 2 * block_size,
+              "chaging the block size breaks encrypted file portability");
+
 AESCryptor::AESCryptor(const uint8_t* key)
     : m_rw_buffer(new char[block_size])
     , m_dst_buffer(new char[block_size])
@@ -683,7 +697,9 @@ void EncryptedFileMapping::refresh_page(size_t local_page_ndx, size_t required)
                 memset(addr + actual, 0x55, size - actual);
             }
             else {
-                throw DecryptionFailed();
+                size_t fs = to_size_t(File::get_size_static(m_file.fd));
+                throw DecryptionFailed(
+                    util::format("failed to decrypt block %1 in file of size %2", local_page_ndx + m_first_page, fs));
             }
         }
     }
@@ -1025,7 +1041,7 @@ void EncryptedFileMapping::read_barrier(const void* addr, size_t size, Header_to
 
 void EncryptedFileMapping::extend_to(size_t offset, size_t new_size)
 {
-    REALM_ASSERT(new_size % (1ULL << m_page_shift) == 0);
+    REALM_ASSERT_EX(new_size % page_size() == 0, new_size, page_size());
     size_t num_pages = new_size >> m_page_shift;
     m_page_state.resize(num_pages, PageState::Clean);
     m_chunk_dont_scan.resize((num_pages + page_to_chunk_factor - 1) >> page_to_chunk_shift, false);
@@ -1084,3 +1100,13 @@ File::SizeType data_size_to_encrypted_size(File::SizeType size) noexcept
 }
 } // namespace realm::util
 #endif // REALM_ENABLE_ENCRYPTION
+
+namespace realm::util {
+std::string DecryptionFailed::get_message_with_bt(std::string_view msg)
+{
+    auto bt = Backtrace::capture();
+    std::stringstream ss;
+    bt.print(ss);
+    return util::format("Decryption failed: %1\n%2\n", msg, ss.str());
+}
+} // namespace realm::util

src/realm/util/encrypted_file_mapping.hpp

@@ -104,6 +104,9 @@ class EncryptedFileMapping {
     {
         m_observer = observer;
     }
+#if REALM_DEBUG
+    std::string print_debug();
+#endif // REALM_DEBUG
 
 private:
     SharedFileInfo& m_file;
@@ -167,12 +170,13 @@ class EncryptedFileMapping {
 
 inline size_t EncryptedFileMapping::get_offset_of_address(const void* addr) const
 {
-    return reinterpret_cast<size_t>(addr) & ((1 << m_page_shift) - 1);
+    REALM_ASSERT_3(reinterpret_cast<size_t>(addr), >=, reinterpret_cast<size_t>(m_addr));
+    return (reinterpret_cast<size_t>(addr) - reinterpret_cast<size_t>(m_addr)) & ((1ULL << m_page_shift) - 1);
 }
 
 inline size_t EncryptedFileMapping::get_local_index_of_address(const void* addr, size_t offset) const
 {
-    REALM_ASSERT_EX(addr >= m_addr, addr, m_addr);
+    REALM_ASSERT_EX(addr >= m_addr, size_t(addr), size_t(m_addr));
 
     size_t local_ndx =
         ((reinterpret_cast<uintptr_t>(addr) - reinterpret_cast<uintptr_t>(m_addr) + offset) >> m_page_shift);
@@ -188,6 +192,46 @@ inline bool EncryptedFileMapping::contains_page(size_t page_in_file) const
     return page_in_file >= m_first_page && page_in_file - m_first_page < m_page_state.size();
 }
 
+#if REALM_DEBUG
+inline std::string EncryptedFileMapping::print_debug()
+{
+    auto state_name = [](const PageState& s) -> std::string {
+        if (s == PageState::Clean) {
+            return "Clean";
+        }
+        std::string state = "{";
+        if (s & PageState::Touched) {
+            state += "Touched";
+        }
+        if (s & PageState::UpToDate) {
+            state += "UpToDate";
+        }
+        if (s & PageState::StaleIV) {
+            state += "StaleIV";
+        }
+        if (s & PageState::Writable) {
+            state += "Writable";
+        }
+        if (s & PageState::Dirty) {
+            state += "Dirty";
+        }
+        state += "}";
+        return state;
+    };
+    std::string page_states;
+    for (PageState& s : m_page_state) {
+        if (!page_states.empty()) {
+            page_states += ", ";
+        }
+        page_states += state_name(s);
+    }
+    return util::format("%1 pages from %2 to %3: %4", m_page_state.size(), m_first_page,
+                        m_page_state.size() + m_first_page, page_states);
+}
+#endif // REALM_DEBUG
+
+constexpr inline size_t c_min_encrypted_file_size = 8192;
+
 } // namespace realm::util
 
 #endif // REALM_ENABLE_ENCRYPTION
@@ -197,13 +241,14 @@ namespace realm::util {
 /// contain valid encrypted data
 struct DecryptionFailed : FileAccessError {
     DecryptionFailed()
-        : FileAccessError(ErrorCodes::DecryptionFailed, "Decryption failed", std::string(), 0)
+        : FileAccessError(ErrorCodes::DecryptionFailed, get_message_with_bt(""), std::string(), 0)
     {
     }
     DecryptionFailed(const std::string& msg)
-        : FileAccessError(ErrorCodes::DecryptionFailed, util::format("Decryption failed: '%1'", msg), std::string())
+        : FileAccessError(ErrorCodes::DecryptionFailed, get_message_with_bt(msg), std::string())
     {
     }
+    static std::string get_message_with_bt(std::string_view msg);
 };
 } // namespace realm::util
 

src/realm/util/file.cpp

@@ -23,6 +23,7 @@
 #include <realm/util/file_mapper.hpp>
 
 #include <algorithm>
+#include <atomic>
 #include <cerrno>
 #include <climits>
 #include <cstddef>
@@ -49,6 +50,7 @@
 using namespace realm::util;
 
 namespace {
+constexpr size_t c_min_supported_page_size = 4096;
 size_t get_page_size()
 {
 #ifdef _WIN32
@@ -60,13 +62,13 @@ size_t get_page_size()
 #else
     long size = sysconf(_SC_PAGESIZE);
 #endif
-    REALM_ASSERT(size > 0 && size % 4096 == 0);
+    REALM_ASSERT(size > 0 && size % c_min_supported_page_size == 0);
     return static_cast<size_t>(size);
 }
 
 // This variable exists such that page_size() can return the page size without having to make any system calls.
 // It could also have been a static local variable, but Valgrind/Helgrind gives a false error on that.
-size_t cached_page_size = get_page_size();
+std::atomic<size_t> cached_page_size = get_page_size();
 
 bool for_each_helper(const std::string& path, const std::string& dir, realm::util::File::ForEachHandler& handler)
 {
@@ -410,7 +412,18 @@ std::string make_temp_file(const char* prefix)
 
 size_t page_size()
 {
-    return cached_page_size;
+    return cached_page_size.load(std::memory_order::memory_order_relaxed);
+}
+
+OnlyForTestingPageSizeChange::OnlyForTestingPageSizeChange(size_t new_page_size)
+{
+    REALM_ASSERT(new_page_size % c_min_supported_page_size == 0);
+    cached_page_size = new_page_size;
+}
+
+OnlyForTestingPageSizeChange::~OnlyForTestingPageSizeChange()
+{
+    cached_page_size = get_page_size();
 }
 
 void File::open_internal(const std::string& path, AccessMode a, CreateMode c, int flags, bool* success)

src/realm/util/file.hpp

@@ -114,6 +114,10 @@ std::string make_temp_file(const char* prefix);
 
 size_t page_size();
 
+struct OnlyForTestingPageSizeChange {
+    OnlyForTestingPageSizeChange(size_t new_page_size);
+    ~OnlyForTestingPageSizeChange();
+};
 
 /// This class provides a RAII abstraction over the concept of a file
 /// descriptor (or file handle).

src/realm/util/file_mapper.cpp

@@ -48,6 +48,7 @@
 #include <sstream>
 #include <regex>
 #include <thread>
+#include <iostream>
 
 #include <realm/util/file.hpp>
 #include <realm/util/errno.hpp>
@@ -128,7 +129,6 @@ int64_t fetch_value_in_file(const std::string& fname, const char* scan_pattern)
     return PageReclaimGovernor::no_match;
 }
 
-
 /* Default reclaim governor
  *
  */
@@ -485,13 +485,14 @@ SharedFileInfo* get_file_info_for_file(File& file)
         return it->info.get();
 }
 
-
 namespace {
 EncryptedFileMapping* add_mapping(void* addr, size_t size, const FileAttributes& file, size_t file_offset)
 {
     size_t fs = to_size_t(File::get_size_static(file.fd));
-    if (fs > 0 && fs < page_size())
-        throw DecryptionFailed();
+    if (fs > 0 && fs < c_min_encrypted_file_size)
+        throw DecryptionFailed(
+            util::format("file size %1 is less than the minimum encrypted file size of %2 for '%3'", fs,
+                         c_min_encrypted_file_size, file.path));
 
     LockGuard lock(mapping_mutex);
 

src/realm/util/file_mapper.hpp

@@ -200,6 +200,7 @@ File::SizeType encrypted_size_to_data_size(File::SizeType size) noexcept;
 File::SizeType data_size_to_encrypted_size(File::SizeType size) noexcept;
 
 size_t round_up_to_page_size(size_t size) noexcept;
+
 } // namespace util
 } // namespace realm
 #endif

test/test_alloc.cpp

@@ -325,6 +325,10 @@ NONCONCURRENT_TEST_IF(Alloc_MapFailureRecovery, _impl::SimulatedFailure::is_enab
     Group().write(path);
 
     SlabAlloc::Config cfg;
+    // mimic a DB initiating a session
+    cfg.is_shared = true;
+    cfg.session_initiator = true;
+
     SlabAlloc alloc;
 
     { // Initial Header mapping fails
@@ -350,19 +354,31 @@ NONCONCURRENT_TEST_IF(Alloc_MapFailureRecovery, _impl::SimulatedFailure::is_enab
         _impl::SimulatedFailure::prime_mmap(nullptr);
         auto top_ref = alloc.attach_file(path, cfg);
         CHECK(alloc.is_attached());
+        if (cfg.encryption_key) {
+            // the internal baseline has capacity for a page but it is not entirely backed by a file (yet)
+            CHECK_EQUAL(alloc.get_baseline(), page_size);
+        }
+        else {
+            CHECK_NOT_EQUAL(alloc.get_baseline(), page_size);
+        }
         // file has not (yet) been expanded to match:
-        CHECK(alloc.get_baseline() != page_size);
+        CHECK_NOT_EQUAL(alloc.get_file_size(), page_size);
         // convert before aligning file size
         alloc.convert_from_streaming_form(top_ref);
         // file is expanded:
         CHECK(alloc.align_filesize_for_mmap(top_ref, cfg));
+        CHECK(!alloc.is_attached());
         // and consequently needs to be reattached:
         alloc.detach();
         alloc.attach_file(path, cfg);
         alloc.init_mapping_management(1);
+        CHECK(alloc.is_attached());
+        // now both the allocator and the backing file agree on the capacity
+        CHECK_EQUAL(alloc.get_baseline(), page_size);
+        CHECK_EQUAL(alloc.get_file_size(), page_size);
     }
 
-    { // Extendind the first mapping
+    { // Extending the first mapping
         const auto initial_baseline = alloc.get_baseline();
         const auto initial_version = alloc.get_mapping_version();
         const char* initial_translated = alloc.translate(1000);