From 916fd793a5cbc28621fa5b2c38cc2869b103d321 Mon Sep 17 00:00:00 2001 From: Ilho Kim Date: Fri, 16 Dec 2022 12:06:30 +0900 Subject: [PATCH] Skip the security registration if it is unnecessary If recovery occurs due to termination of update before security registration performed with information about a new package in the update case the security registration for the original package is skiped because it is unnecessary Change-Id: Ic1ac7f0f4ba213c73d61dd1819f543fb28d0b768 Signed-off-by: Ilho Kim --- src/common/recovery_file.cc | 21 +++++++++- src/common/recovery_file.h | 15 +++++++ src/common/step/filesystem/step_recover_files.cc | 46 ++++++++++++++++++++-- src/common/step/security/step_recover_security.cc | 18 +++++++++ src/common/step/security/step_register_security.cc | 9 +++++ src/common/step/security/step_register_security.h | 3 ++ src/common/step/security/step_update_security.cc | 9 +++++ src/common/step/security/step_update_security.h | 3 ++ 8 files changed, 119 insertions(+), 5 deletions(-) diff --git a/src/common/recovery_file.cc b/src/common/recovery_file.cc index cd1b632..ace2414 100644 --- a/src/common/recovery_file.cc +++ b/src/common/recovery_file.cc @@ -94,7 +94,8 @@ std::unique_ptr RecoveryFile::OpenRecoveryFile( } RecoveryFile::RecoveryFile(const bf::path& path, RequestType type, bool load) - : type_(type), path_(path), backup_done_(false), cleanup_(false) { + : type_(type), path_(path), backup_done_(false), cleanup_(false), + security_operation_done_(false) { backup_path_ = path_.string() + ".bck"; if (load) { if (!ReadFileContent()) { @@ -144,6 +145,10 @@ void RecoveryFile::set_cleanup(bool cleanup) { cleanup_ = cleanup; } +void RecoveryFile::set_security_operation_done(bool security_operation_done) { + security_operation_done_ = security_operation_done; +} + const boost::filesystem::path& RecoveryFile::unpacked_dir() const { return unpacked_dir_; } @@ -164,6 +169,10 @@ bool RecoveryFile::cleanup() const { return cleanup_; } +bool RecoveryFile::security_operation_done() const { + return security_operation_done_; +} + bool RecoveryFile::ReadFileContent() { FILE* handle = fopen(path_.c_str(), "r"); if (!handle) { @@ -213,6 +222,15 @@ bool RecoveryFile::ReadFileContent() { cleanup_ = true; else cleanup_ = false; + if (!fgets(data.data(), data.size(), handle)) { + fclose(handle); + return true; + } + std::string security_operation_done_flag = TruncateNewLine(data.data()); + if (security_operation_done_flag == "true") + security_operation_done_ = true; + else + security_operation_done_ = false; fclose(handle); return true; } @@ -270,6 +288,7 @@ bool RecoveryFile::WriteAndCommitFileContent() { ofs << pkgid_ << std::endl; ofs << (backup_done_ ? "true" : "false") << std::endl; ofs << (cleanup_ ? "cleanup" : "rollback") << std::endl; + ofs << (security_operation_done_ ? "true" : "false") << std::endl; ofs.flush(); ::fsync(ofs->handle()); ofs.close(); diff --git a/src/common/recovery_file.h b/src/common/recovery_file.h index 6aaffb2..d68fb60 100644 --- a/src/common/recovery_file.h +++ b/src/common/recovery_file.h @@ -88,6 +88,13 @@ class RecoveryFile { void set_cleanup(bool cleanup); /** + * setter for security operation done + * + * \param security_operation_done boolean value of security_operation_done + */ + void set_security_operation_done(bool security_operation_done); + + /** * getter for unpacked dir * * \return current unpacked_dir @@ -123,6 +130,13 @@ class RecoveryFile { bool cleanup() const; /** + * getter for security operation done flag + * + * \return true if security operation done flag has set + */ + bool security_operation_done() const; + + /** * Transaction of current RecoveryFile content into recovery file * * \return true if success @@ -143,6 +157,7 @@ class RecoveryFile { boost::filesystem::path backup_path_; bool backup_done_; bool cleanup_; + bool security_operation_done_; }; } // namespace recovery diff --git a/src/common/step/filesystem/step_recover_files.cc b/src/common/step/filesystem/step_recover_files.cc index 5f2f6ca..5bd1ff8 100644 --- a/src/common/step/filesystem/step_recover_files.cc +++ b/src/common/step/filesystem/step_recover_files.cc @@ -16,6 +16,30 @@ namespace bf = boost::filesystem; namespace bs = boost::system; +namespace { + +const char kExternalMemoryMountPoint[] = ".mmc"; + +bool Move(const boost::filesystem::path& from, + const boost::filesystem::path& to, + common_installer::FSFlag flag = common_installer::FSFlag::FS_NONE) { + if (bf::is_directory(from)) { + if (!common_installer::MoveDir(from, to / from.filename(), flag)) { + LOG(ERROR) << "Failed to move directory: " << from; + return false; + } + } else { + if (!common_installer::MoveFile(from, to / from.filename())) { + LOG(ERROR) << "Fail to move file: " << from; + return false; + } + } + + return true; +} + +} // namespace + namespace common_installer { namespace filesystem { @@ -35,15 +59,29 @@ Step::Status StepRecoverFiles::RecoveryUpdate() { if (bf::exists(backup_path)) { // Remove pkgdir only if backup original contents is completely done. if (backup_done) { - if (!RemoveAll(context_->GetPkgPath())) { - LOG(ERROR) << "Cannot restore widget files to its correct location"; - return Status::RECOVERY_ERROR; + for (bf::directory_iterator iter(context_->GetPkgPath()); + iter != bf::directory_iterator(); ++iter) { + if (iter->path().filename() == kExternalMemoryMountPoint) + continue; + + if (!RemoveAll(iter->path())) { + LOG(ERROR) << "Cannot restore widget files to its correct location"; + return Status::RECOVERY_ERROR; + } } // it may fail during recovery. recovery_file->set_backup_done(false); recovery_file->WriteAndCommitFileContent(); } - (void) MoveDir(backup_path, context_->GetPkgPath(), FS_MERGE_OVERWRITE); + + // create copy of old package content skipping the external memory mount point + for (bf::directory_iterator iter(backup_path); + iter != bf::directory_iterator(); ++iter) { + if (!Move(iter->path(), context_->GetPkgPath())) + return Status::RECOVERY_ERROR; + } + + RemoveAll(backup_path); } LOG(INFO) << "Package files recovery done"; return Status::OK; diff --git a/src/common/step/security/step_recover_security.cc b/src/common/step/security/step_recover_security.cc index ffc031e..ae59eaa 100644 --- a/src/common/step/security/step_recover_security.cc +++ b/src/common/step/security/step_recover_security.cc @@ -25,6 +25,12 @@ bool StepRecoverSecurity::Check(bool is_update) { Step::Status StepRecoverSecurity::RecoveryNew() { if (!Check(false)) return Status::OK; + recovery::RecoveryFile* recovery_file = + context_->recovery_info.get().recovery_file.get(); + if (!recovery_file->security_operation_done()) { + LOG(DEBUG) << "security_operation_done false skip recover security"; + return Status::OK; + } std::string error_message; if (!context_->manifest_data.get()) { if (!UnregisterSecurityContextForPkgId(context_->pkgid.get(), @@ -54,6 +60,12 @@ Step::Status StepRecoverSecurity::RecoveryUpdate() { LOG(ERROR) << "Invalid parameters"; return Status::INVALID_VALUE; } + recovery::RecoveryFile* recovery_file = + context_->recovery_info.get().recovery_file.get(); + if (!recovery_file->security_operation_done()) { + LOG(DEBUG) << "security_operation_done false skip recover security"; + return Status::OK; + } std::string error_message; if (!RegisterSecurityContextForManifest(context_, &error_message)) { LOG(ERROR) << "Unsuccessful update"; @@ -80,6 +92,12 @@ Step::Status StepRecoverSecurity::RecoveryReadonlyUpdateInstall() { LOG(ERROR) << "Invalid parameters"; return Status::INVALID_VALUE; } + recovery::RecoveryFile* recovery_file = + context_->recovery_info.get().recovery_file.get(); + if (!recovery_file->security_operation_done()) { + LOG(DEBUG) << "security_operation_done false skip recover security"; + return Status::OK; + } std::string error_message; if (!RegisterSecurityContextForManifest(context_, &error_message)) { LOG(ERROR) << "Unsuccessful update"; diff --git a/src/common/step/security/step_register_security.cc b/src/common/step/security/step_register_security.cc index 917be13..8c655f1 100644 --- a/src/common/step/security/step_register_security.cc +++ b/src/common/step/security/step_register_security.cc @@ -38,6 +38,8 @@ Step::Status StepRegisterSecurity::precheck() { } Step::Status StepRegisterSecurity::process() { + AddRecoveryInfo(); + std::string error_message; if (context_->request_type.get() != RequestType::Move && !RegisterSecurityContextForManifest(context_, &error_message)) { @@ -62,5 +64,12 @@ Step::Status StepRegisterSecurity::process() { return Status::OK; } +void StepRegisterSecurity::AddRecoveryInfo() { + recovery::RecoveryFile* recovery_file = + context_->recovery_info.get().recovery_file.get(); + recovery_file->set_security_operation_done(true); + recovery_file->WriteAndCommitFileContent(); +} + } // namespace security } // namespace common_installer diff --git a/src/common/step/security/step_register_security.h b/src/common/step/security/step_register_security.h index 98f953f..21bc815 100644 --- a/src/common/step/security/step_register_security.h +++ b/src/common/step/security/step_register_security.h @@ -21,6 +21,9 @@ class StepRegisterSecurity : public Step { Status clean() override { return Status::OK; } Status precheck() override; + private: + void AddRecoveryInfo(); + STEP_NAME(RegisterSecurity) }; diff --git a/src/common/step/security/step_update_security.cc b/src/common/step/security/step_update_security.cc index f00f4b2..7d17db2 100644 --- a/src/common/step/security/step_update_security.cc +++ b/src/common/step/security/step_update_security.cc @@ -12,6 +12,8 @@ namespace common_installer { namespace security { Step::Status StepUpdateSecurity::process() { + AddRecoveryInfo(); + std::string error_message; if (!RegisterSecurityContextForManifest(context_, &error_message)) { if (!error_message.empty()) { @@ -56,5 +58,12 @@ Step::Status StepUpdateSecurity::undo() { return Status::OK; } +void StepUpdateSecurity::AddRecoveryInfo() { + recovery::RecoveryFile* recovery_file = + context_->recovery_info.get().recovery_file.get(); + recovery_file->set_security_operation_done(true); + recovery_file->WriteAndCommitFileContent(); +} + } // namespace security } // namespace common_installer diff --git a/src/common/step/security/step_update_security.h b/src/common/step/security/step_update_security.h index 6c78e41..243c98e 100644 --- a/src/common/step/security/step_update_security.h +++ b/src/common/step/security/step_update_security.h @@ -21,6 +21,9 @@ class StepUpdateSecurity : public Step { Status clean() override { return Status::OK; } Status precheck() override { return Status::OK; } + private: + void AddRecoveryInfo(); + STEP_NAME(UpdateSecurity) }; -- 2.7.4