From: Tomasz Swierczek Date: Thu, 28 Apr 2022 09:13:42 +0000 (+0200) Subject: Enhance DB recovery logic X-Git-Tag: submit/sessiond/20220715.092836~7 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=e0180ac2722abcc23f60a4ff22e21579b3bd567c;p=platform%2Fcore%2Fsecurity%2Fsecurity-manager.git Enhance DB recovery logic "If we are wise, let us prepare for the worst." - George Washington Previously, the logic of DB recovery was: 1. Remove the "-recovered" file flag, IF it survived reboot (shouldn't) 2. Check DB for corruption 3. IF corruption occured, then: a. Replace original DB with fallback made at image creation b. Create the "-recovered" file next to DB file that signals rest of the system some apps may be missing If sudden poweroff happens between 3a and 3b, system will not get informed about missing app installation data. This patch changes order of operations 3a and 3b, and also removes operation number 1. From now on, the system-level scripts responsible for recovery should remove the flag, when full recovery was complete. Changing order of 3a with 3b ensures the flag is created when DB error was found and is not prone to sudden power-off. The flag is meant to be used for file-existance signalling of the need to reinstall apps that were not in the backed-up DB. Since its existence can trigger app installation, which in turn, can launch & use security-manager (which will also attemt to access the DB), it MUST be ensured that rules-loader is not running concurrently with any other processes/services that may use security-manager's DB (the recovery of DB from fallback/backup has to be complete). This is achieved by systemd's "Before=" service option in rules-loader service file which prohibits security-manager's socket & service start before rules loader-ends operation. Change-Id: I472c09d9398f69a97e118b69aad61dc016e3d22d --- diff --git a/src/common/include/db-config.h b/src/common/include/db-config.h index 6d06fb00..4a772669 100644 --- a/src/common/include/db-config.h +++ b/src/common/include/db-config.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019-2020 Samsung Electronics Co., Ltd. All rights reserved. + * Copyright (c) 2019-2022 Samsung Electronics Co., Ltd. All rights reserved. * * This file is licensed under the terms of MIT License or the Apache License * Version 2.0 of your choice. See the LICENSE.MIT file for MIT license details. @@ -36,9 +36,8 @@ #define DB_FALLBACK_PATH DATA_INSTALL_DIR "/" DB_FILE -// If database initialization fails, restoration to a fallback snapshot is -// attempted. If the restoration succeeds, a file flag is created to notify -// other system components. +// If database initialization fails, a file flag is created to notify +// other system components & then, the DB restoration is attempted. // For database placed in "$f" the filename is ("$f" DB_RECOVERED_SUFFIX). #define DB_RECOVERED_SUFFIX "-recovered" #define DB_JOURNAL_SUFFIX "-journal" diff --git a/src/common/privilege_db.cpp b/src/common/privilege_db.cpp index 33ffbc00..10318751 100644 --- a/src/common/privilege_db.cpp +++ b/src/common/privilege_db.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014-2020 Samsung Electronics Co., Ltd. All rights reserved. + * Copyright (c) 2014-2022 Samsung Electronics Co., Ltd. All rights reserved. * * This file is licensed under the terms of MIT License or the Apache License * Version 2.0 of your choice. See the LICENSE.MIT file for MIT license details. @@ -137,8 +137,7 @@ void tryCatchDbInit(F &&f) { } //namespace PrivilegeDb::PrivilegeDb(Offline offline, const std::string &dbPath, const char *okMarkerPath, const char *loaderCmd) { - bool didFallback = false; - if (!underlying(offline) && !FS::fileExists(okMarkerPath) && !(didFallback = FS::fileExists(dbPath + DB_RECOVERED_SUFFIX))) + if (!underlying(offline) && !FS::fileExists(okMarkerPath) && !FS::fileExists(dbPath + DB_RECOVERED_SUFFIX)) throwDbInitEx("loader failed to initialize db - giving up"); tryCatchDbInit([&]{ mSqlConnection.Connect(dbPath); }); @@ -147,8 +146,6 @@ PrivilegeDb::PrivilegeDb(Offline offline, const std::string &dbPath, const char } catch (DB::SqlConnection::Exception::Base &e) { if (underlying(offline)) throwDbInitEx("failed to initialize db in offline mode - giving up"); - if (didFallback) - throwDbInitEx("Database initialization error during query preparation on fallback db - giving up"); LogError("Database initialization error during query preparation - attempting fallback"); tryCatchDbInit([&]{ mSqlConnection.Disconnect(); diff --git a/src/server/rules-loader/security-manager-rules-loader.cpp b/src/server/rules-loader/security-manager-rules-loader.cpp index 1eeef960..b0dc3893 100644 --- a/src/server/rules-loader/security-manager-rules-loader.cpp +++ b/src/server/rules-loader/security-manager-rules-loader.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018-2020 Samsung Electronics Co., Ltd. All rights reserved. + * Copyright (c) 2018-2022 Samsung Electronics Co., Ltd. All rights reserved. * * This file is licensed under the terms of MIT License or the Apache License * Version 2.0 of your choice. See the LICENSE.MIT file for MIT license details. @@ -571,7 +571,7 @@ inl void parseOptions(int argc, char *argv[]) { // skip main database bringup and start applying fallback right away // // the option is used by the manager if a fallback has not been applied yet - // ("database successfully recovered" marker file does not exist) + // ("database recovery attempted" marker file does not exist) // and a schema error has been detected case 'f': // exclusive option - cannot be combined w/ anything else @@ -1054,7 +1054,7 @@ int main(int argc, char *argv[]) { // need the following filenames: // dbPath : to open the database - // dbPath + DB_RECOVERED_SUFFIX : to remove/create the "database successfully recovered" marker file + // dbPath + DB_RECOVERED_SUFFIX : to remove/create the "database recovery attempted" marker file // dbPath + DB_JOURNAL_SUFFIX : to potentially truncate the journal file when overwriting database with fallback // // the pkgsInfo tape memory is used to store these names (the tape is otherwise unused during database bringup) @@ -1068,11 +1068,6 @@ int main(int argc, char *argv[]) { // pkgsInfo.t = dbPath (\0-unterminated) memcpy(pkgsInfo.t, dbPath, dbPathLen); - // pkgsInfo.t = dbPath + DB_RECOVERED_SUFFIX (\0-terminated) - acpy(pkgsInfo.t + dbPathLen, DB_RECOVERED_SUFFIX); - // remove the "database successfully recovered" marker file (it's not supposed to survive reboot but is stored on permanent flash) - if (unlikely(!unlinkIfExists(pkgsInfo.t))) - fail("unlink(db" DB_RECOVERED_SUFFIX ") failed"); parseOptions(argc, argv); @@ -1088,6 +1083,11 @@ int main(int argc, char *argv[]) { if (unlikely(0 > creat(dbOkMarker, 0644))) fail("creat(dbOkMarker) failed"); } else { + // create the "database recovery attempted" marker file + acpy(pkgsInfo.t + dbPathLen, DB_RECOVERED_SUFFIX); + if (unlikely(creat(pkgsInfo.t, 0644) < 0)) + fail("creat(.security-manager.db" DB_RECOVERED_SUFFIX ") failed"); + // main db initialization failed or not attempted, apply fallback toStderr("overwriting db with fallback"); overwriteDbFileWithFallback(dbPathLen); @@ -1096,11 +1096,6 @@ int main(int argc, char *argv[]) { if (unlikely(!dbUp(CheckFallback::no))) fail("fallback db bringup failed"); - // create the "database successfully recovered" marker file - acpy(pkgsInfo.t + dbPathLen, DB_RECOVERED_SUFFIX); - if (unlikely(creat(pkgsInfo.t, 0644) < 0)) - fail("creat(.security-manager.db" DB_RECOVERED_SUFFIX ") failed"); - // try to sync DB_INSTALL_DIR dir const int dbdirfd = open(DB_INSTALL_DIR, O_RDONLY); if (unlikely(dbdirfd < 0)) diff --git a/test/privilege_db_fixture.cpp b/test/privilege_db_fixture.cpp index 62557b73..16369e40 100644 --- a/test/privilege_db_fixture.cpp +++ b/test/privilege_db_fixture.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016-2020 Samsung Electronics Co., Ltd. All rights reserved. + * Copyright (c) 2016-2022 Samsung Electronics Co., Ltd. All rights reserved. * * This file is licensed under the terms of MIT License or the Apache License * Version 2.0 of your choice. See the LICENSE.MIT file for MIT license details. @@ -69,8 +69,9 @@ void testMarkerFile(const char *f, bool present) { } } void checkMarker(PrivilegeDBFixture::Marker marker) { - testMarkerFile(TEST_DB_OK_MARKER, underlying(marker) & underlying(PrivilegeDBFixture::Marker::standard)); - testMarkerFile(TEST_DB_PATH DB_RECOVERED_SUFFIX, underlying(marker) & underlying(PrivilegeDBFixture::Marker::fallback)); + const auto broken = marker != PrivilegeDBFixture::Marker::standard; + testMarkerFile(TEST_DB_OK_MARKER, !broken); + testMarkerFile(TEST_DB_PATH DB_RECOVERED_SUFFIX, broken); if (underlying(marker)) { struct stat st; BOOST_REQUIRE(!lstat(TEST_DB_PATH, &st));