Scary warning when running two aktualizr at the same time

src/libaktualizr/bootloader/bootloader.cc

@@ -58,22 +58,12 @@ void BootloaderConfig::writeToStream(std::ostream& out_stream) const {
 Bootloader::Bootloader(const BootloaderConfig& config, INvStorage& storage) : config_(config), storage_(storage) {
   reboot_sentinel_ = config_.reboot_sentinel_dir / config_.reboot_sentinel_name;
 
-  if (mkdir(config_.reboot_sentinel_dir.c_str(), S_IRWXU) == -1) {
-    struct stat st {};
-    int ret = stat(config_.reboot_sentinel_dir.c_str(), &st);
-    // checks: - stat succeeded
-    //         - is a directory
-    //         - no read and write permissions for group and others
-    //         - owner is current user
-    if (ret < 0 || ((st.st_mode & S_IFDIR) == 0) || (st.st_mode & (S_IRGRP | S_IROTH | S_IWGRP | S_IWOTH)) != 0 ||
-        (st.st_uid != getuid())) {
-      LOG_WARNING << "Could not create " << config_.reboot_sentinel_dir
-                  << " securely, reboot detection support disabled";
-      reboot_detect_supported_ = false;
-      return;
-    }
-    // mdkir failed but directory is here with correct permissions
+  if (!Utils::createSecureDirectory(config_.reboot_sentinel_dir)) {
+    LOG_WARNING << "Could not create " << config_.reboot_sentinel_dir << " securely, reboot detection support disabled";
+    reboot_detect_supported_ = false;
+    return;
   }
+
   reboot_detect_supported_ = true;
 }
 

src/libaktualizr/storage/sqlstorage_base.cc

@@ -5,6 +5,27 @@
 
 boost::filesystem::path SQLStorageBase::dbPath() const { return sqldb_path_; }
 
+StorageLock::StorageLock(boost::filesystem::path path) : lock_path(std::move(path)) {
+  {
+    std::fstream fs;
+    fs.open(lock_path.c_str(), std::fstream::in | std::fstream::out | std::fstream::app);
+  }
+  fl_ = lock_path.c_str();
+  if (!fl_.try_lock()) {
+    throw StorageLock::locked_exception();
+  }
+}
+
+StorageLock::~StorageLock() {
+  try {
+    if (!lock_path.empty()) {
+      fl_.unlock();
+      std::remove(lock_path.c_str());
+    }
+  } catch (std::exception& e) {
+  }
+}
+
 SQLStorageBase::SQLStorageBase(boost::filesystem::path sqldb_path, bool readonly,
                                std::vector<std::string> schema_migrations,
                                std::vector<std::string> schema_rollback_migrations, std::string current_schema,
@@ -34,6 +55,15 @@ SQLStorageBase::SQLStorageBase(boost::filesystem::path sqldb_path, bool readonly
     }
   }
 
+  try {
+    lock = StorageLock(db_parent_path / "storage.lock");
+  } catch (StorageLock::locked_exception& e) {
+    LOG_WARNING << "\033[31m"
+                << "Storage in " << db_parent_path
+                << " is already in use, running several instances concurrently may result in data corruption!"
+                << "\033[0m";
+  }
+
   if (!dbMigrate()) {
     throw StorageException("SQLite database migration failed");
   }

src/libaktualizr/storage/sqlstorage_base.h

@@ -2,35 +2,61 @@
 #define SQLSTORAGE_BASE_H_
 
 #include <boost/filesystem.hpp>
+#include <boost/interprocess/sync/file_lock.hpp>
+#include <boost/interprocess/sync/scoped_lock.hpp>
 
 #include "sql_utils.h"
 #include "storage_config.h"
 
 enum class DbVersion : int32_t { kEmpty = -1, kInvalid = -2 };
 
+class StorageLock {
+ public:
+  StorageLock() = default;
+  StorageLock(boost::filesystem::path path);
+  StorageLock(StorageLock &other) = delete;
+  StorageLock &operator=(StorageLock &other) = delete;
+  StorageLock(StorageLock &&other) = default;
+  StorageLock &operator=(StorageLock &&other) = default;
+  virtual ~StorageLock();
+
+  class locked_exception : std::runtime_error {
+   public:
+    locked_exception() : std::runtime_error("locked") {}
+  };
+
+ private:
+  boost::filesystem::path lock_path;
+  boost::interprocess::file_lock fl_;
+};
+
 class SQLStorageBase {
  public:
   explicit SQLStorageBase(boost::filesystem::path sqldb_path, bool readonly, std::vector<std::string> schema_migrations,
                           std::vector<std::string> schema_rollback_migrations, std::string current_schema,
                           int current_schema_version);
   ~SQLStorageBase() = default;
-  std::string getTableSchemaFromDb(const std::string& tablename);
+  std::string getTableSchemaFromDb(const std::string &tablename);
   bool dbMigrateForward(int version_from, int version_to = 0);
   bool dbMigrateBackward(int version_from, int version_to = 0);
   bool dbMigrate();
   DbVersion getVersion();  // non-negative integer on success or -1 on error
   boost::filesystem::path dbPath() const;
 
  protected:
-  SQLite3Guard dbConnection() const;
   boost::filesystem::path sqldb_path_;
   boost::filesystem::path images_path_{sqldb_path_.parent_path() / "images"};
   bool readonly_{false};
+
+  StorageLock lock;
+
   const std::vector<std::string> schema_migrations_;
   std::vector<std::string> schema_rollback_migrations_;
   const std::string current_schema_;
   const int current_schema_version_;
-  bool dbInsertBackMigrations(SQLite3Guard& db, int version_latest);
+
+  SQLite3Guard dbConnection() const;
+  bool dbInsertBackMigrations(SQLite3Guard &db, int version_latest);
 };
 
 #endif  // SQLSTORAGE_BASE_H_

src/libaktualizr/storage/sqlstorage_test.cc

@@ -396,26 +396,29 @@ TEST(sqlstorage, migrate_from_fs) {
   // (anonymized data)
   Utils::copyDir(test_data_dir / "fs_snapshot", config.path);
   ASSERT_GE(chmod(config.path.c_str(), S_IRWXU), 0);
-  auto storage = INvStorage::newStorage(config);
 
-  EXPECT_TRUE(storage->loadPrimaryKeys(nullptr, nullptr));
-  EXPECT_TRUE(storage->loadTlsCreds(nullptr, nullptr, nullptr));
-  EXPECT_TRUE(storage->loadLatestRoot(nullptr, Uptane::RepositoryType::Director()));
-  EXPECT_TRUE(storage->loadDeviceId(nullptr));
+  {
+    auto storage = INvStorage::newStorage(config);
+
+    EXPECT_TRUE(storage->loadPrimaryKeys(nullptr, nullptr));
+    EXPECT_TRUE(storage->loadTlsCreds(nullptr, nullptr, nullptr));
+    EXPECT_TRUE(storage->loadLatestRoot(nullptr, Uptane::RepositoryType::Director()));
+    EXPECT_TRUE(storage->loadDeviceId(nullptr));
 
-  EcuSerials serials;
-  EXPECT_TRUE(storage->loadEcuSerials(&serials));
-  EXPECT_EQ(serials.size(), 3);
+    EcuSerials serials;
+    EXPECT_TRUE(storage->loadEcuSerials(&serials));
+    EXPECT_EQ(serials.size(), 3);
 
-  std::vector<MisconfiguredEcu> misconfigured;
-  EXPECT_TRUE(storage->loadMisconfiguredEcus(&misconfigured));
-  EXPECT_EQ(misconfigured.size(), 1);
+    std::vector<MisconfiguredEcu> misconfigured;
+    EXPECT_TRUE(storage->loadMisconfiguredEcus(&misconfigured));
+    EXPECT_EQ(misconfigured.size(), 1);
 
-  EXPECT_TRUE(storage->loadEcuRegistered());
+    EXPECT_TRUE(storage->loadEcuRegistered());
 
-  std::vector<Uptane::Target> installed;
-  storage->loadPrimaryInstalledVersions(&installed, nullptr, nullptr);
-  EXPECT_NE(installed.size(), 0);
+    std::vector<Uptane::Target> installed;
+    storage->loadPrimaryInstalledVersions(&installed, nullptr, nullptr);
+    EXPECT_NE(installed.size(), 0);
+  }
 
   // note: installation result is not migrated anymore
 

src/libaktualizr/utilities/utils.cc

@@ -675,6 +675,23 @@ void Utils::createDirectories(const boost::filesystem::path &path, mode_t mode)
   std::cout << "created: " << path.native() << "\n";
 }
 
+bool Utils::createSecureDirectory(const boost::filesystem::path &path) {
+  if (mkdir(path.c_str(), S_IRWXU) == 0) {
+    // directory created successfully
+    return true;
+  }
+
+  // mkdir failed, see if the directory already exists with correct permissions
+  struct stat st {};
+  int ret = stat(path.c_str(), &st);
+  // checks: - stat succeeded
+  //         - is a directory
+  //         - no read and write permissions for group and others
+  //         - owner is current user
+  return (ret >= 0 && ((st.st_mode & S_IFDIR) == S_IFDIR) && (st.st_mode & (S_IRWXU | S_IRWXG | S_IRWXO)) == S_IRWXU &&
+          (st.st_uid == getuid()));
+}
+
 std::string Utils::urlEncode(const std::string &input) {
   std::string res;
 

src/libaktualizr/utilities/utils.h

@@ -44,6 +44,7 @@ struct Utils {
   static boost::filesystem::path absolutePath(const boost::filesystem::path &root, const boost::filesystem::path &file);
   static void setSocketPort(sockaddr_storage *addr, in_port_t port);
   static void createDirectories(const boost::filesystem::path &path, mode_t mode);
+  static bool createSecureDirectory(const boost::filesystem::path &path);
   static std::string urlEncode(const std::string &input);
   static CURL *curlDupHandleWrapper(CURL *curl_in, bool using_pkcs11);
   static std::vector<boost::filesystem::path> getDirEntriesByExt(const boost::filesystem::path &dir_path,

src/libaktualizr/utilities/utils_test.cc

@@ -328,6 +328,19 @@ TEST(Utils, shell) {
   EXPECT_NE(statuscode, 0);
 }
 
+TEST(Utils, createSecureDirectory) {
+  TemporaryDirectory temp_dir;
+
+  EXPECT_TRUE(Utils::createSecureDirectory(temp_dir / "sec"));
+  EXPECT_TRUE(boost::filesystem::is_directory(temp_dir / "sec"));
+  // check that it succeeds the second time
+  EXPECT_TRUE(Utils::createSecureDirectory(temp_dir / "sec"));
+
+  // regular directory is insecure
+  boost::filesystem::create_directory(temp_dir / "unsec");
+  EXPECT_FALSE(Utils::createSecureDirectory(temp_dir / "unsec"));
+}
+
 TEST(Utils, urlencode) { EXPECT_EQ(Utils::urlEncode("test! test@ test#"), "test%21%20test%40%20test%23"); }
 
 TEST(Utils, getDirEntriesByExt) {