From 0a7cd42c840c952055ff9378a40709e674100551 Mon Sep 17 00:00:00 2001 From: Patrick Vacek Date: Tue, 15 Jan 2019 16:25:14 +0100 Subject: [PATCH 01/19] WIP draft of basic delegation support. Signed-off-by: Patrick Vacek --- src/libaktualizr/primary/sotauptaneclient.cc | 62 ++++++++++++++++++-- src/libaktualizr/uptane/imagesrepository.cc | 20 ++++--- src/libaktualizr/uptane/imagesrepository.h | 11 +++- src/libaktualizr/uptane/tuf.cc | 32 +++++++++- src/libaktualizr/uptane/tuf.h | 17 +++++- 5 files changed, 124 insertions(+), 18 deletions(-) diff --git a/src/libaktualizr/primary/sotauptaneclient.cc b/src/libaktualizr/primary/sotauptaneclient.cc index d3ac7a1cb6..e033fe2759 100644 --- a/src/libaktualizr/primary/sotauptaneclient.cc +++ b/src/libaktualizr/primary/sotauptaneclient.cc @@ -568,7 +568,7 @@ bool SotaUptaneClient::updateImagesMeta() { local_version = -1; } - if (!images_repo.verifyTargets(images_targets)) { + if (!images_repo.verifyTargets(images_targets, "targets")) { last_exception = images_repo.getLastException(); return false; } @@ -579,11 +579,53 @@ bool SotaUptaneClient::updateImagesMeta() { storage->storeNonRoot(images_targets, Uptane::RepositoryType::Image(), Uptane::Role::Targets()); } - if (images_repo.targetsExpired()) { + if (images_repo.targetsExpired("targets")) { last_exception = Uptane::ExpiredMetadata("repo", "targets"); return false; } } + + // Update Images delegated Targets Metadata + for (const Uptane::Delegation &delegation : images_repo.delegations("targets")) { + std::string images_delegated; + + if (!uptane_fetcher->fetchLatestRole(&images_delegated, Uptane::kMaxImagesTargetsSize, + Uptane::RepositoryType::Image(), Uptane::Role::Delegated(delegation.name_))) { + return false; + } + int remote_version = Uptane::extractVersionUntrusted(images_delegated); + + int local_version; + std::string images_delegated_stored; + // TODO: will this work? + if (storage->loadNonRoot(&images_delegated_stored, Uptane::RepositoryType::Image(), + Uptane::Role::Delegated(delegation.name_))) { + local_version = Uptane::extractVersionUntrusted(images_delegated_stored); + } else { + local_version = -1; + } + + if (!images_repo.verifyTargets(images_delegated, delegation.name_)) { + last_exception = images_repo.getLastException(); + return false; + } + + if (local_version > remote_version) { + return false; + } else if (local_version < remote_version) { + // TODO: will this work? + // Store the metadata (in meta table? with new meta_type with delegated name?) + // Maybe better to use a separate table. + storage->storeNonRoot(images_delegated, Uptane::RepositoryType::Image(), + Uptane::Role::Delegated(delegation.name_)); + } + + if (images_repo.targetsExpired(delegation.name_)) { + last_exception = Uptane::ExpiredMetadata("repo", "delegated targets"); + return false; + } + } + return true; } @@ -693,16 +735,27 @@ bool SotaUptaneClient::checkImagesMetaOffline() { return false; } - if (!images_repo.verifyTargets(images_targets)) { + if (!images_repo.verifyTargets(images_targets, "targets")) { last_exception = images_repo.getLastException(); return false; } - if (images_repo.targetsExpired()) { + if (images_repo.targetsExpired("targets")) { last_exception = Uptane::ExpiredMetadata("repo", "targets"); return false; } } + + // TODO: delegations + // Is rechecking for delegations necessary, or how do we know if we need to + // check/look for them? + // Check for delegations in targets.json. + // Store keys + // Store roles + // Update delegated_targets metadata + // Load delegated targets metadata + // Verify targets + // Check expiration return true; } @@ -789,7 +842,6 @@ result::Download SotaUptaneClient::downloadImages(const std::vector downloaded_targets; for (auto it = targets.cbegin(); it != targets.cend(); ++it) { - // TODO: delegations auto images_target = images_repo.getTarget(*it); if (images_target == nullptr) { last_exception = Uptane::TargetHashMismatch(it->filename()); diff --git a/src/libaktualizr/uptane/imagesrepository.cc b/src/libaktualizr/uptane/imagesrepository.cc index 52a31d06dc..8017001d5d 100644 --- a/src/libaktualizr/uptane/imagesrepository.cc +++ b/src/libaktualizr/uptane/imagesrepository.cc @@ -4,7 +4,7 @@ namespace Uptane { void ImagesRepository::resetMeta() { resetRoot(); - targets = Targets(); + targets.clear(); snapshot = Snapshot(); timestamp = TimestampMeta(); } @@ -61,9 +61,10 @@ bool ImagesRepository::verifySnapshot(const std::string& snapshot_raw) { return true; } -bool ImagesRepository::verifyTargets(const std::string& targets_raw) { +bool ImagesRepository::verifyTargets(const std::string& targets_raw, const std::string& role_name) { try { - const std::string canonical = Utils::jsonToCanonicalStr(Utils::parseJSON(targets_raw)); + const Json::Value targets_json = Utils::parseJSON(targets_raw); + const std::string canonical = Utils::jsonToCanonicalStr(targets_json); bool hash_exists = false; for (const auto& it : snapshot.targets_hashes()) { switch (it.type()) { @@ -89,8 +90,11 @@ bool ImagesRepository::verifyTargets(const std::string& targets_raw) { LOG_ERROR << "No hash found for targets.json"; return false; } - targets = Targets(RepositoryType::Image(), Utils::parseJSON(targets_raw), root); // signature verification - if (targets.version() != snapshot.targets_version()) { + targets[role_name] = Targets(RepositoryType::Image(), targets_json, root); // signature verification + // Only compare targets version in snapshot metadata for top-level + // targets.json. Delegated target metadata versions are not tracked outside + // of their own metadata. + if (role_name == "targets" && targets[role_name].version() != snapshot.targets_version()) { return false; } } catch (const Exception& e) { @@ -101,9 +105,11 @@ bool ImagesRepository::verifyTargets(const std::string& targets_raw) { return true; } +// TODO: Delegation support. std::unique_ptr ImagesRepository::getTarget(const Uptane::Target& director_target) { - auto it = std::find(targets.targets.cbegin(), targets.targets.cend(), director_target); - if (it == targets.targets.cend()) { + auto it = std::find(targets["targets"].targets.cbegin(), targets["targets"].targets.cend(), director_target); + if (it == targets["targets"].targets.cend()) { + // TODO: check delegation paths, etc. return std::unique_ptr(nullptr); } else { return std_::make_unique(*it); diff --git a/src/libaktualizr/uptane/imagesrepository.h b/src/libaktualizr/uptane/imagesrepository.h index f7098ff62e..c505a5b663 100644 --- a/src/libaktualizr/uptane/imagesrepository.h +++ b/src/libaktualizr/uptane/imagesrepository.h @@ -1,6 +1,9 @@ #ifndef IMAGES_REPOSITORY_H_ #define IMAGES_REPOSITORY_H_ +#include +#include + #include "uptanerepository.h" namespace Uptane { @@ -11,9 +14,10 @@ class ImagesRepository : public RepositoryCommon { void resetMeta(); - bool verifyTargets(const std::string& targets_raw); - bool targetsExpired() { return targets.isExpired(TimeStamp::Now()); } + bool verifyTargets(const std::string& targets_raw, const std::string& role_name); + bool targetsExpired(const std::string& role_name) { return targets[role_name].isExpired(TimeStamp::Now()); } int64_t targetsSize() { return snapshot.targets_size(); } + std::vector delegations(const std::string& role_name) { return targets[role_name].delegations_; } std::unique_ptr getTarget(const Uptane::Target& director_target); bool verifyTimestamp(const std::string& timestamp_raw); @@ -26,7 +30,8 @@ class ImagesRepository : public RepositoryCommon { Exception getLastException() const { return last_exception; } private: - Uptane::Targets targets; + // Map from role name -> metadata ("targets" for top-level): + std::map targets; Uptane::TimestampMeta timestamp; Uptane::Snapshot snapshot; diff --git a/src/libaktualizr/uptane/tuf.cc b/src/libaktualizr/uptane/tuf.cc index 18f1e11c45..3359b735e0 100644 --- a/src/libaktualizr/uptane/tuf.cc +++ b/src/libaktualizr/uptane/tuf.cc @@ -265,12 +265,42 @@ void Uptane::Targets::init(const Json::Value &json) { throw Uptane::InvalidMetadata("", "targets", "invalid targets.json"); } - Json::Value target_list = json["signed"]["targets"]; + const Json::Value target_list = json["signed"]["targets"]; for (Json::ValueIterator t_it = target_list.begin(); t_it != target_list.end(); t_it++) { Target t(t_it.key().asString(), *t_it); targets.push_back(t); } + if (json["signed"]["delegations"].isObject()) { + const Json::Value key_list = json["signed"]["delegations"]["keys"]; + for (Json::ValueIterator k_it = key_list.begin(); k_it != key_list.end(); k_it++) { + const std::string key_type = boost::algorithm::to_lower_copy((*k_it)["keytype"].asString()); + if (key_type != "rsa" && key_type != "ed25519") { + throw SecurityException("image", "Unsupported key type: " + (*k_it)["keytype"].asString()); + } + const KeyId keyid = k_it.key().asString(); + PublicKey key(*k_it); + keys_[keyid] = key; + } + + const Json::Value role_list = json["signed"]["delegations"]["roles"]; + for (Json::ValueIterator r_it = role_list.begin(); r_it != role_list.end(); r_it++) { + Delegation delegate; + delegate.name_ = (*r_it)["name"].asString(); + const Json::Value keyid_list = (*r_it)["keyids"]; + for (Json::ValueIterator kid_it = keyid_list.begin(); kid_it != keyid_list.end(); kid_it++) { + delegate.key_ids_.emplace_back(kid_it.key().asString()); + } + const Json::Value paths_list = (*r_it)["paths"]; + for (Json::ValueIterator p_it = paths_list.begin(); p_it != paths_list.end(); p_it++) { + delegate.paths_.emplace_back(p_it.key().asString()); + } + delegate.terminating_ = (*r_it)["terminating"].asBool(); + delegate.threshold_ = (*r_it)["threshold"].asInt64(); + delegations_.push_back(delegate); + } + } + if (json["signed"]["custom"].isObject()) { correlation_id_ = json["signed"]["custom"]["correlationId"].asString(); } else { diff --git a/src/libaktualizr/uptane/tuf.h b/src/libaktualizr/uptane/tuf.h index efdb220f43..2a01f41c37 100644 --- a/src/libaktualizr/uptane/tuf.h +++ b/src/libaktualizr/uptane/tuf.h @@ -8,6 +8,7 @@ #include #include #include +#include #include "uptane/exceptions.h" #include "crypto/crypto.h" @@ -60,6 +61,7 @@ class Role { static Role Timestamp() { return Role{RoleEnum::kTimestamp}; } static Role Delegated(const std::string &name) { return Role(name, true); } static Role InvalidRole() { return Role{RoleEnum::kInvalidRole}; } + // TODO: add delegated? static std::vector Roles() { return {Root(), Snapshot(), Targets(), Timestamp()}; } explicit Role(const std::string &role_name, bool delegation = false); @@ -359,22 +361,33 @@ class Root : public BaseMeta { std::map thresholds_for_role_; }; +struct Delegation { + std::string name_; + std::vector key_ids_; + std::vector paths_; + bool terminating_{true}; + int64_t threshold_{0}; +}; + class Targets : public BaseMeta { public: explicit Targets(const Json::Value &json); Targets(RepositoryType repo, const Json::Value &json, Root &root); Targets() = default; - std::vector targets; bool operator==(const Targets &rhs) const { return version_ == rhs.version() && expiry_ == rhs.expiry() && targets == rhs.targets; } const std::string &correlation_id() const { return correlation_id_; } + std::vector targets; + std::vector delegations_; + private: void init(const Json::Value &json); - std::string correlation_id_; // custom non-tuf + std::map keys_; // for delegated roles + std::string correlation_id_; // custom non-tuf }; class TimestampMeta : public BaseMeta { From 9d2e1959ba89c26f09854b33028d6f77c4bc2bcd Mon Sep 17 00:00:00 2001 From: Patrick Vacek Date: Thu, 17 Jan 2019 15:41:00 +0100 Subject: [PATCH 02/19] Delegation support in SQL database. Signed-off-by: Patrick Vacek --- config/sql/migration/migrate.15.sql | 1 + config/sql/migration/migrate.16.sql | 1 + config/sql/migration/migrate.17.sql | 9 +++++ config/sql/rollback/rollback.17.sql | 9 +++++ config/sql/schema.sql | 3 +- src/libaktualizr/storage/invstorage.h | 3 ++ src/libaktualizr/storage/sqlstorage.cc | 55 ++++++++++++++++++++++++++ src/libaktualizr/storage/sqlstorage.h | 3 ++ 8 files changed, 83 insertions(+), 1 deletion(-) create mode 100644 config/sql/migration/migrate.17.sql create mode 100644 config/sql/rollback/rollback.17.sql diff --git a/config/sql/migration/migrate.15.sql b/config/sql/migration/migrate.15.sql index 196664cb1f..1efc0d97b4 100644 --- a/config/sql/migration/migrate.15.sql +++ b/config/sql/migration/migrate.15.sql @@ -1,3 +1,4 @@ +-- Don't modify this! Create a new migration instead--see docs/schema-migrations.adoc SAVEPOINT MIGRATION; CREATE TABLE rollback_migrations(version_from INT PRIMARY KEY, migration TEXT NOT NULL); diff --git a/config/sql/migration/migrate.16.sql b/config/sql/migration/migrate.16.sql index e6b5044c7e..4b5b8f60ee 100644 --- a/config/sql/migration/migrate.16.sql +++ b/config/sql/migration/migrate.16.sql @@ -1,3 +1,4 @@ +-- Don't modify this! Create a new migration instead--see docs/schema-migrations.adoc SAVEPOINT MIGRATION; DROP TABLE installation_result; diff --git a/config/sql/migration/migrate.17.sql b/config/sql/migration/migrate.17.sql new file mode 100644 index 0000000000..edb76255b7 --- /dev/null +++ b/config/sql/migration/migrate.17.sql @@ -0,0 +1,9 @@ +-- Don't modify this! Create a new migration instead--see docs/schema-migrations.adoc +SAVEPOINT MIGRATION; + +CREATE TABLE delegations(meta BLOB NOT NULL, role_name TEXT NOT NULL, UNIQUE(role_name)); + +DELETE FROM version; +INSERT INTO version VALUES(17); + +RELEASE MIGRATION; diff --git a/config/sql/rollback/rollback.17.sql b/config/sql/rollback/rollback.17.sql new file mode 100644 index 0000000000..989b33c3f9 --- /dev/null +++ b/config/sql/rollback/rollback.17.sql @@ -0,0 +1,9 @@ +-- Don't modify this! Create a new migration instead--see docs/schema-migrations.adoc +SAVEPOINT ROLLBACK_MIGRATION; + +DROP TABLE delegations; + +DELETE FROM version; +INSERT INTO version VALUES(16); + +RELEASE ROLLBACK_MIGRATION; diff --git a/config/sql/schema.sql b/config/sql/schema.sql index 1ebcce4d7a..76af6cb834 100644 --- a/config/sql/schema.sql +++ b/config/sql/schema.sql @@ -1,5 +1,5 @@ CREATE TABLE version(version INTEGER); -INSERT INTO version(rowid,version) VALUES(1,16); +INSERT INTO version(rowid,version) VALUES(1,17); CREATE TABLE device_info(unique_mark INTEGER PRIMARY KEY CHECK (unique_mark = 0), device_id TEXT, is_registered INTEGER NOT NULL DEFAULT 0 CHECK (is_registered IN (0,1))); CREATE TABLE ecu_serials(serial TEXT UNIQUE, hardware_id TEXT NOT NULL, is_primary INTEGER NOT NULL CHECK (is_primary IN (0,1))); CREATE TABLE misconfigured_ecus(serial TEXT UNIQUE, hardware_id TEXT NOT NULL, state INTEGER NOT NULL CHECK (state IN (0,1))); @@ -23,3 +23,4 @@ CREATE TABLE device_installation_result(unique_mark INTEGER PRIMARY KEY CHECK (u CREATE TABLE ecu_installation_results(ecu_serial TEXT NOT NULL PRIMARY KEY, success INTEGER NOT NULL DEFAULT 0, result_code TEXT NOT NULL DEFAULT "", description TEXT NOT NULL DEFAULT ""); CREATE TABLE need_reboot(unique_mark INTEGER PRIMARY KEY CHECK (unique_mark = 0), flag INTEGER NOT NULL DEFAULT 0); CREATE TABLE rollback_migrations(version_from INT PRIMARY KEY, migration TEXT NOT NULL); +CREATE TABLE delegations(meta BLOB NOT NULL, role_name TEXT NOT NULL, UNIQUE(role_name)); diff --git a/src/libaktualizr/storage/invstorage.h b/src/libaktualizr/storage/invstorage.h index ad10ee7d17..35e84d3388 100644 --- a/src/libaktualizr/storage/invstorage.h +++ b/src/libaktualizr/storage/invstorage.h @@ -136,6 +136,9 @@ class INvStorage { virtual bool loadNonRoot(std::string* data, Uptane::RepositoryType repo, Uptane::Role role) = 0; virtual void clearNonRootMeta(Uptane::RepositoryType repo) = 0; virtual void clearMetadata() = 0; + virtual void storeDelegation(const std::string& data, const Uptane::Role role) = 0; + virtual bool loadDelegation(std::string* data, const Uptane::Role role) = 0; + virtual void clearDelegations() = 0; virtual void storeDeviceId(const std::string& device_id) = 0; virtual bool loadDeviceId(std::string* device_id) = 0; diff --git a/src/libaktualizr/storage/sqlstorage.cc b/src/libaktualizr/storage/sqlstorage.cc index ffb52e43e2..65ac3c92d5 100644 --- a/src/libaktualizr/storage/sqlstorage.cc +++ b/src/libaktualizr/storage/sqlstorage.cc @@ -514,6 +514,61 @@ void SQLStorage::clearMetadata() { } } +void SQLStorage::storeDelegation(const std::string& data, const Uptane::Role role) { + SQLite3Guard db = dbConnection(); + + if (!db.beginTransaction()) { + LOG_ERROR << "Can't start transaction: " << db.errmsg(); + return; + } + + auto del_statement = db.prepareStatement("DELETE FROM delegations WHERE role_name=?;", role.ToString()); + + if (del_statement.step() != SQLITE_DONE) { + LOG_ERROR << "Can't clear delegation metadata: " << db.errmsg(); + return; + } + + auto ins_statement = db.prepareStatement("INSERT INTO delegations VALUES (?, ?);", + SQLBlob(data), role.ToString()); + + if (ins_statement.step() != SQLITE_DONE) { + LOG_ERROR << "Can't add delegation metadata: " << db.errmsg(); + return; + } + + db.commitTransaction(); +} + +bool SQLStorage::loadDelegation(std::string* data, const Uptane::Role role) { + SQLite3Guard db = dbConnection(); + + auto statement = db.prepareStatement( + "SELECT meta FROM delegations WHERE role_name=? ORDER BY role_name DESC LIMIT 1;", role.ToString()); + int result = statement.step(); + + if (result == SQLITE_DONE) { + LOG_TRACE << "Delegations metadata not present"; + return false; + } else if (result != SQLITE_ROW) { + LOG_ERROR << "Can't get delegations metadata: " << db.errmsg(); + return false; + } + if (data != nullptr) { + *data = std::string(reinterpret_cast(sqlite3_column_blob(statement.get(), 0))); + } + + return true; +} + +void SQLStorage::clearDelegations() { + SQLite3Guard db = dbConnection(); + + if (db.exec("DELETE FROM delegations;", nullptr, nullptr) != SQLITE_OK) { + LOG_ERROR << "Can't clear delegations metadata: " << db.errmsg(); + } +} + void SQLStorage::storeDeviceId(const std::string& device_id) { SQLite3Guard db = dbConnection(); diff --git a/src/libaktualizr/storage/sqlstorage.h b/src/libaktualizr/storage/sqlstorage.h index 691c7d23ae..ec75c0c8b5 100644 --- a/src/libaktualizr/storage/sqlstorage.h +++ b/src/libaktualizr/storage/sqlstorage.h @@ -42,6 +42,9 @@ class SQLStorage : public SQLStorageBase, public INvStorage { bool loadNonRoot(std::string* data, Uptane::RepositoryType repo, Uptane::Role role) override; void clearNonRootMeta(Uptane::RepositoryType repo) override; void clearMetadata() override; + void storeDelegation(const std::string& data, const Uptane::Role role) override; + bool loadDelegation(std::string* data, const Uptane::Role role) override; + void clearDelegations() override; void storeDeviceId(const std::string& device_id) override; bool loadDeviceId(std::string* device_id) override; From fda5515550b06c3465078bcb0fe0e199f1bcfbb7 Mon Sep 17 00:00:00 2001 From: Patrick Vacek Date: Thu, 17 Jan 2019 15:50:44 +0100 Subject: [PATCH 03/19] Const-correctness in storage and clang-tidy recommendations. Signed-off-by: Patrick Vacek --- src/libaktualizr/package_manager/packagemanagerfake.cc | 2 +- src/libaktualizr/storage/invstorage.h | 4 ++-- src/libaktualizr/storage/sqlstorage.cc | 4 ++-- src/libaktualizr/storage/sqlstorage.h | 4 ++-- src/libaktualizr/uptane/tuf.h | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/libaktualizr/package_manager/packagemanagerfake.cc b/src/libaktualizr/package_manager/packagemanagerfake.cc index 74fba2e410..59ea2bc8d2 100644 --- a/src/libaktualizr/package_manager/packagemanagerfake.cc +++ b/src/libaktualizr/package_manager/packagemanagerfake.cc @@ -25,7 +25,7 @@ Uptane::Target PackageManagerFake::getCurrent() const { data::InstallationResult PackageManagerFake::install(const Uptane::Target &target) const { // fault injection: only enabled with FIU_ENABLE defined - if (fiu_fail("fake_package_install")) { + if (fiu_fail("fake_package_install") != 0) { std::string failure_cause = fault_injection_get_parameter("fault_fake_package_install_cause"); if (failure_cause.empty()) { failure_cause = "Installation failed"; diff --git a/src/libaktualizr/storage/invstorage.h b/src/libaktualizr/storage/invstorage.h index 35e84d3388..232ca07e57 100644 --- a/src/libaktualizr/storage/invstorage.h +++ b/src/libaktualizr/storage/invstorage.h @@ -136,8 +136,8 @@ class INvStorage { virtual bool loadNonRoot(std::string* data, Uptane::RepositoryType repo, Uptane::Role role) = 0; virtual void clearNonRootMeta(Uptane::RepositoryType repo) = 0; virtual void clearMetadata() = 0; - virtual void storeDelegation(const std::string& data, const Uptane::Role role) = 0; - virtual bool loadDelegation(std::string* data, const Uptane::Role role) = 0; + virtual void storeDelegation(const std::string& data, Uptane::Role role) = 0; + virtual bool loadDelegation(std::string* data, Uptane::Role role) = 0; virtual void clearDelegations() = 0; virtual void storeDeviceId(const std::string& device_id) = 0; diff --git a/src/libaktualizr/storage/sqlstorage.cc b/src/libaktualizr/storage/sqlstorage.cc index 65ac3c92d5..7d9fe848f5 100644 --- a/src/libaktualizr/storage/sqlstorage.cc +++ b/src/libaktualizr/storage/sqlstorage.cc @@ -402,7 +402,7 @@ void SQLStorage::storeRoot(const std::string& data, Uptane::RepositoryType repo, db.commitTransaction(); } -void SQLStorage::storeNonRoot(const std::string& data, Uptane::RepositoryType repo, Uptane::Role role) { +void SQLStorage::storeNonRoot(const std::string& data, Uptane::RepositoryType repo, const Uptane::Role role) { SQLite3Guard db = dbConnection(); if (!db.beginTransaction()) { @@ -472,7 +472,7 @@ bool SQLStorage::loadRoot(std::string* data, Uptane::RepositoryType repo, Uptane return true; } -bool SQLStorage::loadNonRoot(std::string* data, Uptane::RepositoryType repo, Uptane::Role role) { +bool SQLStorage::loadNonRoot(std::string* data, Uptane::RepositoryType repo, const Uptane::Role role) { SQLite3Guard db = dbConnection(); auto statement = db.prepareStatement( diff --git a/src/libaktualizr/storage/sqlstorage.h b/src/libaktualizr/storage/sqlstorage.h index ec75c0c8b5..b516ab2859 100644 --- a/src/libaktualizr/storage/sqlstorage.h +++ b/src/libaktualizr/storage/sqlstorage.h @@ -42,8 +42,8 @@ class SQLStorage : public SQLStorageBase, public INvStorage { bool loadNonRoot(std::string* data, Uptane::RepositoryType repo, Uptane::Role role) override; void clearNonRootMeta(Uptane::RepositoryType repo) override; void clearMetadata() override; - void storeDelegation(const std::string& data, const Uptane::Role role) override; - bool loadDelegation(std::string* data, const Uptane::Role role) override; + void storeDelegation(const std::string& data, Uptane::Role role) override; + bool loadDelegation(std::string* data, Uptane::Role role) override; void clearDelegations() override; void storeDeviceId(const std::string& device_id) override; diff --git a/src/libaktualizr/uptane/tuf.h b/src/libaktualizr/uptane/tuf.h index 2a01f41c37..8454587d81 100644 --- a/src/libaktualizr/uptane/tuf.h +++ b/src/libaktualizr/uptane/tuf.h @@ -106,7 +106,7 @@ class Version { Version() : version_(ANY_VERSION) {} explicit Version(int v) : version_(v) {} std::string RoleFileName(const Role &role) const; - int version() { return version_; } + int version() const { return version_; } private: static const int ANY_VERSION = -1; From 91e5388785763a87135171e085bcbd4a0efdf753 Mon Sep 17 00:00:00 2001 From: Patrick Vacek Date: Thu, 17 Jan 2019 17:25:56 +0100 Subject: [PATCH 04/19] Fix checkImagesMetaOffline and delegation storage calls. Signed-off-by: Patrick Vacek --- src/libaktualizr/primary/sotauptaneclient.cc | 40 ++++++++++---------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/src/libaktualizr/primary/sotauptaneclient.cc b/src/libaktualizr/primary/sotauptaneclient.cc index e033fe2759..0bd4e01ca4 100644 --- a/src/libaktualizr/primary/sotauptaneclient.cc +++ b/src/libaktualizr/primary/sotauptaneclient.cc @@ -597,9 +597,7 @@ bool SotaUptaneClient::updateImagesMeta() { int local_version; std::string images_delegated_stored; - // TODO: will this work? - if (storage->loadNonRoot(&images_delegated_stored, Uptane::RepositoryType::Image(), - Uptane::Role::Delegated(delegation.name_))) { + if (storage->loadDelegation(&images_delegated_stored, Uptane::Role::Delegated(delegation.name_))) { local_version = Uptane::extractVersionUntrusted(images_delegated_stored); } else { local_version = -1; @@ -613,15 +611,11 @@ bool SotaUptaneClient::updateImagesMeta() { if (local_version > remote_version) { return false; } else if (local_version < remote_version) { - // TODO: will this work? - // Store the metadata (in meta table? with new meta_type with delegated name?) - // Maybe better to use a separate table. - storage->storeNonRoot(images_delegated, Uptane::RepositoryType::Image(), - Uptane::Role::Delegated(delegation.name_)); + storage->storeDelegation(images_delegated, Uptane::Role::Delegated(delegation.name_)); } if (images_repo.targetsExpired(delegation.name_)) { - last_exception = Uptane::ExpiredMetadata("repo", "delegated targets"); + last_exception = Uptane::ExpiredMetadata("repo", delegation.name_); return false; } } @@ -746,16 +740,24 @@ bool SotaUptaneClient::checkImagesMetaOffline() { } } - // TODO: delegations - // Is rechecking for delegations necessary, or how do we know if we need to - // check/look for them? - // Check for delegations in targets.json. - // Store keys - // Store roles - // Update delegated_targets metadata - // Load delegated targets metadata - // Verify targets - // Check expiration + // Update Images delegated Targets Metadata + for (const Uptane::Delegation &delegation : images_repo.delegations("targets")) { + std::string images_delegated; + if (!storage->loadDelegation(&images_delegated, Uptane::Role::Delegated(delegation.name_))) { + return false; + } + + if (!images_repo.verifyTargets(images_delegated, delegation.name_)) { + last_exception = images_repo.getLastException(); + return false; + } + + if (images_repo.targetsExpired(delegation.name_)) { + last_exception = Uptane::ExpiredMetadata("repo", delegation.name_); + return false; + } + } + return true; } From 8f2cd1aa7d1e5a6c018699102942669e469ef346 Mon Sep 17 00:00:00 2001 From: Patrick Vacek Date: Fri, 18 Jan 2019 11:41:01 +0100 Subject: [PATCH 05/19] Better role error checking with tests. Signed-off-by: Patrick Vacek --- src/libaktualizr/uptane/role.cc | 22 +++++++++++++++------- src/libaktualizr/uptane/tuf_test.cc | 14 ++++++++++++++ 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/src/libaktualizr/uptane/role.cc b/src/libaktualizr/uptane/role.cc index 9123a6a43e..e40a549f0b 100644 --- a/src/libaktualizr/uptane/role.cc +++ b/src/libaktualizr/uptane/role.cc @@ -5,21 +5,29 @@ using Uptane::Role; using Uptane::Version; +const std::string UPTANE_ROLE_ROOT = "root"; +const std::string UPTANE_ROLE_SNAPSHOT = "snapshot"; +const std::string UPTANE_ROLE_TARGETS = "targets"; +const std::string UPTANE_ROLE_TIMESTAMP = "timestamp"; + Role::Role(const std::string &role_name, const bool delegation) { std::string role_name_lower; std::transform(role_name.begin(), role_name.end(), std::back_inserter(role_name_lower), ::tolower); name_ = role_name_lower; - if (role_name_lower == "root") { + if (delegation) { + if (role_name_lower == UPTANE_ROLE_ROOT || role_name_lower == UPTANE_ROLE_SNAPSHOT || + role_name_lower == UPTANE_ROLE_TARGETS || role_name_lower == UPTANE_ROLE_TIMESTAMP) { + throw Uptane::Exception("", "Invalid delegated role name " + role_name); + } + role_ = RoleEnum::kDelegated; + } else if (role_name_lower == UPTANE_ROLE_ROOT) { role_ = RoleEnum::kRoot; - } else if (role_name_lower == "snapshot") { + } else if (role_name_lower == UPTANE_ROLE_SNAPSHOT) { role_ = RoleEnum::kSnapshot; - } else if (role_name_lower == "targets") { + } else if (role_name_lower == UPTANE_ROLE_TARGETS) { role_ = RoleEnum::kTargets; - } else if (role_name_lower == "timestamp") { + } else if (role_name_lower == UPTANE_ROLE_TIMESTAMP) { role_ = RoleEnum::kTimestamp; - } else if (delegation) { - role_ = RoleEnum::kDelegated; - name_ = role_name; } else { role_ = RoleEnum::kInvalidRole; name_ = "invalidrole"; diff --git a/src/libaktualizr/uptane/tuf_test.cc b/src/libaktualizr/uptane/tuf_test.cc index ec27583d7d..5f05e6eb89 100644 --- a/src/libaktualizr/uptane/tuf_test.cc +++ b/src/libaktualizr/uptane/tuf_test.cc @@ -47,6 +47,20 @@ TEST(Root, RootJsonRsassaPssSha256) { EXPECT_NO_THROW(Uptane::Root(Uptane::RepositoryType::Director(), initial_root, root)); } +/* Reject delegated role names that are identical to other roles. */ +TEST(Role, InvalidDelegationName) { + EXPECT_THROW(Uptane::Role::Delegated("root"), Uptane::Exception); + EXPECT_THROW(Uptane::Role::Delegated("snapshot"), Uptane::Exception); + EXPECT_THROW(Uptane::Role::Delegated("targets"), Uptane::Exception); + EXPECT_THROW(Uptane::Role::Delegated("timestamp"), Uptane::Exception); +} + +/* Delegated role has custom name. */ +TEST(Role, ValidDelegationName) { + Uptane::Role delegated = Uptane::Role::Delegated("whatever"); + EXPECT_EQ(delegated.ToString(), "whatever"); +} + #ifndef __NO_MAIN__ int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); From d08c763447d411fcfe9adb6a7e83b35a9098499b Mon Sep 17 00:00:00 2001 From: Patrick Vacek Date: Fri, 18 Jan 2019 18:14:08 +0100 Subject: [PATCH 06/19] Refactor signing code from Root for use by Targets too. Signed-off-by: Patrick Vacek --- .../aktualizr_secondary.cc | 3 +- src/libaktualizr/primary/sotauptaneclient.cc | 24 ++-- src/libaktualizr/uptane/CMakeLists.txt | 7 +- src/libaktualizr/uptane/directorrepository.cc | 3 +- src/libaktualizr/uptane/imagesrepository.cc | 10 +- src/libaktualizr/uptane/imagesrepository.h | 2 +- src/libaktualizr/uptane/metawithkeys.cc | 120 +++++++++++++++++ .../uptane/partialverificationsecondary.cc | 3 +- src/libaktualizr/uptane/root.cc | 122 ++---------------- src/libaktualizr/uptane/tuf.cc | 56 ++++---- src/libaktualizr/uptane/tuf.h | 94 ++++++++++---- 11 files changed, 256 insertions(+), 188 deletions(-) create mode 100644 src/libaktualizr/uptane/metawithkeys.cc diff --git a/src/aktualizr_secondary/aktualizr_secondary.cc b/src/aktualizr_secondary/aktualizr_secondary.cc index e4bdede562..0372c58b07 100644 --- a/src/aktualizr_secondary/aktualizr_secondary.cc +++ b/src/aktualizr_secondary/aktualizr_secondary.cc @@ -70,7 +70,8 @@ bool AktualizrSecondary::putMetadataResp(const Uptane::RawMetaPack& meta_pack) { // TODO: proper partial verification root_ = Uptane::Root(Uptane::RepositoryType::Director(), Utils::parseJSON(meta_pack.director_root), root_); - Uptane::Targets targets(Uptane::RepositoryType::Director(), Utils::parseJSON(meta_pack.director_targets), root_); + Uptane::Targets targets(Uptane::RepositoryType::Director(), Utils::parseJSON(meta_pack.director_targets), + std::make_shared(root_)); if (meta_targets_.version() > targets.version()) { detected_attack_ = "Rollback attack detected"; return true; diff --git a/src/libaktualizr/primary/sotauptaneclient.cc b/src/libaktualizr/primary/sotauptaneclient.cc index 0bd4e01ca4..250f07f015 100644 --- a/src/libaktualizr/primary/sotauptaneclient.cc +++ b/src/libaktualizr/primary/sotauptaneclient.cc @@ -586,24 +586,24 @@ bool SotaUptaneClient::updateImagesMeta() { } // Update Images delegated Targets Metadata - for (const Uptane::Delegation &delegation : images_repo.delegations("targets")) { + for (const std::string &delegation : images_repo.delegations("targets")) { std::string images_delegated; if (!uptane_fetcher->fetchLatestRole(&images_delegated, Uptane::kMaxImagesTargetsSize, - Uptane::RepositoryType::Image(), Uptane::Role::Delegated(delegation.name_))) { + Uptane::RepositoryType::Image(), Uptane::Role::Delegated(delegation))) { return false; } int remote_version = Uptane::extractVersionUntrusted(images_delegated); int local_version; std::string images_delegated_stored; - if (storage->loadDelegation(&images_delegated_stored, Uptane::Role::Delegated(delegation.name_))) { + if (storage->loadDelegation(&images_delegated_stored, Uptane::Role::Delegated(delegation))) { local_version = Uptane::extractVersionUntrusted(images_delegated_stored); } else { local_version = -1; } - if (!images_repo.verifyTargets(images_delegated, delegation.name_)) { + if (!images_repo.verifyTargets(images_delegated, delegation)) { last_exception = images_repo.getLastException(); return false; } @@ -611,11 +611,11 @@ bool SotaUptaneClient::updateImagesMeta() { if (local_version > remote_version) { return false; } else if (local_version < remote_version) { - storage->storeDelegation(images_delegated, Uptane::Role::Delegated(delegation.name_)); + storage->storeDelegation(images_delegated, Uptane::Role::Delegated(delegation)); } - if (images_repo.targetsExpired(delegation.name_)) { - last_exception = Uptane::ExpiredMetadata("repo", delegation.name_); + if (images_repo.targetsExpired(delegation)) { + last_exception = Uptane::ExpiredMetadata("repo", delegation); return false; } } @@ -741,19 +741,19 @@ bool SotaUptaneClient::checkImagesMetaOffline() { } // Update Images delegated Targets Metadata - for (const Uptane::Delegation &delegation : images_repo.delegations("targets")) { + for (const std::string &delegation : images_repo.delegations("targets")) { std::string images_delegated; - if (!storage->loadDelegation(&images_delegated, Uptane::Role::Delegated(delegation.name_))) { + if (!storage->loadDelegation(&images_delegated, Uptane::Role::Delegated(delegation))) { return false; } - if (!images_repo.verifyTargets(images_delegated, delegation.name_)) { + if (!images_repo.verifyTargets(images_delegated, delegation)) { last_exception = images_repo.getLastException(); return false; } - if (images_repo.targetsExpired(delegation.name_)) { - last_exception = Uptane::ExpiredMetadata("repo", delegation.name_); + if (images_repo.targetsExpired(delegation)) { + last_exception = Uptane::ExpiredMetadata("repo", delegation); return false; } } diff --git a/src/libaktualizr/uptane/CMakeLists.txt b/src/libaktualizr/uptane/CMakeLists.txt index e2eda0340e..e125c9c1a6 100644 --- a/src/libaktualizr/uptane/CMakeLists.txt +++ b/src/libaktualizr/uptane/CMakeLists.txt @@ -1,7 +1,9 @@ -set(SOURCES fetcher.cc +set(SOURCES + fetcher.cc ipsecondarydiscovery.cc ipuptanesecondary.cc managedsecondary.cc + metawithkeys.cc partialverificationsecondary.cc role.cc root.cc @@ -12,7 +14,8 @@ set(SOURCES fetcher.cc imagesrepository.cc virtualsecondary.cc) -set(HEADERS exceptions.h +set(HEADERS + exceptions.h fetcher.h ipsecondarydiscovery.h ipuptanesecondary.h diff --git a/src/libaktualizr/uptane/directorrepository.cc b/src/libaktualizr/uptane/directorrepository.cc index 756a91e836..19da8ac2c4 100644 --- a/src/libaktualizr/uptane/directorrepository.cc +++ b/src/libaktualizr/uptane/directorrepository.cc @@ -9,7 +9,8 @@ void DirectorRepository::resetMeta() { bool DirectorRepository::verifyTargets(const std::string& targets_raw) { try { - targets = Targets(RepositoryType::Director(), Utils::parseJSON(targets_raw), root); // signature verification + // Verify the signature: + targets = Targets(RepositoryType::Director(), Utils::parseJSON(targets_raw), std::make_shared(root)); } catch (const Uptane::Exception& e) { LOG_ERROR << "Signature verification for director targets metadata failed"; last_exception = e; diff --git a/src/libaktualizr/uptane/imagesrepository.cc b/src/libaktualizr/uptane/imagesrepository.cc index 8017001d5d..1a0cbcedac 100644 --- a/src/libaktualizr/uptane/imagesrepository.cc +++ b/src/libaktualizr/uptane/imagesrepository.cc @@ -11,8 +11,9 @@ void ImagesRepository::resetMeta() { bool ImagesRepository::verifyTimestamp(const std::string& timestamp_raw) { try { + // Verify the signature: timestamp = - TimestampMeta(RepositoryType::Image(), Utils::parseJSON(timestamp_raw), root); // signature verification + TimestampMeta(RepositoryType::Image(), Utils::parseJSON(timestamp_raw), std::make_shared(root)); } catch (const Exception& e) { LOG_ERROR << "Signature verification for timestamp metadata failed"; last_exception = e; @@ -49,7 +50,8 @@ bool ImagesRepository::verifySnapshot(const std::string& snapshot_raw) { LOG_ERROR << "No hash found for shapshot.json"; return false; } - snapshot = Snapshot(RepositoryType::Image(), Utils::parseJSON(snapshot_raw), root); // signature verification + // Verify the signature: + snapshot = Snapshot(RepositoryType::Image(), Utils::parseJSON(snapshot_raw), std::make_shared(root)); if (snapshot.version() != timestamp.snapshot_version()) { return false; } @@ -90,7 +92,9 @@ bool ImagesRepository::verifyTargets(const std::string& targets_raw, const std:: LOG_ERROR << "No hash found for targets.json"; return false; } - targets[role_name] = Targets(RepositoryType::Image(), targets_json, root); // signature verification + // Verify the signature: + // TODO: delegated targets are checked with parent Targets, not Root. + targets[role_name] = Targets(RepositoryType::Image(), targets_json, std::make_shared(root)); // Only compare targets version in snapshot metadata for top-level // targets.json. Delegated target metadata versions are not tracked outside // of their own metadata. diff --git a/src/libaktualizr/uptane/imagesrepository.h b/src/libaktualizr/uptane/imagesrepository.h index c505a5b663..0b82107bf2 100644 --- a/src/libaktualizr/uptane/imagesrepository.h +++ b/src/libaktualizr/uptane/imagesrepository.h @@ -17,7 +17,7 @@ class ImagesRepository : public RepositoryCommon { bool verifyTargets(const std::string& targets_raw, const std::string& role_name); bool targetsExpired(const std::string& role_name) { return targets[role_name].isExpired(TimeStamp::Now()); } int64_t targetsSize() { return snapshot.targets_size(); } - std::vector delegations(const std::string& role_name) { return targets[role_name].delegations_; } + std::set delegations(const std::string& role_name) { return targets[role_name].delegated_role_names_; } std::unique_ptr getTarget(const Uptane::Target& director_target); bool verifyTimestamp(const std::string& timestamp_raw); diff --git a/src/libaktualizr/uptane/metawithkeys.cc b/src/libaktualizr/uptane/metawithkeys.cc new file mode 100644 index 0000000000..6b80bbf803 --- /dev/null +++ b/src/libaktualizr/uptane/metawithkeys.cc @@ -0,0 +1,120 @@ +#include "logging/logging.h" +#include "uptane/exceptions.h" +#include "uptane/tuf.h" + +using Uptane::MetaWithKeys; + +MetaWithKeys::MetaWithKeys(const Json::Value &json) : BaseMeta(json) {} +MetaWithKeys::MetaWithKeys(RepositoryType repo, const Json::Value &json, const std::shared_ptr &root) + : BaseMeta(repo, json, root) {} + +void Uptane::MetaWithKeys::ParseKeys(const RepositoryType repo, const Json::Value &keys) { + for (Json::ValueIterator it = keys.begin(); it != keys.end(); ++it) { + const std::string key_type = boost::algorithm::to_lower_copy((*it)["keytype"].asString()); + if (key_type != "rsa" && key_type != "ed25519") { + throw SecurityException(repo, "Unsupported key type: " + (*it)["keytype"].asString()); + } + const KeyId keyid = it.key().asString(); + PublicKey key(*it); + keys_[keyid] = key; + } +} + +void Uptane::MetaWithKeys::ParseRole(const RepositoryType repo, const Json::ValueIterator &it, const Role &role, + const std::string &meta_role) { + if (role == Role::InvalidRole()) { + LOG_WARNING << "Invalid role in " << meta_role << ".json"; + LOG_TRACE << "Role name:" << role.ToString(); + return; + } + // Threshold + const int64_t requiredThreshold = (*it)["threshold"].asInt64(); + if (requiredThreshold < kMinSignatures) { + // static_cast is to stop << taking a reference to kMinSignatures + // http://www.stroustrup.com/bs_faq2.html#in-class + // this occurs in Boost 1.62 (and possibly other versions) + LOG_DEBUG << "Failing with threshold for role " << role << " too small: " << requiredThreshold << " < " + << static_cast(kMinSignatures); + throw IllegalThreshold(repo, "The role " + role.ToString() + " had an illegal signature threshold."); + } + if (kMaxSignatures < requiredThreshold) { + // static_cast is to stop << taking a reference to kMaxSignatures + // http://www.stroustrup.com/bs_faq2.html#in-class + // this occurs in Boost 1.62 (and possibly other versions) + LOG_DEBUG << "Failing with threshold for role " << role << " too large: " << static_cast(kMaxSignatures) + << " < " << requiredThreshold; + throw IllegalThreshold(repo, meta_role + ".json contains a role that requires too many signatures"); + } + thresholds_for_role_[role] = requiredThreshold; + + // KeyIds + const Json::Value keyids = (*it)["keyids"]; + for (Json::ValueIterator itk = keyids.begin(); itk != keyids.end(); ++itk) { + keys_for_role_.insert(std::make_pair(role, (*itk).asString())); + } +} + +void Uptane::MetaWithKeys::UnpackSignedObject(const RepositoryType repo, const Json::Value &signed_object) { + const std::string repository = repo; + + const std::string canonical = Json::FastWriter().write(signed_object["signed"]); + const Uptane::Role role(signed_object["signed"]["_type"].asString()); + const Json::Value signatures = signed_object["signatures"]; + int valid_signatures = 0; + + std::set used_keyids; + for (Json::ValueIterator sig = signatures.begin(); sig != signatures.end(); ++sig) { + const std::string keyid = (*sig)["keyid"].asString(); + if (used_keyids.count(keyid) != 0) { + throw NonUniqueSignatures(repository, role.ToString()); + } + used_keyids.insert(keyid); + + std::string method((*sig)["method"].asString()); + std::transform(method.begin(), method.end(), method.begin(), ::tolower); + + if (method != "rsassa-pss" && method != "rsassa-pss-sha256" && method != "ed25519") { + throw SecurityException(repository, std::string("Unsupported sign method: ") + (*sig)["method"].asString()); + } + + if (keys_.count(keyid) == 0u) { + LOG_DEBUG << "Signed by unknown KeyId: " << keyid << ". Skipping."; + continue; + } + + if (keys_for_role_.count(std::make_pair(role, keyid)) == 0u) { + LOG_WARNING << "KeyId " << keyid << " is not valid to sign for this role (" << role.ToString() << ")."; + continue; + } + const std::string signature = (*sig)["sig"].asString(); + if (keys_[keyid].VerifySignature(signature, canonical)) { + valid_signatures++; + } else { + LOG_WARNING << "Signature was present but invalid: " << signature << " with KeyId: " << keyid; + } + } + const int64_t threshold = thresholds_for_role_[role]; + if (threshold < kMinSignatures || kMaxSignatures < threshold) { + throw IllegalThreshold(repository, "Invalid signature threshold"); + } + // One signature and it is bad: throw bad key ID. + // Multiple signatures but not enough good ones to pass threshold: throw unmet threshold. + if (signatures.size() == 1 && valid_signatures == 0) { + throw BadKeyId(repository); + } + if (valid_signatures < threshold) { + throw UnmetThreshold(repository, role.ToString()); + } + + // TODO: fix for delegated targets. + // Actually, just fix this for everything; it's broken. + // It just checks the an object is equal to itself. role should come from the + // roles of this object, not the signed_object. + const Uptane::Role actual_role(Uptane::Role(signed_object["signed"]["_type"].asString())); + if (role != actual_role) { + LOG_ERROR << "Object was signed for a different role"; + LOG_TRACE << " role:" << role; + LOG_TRACE << " actual_role:" << actual_role; + throw SecurityException(repository, "Object was signed for a different role"); + } +} diff --git a/src/libaktualizr/uptane/partialverificationsecondary.cc b/src/libaktualizr/uptane/partialverificationsecondary.cc index ab3c952c85..8b1b21a70c 100644 --- a/src/libaktualizr/uptane/partialverificationsecondary.cc +++ b/src/libaktualizr/uptane/partialverificationsecondary.cc @@ -37,7 +37,8 @@ bool PartialVerificationSecondary::putMetadata(const RawMetaPack &meta) { // TODO: check for expiration and version downgrade root_ = Uptane::Root(RepositoryType::Director(), Utils::parseJSON(meta.director_root), root_); - Uptane::Targets targets(RepositoryType::Director(), Utils::parseJSON(meta.director_targets), root_); + Uptane::Targets targets(RepositoryType::Director(), Utils::parseJSON(meta.director_targets), + std::make_shared(root_)); if (meta_targets_.version() > targets.version()) { detected_attack_ = "Rollback attack detected"; return true; diff --git a/src/libaktualizr/uptane/root.cc b/src/libaktualizr/uptane/root.cc index 43563f0b05..d52b6245ed 100644 --- a/src/libaktualizr/uptane/root.cc +++ b/src/libaktualizr/uptane/root.cc @@ -1,4 +1,3 @@ - #include "logging/logging.h" #include "uptane/exceptions.h" #include "uptane/tuf.h" @@ -7,76 +6,29 @@ using Uptane::Root; Root::Root(const RepositoryType repo, const Json::Value &json, Root &root) : Root(repo, json) { root.UnpackSignedObject(repo, json); - this->UnpackSignedObject(repo, json); + this->Root::UnpackSignedObject(repo, json); } -Root::Root(const RepositoryType repo, const Json::Value &json) : policy_(Policy::kCheck) { - if (!json.isObject() || !json.isMember("signed")) { - throw InvalidMetadata("", "", "invalid metadata json"); - } - - version_ = json["signed"]["version"].asInt(); - - expiry_ = TimeStamp(json["signed"]["expires"].asString()); - original_object_ = json; - - if (!json.isObject() || !json["signed"].isMember("keys") || !json["signed"].isMember("roles")) { - throw InvalidMetadata(repo, "root", "missing keys/roles field"); +Root::Root(const RepositoryType repo, const Json::Value &json) : MetaWithKeys(json), policy_(Policy::kCheck) { + if (!json["signed"].isMember("keys")) { + throw InvalidMetadata(repo, "root", "missing keys field"); + } else if (!json["signed"].isMember("roles")) { + throw InvalidMetadata(repo, "root", "missing roles field"); } const Json::Value keys = json["signed"]["keys"]; - for (Json::ValueIterator it = keys.begin(); it != keys.end(); ++it) { - const std::string key_type = boost::algorithm::to_lower_copy((*it)["keytype"].asString()); - if (key_type != "rsa" && key_type != "ed25519") { - throw SecurityException(repo, "Unsupported key type: " + (*it)["keytype"].asString()); - } - const KeyId keyid = it.key().asString(); - PublicKey key(*it); - keys_[keyid] = key; - } + ParseKeys(repo, keys); const Json::Value roles = json["signed"]["roles"]; for (Json::ValueIterator it = roles.begin(); it != roles.end(); it++) { - const std::string role_name = it.key().asString(); - const Role role = Role(role_name); - if (role == Role::InvalidRole()) { - LOG_WARNING << "Invalid role in root.json"; - LOG_TRACE << "Role name:" << role_name; - LOG_TRACE << "root.json is:" << json; - continue; - } - // Threshold - const int64_t requiredThreshold = (*it)["threshold"].asInt64(); - if (requiredThreshold < kMinSignatures) { - // static_cast is to stop << taking a reference to kMinSignatures - // http://www.stroustrup.com/bs_faq2.html#in-class - // this occurs in Boost 1.62 (and possibly other versions) - LOG_DEBUG << "Failing with threshold for role " << role << " too small: " << requiredThreshold << " < " - << static_cast(kMinSignatures); - throw IllegalThreshold(repo, "The role " + role_name + " had an illegal signature threshold."); - } - if (kMaxSignatures < requiredThreshold) { - // static_cast is to stop << taking a reference to kMaxSignatures - // http://www.stroustrup.com/bs_faq2.html#in-class - // this occurs in Boost 1.62 (and possibly other versions) - LOG_DEBUG << "Failing with threshold for role " << role << " too large: " << static_cast(kMaxSignatures) - << " < " << requiredThreshold; - throw IllegalThreshold(repo, "root.json contains a role that requires too many signatures"); - } - thresholds_for_role_[role] = requiredThreshold; - - // KeyIds - const Json::Value keyids = (*it)["keyids"]; - for (Json::ValueIterator itk = keyids.begin(); itk != keyids.end(); ++itk) { - keys_for_role_.insert(std::make_pair(role, (*itk).asString())); - } + const Role role = Role(it.key().asString()); + ParseRole(repo, it, role, "root"); } } void Uptane::Root::UnpackSignedObject(const RepositoryType repo, const Json::Value &signed_object) { const std::string repository = repo; - const Uptane::Role role(signed_object["signed"]["_type"].asString()); if (policy_ == Policy::kAcceptAll) { return; } @@ -85,59 +37,5 @@ void Uptane::Root::UnpackSignedObject(const RepositoryType repo, const Json::Val } assert(policy_ == Policy::kCheck); - const std::string canonical = Json::FastWriter().write(signed_object["signed"]); - const Json::Value signatures = signed_object["signatures"]; - int valid_signatures = 0; - - std::set used_keyids; - for (Json::ValueIterator sig = signatures.begin(); sig != signatures.end(); ++sig) { - const std::string keyid = (*sig)["keyid"].asString(); - if (used_keyids.count(keyid) != 0) { - throw NonUniqueSignatures(repository, role.ToString()); - } - used_keyids.insert(keyid); - - std::string method((*sig)["method"].asString()); - std::transform(method.begin(), method.end(), method.begin(), ::tolower); - - if (method != "rsassa-pss" && method != "rsassa-pss-sha256" && method != "ed25519") { - throw SecurityException(repository, std::string("Unsupported sign method: ") + (*sig)["method"].asString()); - } - - if (keys_.count(keyid) == 0u) { - LOG_DEBUG << "Signed by unknown KeyId: " << keyid << ". Skipping."; - continue; - } - - if (keys_for_role_.count(std::make_pair(role, keyid)) == 0u) { - LOG_WARNING << "KeyId " << keyid << " is not valid to sign for this role (" << role.ToString() << ")."; - continue; - } - const std::string signature = (*sig)["sig"].asString(); - if (keys_[keyid].VerifySignature(signature, canonical)) { - valid_signatures++; - } else { - LOG_WARNING << "Signature was present but invalid: " << signature << " with KeyId: " << keyid; - } - } - const int64_t threshold = thresholds_for_role_[role]; - if (threshold < kMinSignatures || kMaxSignatures < threshold) { - throw IllegalThreshold(repository, "Invalid signature threshold"); - } - // One signature and it is bad: throw bad key ID. - // Multiple signatures but not enough good ones to pass threshold: throw unmet threshold. - if (signatures.size() == 1 && valid_signatures == 0) { - throw BadKeyId(repository); - } - if (valid_signatures < threshold) { - throw UnmetThreshold(repository, role.ToString()); - } - - const Uptane::Role actual_role(Uptane::Role(signed_object["signed"]["_type"].asString())); - if (role != actual_role) { - LOG_ERROR << "Object was signed for a different role"; - LOG_TRACE << " role:" << role; - LOG_TRACE << " actual_role:" << actual_role; - throw SecurityException(repository, "Object was signed for a different role"); - } + Uptane::MetaWithKeys::UnpackSignedObject(repo, signed_object); } diff --git a/src/libaktualizr/uptane/tuf.cc b/src/libaktualizr/uptane/tuf.cc index 3359b735e0..4acec46d1d 100644 --- a/src/libaktualizr/uptane/tuf.cc +++ b/src/libaktualizr/uptane/tuf.cc @@ -236,7 +236,7 @@ std::ostream &Uptane::operator<<(std::ostream &os, const Target &t) { void Uptane::BaseMeta::init(const Json::Value &json) { if (!json.isObject() || !json.isMember("signed")) { - LOG_ERROR << "BM FAILURE"; + LOG_ERROR << "Failure during base metadata initialization from json"; throw Uptane::InvalidMetadata("", "", "invalid metadata json"); } @@ -250,12 +250,12 @@ void Uptane::BaseMeta::init(const Json::Value &json) { } Uptane::BaseMeta::BaseMeta(const Json::Value &json) { init(json); } -Uptane::BaseMeta::BaseMeta(RepositoryType repo, const Json::Value &json, Root &root) { +Uptane::BaseMeta::BaseMeta(RepositoryType repo, const Json::Value &json, const std::shared_ptr &root) { if (!json.isObject() || !json.isMember("signed")) { throw Uptane::InvalidMetadata("", "", "invalid metadata json"); } - root.UnpackSignedObject(repo, json); + root->UnpackSignedObject(repo, json); init(json); } @@ -273,31 +273,25 @@ void Uptane::Targets::init(const Json::Value &json) { if (json["signed"]["delegations"].isObject()) { const Json::Value key_list = json["signed"]["delegations"]["keys"]; - for (Json::ValueIterator k_it = key_list.begin(); k_it != key_list.end(); k_it++) { - const std::string key_type = boost::algorithm::to_lower_copy((*k_it)["keytype"].asString()); - if (key_type != "rsa" && key_type != "ed25519") { - throw SecurityException("image", "Unsupported key type: " + (*k_it)["keytype"].asString()); - } - const KeyId keyid = k_it.key().asString(); - PublicKey key(*k_it); - keys_[keyid] = key; - } + ParseKeys(Uptane::RepositoryType::Image(), key_list); const Json::Value role_list = json["signed"]["delegations"]["roles"]; - for (Json::ValueIterator r_it = role_list.begin(); r_it != role_list.end(); r_it++) { - Delegation delegate; - delegate.name_ = (*r_it)["name"].asString(); - const Json::Value keyid_list = (*r_it)["keyids"]; - for (Json::ValueIterator kid_it = keyid_list.begin(); kid_it != keyid_list.end(); kid_it++) { - delegate.key_ids_.emplace_back(kid_it.key().asString()); - } - const Json::Value paths_list = (*r_it)["paths"]; + for (Json::ValueIterator it = role_list.begin(); it != role_list.end(); it++) { + const std::string role_name = (*it)["name"].asString(); + const Role role = Role(role_name); + delegated_role_names_.insert(role_name); + // TODO: use actual parent object role name, don't just assume "targets" + // (for printing errors only though) + ParseRole(Uptane::RepositoryType::Image(), it, role, "targets"); + + const Json::Value paths_list = (*it)["paths"]; + std::vector paths; for (Json::ValueIterator p_it = paths_list.begin(); p_it != paths_list.end(); p_it++) { - delegate.paths_.emplace_back(p_it.key().asString()); + paths.emplace_back((*p_it).asString()); } - delegate.terminating_ = (*r_it)["terminating"].asBool(); - delegate.threshold_ = (*r_it)["threshold"].asInt64(); - delegations_.push_back(delegate); + paths_for_role_[role] = paths; + + terminating_role_[role] = (*it)["terminating"].asBool(); } } @@ -308,9 +302,10 @@ void Uptane::Targets::init(const Json::Value &json) { } } -Uptane::Targets::Targets(const Json::Value &json) : BaseMeta(json) { init(json); } +Uptane::Targets::Targets(const Json::Value &json) : MetaWithKeys(json) { init(json); } -Uptane::Targets::Targets(RepositoryType repo, const Json::Value &json, Root &root) : BaseMeta(repo, json, root) { +Uptane::Targets::Targets(RepositoryType repo, const Json::Value &json, const std::shared_ptr &root) + : MetaWithKeys(repo, json, root) { init(json); } @@ -333,7 +328,8 @@ void Uptane::TimestampMeta::init(const Json::Value &json) { Uptane::TimestampMeta::TimestampMeta(const Json::Value &json) : BaseMeta(json) { init(json); } -Uptane::TimestampMeta::TimestampMeta(RepositoryType repo, const Json::Value &json, Root &root) +Uptane::TimestampMeta::TimestampMeta(RepositoryType repo, const Json::Value &json, + const std::shared_ptr &root) : BaseMeta(repo, json, root) { init(json); } @@ -364,7 +360,8 @@ void Uptane::Snapshot::init(const Json::Value &json) { Uptane::Snapshot::Snapshot(const Json::Value &json) : BaseMeta(json) { init(json); } -Uptane::Snapshot::Snapshot(RepositoryType repo, const Json::Value &json, Root &root) : BaseMeta(repo, json, root) { +Uptane::Snapshot::Snapshot(RepositoryType repo, const Json::Value &json, const std::shared_ptr &root) + : BaseMeta(repo, json, root) { init(json); } @@ -375,7 +372,8 @@ bool MetaPack::isConsistent() const { Uptane::Root original_root(director_root); Uptane::Root new_root(RepositoryType::Director(), director_root.original(), new_root); if (director_targets.original() != Json::nullValue) { - Uptane::Targets(RepositoryType::Director(), director_targets.original(), original_root); + Uptane::Targets(RepositoryType::Director(), director_targets.original(), + std::make_shared(original_root)); } } } catch (const std::logic_error &exc) { diff --git a/src/libaktualizr/uptane/tuf.h b/src/libaktualizr/uptane/tuf.h index 8454587d81..b067de35ce 100644 --- a/src/libaktualizr/uptane/tuf.h +++ b/src/libaktualizr/uptane/tuf.h @@ -293,12 +293,12 @@ class Target { std::ostream &operator<<(std::ostream &os, const Target &t); /* Metadata objects */ -class Root; +class MetaWithKeys; class BaseMeta { public: BaseMeta() = default; explicit BaseMeta(const Json::Value &json); - BaseMeta(RepositoryType repo, const Json::Value &json, Root &root); + BaseMeta(RepositoryType repo, const Json::Value &json, const std::shared_ptr &root); int version() const { return version_; } TimeStamp expiry() const { return expiry_; } bool isExpired(const TimeStamp &now) const { return expiry_.IsExpiredAt(now); } @@ -315,10 +315,60 @@ class BaseMeta { void init(const Json::Value &json); }; -// Implemented in uptane/root.cc -class Root : public BaseMeta { +class MetaWithKeys : public BaseMeta { public: enum class Policy { kRejectAll, kAcceptAll, kCheck }; + /** + * An empty metadata object that could contain keys. + */ + MetaWithKeys() { version_ = 0; } + /** + * A 'real' metadata object that can contain keys (root or targets with + * delegations) and that implements TUF signature validation. + * @param json - The contents of the 'signed' portion + */ + MetaWithKeys(const Json::Value &json); + MetaWithKeys(RepositoryType repo, const Json::Value &json, const std::shared_ptr &root); + + virtual ~MetaWithKeys() = default; + + void ParseKeys(RepositoryType repo, const Json::Value &keys); + // role is the name of a role described in this object's metadata. + // meta_role is the name of this object's role. + void ParseRole(RepositoryType repo, const Json::ValueIterator &it, const Role &role, const std::string &meta_role); + + /** + * Take a JSON blob that contains a signatures/signed component that is supposedly for a given role, and check that is + * suitably signed. + * If it is, it returns the contents of the 'signed' part. + * + * It performs the following checks: + * * "_type" matches the given role + * * "expires" is in the past (vs 'now') + * * The blob has valid signatures from enough keys to cross the threshold for this role + * @param repo - Repository type (only used to improve the error messages) + * @param signed_object + * @return + */ + virtual void UnpackSignedObject(RepositoryType repo, const Json::Value &signed_object); + + bool operator==(const MetaWithKeys &rhs) const { + return version_ == rhs.version_ && expiry_ == rhs.expiry_ && keys_ == rhs.keys_ && + keys_for_role_ == rhs.keys_for_role_ && thresholds_for_role_ == rhs.thresholds_for_role_; + } + + protected: + static const int64_t kMinSignatures = 1; + static const int64_t kMaxSignatures = 1000; + + std::map keys_; + std::set > keys_for_role_; + std::map thresholds_for_role_; +}; + +// Implemented in uptane/root.cc +class Root : public MetaWithKeys { + public: /** * An empty Root, that either accepts or rejects everything */ @@ -331,6 +381,8 @@ class Root : public BaseMeta { Root(RepositoryType repo, const Json::Value &json); Root(RepositoryType repo, const Json::Value &json, Root &root); + ~Root() override = default; + /** * Take a JSON blob that contains a signatures/signed component that is supposedly for a given role, and check that is * suitably signed. @@ -344,7 +396,8 @@ class Root : public BaseMeta { * @param signed_object * @return */ - void UnpackSignedObject(RepositoryType repo, const Json::Value &signed_object); + void UnpackSignedObject(RepositoryType repo, const Json::Value &signed_object) override; + bool operator==(const Root &rhs) const { return version_ == rhs.version_ && expiry_ == rhs.expiry_ && keys_ == rhs.keys_ && keys_for_role_ == rhs.keys_for_role_ && thresholds_for_role_ == rhs.thresholds_for_role_ && @@ -352,28 +405,16 @@ class Root : public BaseMeta { } private: - static const int64_t kMinSignatures = 1; - static const int64_t kMaxSignatures = 1000; - Policy policy_; - std::map keys_; - std::set > keys_for_role_; - std::map thresholds_for_role_; -}; - -struct Delegation { - std::string name_; - std::vector key_ids_; - std::vector paths_; - bool terminating_{true}; - int64_t threshold_{0}; }; -class Targets : public BaseMeta { +// Also used for delegated targets. +class Targets : public MetaWithKeys { public: explicit Targets(const Json::Value &json); - Targets(RepositoryType repo, const Json::Value &json, Root &root); + Targets(RepositoryType repo, const Json::Value &json, const std::shared_ptr &root); Targets() = default; + ~Targets() override = default; bool operator==(const Targets &rhs) const { return version_ == rhs.version() && expiry_ == rhs.expiry() && targets == rhs.targets; @@ -381,19 +422,20 @@ class Targets : public BaseMeta { const std::string &correlation_id() const { return correlation_id_; } std::vector targets; - std::vector delegations_; + std::set delegated_role_names_; private: void init(const Json::Value &json); - std::map keys_; // for delegated roles - std::string correlation_id_; // custom non-tuf + std::map > paths_for_role_; + std::map terminating_role_; + std::string correlation_id_; // custom non-tuf }; class TimestampMeta : public BaseMeta { public: explicit TimestampMeta(const Json::Value &json); - TimestampMeta(RepositoryType repo, const Json::Value &json, Root &root); + TimestampMeta(RepositoryType repo, const Json::Value &json, const std::shared_ptr &root); TimestampMeta() = default; std::vector snapshot_hashes() const { return snapshot_hashes_; }; int64_t snapshot_size() const { return snapshot_size_; }; @@ -410,7 +452,7 @@ class TimestampMeta : public BaseMeta { class Snapshot : public BaseMeta { public: explicit Snapshot(const Json::Value &json); - Snapshot(RepositoryType repo, const Json::Value &json, Root &root); + Snapshot(RepositoryType repo, const Json::Value &json, const std::shared_ptr &root); Snapshot() = default; std::vector targets_hashes() const { return targets_hashes_; }; int64_t targets_size() const { return targets_size_; }; From 0092f6cd02943fe3e6db943860d19ac5bbe57c43 Mon Sep 17 00:00:00 2001 From: Patrick Vacek Date: Thu, 24 Jan 2019 16:51:39 +0100 Subject: [PATCH 07/19] Refactoring and removal of a redundant consistency check. Signed-off-by: Patrick Vacek --- src/libaktualizr/uptane/metawithkeys.cc | 16 ++-------------- src/libaktualizr/uptane/tuf.cc | 9 ++++----- src/libaktualizr/uptane/tuf.h | 6 ++++-- 3 files changed, 10 insertions(+), 21 deletions(-) diff --git a/src/libaktualizr/uptane/metawithkeys.cc b/src/libaktualizr/uptane/metawithkeys.cc index 6b80bbf803..bee89bb4fe 100644 --- a/src/libaktualizr/uptane/metawithkeys.cc +++ b/src/libaktualizr/uptane/metawithkeys.cc @@ -5,8 +5,8 @@ using Uptane::MetaWithKeys; MetaWithKeys::MetaWithKeys(const Json::Value &json) : BaseMeta(json) {} -MetaWithKeys::MetaWithKeys(RepositoryType repo, const Json::Value &json, const std::shared_ptr &root) - : BaseMeta(repo, json, root) {} +MetaWithKeys::MetaWithKeys(RepositoryType repo, const Json::Value &json, const std::shared_ptr &signer) + : BaseMeta(repo, json, signer) {} void Uptane::MetaWithKeys::ParseKeys(const RepositoryType repo, const Json::Value &keys) { for (Json::ValueIterator it = keys.begin(); it != keys.end(); ++it) { @@ -105,16 +105,4 @@ void Uptane::MetaWithKeys::UnpackSignedObject(const RepositoryType repo, const J if (valid_signatures < threshold) { throw UnmetThreshold(repository, role.ToString()); } - - // TODO: fix for delegated targets. - // Actually, just fix this for everything; it's broken. - // It just checks the an object is equal to itself. role should come from the - // roles of this object, not the signed_object. - const Uptane::Role actual_role(Uptane::Role(signed_object["signed"]["_type"].asString())); - if (role != actual_role) { - LOG_ERROR << "Object was signed for a different role"; - LOG_TRACE << " role:" << role; - LOG_TRACE << " actual_role:" << actual_role; - throw SecurityException(repository, "Object was signed for a different role"); - } } diff --git a/src/libaktualizr/uptane/tuf.cc b/src/libaktualizr/uptane/tuf.cc index 4acec46d1d..baefc67ceb 100644 --- a/src/libaktualizr/uptane/tuf.cc +++ b/src/libaktualizr/uptane/tuf.cc @@ -280,9 +280,7 @@ void Uptane::Targets::init(const Json::Value &json) { const std::string role_name = (*it)["name"].asString(); const Role role = Role(role_name); delegated_role_names_.insert(role_name); - // TODO: use actual parent object role name, don't just assume "targets" - // (for printing errors only though) - ParseRole(Uptane::RepositoryType::Image(), it, role, "targets"); + ParseRole(Uptane::RepositoryType::Image(), it, role, name_); const Json::Value paths_list = (*it)["paths"]; std::vector paths; @@ -304,8 +302,9 @@ void Uptane::Targets::init(const Json::Value &json) { Uptane::Targets::Targets(const Json::Value &json) : MetaWithKeys(json) { init(json); } -Uptane::Targets::Targets(RepositoryType repo, const Json::Value &json, const std::shared_ptr &root) - : MetaWithKeys(repo, json, root) { +Uptane::Targets::Targets(RepositoryType repo, const Json::Value &json, const std::shared_ptr &signer, + std::string name) + : MetaWithKeys(repo, json, signer), name_(std::move(name)) { init(json); } diff --git a/src/libaktualizr/uptane/tuf.h b/src/libaktualizr/uptane/tuf.h index b067de35ce..7cf3ad74e3 100644 --- a/src/libaktualizr/uptane/tuf.h +++ b/src/libaktualizr/uptane/tuf.h @@ -328,7 +328,7 @@ class MetaWithKeys : public BaseMeta { * @param json - The contents of the 'signed' portion */ MetaWithKeys(const Json::Value &json); - MetaWithKeys(RepositoryType repo, const Json::Value &json, const std::shared_ptr &root); + MetaWithKeys(RepositoryType repo, const Json::Value &json, const std::shared_ptr &signer); virtual ~MetaWithKeys() = default; @@ -412,7 +412,8 @@ class Root : public MetaWithKeys { class Targets : public MetaWithKeys { public: explicit Targets(const Json::Value &json); - Targets(RepositoryType repo, const Json::Value &json, const std::shared_ptr &root); + Targets(RepositoryType repo, const Json::Value &json, const std::shared_ptr &signer, + std::string name = "targets"); Targets() = default; ~Targets() override = default; @@ -427,6 +428,7 @@ class Targets : public MetaWithKeys { private: void init(const Json::Value &json); + std::string name_; std::map > paths_for_role_; std::map terminating_role_; std::string correlation_id_; // custom non-tuf From b67bc57489947c7c60eeb5f39d583a1ad003dbb7 Mon Sep 17 00:00:00 2001 From: Patrick Vacek Date: Thu, 24 Jan 2019 17:35:07 +0100 Subject: [PATCH 08/19] Proper (non-nested) delegation searching. Signed-off-by: Patrick Vacek --- src/libaktualizr/uptane/imagesrepository.cc | 45 ++++++++++++++++++--- src/libaktualizr/uptane/tuf.h | 4 +- 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/src/libaktualizr/uptane/imagesrepository.cc b/src/libaktualizr/uptane/imagesrepository.cc index 1a0cbcedac..3879b7b7de 100644 --- a/src/libaktualizr/uptane/imagesrepository.cc +++ b/src/libaktualizr/uptane/imagesrepository.cc @@ -1,5 +1,7 @@ #include "imagesrepository.h" +#include + namespace Uptane { void ImagesRepository::resetMeta() { @@ -92,9 +94,18 @@ bool ImagesRepository::verifyTargets(const std::string& targets_raw, const std:: LOG_ERROR << "No hash found for targets.json"; return false; } + // Verify the signature: - // TODO: delegated targets are checked with parent Targets, not Root. - targets[role_name] = Targets(RepositoryType::Image(), targets_json, std::make_shared(root)); + std::shared_ptr signer; + if (role_name == "targets") { + signer = std::make_shared(root); + } else { + // TODO: support nested delegations here. This currently assumes that all + // delegated targets are signed by the top-level targets. + signer = std::make_shared(targets["targets"]); + } + targets[role_name] = Targets(RepositoryType::Image(), targets_json, signer); + // Only compare targets version in snapshot metadata for top-level // targets.json. Delegated target metadata versions are not tracked outside // of their own metadata. @@ -109,11 +120,33 @@ bool ImagesRepository::verifyTargets(const std::string& targets_raw, const std:: return true; } -// TODO: Delegation support. std::unique_ptr ImagesRepository::getTarget(const Uptane::Target& director_target) { - auto it = std::find(targets["targets"].targets.cbegin(), targets["targets"].targets.cend(), director_target); - if (it == targets["targets"].targets.cend()) { - // TODO: check delegation paths, etc. + // Search for the target in the top-level targets metadata. + Uptane::Targets toplevel = targets["targets"]; + auto it = std::find(toplevel.targets.cbegin(), toplevel.targets.cend(), director_target); + if (it == toplevel.targets.cend()) { + // Check if the target matches any of the delegation paths. + for (const auto& delegate_name : toplevel.delegated_role_names_) { + Role delegate_role = Role::Delegated(delegate_name); + std::vector patterns = toplevel.paths_for_role_[delegate_role]; + for (const auto& pattern : patterns) { + if (fnmatch(pattern.c_str(), director_target.filename().c_str(), 0) == 0) { + // Match! Check delegation. + Uptane::Targets delegate = targets[delegate_name]; + auto d_it = std::find(delegate.targets.cbegin(), delegate.targets.cend(), director_target); + if (d_it == delegate.targets.cend()) { + // TODO: recurse here if the delegation is non-terminating. For now, + // assume that all delegations are terminating. + if (!toplevel.terminating_role_[delegate_role]) { + LOG_ERROR << "Nested delegations are not currently supported."; + } + return std::unique_ptr(nullptr); + } else { + return std_::make_unique(*d_it); + } + } + } + } return std::unique_ptr(nullptr); } else { return std_::make_unique(*it); diff --git a/src/libaktualizr/uptane/tuf.h b/src/libaktualizr/uptane/tuf.h index 7cf3ad74e3..f2f9efaffa 100644 --- a/src/libaktualizr/uptane/tuf.h +++ b/src/libaktualizr/uptane/tuf.h @@ -424,13 +424,13 @@ class Targets : public MetaWithKeys { std::vector targets; std::set delegated_role_names_; + std::map > paths_for_role_; + std::map terminating_role_; private: void init(const Json::Value &json); std::string name_; - std::map > paths_for_role_; - std::map terminating_role_; std::string correlation_id_; // custom non-tuf }; From e9290c8bc260c38cadf36e204593b13f65629ad1 Mon Sep 17 00:00:00 2001 From: Patrick Vacek Date: Fri, 25 Jan 2019 18:08:09 +0100 Subject: [PATCH 09/19] Basic delegation test. Copied shamelessly from the full_no_correlation_id test. Signed-off-by: Patrick Vacek --- src/libaktualizr/primary/aktualizr.h | 1 + src/libaktualizr/primary/aktualizr_test.cc | 37 +++------ src/libaktualizr/uptane/CMakeLists.txt | 5 ++ .../uptane/uptane_delegation_test.cc | 78 +++++++++++++++++++ tests/uptane_repo_generation/CMakeLists.txt | 5 ++ .../delegation_basic.sh | 37 +++++++++ tests/uptane_test_common.h | 13 ++++ 7 files changed, 151 insertions(+), 25 deletions(-) create mode 100644 src/libaktualizr/uptane/uptane_delegation_test.cc create mode 100755 tests/uptane_repo_generation/delegation_basic.sh diff --git a/src/libaktualizr/primary/aktualizr.h b/src/libaktualizr/primary/aktualizr.h index 7bd6955b5f..28f4fbb253 100644 --- a/src/libaktualizr/primary/aktualizr.h +++ b/src/libaktualizr/primary/aktualizr.h @@ -182,6 +182,7 @@ class Aktualizr { FRIEND_TEST(Aktualizr, UpdateCheckCompleteError); FRIEND_TEST(Aktualizr, PauseResumeEvents); FRIEND_TEST(Aktualizr, AddSecondary); + FRIEND_TEST(Delegation, Basic); // This constructor is only being used in tests Aktualizr(Config& config, std::shared_ptr storage_in, std::shared_ptr http_in); diff --git a/src/libaktualizr/primary/aktualizr_test.cc b/src/libaktualizr/primary/aktualizr_test.cc index e9ca1b3785..3ffd46c539 100644 --- a/src/libaktualizr/primary/aktualizr_test.cc +++ b/src/libaktualizr/primary/aktualizr_test.cc @@ -19,19 +19,6 @@ boost::filesystem::path uptane_repos_dir; -Config makeTestConfig(const TemporaryDirectory& temp_dir, const std::string& url) { - Config conf("tests/config/basic.toml"); - conf.uptane.director_server = url + "/director"; - conf.uptane.repo_server = url + "/repo"; - conf.provision.server = url; - conf.provision.primary_ecu_serial = "CA:FE:A6:D2:84:9D"; - conf.provision.primary_ecu_hardware_id = "primary_hw"; - conf.storage.path = temp_dir.Path(); - conf.tls.server = url; - UptaneTestCommon::addDefaultSecondary(conf, temp_dir, "secondary_ecu_serial", "secondary_hw"); - return conf; -} - void verifyNothingInstalled(const Json::Value& manifest) { // Verify nothing has installed for the primary. EXPECT_EQ( @@ -96,7 +83,7 @@ TEST(Aktualizr, FullNoUpdates) { future_FullNoUpdates = promise_FullNoUpdates.get_future(); TemporaryDirectory temp_dir; auto http = std::make_shared(temp_dir.Path(), "noupdates"); - Config conf = makeTestConfig(temp_dir, http->tls_server); + Config conf = UptaneTestCommon::makeTestConfig(temp_dir, http->tls_server); auto storage = INvStorage::newStorage(conf.storage); Aktualizr aktualizr(conf, storage, http); @@ -122,7 +109,7 @@ TEST(Aktualizr, FullNoUpdates) { TEST(Aktualizr, AddSecondary) { TemporaryDirectory temp_dir; auto http = std::make_shared(temp_dir.Path(), "noupdates"); - Config conf = makeTestConfig(temp_dir, http->tls_server); + Config conf = UptaneTestCommon::makeTestConfig(temp_dir, http->tls_server); auto storage = INvStorage::newStorage(conf.storage); Aktualizr aktualizr(conf, storage, http); @@ -296,7 +283,7 @@ TEST(Aktualizr, FullWithUpdates) { future_FullWithUpdates = promise_FullWithUpdates.get_future(); TemporaryDirectory temp_dir; auto http = std::make_shared(temp_dir.Path()); - Config conf = makeTestConfig(temp_dir, http->tls_server); + Config conf = UptaneTestCommon::makeTestConfig(temp_dir, http->tls_server); auto storage = INvStorage::newStorage(conf.storage); Aktualizr aktualizr(conf, storage, http); @@ -366,7 +353,7 @@ class HttpFakePutCounter : public HttpFake { TEST(Aktualizr, FullWithUpdatesNeedReboot) { TemporaryDirectory temp_dir; auto http = std::make_shared(temp_dir.Path()); - Config conf = makeTestConfig(temp_dir, http->tls_server); + Config conf = UptaneTestCommon::makeTestConfig(temp_dir, http->tls_server); conf.pacman.fake_need_reboot = true; conf.bootloader.reboot_sentinel_dir = temp_dir.Path(); @@ -578,7 +565,7 @@ TEST(Aktualizr, CheckNoUpdates) { future_CheckNoUpdates = promise_CheckNoUpdates.get_future(); TemporaryDirectory temp_dir; auto http = std::make_shared(temp_dir.Path(), "noupdates"); - Config conf = makeTestConfig(temp_dir, http->tls_server); + Config conf = UptaneTestCommon::makeTestConfig(temp_dir, http->tls_server); auto storage = INvStorage::newStorage(conf.storage); Aktualizr aktualizr(conf, storage, http); @@ -672,7 +659,7 @@ TEST(Aktualizr, DownloadWithUpdates) { future_DownloadWithUpdates = promise_DownloadWithUpdates.get_future(); TemporaryDirectory temp_dir; auto http = std::make_shared(temp_dir.Path(), "hasupdates"); - Config conf = makeTestConfig(temp_dir, http->tls_server); + Config conf = UptaneTestCommon::makeTestConfig(temp_dir, http->tls_server); auto storage = INvStorage::newStorage(conf.storage); Aktualizr aktualizr(conf, storage, http); @@ -804,7 +791,7 @@ TEST(Aktualizr, InstallWithUpdates) { future_InstallWithUpdates = promise_InstallWithUpdates.get_future(); TemporaryDirectory temp_dir; auto http = std::make_shared(temp_dir.Path(), "hasupdates"); - Config conf = makeTestConfig(temp_dir, http->tls_server); + Config conf = UptaneTestCommon::makeTestConfig(temp_dir, http->tls_server); auto storage = INvStorage::newStorage(conf.storage); Aktualizr aktualizr(conf, storage, http); @@ -907,7 +894,7 @@ void CampaignCheck_events(const std::shared_ptr& event) { TEST(Aktualizr, CampaignCheckAndAccept) { TemporaryDirectory temp_dir; auto http = std::make_shared(temp_dir.Path()); - Config conf = makeTestConfig(temp_dir, http->tls_server); + Config conf = UptaneTestCommon::makeTestConfig(temp_dir, http->tls_server); { auto storage = INvStorage::newStorage(conf.storage); @@ -954,7 +941,7 @@ TEST(Aktualizr, FullNoCorrelationId) { // scope `Aktualizr` object, so that the ReportQueue flushes its events before // we count them at the end { - Config conf = makeTestConfig(temp_dir, http->tls_server); + Config conf = UptaneTestCommon::makeTestConfig(temp_dir, http->tls_server); auto storage = INvStorage::newStorage(conf.storage); Aktualizr aktualizr(conf, storage, http); @@ -1008,7 +995,7 @@ TEST(Aktualizr, APICheck) { TemporaryDirectory temp_dir; auto http = std::make_shared(temp_dir.Path(), "hasupdates"); - Config conf = makeTestConfig(temp_dir, http->tls_server); + Config conf = UptaneTestCommon::makeTestConfig(temp_dir, http->tls_server); auto storage = INvStorage::newStorage(conf.storage); @@ -1058,7 +1045,7 @@ TEST(Aktualizr, UpdateCheckCompleteError) { TemporaryDirectory temp_dir; auto http = std::make_shared(temp_dir.Path()); - Config conf = makeTestConfig(temp_dir, "http://updatefail"); + Config conf = UptaneTestCommon::makeTestConfig(temp_dir, "http://updatefail"); auto storage = INvStorage::newStorage(conf.storage); CountUpdateCheckEvents counter; @@ -1075,7 +1062,7 @@ TEST(Aktualizr, UpdateCheckCompleteError) { TEST(Aktualizr, PauseResumeEvents) { TemporaryDirectory temp_dir; auto http = std::make_shared(temp_dir.Path(), "noupdates"); - Config conf = makeTestConfig(temp_dir, http->tls_server); + Config conf = UptaneTestCommon::makeTestConfig(temp_dir, http->tls_server); auto storage = INvStorage::newStorage(conf.storage); Aktualizr aktualizr(conf, storage, http); diff --git a/src/libaktualizr/uptane/CMakeLists.txt b/src/libaktualizr/uptane/CMakeLists.txt index e125c9c1a6..1b21a1c4d6 100644 --- a/src/libaktualizr/uptane/CMakeLists.txt +++ b/src/libaktualizr/uptane/CMakeLists.txt @@ -63,6 +63,11 @@ endif(BUILD_OSTREE AND SOTA_PACKED_CREDENTIALS) add_aktualizr_test(NAME uptane SOURCES uptane_test.cc PROJECT_WORKING_DIRECTORY) set_tests_properties(test_uptane PROPERTIES LABELS "crypto") +add_aktualizr_test(NAME uptane_delegation SOURCES uptane_delegation_test.cc PROJECT_WORKING_DIRECTORY + ARGS ${PROJECT_BINARY_DIR}/uptane_repos) +add_dependencies(t_uptane_delegation uptane_repo_delegation_basic) +set_tests_properties(test_uptane_delegation PROPERTIES LABELS "crypto") + add_aktualizr_test(NAME uptane_implicit SOURCES uptane_implicit_test.cc PROJECT_WORKING_DIRECTORY) set_tests_properties(test_uptane_implicit PROPERTIES LABELS "crypto") diff --git a/src/libaktualizr/uptane/uptane_delegation_test.cc b/src/libaktualizr/uptane/uptane_delegation_test.cc new file mode 100644 index 0000000000..7d61bb1bfa --- /dev/null +++ b/src/libaktualizr/uptane/uptane_delegation_test.cc @@ -0,0 +1,78 @@ +#include + +#include + +#include +#include "json/json.h" + +#include "config/config.h" +#include "httpfake.h" +#include "primary/aktualizr.h" +#include "primary/events.h" +#include "uptane_test_common.h" + +boost::filesystem::path uptane_repos_dir; + +class HttpFakeDelegationBasic : public HttpFake { + public: + HttpFakeDelegationBasic(const boost::filesystem::path& test_dir_in) + : HttpFake(test_dir_in, "", uptane_repos_dir / "delegation_basic") {} + + HttpResponse handle_event(const std::string& url, const Json::Value& data) override { + (void)url; + for (const Json::Value& event : data) { + ++events_seen; + EXPECT_EQ(event["event"]["correlationId"].asString(), ""); + } + return HttpResponse("", 200, CURLE_OK, ""); + } + + unsigned int events_seen{0}; +}; + +/* Correlation ID is empty if none was provided in targets metadata. */ +TEST(Delegation, Basic) { + TemporaryDirectory temp_dir; + auto http = std::make_shared(temp_dir.Path()); + + // scope `Aktualizr` object, so that the ReportQueue flushes its events before + // we count them at the end + { + Config conf = UptaneTestCommon::makeTestConfig(temp_dir, http->tls_server); + + auto storage = INvStorage::newStorage(conf.storage); + Aktualizr aktualizr(conf, storage, http); + + aktualizr.Initialize(); + result::UpdateCheck update_result = aktualizr.CheckUpdates().get(); + EXPECT_EQ(update_result.status, result::UpdateStatus::kUpdatesAvailable); + + result::Download download_result = aktualizr.Download(update_result.updates).get(); + EXPECT_EQ(download_result.status, result::DownloadStatus::kSuccess); + + result::Install install_result = aktualizr.Install(download_result.updates).get(); + for (const auto& r : install_result.ecu_reports) { + EXPECT_EQ(r.install_res.result_code.num_code, data::ResultCode::Numeric::kOk); + } + } + + EXPECT_EQ(http->events_seen, 8); +} + +#ifndef __NO_MAIN__ +int main(int argc, char** argv) { + ::testing::InitGoogleTest(&argc, argv); + if (argc != 2) { + std::cerr << "Error: " << argv[0] << " requires the path to the base directory of uptane repos.\n"; + return EXIT_FAILURE; + } + uptane_repos_dir = argv[1]; + + logger_init(); + logger_set_threshold(boost::log::trivial::trace); + + return RUN_ALL_TESTS(); +} +#endif + +// vim: set tabstop=2 shiftwidth=2 expandtab: diff --git a/tests/uptane_repo_generation/CMakeLists.txt b/tests/uptane_repo_generation/CMakeLists.txt index 83c142304f..101f5e0f22 100644 --- a/tests/uptane_repo_generation/CMakeLists.txt +++ b/tests/uptane_repo_generation/CMakeLists.txt @@ -2,3 +2,8 @@ add_custom_target(uptane_repo_full_no_correlation_id COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/full_no_correlation_id.sh $ ${PROJECT_BINARY_DIR}/uptane_repos/full_no_correlation_id) add_dependencies(uptane_repo_full_no_correlation_id aktualizr-repo) + +add_custom_target(uptane_repo_delegation_basic + COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/delegation_basic.sh + $ ${PROJECT_BINARY_DIR}/uptane_repos/delegation_basic) +add_dependencies(uptane_repo_delegation_basic aktualizr-repo) diff --git a/tests/uptane_repo_generation/delegation_basic.sh b/tests/uptane_repo_generation/delegation_basic.sh new file mode 100755 index 0000000000..79647ae6dc --- /dev/null +++ b/tests/uptane_repo_generation/delegation_basic.sh @@ -0,0 +1,37 @@ +#! /bin/bash +set -eEuo pipefail + +if [ "$#" -ne 2 ]; then + echo "Usage: $0 " + exit 1 +fi + +AKTUALIZR_REPO="$1" +DEST_DIR="$2" + +akrepo() { + "$AKTUALIZR_REPO" --path "$DEST_DIR" "$@" +} + +if [ -d "$DEST_DIR" ]; then + # already here, bailing + exit 0 +fi + +mkdir -p "$DEST_DIR" +trap 'rm -rf "$DEST_DIR"' ERR + +IMAGES=$(mktemp -d) +trap 'rm -rf "$IMAGES"' exit +PRIMARY_FIRMWARE="$IMAGES/primary.txt" +echo "primary" > "$PRIMARY_FIRMWARE" +SECONDARY_FIRMWARE="$IMAGES/secondary.txt" +echo "secondary" > "$SECONDARY_FIRMWARE" + +akrepo --command generate --expires 2021-07-04T16:33:27Z +akrepo --command adddelegation --dname new-role --dpattern "abc/*" +akrepo --command image --filename "$PRIMARY_FIRMWARE" --targetname primary.txt +akrepo --command image --filename "$SECONDARY_FIRMWARE" --targetname "abc/secondary.txt" --dname new-role +akrepo --command addtarget --hwid primary_hw --serial CA:FE:A6:D2:84:9D --targetname primary.txt +akrepo --command addtarget --hwid secondary_hw --serial secondary_ecu_serial --targetname "abc/secondary.txt" +akrepo --command signtargets diff --git a/tests/uptane_test_common.h b/tests/uptane_test_common.h index 4a8f1da812..50132b4477 100644 --- a/tests/uptane_test_common.h +++ b/tests/uptane_test_common.h @@ -29,6 +29,19 @@ struct UptaneTestCommon { return ecu_config; } + static Config makeTestConfig(const TemporaryDirectory& temp_dir, const std::string& url) { + Config conf("tests/config/basic.toml"); + conf.uptane.director_server = url + "/director"; + conf.uptane.repo_server = url + "/repo"; + conf.provision.server = url; + conf.provision.primary_ecu_serial = "CA:FE:A6:D2:84:9D"; + conf.provision.primary_ecu_hardware_id = "primary_hw"; + conf.storage.path = temp_dir.Path(); + conf.tls.server = url; + UptaneTestCommon::addDefaultSecondary(conf, temp_dir, "secondary_ecu_serial", "secondary_hw"); + return conf; + } + static std::vector makePackage(const std::string &serial, const std::string &hw_id) { std::vector packages_to_install; Json::Value ot_json; From f4d7d4ef1077b9ee3fe34c1c29bb9bf6ce2b12f7 Mon Sep 17 00:00:00 2001 From: Patrick Vacek Date: Mon, 28 Jan 2019 15:19:02 +0100 Subject: [PATCH 10/19] Specify delegated role when parsing delegations. Signed-off-by: Patrick Vacek --- src/libaktualizr/uptane/tuf.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libaktualizr/uptane/tuf.cc b/src/libaktualizr/uptane/tuf.cc index baefc67ceb..d3a59d4ccf 100644 --- a/src/libaktualizr/uptane/tuf.cc +++ b/src/libaktualizr/uptane/tuf.cc @@ -278,7 +278,7 @@ void Uptane::Targets::init(const Json::Value &json) { const Json::Value role_list = json["signed"]["delegations"]["roles"]; for (Json::ValueIterator it = role_list.begin(); it != role_list.end(); it++) { const std::string role_name = (*it)["name"].asString(); - const Role role = Role(role_name); + const Role role = Role::Delegated(role_name); delegated_role_names_.insert(role_name); ParseRole(Uptane::RepositoryType::Image(), it, role, name_); From f76eff142d9f24d49189dfc223b36cf50bcbb130 Mon Sep 17 00:00:00 2001 From: Patrick Vacek Date: Mon, 28 Jan 2019 15:20:58 +0100 Subject: [PATCH 11/19] Refactor: use more generic "signer" instead of "root". Targets can also sign. Signed-off-by: Patrick Vacek --- src/libaktualizr/uptane/tuf.cc | 12 ++++++------ src/libaktualizr/uptane/tuf.h | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/libaktualizr/uptane/tuf.cc b/src/libaktualizr/uptane/tuf.cc index d3a59d4ccf..8f4ba21968 100644 --- a/src/libaktualizr/uptane/tuf.cc +++ b/src/libaktualizr/uptane/tuf.cc @@ -250,12 +250,12 @@ void Uptane::BaseMeta::init(const Json::Value &json) { } Uptane::BaseMeta::BaseMeta(const Json::Value &json) { init(json); } -Uptane::BaseMeta::BaseMeta(RepositoryType repo, const Json::Value &json, const std::shared_ptr &root) { +Uptane::BaseMeta::BaseMeta(RepositoryType repo, const Json::Value &json, const std::shared_ptr &signer) { if (!json.isObject() || !json.isMember("signed")) { throw Uptane::InvalidMetadata("", "", "invalid metadata json"); } - root->UnpackSignedObject(repo, json); + signer->UnpackSignedObject(repo, json); init(json); } @@ -328,8 +328,8 @@ void Uptane::TimestampMeta::init(const Json::Value &json) { Uptane::TimestampMeta::TimestampMeta(const Json::Value &json) : BaseMeta(json) { init(json); } Uptane::TimestampMeta::TimestampMeta(RepositoryType repo, const Json::Value &json, - const std::shared_ptr &root) - : BaseMeta(repo, json, root) { + const std::shared_ptr &signer) + : BaseMeta(repo, json, signer) { init(json); } @@ -359,8 +359,8 @@ void Uptane::Snapshot::init(const Json::Value &json) { Uptane::Snapshot::Snapshot(const Json::Value &json) : BaseMeta(json) { init(json); } -Uptane::Snapshot::Snapshot(RepositoryType repo, const Json::Value &json, const std::shared_ptr &root) - : BaseMeta(repo, json, root) { +Uptane::Snapshot::Snapshot(RepositoryType repo, const Json::Value &json, const std::shared_ptr &signer) + : BaseMeta(repo, json, signer) { init(json); } diff --git a/src/libaktualizr/uptane/tuf.h b/src/libaktualizr/uptane/tuf.h index f2f9efaffa..a67327cf28 100644 --- a/src/libaktualizr/uptane/tuf.h +++ b/src/libaktualizr/uptane/tuf.h @@ -298,7 +298,7 @@ class BaseMeta { public: BaseMeta() = default; explicit BaseMeta(const Json::Value &json); - BaseMeta(RepositoryType repo, const Json::Value &json, const std::shared_ptr &root); + BaseMeta(RepositoryType repo, const Json::Value &json, const std::shared_ptr &signer); int version() const { return version_; } TimeStamp expiry() const { return expiry_; } bool isExpired(const TimeStamp &now) const { return expiry_.IsExpiredAt(now); } @@ -437,7 +437,7 @@ class Targets : public MetaWithKeys { class TimestampMeta : public BaseMeta { public: explicit TimestampMeta(const Json::Value &json); - TimestampMeta(RepositoryType repo, const Json::Value &json, const std::shared_ptr &root); + TimestampMeta(RepositoryType repo, const Json::Value &json, const std::shared_ptr &signer); TimestampMeta() = default; std::vector snapshot_hashes() const { return snapshot_hashes_; }; int64_t snapshot_size() const { return snapshot_size_; }; @@ -454,7 +454,7 @@ class TimestampMeta : public BaseMeta { class Snapshot : public BaseMeta { public: explicit Snapshot(const Json::Value &json); - Snapshot(RepositoryType repo, const Json::Value &json, const std::shared_ptr &root); + Snapshot(RepositoryType repo, const Json::Value &json, const std::shared_ptr &signer); Snapshot() = default; std::vector targets_hashes() const { return targets_hashes_; }; int64_t targets_size() const { return targets_size_; }; From 8c6fb548c05eb7c51d8a41de0a908c34e24374cf Mon Sep 17 00:00:00 2001 From: Patrick Vacek Date: Mon, 28 Jan 2019 17:22:27 +0100 Subject: [PATCH 12/19] Refactor to send Role into UnpackSignedObject. Signed-off-by: Patrick Vacek --- src/aktualizr_repo/repo_test.cc | 4 +-- .../aktualizr_secondary.cc | 4 +-- .../opcuaserver_secondary_delegate.cc | 5 ++-- src/libaktualizr/primary/sotauptaneclient.cc | 28 +++++++++++-------- src/libaktualizr/uptane/directorrepository.cc | 3 +- src/libaktualizr/uptane/imagesrepository.cc | 8 +++--- src/libaktualizr/uptane/imagesrepository.h | 2 +- src/libaktualizr/uptane/metawithkeys.cc | 21 +++++++++++--- .../uptane/partialverificationsecondary.cc | 2 +- src/libaktualizr/uptane/root.cc | 8 +++--- src/libaktualizr/uptane/tuf.cc | 17 +++++------ src/libaktualizr/uptane/tuf.h | 12 ++++---- 12 files changed, 67 insertions(+), 47 deletions(-) diff --git a/src/aktualizr_repo/repo_test.cc b/src/aktualizr_repo/repo_test.cc index 3dbb457fe1..237ebd9de3 100644 --- a/src/aktualizr_repo/repo_test.cc +++ b/src/aktualizr_repo/repo_test.cc @@ -221,8 +221,8 @@ TEST(aktualizr_repo, sign) { auto json = Utils::parseJSON(output); Uptane::Root root(Uptane::RepositoryType::Director(), Utils::parseJSONFile(temp_dir.Path() / "repo/director/root.json")); - root.UnpackSignedObject(Uptane::RepositoryType::Director(), json); - EXPECT_NO_THROW(root.UnpackSignedObject(Uptane::RepositoryType::Director(), json)); + root.UnpackSignedObject(Uptane::RepositoryType::Director(), Uptane::Role::Snapshot(), json); + EXPECT_NO_THROW(root.UnpackSignedObject(Uptane::RepositoryType::Director(), Uptane::Role::Snapshot(), json)); check_repo(temp_dir); } diff --git a/src/aktualizr_secondary/aktualizr_secondary.cc b/src/aktualizr_secondary/aktualizr_secondary.cc index 0372c58b07..4ab5f5c94f 100644 --- a/src/aktualizr_secondary/aktualizr_secondary.cc +++ b/src/aktualizr_secondary/aktualizr_secondary.cc @@ -70,8 +70,8 @@ bool AktualizrSecondary::putMetadataResp(const Uptane::RawMetaPack& meta_pack) { // TODO: proper partial verification root_ = Uptane::Root(Uptane::RepositoryType::Director(), Utils::parseJSON(meta_pack.director_root), root_); - Uptane::Targets targets(Uptane::RepositoryType::Director(), Utils::parseJSON(meta_pack.director_targets), - std::make_shared(root_)); + Uptane::Targets targets(Uptane::RepositoryType::Director(), Uptane::Role::Targets(), + Utils::parseJSON(meta_pack.director_targets), std::make_shared(root_)); if (meta_targets_.version() > targets.version()) { detected_attack_ = "Rollback attack detected"; return true; diff --git a/src/aktualizr_secondary/opcuaserver_secondary_delegate.cc b/src/aktualizr_secondary/opcuaserver_secondary_delegate.cc index edb7be66cd..a1688b6656 100644 --- a/src/aktualizr_secondary/opcuaserver_secondary_delegate.cc +++ b/src/aktualizr_secondary/opcuaserver_secondary_delegate.cc @@ -76,8 +76,9 @@ void OpcuaServerSecondaryDelegate::handleAllMetaDataFilesReceived(opcuabridge::S // TODO: proper root metadata rotation secondary_->root_ = Uptane::Root(Uptane::RepositoryType::Director(), Utils::parseJSON(received_meta_pack_.director_root), secondary_->root_); - Uptane::Targets targets(Uptane::RepositoryType::Director(), Utils::parseJSON(received_meta_pack_.director_targets), - secondary_->root_); + Uptane::Targets targets(Uptane::RepositoryType::Director(), Uptane::Role::Targets(), + Utils::parseJSON(received_meta_pack_.director_targets), + std::make_shared(secondary_->root_)); if (secondary_->meta_targets_.version() > targets.version()) { secondary_->detected_attack_ = "Rollback attack detected"; LOG_ERROR << "Uptane security check: " << secondary_->detected_attack_; diff --git a/src/libaktualizr/primary/sotauptaneclient.cc b/src/libaktualizr/primary/sotauptaneclient.cc index 250f07f015..949245815b 100644 --- a/src/libaktualizr/primary/sotauptaneclient.cc +++ b/src/libaktualizr/primary/sotauptaneclient.cc @@ -552,23 +552,24 @@ bool SotaUptaneClient::updateImagesMeta() { // Update Images Targets Metadata { std::string images_targets; + Uptane::Role targets_role = Uptane::Role::Targets(); int64_t targets_size = (images_repo.targetsSize() > 0) ? images_repo.targetsSize() : Uptane::kMaxImagesTargetsSize; if (!uptane_fetcher->fetchLatestRole(&images_targets, targets_size, Uptane::RepositoryType::Image(), - Uptane::Role::Targets())) { + targets_role)) { return false; } int remote_version = Uptane::extractVersionUntrusted(images_targets); int local_version; std::string images_targets_stored; - if (storage->loadNonRoot(&images_targets_stored, Uptane::RepositoryType::Image(), Uptane::Role::Targets())) { + if (storage->loadNonRoot(&images_targets_stored, Uptane::RepositoryType::Image(), targets_role)) { local_version = Uptane::extractVersionUntrusted(images_targets_stored); } else { local_version = -1; } - if (!images_repo.verifyTargets(images_targets, "targets")) { + if (!images_repo.verifyTargets(images_targets, targets_role)) { last_exception = images_repo.getLastException(); return false; } @@ -576,7 +577,7 @@ bool SotaUptaneClient::updateImagesMeta() { if (local_version > remote_version) { return false; } else if (local_version < remote_version) { - storage->storeNonRoot(images_targets, Uptane::RepositoryType::Image(), Uptane::Role::Targets()); + storage->storeNonRoot(images_targets, Uptane::RepositoryType::Image(), targets_role); } if (images_repo.targetsExpired("targets")) { @@ -588,22 +589,23 @@ bool SotaUptaneClient::updateImagesMeta() { // Update Images delegated Targets Metadata for (const std::string &delegation : images_repo.delegations("targets")) { std::string images_delegated; + Uptane::Role delegated_role = Uptane::Role::Delegated(delegation); if (!uptane_fetcher->fetchLatestRole(&images_delegated, Uptane::kMaxImagesTargetsSize, - Uptane::RepositoryType::Image(), Uptane::Role::Delegated(delegation))) { + Uptane::RepositoryType::Image(), delegated_role)) { return false; } int remote_version = Uptane::extractVersionUntrusted(images_delegated); int local_version; std::string images_delegated_stored; - if (storage->loadDelegation(&images_delegated_stored, Uptane::Role::Delegated(delegation))) { + if (storage->loadDelegation(&images_delegated_stored, delegated_role)) { local_version = Uptane::extractVersionUntrusted(images_delegated_stored); } else { local_version = -1; } - if (!images_repo.verifyTargets(images_delegated, delegation)) { + if (!images_repo.verifyTargets(images_delegated, delegated_role)) { last_exception = images_repo.getLastException(); return false; } @@ -611,7 +613,7 @@ bool SotaUptaneClient::updateImagesMeta() { if (local_version > remote_version) { return false; } else if (local_version < remote_version) { - storage->storeDelegation(images_delegated, Uptane::Role::Delegated(delegation)); + storage->storeDelegation(images_delegated, delegated_role); } if (images_repo.targetsExpired(delegation)) { @@ -725,11 +727,12 @@ bool SotaUptaneClient::checkImagesMetaOffline() { // Load Images Targets Metadata { std::string images_targets; - if (!storage->loadNonRoot(&images_targets, Uptane::RepositoryType::Image(), Uptane::Role::Targets())) { + Uptane::Role targets_role = Uptane::Role::Targets(); + if (!storage->loadNonRoot(&images_targets, Uptane::RepositoryType::Image(), targets_role)) { return false; } - if (!images_repo.verifyTargets(images_targets, "targets")) { + if (!images_repo.verifyTargets(images_targets, targets_role)) { last_exception = images_repo.getLastException(); return false; } @@ -743,11 +746,12 @@ bool SotaUptaneClient::checkImagesMetaOffline() { // Update Images delegated Targets Metadata for (const std::string &delegation : images_repo.delegations("targets")) { std::string images_delegated; - if (!storage->loadDelegation(&images_delegated, Uptane::Role::Delegated(delegation))) { + Uptane::Role delegated_role = Uptane::Role::Delegated(delegation); + if (!storage->loadDelegation(&images_delegated, delegated_role)) { return false; } - if (!images_repo.verifyTargets(images_delegated, delegation)) { + if (!images_repo.verifyTargets(images_delegated, delegated_role)) { last_exception = images_repo.getLastException(); return false; } diff --git a/src/libaktualizr/uptane/directorrepository.cc b/src/libaktualizr/uptane/directorrepository.cc index 19da8ac2c4..1fd2fe30c1 100644 --- a/src/libaktualizr/uptane/directorrepository.cc +++ b/src/libaktualizr/uptane/directorrepository.cc @@ -10,7 +10,8 @@ void DirectorRepository::resetMeta() { bool DirectorRepository::verifyTargets(const std::string& targets_raw) { try { // Verify the signature: - targets = Targets(RepositoryType::Director(), Utils::parseJSON(targets_raw), std::make_shared(root)); + targets = Targets(RepositoryType::Director(), Role::Targets(), Utils::parseJSON(targets_raw), + std::make_shared(root)); } catch (const Uptane::Exception& e) { LOG_ERROR << "Signature verification for director targets metadata failed"; last_exception = e; diff --git a/src/libaktualizr/uptane/imagesrepository.cc b/src/libaktualizr/uptane/imagesrepository.cc index 3879b7b7de..3e692af675 100644 --- a/src/libaktualizr/uptane/imagesrepository.cc +++ b/src/libaktualizr/uptane/imagesrepository.cc @@ -65,7 +65,7 @@ bool ImagesRepository::verifySnapshot(const std::string& snapshot_raw) { return true; } -bool ImagesRepository::verifyTargets(const std::string& targets_raw, const std::string& role_name) { +bool ImagesRepository::verifyTargets(const std::string& targets_raw, const Uptane::Role role) { try { const Json::Value targets_json = Utils::parseJSON(targets_raw); const std::string canonical = Utils::jsonToCanonicalStr(targets_json); @@ -97,19 +97,19 @@ bool ImagesRepository::verifyTargets(const std::string& targets_raw, const std:: // Verify the signature: std::shared_ptr signer; - if (role_name == "targets") { + if (role == Uptane::Role::Targets()) { signer = std::make_shared(root); } else { // TODO: support nested delegations here. This currently assumes that all // delegated targets are signed by the top-level targets. signer = std::make_shared(targets["targets"]); } - targets[role_name] = Targets(RepositoryType::Image(), targets_json, signer); + targets[role.ToString()] = Targets(RepositoryType::Image(), role, targets_json, signer); // Only compare targets version in snapshot metadata for top-level // targets.json. Delegated target metadata versions are not tracked outside // of their own metadata. - if (role_name == "targets" && targets[role_name].version() != snapshot.targets_version()) { + if (role == Uptane::Role::Targets() && targets[role.ToString()].version() != snapshot.targets_version()) { return false; } } catch (const Exception& e) { diff --git a/src/libaktualizr/uptane/imagesrepository.h b/src/libaktualizr/uptane/imagesrepository.h index 0b82107bf2..46a7c8ff03 100644 --- a/src/libaktualizr/uptane/imagesrepository.h +++ b/src/libaktualizr/uptane/imagesrepository.h @@ -14,7 +14,7 @@ class ImagesRepository : public RepositoryCommon { void resetMeta(); - bool verifyTargets(const std::string& targets_raw, const std::string& role_name); + bool verifyTargets(const std::string& targets_raw, Uptane::Role role); bool targetsExpired(const std::string& role_name) { return targets[role_name].isExpired(TimeStamp::Now()); } int64_t targetsSize() { return snapshot.targets_size(); } std::set delegations(const std::string& role_name) { return targets[role_name].delegated_role_names_; } diff --git a/src/libaktualizr/uptane/metawithkeys.cc b/src/libaktualizr/uptane/metawithkeys.cc index bee89bb4fe..1b6c975ac7 100644 --- a/src/libaktualizr/uptane/metawithkeys.cc +++ b/src/libaktualizr/uptane/metawithkeys.cc @@ -5,8 +5,9 @@ using Uptane::MetaWithKeys; MetaWithKeys::MetaWithKeys(const Json::Value &json) : BaseMeta(json) {} -MetaWithKeys::MetaWithKeys(RepositoryType repo, const Json::Value &json, const std::shared_ptr &signer) - : BaseMeta(repo, json, signer) {} +MetaWithKeys::MetaWithKeys(RepositoryType repo, const Role role, const Json::Value &json, + const std::shared_ptr &signer) + : BaseMeta(repo, role, json, signer) {} void Uptane::MetaWithKeys::ParseKeys(const RepositoryType repo, const Json::Value &keys) { for (Json::ValueIterator it = keys.begin(); it != keys.end(); ++it) { @@ -54,11 +55,23 @@ void Uptane::MetaWithKeys::ParseRole(const RepositoryType repo, const Json::Valu } } -void Uptane::MetaWithKeys::UnpackSignedObject(const RepositoryType repo, const Json::Value &signed_object) { +void Uptane::MetaWithKeys::UnpackSignedObject(const RepositoryType repo, const Role role, + const Json::Value &signed_object) { const std::string repository = repo; + const Uptane::Role type(signed_object["signed"]["_type"].asString()); + if (role.IsDelegation()) { + if (type != Uptane::Role::Targets()) { + LOG_ERROR << "Delegated role " << role << " has an invalid type: " << type; + throw SecurityException(repo, "Delegated role " + role.ToString() + " has invalid type " + type.ToString()); + } + } else if (role != type) { + LOG_ERROR << "Metadata type " << type << " does not match expected role: " << role; + throw SecurityException(repo, + "Metadata type " + type.ToString() + " does not match expected role " + role.ToString()); + } + const std::string canonical = Json::FastWriter().write(signed_object["signed"]); - const Uptane::Role role(signed_object["signed"]["_type"].asString()); const Json::Value signatures = signed_object["signatures"]; int valid_signatures = 0; diff --git a/src/libaktualizr/uptane/partialverificationsecondary.cc b/src/libaktualizr/uptane/partialverificationsecondary.cc index 8b1b21a70c..d92966ebb9 100644 --- a/src/libaktualizr/uptane/partialverificationsecondary.cc +++ b/src/libaktualizr/uptane/partialverificationsecondary.cc @@ -37,7 +37,7 @@ bool PartialVerificationSecondary::putMetadata(const RawMetaPack &meta) { // TODO: check for expiration and version downgrade root_ = Uptane::Root(RepositoryType::Director(), Utils::parseJSON(meta.director_root), root_); - Uptane::Targets targets(RepositoryType::Director(), Utils::parseJSON(meta.director_targets), + Uptane::Targets targets(RepositoryType::Director(), Role::Targets(), Utils::parseJSON(meta.director_targets), std::make_shared(root_)); if (meta_targets_.version() > targets.version()) { detected_attack_ = "Rollback attack detected"; diff --git a/src/libaktualizr/uptane/root.cc b/src/libaktualizr/uptane/root.cc index d52b6245ed..99b71a0eea 100644 --- a/src/libaktualizr/uptane/root.cc +++ b/src/libaktualizr/uptane/root.cc @@ -5,8 +5,8 @@ using Uptane::Root; Root::Root(const RepositoryType repo, const Json::Value &json, Root &root) : Root(repo, json) { - root.UnpackSignedObject(repo, json); - this->Root::UnpackSignedObject(repo, json); + root.UnpackSignedObject(repo, Role::Root(), json); + this->Root::UnpackSignedObject(repo, Role::Root(), json); } Root::Root(const RepositoryType repo, const Json::Value &json) : MetaWithKeys(json), policy_(Policy::kCheck) { @@ -26,7 +26,7 @@ Root::Root(const RepositoryType repo, const Json::Value &json) : MetaWithKeys(js } } -void Uptane::Root::UnpackSignedObject(const RepositoryType repo, const Json::Value &signed_object) { +void Uptane::Root::UnpackSignedObject(const RepositoryType repo, const Role role, const Json::Value &signed_object) { const std::string repository = repo; if (policy_ == Policy::kAcceptAll) { @@ -37,5 +37,5 @@ void Uptane::Root::UnpackSignedObject(const RepositoryType repo, const Json::Val } assert(policy_ == Policy::kCheck); - Uptane::MetaWithKeys::UnpackSignedObject(repo, signed_object); + Uptane::MetaWithKeys::UnpackSignedObject(repo, role, signed_object); } diff --git a/src/libaktualizr/uptane/tuf.cc b/src/libaktualizr/uptane/tuf.cc index 8f4ba21968..ef7625fc88 100644 --- a/src/libaktualizr/uptane/tuf.cc +++ b/src/libaktualizr/uptane/tuf.cc @@ -250,12 +250,13 @@ void Uptane::BaseMeta::init(const Json::Value &json) { } Uptane::BaseMeta::BaseMeta(const Json::Value &json) { init(json); } -Uptane::BaseMeta::BaseMeta(RepositoryType repo, const Json::Value &json, const std::shared_ptr &signer) { +Uptane::BaseMeta::BaseMeta(RepositoryType repo, const Role role, const Json::Value &json, + const std::shared_ptr &signer) { if (!json.isObject() || !json.isMember("signed")) { throw Uptane::InvalidMetadata("", "", "invalid metadata json"); } - signer->UnpackSignedObject(repo, json); + signer->UnpackSignedObject(repo, role, json); init(json); } @@ -302,9 +303,9 @@ void Uptane::Targets::init(const Json::Value &json) { Uptane::Targets::Targets(const Json::Value &json) : MetaWithKeys(json) { init(json); } -Uptane::Targets::Targets(RepositoryType repo, const Json::Value &json, const std::shared_ptr &signer, - std::string name) - : MetaWithKeys(repo, json, signer), name_(std::move(name)) { +Uptane::Targets::Targets(RepositoryType repo, const Role role, const Json::Value &json, + const std::shared_ptr &signer) + : MetaWithKeys(repo, role, json, signer), name_(role.ToString()) { init(json); } @@ -329,7 +330,7 @@ Uptane::TimestampMeta::TimestampMeta(const Json::Value &json) : BaseMeta(json) { Uptane::TimestampMeta::TimestampMeta(RepositoryType repo, const Json::Value &json, const std::shared_ptr &signer) - : BaseMeta(repo, json, signer) { + : BaseMeta(repo, Role::Timestamp(), json, signer) { init(json); } @@ -360,7 +361,7 @@ void Uptane::Snapshot::init(const Json::Value &json) { Uptane::Snapshot::Snapshot(const Json::Value &json) : BaseMeta(json) { init(json); } Uptane::Snapshot::Snapshot(RepositoryType repo, const Json::Value &json, const std::shared_ptr &signer) - : BaseMeta(repo, json, signer) { + : BaseMeta(repo, Role::Snapshot(), json, signer) { init(json); } @@ -371,7 +372,7 @@ bool MetaPack::isConsistent() const { Uptane::Root original_root(director_root); Uptane::Root new_root(RepositoryType::Director(), director_root.original(), new_root); if (director_targets.original() != Json::nullValue) { - Uptane::Targets(RepositoryType::Director(), director_targets.original(), + Uptane::Targets(RepositoryType::Director(), Role::Targets(), director_targets.original(), std::make_shared(original_root)); } } diff --git a/src/libaktualizr/uptane/tuf.h b/src/libaktualizr/uptane/tuf.h index a67327cf28..3617ed8f56 100644 --- a/src/libaktualizr/uptane/tuf.h +++ b/src/libaktualizr/uptane/tuf.h @@ -67,6 +67,7 @@ class Role { explicit Role(const std::string &role_name, bool delegation = false); std::string ToString() const; int ToInt() const { return static_cast(role_); } + bool IsDelegation() const { return role_ == RoleEnum::kDelegated; } bool operator==(const Role &other) const { return name_ == other.name_; } bool operator!=(const Role &other) const { return !(*this == other); } bool operator<(const Role &other) const { return name_ < other.name_; } @@ -298,7 +299,7 @@ class BaseMeta { public: BaseMeta() = default; explicit BaseMeta(const Json::Value &json); - BaseMeta(RepositoryType repo, const Json::Value &json, const std::shared_ptr &signer); + BaseMeta(RepositoryType repo, Role role, const Json::Value &json, const std::shared_ptr &signer); int version() const { return version_; } TimeStamp expiry() const { return expiry_; } bool isExpired(const TimeStamp &now) const { return expiry_.IsExpiredAt(now); } @@ -328,7 +329,7 @@ class MetaWithKeys : public BaseMeta { * @param json - The contents of the 'signed' portion */ MetaWithKeys(const Json::Value &json); - MetaWithKeys(RepositoryType repo, const Json::Value &json, const std::shared_ptr &signer); + MetaWithKeys(RepositoryType repo, Role role, const Json::Value &json, const std::shared_ptr &signer); virtual ~MetaWithKeys() = default; @@ -350,7 +351,7 @@ class MetaWithKeys : public BaseMeta { * @param signed_object * @return */ - virtual void UnpackSignedObject(RepositoryType repo, const Json::Value &signed_object); + virtual void UnpackSignedObject(RepositoryType repo, Role role, const Json::Value &signed_object); bool operator==(const MetaWithKeys &rhs) const { return version_ == rhs.version_ && expiry_ == rhs.expiry_ && keys_ == rhs.keys_ && @@ -396,7 +397,7 @@ class Root : public MetaWithKeys { * @param signed_object * @return */ - void UnpackSignedObject(RepositoryType repo, const Json::Value &signed_object) override; + void UnpackSignedObject(RepositoryType repo, Role role, const Json::Value &signed_object) override; bool operator==(const Root &rhs) const { return version_ == rhs.version_ && expiry_ == rhs.expiry_ && keys_ == rhs.keys_ && @@ -412,8 +413,7 @@ class Root : public MetaWithKeys { class Targets : public MetaWithKeys { public: explicit Targets(const Json::Value &json); - Targets(RepositoryType repo, const Json::Value &json, const std::shared_ptr &signer, - std::string name = "targets"); + Targets(RepositoryType repo, Role role, const Json::Value &json, const std::shared_ptr &signer); Targets() = default; ~Targets() override = default; From 7307be866243e1576bd86be3d508dab548e634d7 Mon Sep 17 00:00:00 2001 From: Patrick Vacek Date: Mon, 28 Jan 2019 17:23:41 +0100 Subject: [PATCH 13/19] Allow / to pass unchanged in encoded URLs. This matches my understanding of the expected behavior for directory structures in target names. Signed-off-by: Patrick Vacek --- src/libaktualizr/utilities/utils.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libaktualizr/utilities/utils.cc b/src/libaktualizr/utilities/utils.cc index fbd0e485a8..28bb4a58d2 100644 --- a/src/libaktualizr/utilities/utils.cc +++ b/src/libaktualizr/utilities/utils.cc @@ -675,7 +675,7 @@ std::string Utils::urlEncode(const std::string &input) { for (char c : input) { if ((c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') || c == '-' || c == '.' || - c == '_' || c == '~') { + c == '_' || c == '~' || c == '/') { res.push_back(c); } else { res.push_back('%'); From 250435e4f1b271c34bd6b2405cf0f60b477ab190 Mon Sep 17 00:00:00 2001 From: Patrick Vacek Date: Tue, 29 Jan 2019 14:06:01 +0100 Subject: [PATCH 14/19] Only check hashes for top-level targets.json. Delegated targets do not have their hashes stored in their parent. Signed-off-by: Patrick Vacek --- src/libaktualizr/uptane/imagesrepository.cc | 49 +++++++++++---------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/src/libaktualizr/uptane/imagesrepository.cc b/src/libaktualizr/uptane/imagesrepository.cc index 3e692af675..6b45cde9f3 100644 --- a/src/libaktualizr/uptane/imagesrepository.cc +++ b/src/libaktualizr/uptane/imagesrepository.cc @@ -69,30 +69,33 @@ bool ImagesRepository::verifyTargets(const std::string& targets_raw, const Uptan try { const Json::Value targets_json = Utils::parseJSON(targets_raw); const std::string canonical = Utils::jsonToCanonicalStr(targets_json); - bool hash_exists = false; - for (const auto& it : snapshot.targets_hashes()) { - switch (it.type()) { - case Hash::Type::kSha256: - if (Hash(Hash::Type::kSha256, boost::algorithm::hex(Crypto::sha256digest(canonical))) != it) { - LOG_ERROR << "Hash verification for targets metadata failed"; - return false; - } - hash_exists = true; - break; - case Hash::Type::kSha512: - if (Hash(Hash::Type::kSha512, boost::algorithm::hex(Crypto::sha512digest(canonical))) != it) { - LOG_ERROR << "Hash verification for targets metadata failed"; - return false; - } - hash_exists = true; - break; - default: - break; + // Delegated targets do not have their hashes stored in their parent. + if (role == Uptane::Role::Targets()) { + bool hash_exists = false; + for (const auto& it : snapshot.targets_hashes()) { + switch (it.type()) { + case Hash::Type::kSha256: + if (Hash(Hash::Type::kSha256, boost::algorithm::hex(Crypto::sha256digest(canonical))) != it) { + LOG_ERROR << "Hash verification for targets metadata failed"; + return false; + } + hash_exists = true; + break; + case Hash::Type::kSha512: + if (Hash(Hash::Type::kSha512, boost::algorithm::hex(Crypto::sha512digest(canonical))) != it) { + LOG_ERROR << "Hash verification for targets metadata failed"; + return false; + } + hash_exists = true; + break; + default: + break; + } + } + if (!hash_exists) { + LOG_ERROR << "No hash found for targets.json"; + return false; } - } - if (!hash_exists) { - LOG_ERROR << "No hash found for targets.json"; - return false; } // Verify the signature: From 72d6b589d04fa673291e0f0ebc551e118cd4258c Mon Sep 17 00:00:00 2001 From: Patrick Vacek Date: Tue, 29 Jan 2019 15:28:15 +0100 Subject: [PATCH 15/19] Refactor reserved role names and unify IsReserved() function. Signed-off-by: Patrick Vacek --- src/aktualizr_repo/image_repo.cc | 6 +++--- src/aktualizr_repo/repo.cc | 4 ++-- src/aktualizr_repo/repo.h | 8 ++------ src/libaktualizr/uptane/role.cc | 22 +++++++++++----------- src/libaktualizr/uptane/tuf.h | 22 ++++++++++++++++------ 5 files changed, 34 insertions(+), 28 deletions(-) diff --git a/src/aktualizr_repo/image_repo.cc b/src/aktualizr_repo/image_repo.cc index 06c1bcb746..15bfc42ea3 100644 --- a/src/aktualizr_repo/image_repo.cc +++ b/src/aktualizr_repo/image_repo.cc @@ -42,8 +42,8 @@ void ImageRepo::addDelegation(const Uptane::Role &name, const std::string &path, if (keys_.count(name) != 0) { throw std::runtime_error("Delegation with the same name already exist."); } - if (Delegation::isBadName(name.ToString())) { - throw std::runtime_error("Delegation with the wrong name, this name is reserved."); + if (Uptane::Role::IsReserved(name.ToString())) { + throw std::runtime_error("Delegation name " + name.ToString() + " is reserved."); } generateKeyPair(key_type, name); Json::Value delegate; @@ -83,4 +83,4 @@ void ImageRepo::addCustomImage(const std::string &name, const Uptane::Hash &hash target["hashes"]["sha512"] = hash.HashString(); } addImage(name, target, delegation); -} \ No newline at end of file +} diff --git a/src/aktualizr_repo/repo.cc b/src/aktualizr_repo/repo.cc index 91b0e02ddc..7187289e61 100644 --- a/src/aktualizr_repo/repo.cc +++ b/src/aktualizr_repo/repo.cc @@ -203,7 +203,7 @@ Json::Value Repo::getTarget(const std::string &target_name) { return image_targets["targets"][target_name]; } else { for (auto &p : boost::filesystem::directory_iterator(path_ / "repo" / repo_type_.toString())) { - if (!Delegation::isBadName(p.path().stem().string())) { + if (!Uptane::Role::IsReserved(p.path().stem().string())) { auto targets = Utils::parseJSONFile(p)["signed"]; if (targets["targets"].isMember(target_name)) { return targets["targets"][target_name]; @@ -224,7 +224,7 @@ void Repo::readKeys() { key_type_str >> key_type; std::string private_key_string(Utils::readFile(p / "private.key")); auto name = p.path().filename().string(); - keys_[Uptane::Role(name, !Delegation::isBadName(name))] = + keys_[Uptane::Role(name, !Uptane::Role::IsReserved(name))] = KeyPair(PublicKey(public_key_string, key_type), private_key_string); } } diff --git a/src/aktualizr_repo/repo.h b/src/aktualizr_repo/repo.h index 213bd3a6c1..25610af13e 100644 --- a/src/aktualizr_repo/repo.h +++ b/src/aktualizr_repo/repo.h @@ -19,8 +19,8 @@ struct KeyPair { struct Delegation { Delegation() = default; Delegation(const boost::filesystem::path &repo_path, std::string delegation_name) : name(std::move(delegation_name)) { - if (isBadName(name)) { - throw std::runtime_error("Delegation with the wrong name, this name is reserved."); + if (Uptane::Role::IsReserved(name)) { + throw std::runtime_error("Delegation name " + name + " is reserved."); } boost::filesystem::path delegation_path(((repo_path / "repo/image") / name).string() + ".json"); boost::filesystem::path targets_path(repo_path / "repo/image/targets.json"); @@ -44,10 +44,6 @@ struct Delegation { bool isMatched(const boost::filesystem::path &image_path) { return (fnmatch(pattern.c_str(), image_path.c_str(), 0) == 0); } - static bool isBadName(const std::string &delegation_name) { - return (delegation_name == "root" || delegation_name == "targets" || delegation_name == "snapshot" || - delegation_name == "timestamp"); - } operator bool() const { return (!name.empty() && !pattern.empty()); } std::string name; std::string pattern; diff --git a/src/libaktualizr/uptane/role.cc b/src/libaktualizr/uptane/role.cc index e40a549f0b..0f7bdd6815 100644 --- a/src/libaktualizr/uptane/role.cc +++ b/src/libaktualizr/uptane/role.cc @@ -5,28 +5,28 @@ using Uptane::Role; using Uptane::Version; -const std::string UPTANE_ROLE_ROOT = "root"; -const std::string UPTANE_ROLE_SNAPSHOT = "snapshot"; -const std::string UPTANE_ROLE_TARGETS = "targets"; -const std::string UPTANE_ROLE_TIMESTAMP = "timestamp"; +const std::string Role::ROOT = "root"; +const std::string Role::SNAPSHOT = "snapshot"; +const std::string Role::TARGETS = "targets"; +const std::string Role::TIMESTAMP = "timestamp"; Role::Role(const std::string &role_name, const bool delegation) { std::string role_name_lower; std::transform(role_name.begin(), role_name.end(), std::back_inserter(role_name_lower), ::tolower); name_ = role_name_lower; if (delegation) { - if (role_name_lower == UPTANE_ROLE_ROOT || role_name_lower == UPTANE_ROLE_SNAPSHOT || - role_name_lower == UPTANE_ROLE_TARGETS || role_name_lower == UPTANE_ROLE_TIMESTAMP) { - throw Uptane::Exception("", "Invalid delegated role name " + role_name); + if (IsReserved(name_)) { + throw Uptane::Exception("", "Delegated role name " + role_name + " is reserved."); } role_ = RoleEnum::kDelegated; - } else if (role_name_lower == UPTANE_ROLE_ROOT) { + name_ = role_name; + } else if (role_name_lower == ROOT) { role_ = RoleEnum::kRoot; - } else if (role_name_lower == UPTANE_ROLE_SNAPSHOT) { + } else if (role_name_lower == SNAPSHOT) { role_ = RoleEnum::kSnapshot; - } else if (role_name_lower == UPTANE_ROLE_TARGETS) { + } else if (role_name_lower == TARGETS) { role_ = RoleEnum::kTargets; - } else if (role_name_lower == UPTANE_ROLE_TIMESTAMP) { + } else if (role_name_lower == TIMESTAMP) { role_ = RoleEnum::kTimestamp; } else { role_ = RoleEnum::kInvalidRole; diff --git a/src/libaktualizr/uptane/tuf.h b/src/libaktualizr/uptane/tuf.h index 3617ed8f56..191caf325c 100644 --- a/src/libaktualizr/uptane/tuf.h +++ b/src/libaktualizr/uptane/tuf.h @@ -55,14 +55,23 @@ using KeyId = std::string; */ class Role { public: + static const std::string ROOT; + static const std::string SNAPSHOT; + static const std::string TARGETS; + static const std::string TIMESTAMP; + static Role Root() { return Role{RoleEnum::kRoot}; } static Role Snapshot() { return Role{RoleEnum::kSnapshot}; } static Role Targets() { return Role{RoleEnum::kTargets}; } static Role Timestamp() { return Role{RoleEnum::kTimestamp}; } static Role Delegated(const std::string &name) { return Role(name, true); } static Role InvalidRole() { return Role{RoleEnum::kInvalidRole}; } - // TODO: add delegated? + // Delegated is not included because this is only used for a metadata table + // that doesn't include delegations. static std::vector Roles() { return {Root(), Snapshot(), Targets(), Timestamp()}; } + static bool IsReserved(const std::string &name) { + return (name == ROOT || name == TARGETS || name == SNAPSHOT || name == TIMESTAMP); + } explicit Role(const std::string &role_name, bool delegation = false); std::string ToString() const; @@ -75,18 +84,19 @@ class Role { friend std::ostream &operator<<(std::ostream &os, const Role &role); private: - /** This must match the meta_types table in sqlstorage */ + /** The four standard roles must match the meta_types table in sqlstorage. + * Delegations are special and handled differently. */ enum class RoleEnum { kRoot = 0, kSnapshot = 1, kTargets = 2, kTimestamp = 3, kDelegated = 4, kInvalidRole = -1 }; explicit Role(RoleEnum role) : role_(role) { if (role_ == RoleEnum::kRoot) { - name_ = "root"; + name_ = ROOT; } else if (role_ == RoleEnum::kSnapshot) { - name_ = "snapshot"; + name_ = SNAPSHOT; } else if (role_ == RoleEnum::kTargets) { - name_ = "targets"; + name_ = TARGETS; } else if (role_ == RoleEnum::kTimestamp) { - name_ = "timestamp"; + name_ = TIMESTAMP; } else { role_ = RoleEnum::kInvalidRole; name_ = "invalidrole"; From 07e18baee517e709ea7378c7cb8c94d88048e462 Mon Sep 17 00:00:00 2001 From: Patrick Vacek Date: Tue, 29 Jan 2019 15:58:12 +0100 Subject: [PATCH 16/19] More TUF role testing. Update actions and changelog. Signed-off-by: Patrick Vacek --- CHANGELOG.md | 4 ++ actions.md | 6 +++ src/libaktualizr/uptane/tuf_test.cc | 38 +++++++++++++++---- .../uptane/uptane_delegation_test.cc | 4 +- 4 files changed, 44 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e8865b8b25..189ef2150a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ Our versioning scheme is `YEAR.N` where `N` is incremented whenever a new releas ## [??? (unreleased)] +### Added + +- Basic first-order delegation support. + ## [2019.1] - 2019-01-10 ### Changed diff --git a/actions.md b/actions.md index cfb105ffbb..2faf1abe40 100644 --- a/actions.md +++ b/actions.md @@ -94,6 +94,8 @@ These are the primary actions that a user of libaktualizr can perform through th - [x] Send UpdateCheckComplete event after successful check with no available updates (aktualizr_test.cc) - [x] Send UpdateCheckComplete event after failure (aktualizr_test.cc) - [x] Download updates + - [x] Find requested target + - [x] Search first-order delegations (uptane_delegation_test.cc) - [x] Download an update - [x] Download an OSTree package (fetcher_test.cc) - [x] Download a binary package (uptane_vector_tests.cc, aktualizr_test.cc) @@ -178,6 +180,9 @@ These are internal requirements that are relatively opaque to the user and/or co - [x] Sign TUF metadata - [x] Sign TUF metadata with RSA2048 (keymanager_test.cc) - [x] Sign TUF metadata with ED25519 (keymanager_test.cc) + - [x] Validate TUF roles (tuf_test.cc) + - [x] Delegated roles have custom names (tuf_test.cc) + - [x] Reject delegated role names that are identical to reserved role names (tuf_test.cc) - [x] Validate a TUF root (tuf_test.cc, uptane_test.cc) - [x] Throw an exception if a TUF root is invalid - [x] Throw an exception if a TUF root is unsigned (tuf_test.cc, uptane_test.cc) @@ -188,6 +193,7 @@ These are internal requirements that are relatively opaque to the user and/or co - [x] Parse Uptane timestamps (types_test.cc) - [x] Throw an exception if an Uptane timestamp is invalid (types_test.cc) - [x] Get current time (types_test.cc) + - [x] Validate first-order target delegations (uptane_delegation_test.cc) - [x] Reject http GET responses that exceed size limit (httpclient_test.cc) - [x] Reject http GET responses that do not meet speed limit (httpclient_test.cc) - [x] Abort update if any signature threshold is <= 0 (REQ-153, uptane_vector_tests.cc) diff --git a/src/libaktualizr/uptane/tuf_test.cc b/src/libaktualizr/uptane/tuf_test.cc index 5f05e6eb89..248d232f78 100644 --- a/src/libaktualizr/uptane/tuf_test.cc +++ b/src/libaktualizr/uptane/tuf_test.cc @@ -47,7 +47,37 @@ TEST(Root, RootJsonRsassaPssSha256) { EXPECT_NO_THROW(Uptane::Root(Uptane::RepositoryType::Director(), initial_root, root)); } -/* Reject delegated role names that are identical to other roles. */ +/* Validate TUF roles. */ +TEST(Role, ValidateRoles) { + Uptane::Role root = Uptane::Role::Root(); + EXPECT_EQ(root.ToInt(), 0); + EXPECT_EQ(root.ToString(), "root"); + EXPECT_EQ(root.IsDelegation(), false); + + Uptane::Role snapshot = Uptane::Role::Snapshot(); + EXPECT_EQ(snapshot.ToInt(), 1); + EXPECT_EQ(snapshot.ToString(), "snapshot"); + EXPECT_EQ(snapshot.IsDelegation(), false); + + Uptane::Role targets = Uptane::Role::Targets(); + EXPECT_EQ(targets.ToInt(), 2); + EXPECT_EQ(targets.ToString(), "targets"); + EXPECT_EQ(targets.IsDelegation(), false); + + Uptane::Role timestamp = Uptane::Role::Timestamp(); + EXPECT_EQ(timestamp.ToInt(), 3); + EXPECT_EQ(timestamp.ToString(), "timestamp"); + EXPECT_EQ(timestamp.IsDelegation(), false); +} + +/* Delegated roles have custom names. */ +TEST(Role, ValidateDelegation) { + Uptane::Role delegated = Uptane::Role::Delegated("whatever"); + EXPECT_EQ(delegated.ToString(), "whatever"); + EXPECT_EQ(delegated.IsDelegation(), true); +} + +/* Reject delegated role names that are identical to reserved role names. */ TEST(Role, InvalidDelegationName) { EXPECT_THROW(Uptane::Role::Delegated("root"), Uptane::Exception); EXPECT_THROW(Uptane::Role::Delegated("snapshot"), Uptane::Exception); @@ -55,12 +85,6 @@ TEST(Role, InvalidDelegationName) { EXPECT_THROW(Uptane::Role::Delegated("timestamp"), Uptane::Exception); } -/* Delegated role has custom name. */ -TEST(Role, ValidDelegationName) { - Uptane::Role delegated = Uptane::Role::Delegated("whatever"); - EXPECT_EQ(delegated.ToString(), "whatever"); -} - #ifndef __NO_MAIN__ int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); diff --git a/src/libaktualizr/uptane/uptane_delegation_test.cc b/src/libaktualizr/uptane/uptane_delegation_test.cc index 7d61bb1bfa..901962e26d 100644 --- a/src/libaktualizr/uptane/uptane_delegation_test.cc +++ b/src/libaktualizr/uptane/uptane_delegation_test.cc @@ -30,7 +30,9 @@ class HttpFakeDelegationBasic : public HttpFake { unsigned int events_seen{0}; }; -/* Correlation ID is empty if none was provided in targets metadata. */ +/* Validate first-order target delegations. + * Search first-order delegations. + * Correlation ID is empty if none was provided in targets metadata. */ TEST(Delegation, Basic) { TemporaryDirectory temp_dir; auto http = std::make_shared(temp_dir.Path()); From 508e22a3016e5039062fbbd872454c322aba7988 Mon Sep 17 00:00:00 2001 From: Patrick Vacek Date: Tue, 29 Jan 2019 16:03:12 +0100 Subject: [PATCH 17/19] Rename Delegated to Delegation. Signed-off-by: Patrick Vacek --- src/libaktualizr/primary/sotauptaneclient.cc | 4 ++-- src/libaktualizr/uptane/imagesrepository.cc | 2 +- src/libaktualizr/uptane/role.cc | 2 +- src/libaktualizr/uptane/tuf.cc | 2 +- src/libaktualizr/uptane/tuf.h | 8 ++++---- src/libaktualizr/uptane/tuf_test.cc | 10 +++++----- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/libaktualizr/primary/sotauptaneclient.cc b/src/libaktualizr/primary/sotauptaneclient.cc index 949245815b..4b1835ae7b 100644 --- a/src/libaktualizr/primary/sotauptaneclient.cc +++ b/src/libaktualizr/primary/sotauptaneclient.cc @@ -589,7 +589,7 @@ bool SotaUptaneClient::updateImagesMeta() { // Update Images delegated Targets Metadata for (const std::string &delegation : images_repo.delegations("targets")) { std::string images_delegated; - Uptane::Role delegated_role = Uptane::Role::Delegated(delegation); + Uptane::Role delegated_role = Uptane::Role::Delegation(delegation); if (!uptane_fetcher->fetchLatestRole(&images_delegated, Uptane::kMaxImagesTargetsSize, Uptane::RepositoryType::Image(), delegated_role)) { @@ -746,7 +746,7 @@ bool SotaUptaneClient::checkImagesMetaOffline() { // Update Images delegated Targets Metadata for (const std::string &delegation : images_repo.delegations("targets")) { std::string images_delegated; - Uptane::Role delegated_role = Uptane::Role::Delegated(delegation); + Uptane::Role delegated_role = Uptane::Role::Delegation(delegation); if (!storage->loadDelegation(&images_delegated, delegated_role)) { return false; } diff --git a/src/libaktualizr/uptane/imagesrepository.cc b/src/libaktualizr/uptane/imagesrepository.cc index 6b45cde9f3..5e308229c7 100644 --- a/src/libaktualizr/uptane/imagesrepository.cc +++ b/src/libaktualizr/uptane/imagesrepository.cc @@ -130,7 +130,7 @@ std::unique_ptr ImagesRepository::getTarget(const Uptane::Target if (it == toplevel.targets.cend()) { // Check if the target matches any of the delegation paths. for (const auto& delegate_name : toplevel.delegated_role_names_) { - Role delegate_role = Role::Delegated(delegate_name); + Role delegate_role = Role::Delegation(delegate_name); std::vector patterns = toplevel.paths_for_role_[delegate_role]; for (const auto& pattern : patterns) { if (fnmatch(pattern.c_str(), director_target.filename().c_str(), 0) == 0) { diff --git a/src/libaktualizr/uptane/role.cc b/src/libaktualizr/uptane/role.cc index 0f7bdd6815..928c951e1a 100644 --- a/src/libaktualizr/uptane/role.cc +++ b/src/libaktualizr/uptane/role.cc @@ -18,7 +18,7 @@ Role::Role(const std::string &role_name, const bool delegation) { if (IsReserved(name_)) { throw Uptane::Exception("", "Delegated role name " + role_name + " is reserved."); } - role_ = RoleEnum::kDelegated; + role_ = RoleEnum::kDelegation; name_ = role_name; } else if (role_name_lower == ROOT) { role_ = RoleEnum::kRoot; diff --git a/src/libaktualizr/uptane/tuf.cc b/src/libaktualizr/uptane/tuf.cc index ef7625fc88..a2b1bead31 100644 --- a/src/libaktualizr/uptane/tuf.cc +++ b/src/libaktualizr/uptane/tuf.cc @@ -279,7 +279,7 @@ void Uptane::Targets::init(const Json::Value &json) { const Json::Value role_list = json["signed"]["delegations"]["roles"]; for (Json::ValueIterator it = role_list.begin(); it != role_list.end(); it++) { const std::string role_name = (*it)["name"].asString(); - const Role role = Role::Delegated(role_name); + const Role role = Role::Delegation(role_name); delegated_role_names_.insert(role_name); ParseRole(Uptane::RepositoryType::Image(), it, role, name_); diff --git a/src/libaktualizr/uptane/tuf.h b/src/libaktualizr/uptane/tuf.h index 191caf325c..0d13dd9679 100644 --- a/src/libaktualizr/uptane/tuf.h +++ b/src/libaktualizr/uptane/tuf.h @@ -64,9 +64,9 @@ class Role { static Role Snapshot() { return Role{RoleEnum::kSnapshot}; } static Role Targets() { return Role{RoleEnum::kTargets}; } static Role Timestamp() { return Role{RoleEnum::kTimestamp}; } - static Role Delegated(const std::string &name) { return Role(name, true); } + static Role Delegation(const std::string &name) { return Role(name, true); } static Role InvalidRole() { return Role{RoleEnum::kInvalidRole}; } - // Delegated is not included because this is only used for a metadata table + // Delegation is not included because this is only used for a metadata table // that doesn't include delegations. static std::vector Roles() { return {Root(), Snapshot(), Targets(), Timestamp()}; } static bool IsReserved(const std::string &name) { @@ -76,7 +76,7 @@ class Role { explicit Role(const std::string &role_name, bool delegation = false); std::string ToString() const; int ToInt() const { return static_cast(role_); } - bool IsDelegation() const { return role_ == RoleEnum::kDelegated; } + bool IsDelegation() const { return role_ == RoleEnum::kDelegation; } bool operator==(const Role &other) const { return name_ == other.name_; } bool operator!=(const Role &other) const { return !(*this == other); } bool operator<(const Role &other) const { return name_ < other.name_; } @@ -86,7 +86,7 @@ class Role { private: /** The four standard roles must match the meta_types table in sqlstorage. * Delegations are special and handled differently. */ - enum class RoleEnum { kRoot = 0, kSnapshot = 1, kTargets = 2, kTimestamp = 3, kDelegated = 4, kInvalidRole = -1 }; + enum class RoleEnum { kRoot = 0, kSnapshot = 1, kTargets = 2, kTimestamp = 3, kDelegation = 4, kInvalidRole = -1 }; explicit Role(RoleEnum role) : role_(role) { if (role_ == RoleEnum::kRoot) { diff --git a/src/libaktualizr/uptane/tuf_test.cc b/src/libaktualizr/uptane/tuf_test.cc index 248d232f78..2080d0848e 100644 --- a/src/libaktualizr/uptane/tuf_test.cc +++ b/src/libaktualizr/uptane/tuf_test.cc @@ -72,17 +72,17 @@ TEST(Role, ValidateRoles) { /* Delegated roles have custom names. */ TEST(Role, ValidateDelegation) { - Uptane::Role delegated = Uptane::Role::Delegated("whatever"); + Uptane::Role delegated = Uptane::Role::Delegation("whatever"); EXPECT_EQ(delegated.ToString(), "whatever"); EXPECT_EQ(delegated.IsDelegation(), true); } /* Reject delegated role names that are identical to reserved role names. */ TEST(Role, InvalidDelegationName) { - EXPECT_THROW(Uptane::Role::Delegated("root"), Uptane::Exception); - EXPECT_THROW(Uptane::Role::Delegated("snapshot"), Uptane::Exception); - EXPECT_THROW(Uptane::Role::Delegated("targets"), Uptane::Exception); - EXPECT_THROW(Uptane::Role::Delegated("timestamp"), Uptane::Exception); + EXPECT_THROW(Uptane::Role::Delegation("root"), Uptane::Exception); + EXPECT_THROW(Uptane::Role::Delegation("snapshot"), Uptane::Exception); + EXPECT_THROW(Uptane::Role::Delegation("targets"), Uptane::Exception); + EXPECT_THROW(Uptane::Role::Delegation("timestamp"), Uptane::Exception); } #ifndef __NO_MAIN__ From e655ae58f46b7673cffc4b50a380f6d7d657e1db Mon Sep 17 00:00:00 2001 From: Patrick Vacek Date: Tue, 29 Jan 2019 16:19:58 +0100 Subject: [PATCH 18/19] Pass Roles by reference after all. Based on clang-tidy's advice. Signed-off-by: Patrick Vacek --- src/libaktualizr/uptane/imagesrepository.cc | 2 +- src/libaktualizr/uptane/imagesrepository.h | 2 +- src/libaktualizr/uptane/metawithkeys.cc | 4 ++-- src/libaktualizr/uptane/root.cc | 2 +- src/libaktualizr/uptane/tuf.cc | 4 ++-- src/libaktualizr/uptane/tuf.h | 11 ++++++----- 6 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/libaktualizr/uptane/imagesrepository.cc b/src/libaktualizr/uptane/imagesrepository.cc index 5e308229c7..2b743c74e4 100644 --- a/src/libaktualizr/uptane/imagesrepository.cc +++ b/src/libaktualizr/uptane/imagesrepository.cc @@ -65,7 +65,7 @@ bool ImagesRepository::verifySnapshot(const std::string& snapshot_raw) { return true; } -bool ImagesRepository::verifyTargets(const std::string& targets_raw, const Uptane::Role role) { +bool ImagesRepository::verifyTargets(const std::string& targets_raw, const Uptane::Role& role) { try { const Json::Value targets_json = Utils::parseJSON(targets_raw); const std::string canonical = Utils::jsonToCanonicalStr(targets_json); diff --git a/src/libaktualizr/uptane/imagesrepository.h b/src/libaktualizr/uptane/imagesrepository.h index 46a7c8ff03..c23975687d 100644 --- a/src/libaktualizr/uptane/imagesrepository.h +++ b/src/libaktualizr/uptane/imagesrepository.h @@ -14,7 +14,7 @@ class ImagesRepository : public RepositoryCommon { void resetMeta(); - bool verifyTargets(const std::string& targets_raw, Uptane::Role role); + bool verifyTargets(const std::string& targets_raw, const Uptane::Role& role); bool targetsExpired(const std::string& role_name) { return targets[role_name].isExpired(TimeStamp::Now()); } int64_t targetsSize() { return snapshot.targets_size(); } std::set delegations(const std::string& role_name) { return targets[role_name].delegated_role_names_; } diff --git a/src/libaktualizr/uptane/metawithkeys.cc b/src/libaktualizr/uptane/metawithkeys.cc index 1b6c975ac7..f292407d8e 100644 --- a/src/libaktualizr/uptane/metawithkeys.cc +++ b/src/libaktualizr/uptane/metawithkeys.cc @@ -5,7 +5,7 @@ using Uptane::MetaWithKeys; MetaWithKeys::MetaWithKeys(const Json::Value &json) : BaseMeta(json) {} -MetaWithKeys::MetaWithKeys(RepositoryType repo, const Role role, const Json::Value &json, +MetaWithKeys::MetaWithKeys(RepositoryType repo, const Role &role, const Json::Value &json, const std::shared_ptr &signer) : BaseMeta(repo, role, json, signer) {} @@ -55,7 +55,7 @@ void Uptane::MetaWithKeys::ParseRole(const RepositoryType repo, const Json::Valu } } -void Uptane::MetaWithKeys::UnpackSignedObject(const RepositoryType repo, const Role role, +void Uptane::MetaWithKeys::UnpackSignedObject(const RepositoryType repo, const Role &role, const Json::Value &signed_object) { const std::string repository = repo; diff --git a/src/libaktualizr/uptane/root.cc b/src/libaktualizr/uptane/root.cc index 99b71a0eea..d1ca94b59e 100644 --- a/src/libaktualizr/uptane/root.cc +++ b/src/libaktualizr/uptane/root.cc @@ -26,7 +26,7 @@ Root::Root(const RepositoryType repo, const Json::Value &json) : MetaWithKeys(js } } -void Uptane::Root::UnpackSignedObject(const RepositoryType repo, const Role role, const Json::Value &signed_object) { +void Uptane::Root::UnpackSignedObject(const RepositoryType repo, const Role &role, const Json::Value &signed_object) { const std::string repository = repo; if (policy_ == Policy::kAcceptAll) { diff --git a/src/libaktualizr/uptane/tuf.cc b/src/libaktualizr/uptane/tuf.cc index a2b1bead31..4691fc7be0 100644 --- a/src/libaktualizr/uptane/tuf.cc +++ b/src/libaktualizr/uptane/tuf.cc @@ -250,7 +250,7 @@ void Uptane::BaseMeta::init(const Json::Value &json) { } Uptane::BaseMeta::BaseMeta(const Json::Value &json) { init(json); } -Uptane::BaseMeta::BaseMeta(RepositoryType repo, const Role role, const Json::Value &json, +Uptane::BaseMeta::BaseMeta(RepositoryType repo, const Role &role, const Json::Value &json, const std::shared_ptr &signer) { if (!json.isObject() || !json.isMember("signed")) { throw Uptane::InvalidMetadata("", "", "invalid metadata json"); @@ -303,7 +303,7 @@ void Uptane::Targets::init(const Json::Value &json) { Uptane::Targets::Targets(const Json::Value &json) : MetaWithKeys(json) { init(json); } -Uptane::Targets::Targets(RepositoryType repo, const Role role, const Json::Value &json, +Uptane::Targets::Targets(RepositoryType repo, const Role &role, const Json::Value &json, const std::shared_ptr &signer) : MetaWithKeys(repo, role, json, signer), name_(role.ToString()) { init(json); diff --git a/src/libaktualizr/uptane/tuf.h b/src/libaktualizr/uptane/tuf.h index 0d13dd9679..b9bc41ba69 100644 --- a/src/libaktualizr/uptane/tuf.h +++ b/src/libaktualizr/uptane/tuf.h @@ -309,7 +309,7 @@ class BaseMeta { public: BaseMeta() = default; explicit BaseMeta(const Json::Value &json); - BaseMeta(RepositoryType repo, Role role, const Json::Value &json, const std::shared_ptr &signer); + BaseMeta(RepositoryType repo, const Role &role, const Json::Value &json, const std::shared_ptr &signer); int version() const { return version_; } TimeStamp expiry() const { return expiry_; } bool isExpired(const TimeStamp &now) const { return expiry_.IsExpiredAt(now); } @@ -339,7 +339,8 @@ class MetaWithKeys : public BaseMeta { * @param json - The contents of the 'signed' portion */ MetaWithKeys(const Json::Value &json); - MetaWithKeys(RepositoryType repo, Role role, const Json::Value &json, const std::shared_ptr &signer); + MetaWithKeys(RepositoryType repo, const Role &role, const Json::Value &json, + const std::shared_ptr &signer); virtual ~MetaWithKeys() = default; @@ -361,7 +362,7 @@ class MetaWithKeys : public BaseMeta { * @param signed_object * @return */ - virtual void UnpackSignedObject(RepositoryType repo, Role role, const Json::Value &signed_object); + virtual void UnpackSignedObject(RepositoryType repo, const Role &role, const Json::Value &signed_object); bool operator==(const MetaWithKeys &rhs) const { return version_ == rhs.version_ && expiry_ == rhs.expiry_ && keys_ == rhs.keys_ && @@ -407,7 +408,7 @@ class Root : public MetaWithKeys { * @param signed_object * @return */ - void UnpackSignedObject(RepositoryType repo, Role role, const Json::Value &signed_object) override; + void UnpackSignedObject(RepositoryType repo, const Role &role, const Json::Value &signed_object) override; bool operator==(const Root &rhs) const { return version_ == rhs.version_ && expiry_ == rhs.expiry_ && keys_ == rhs.keys_ && @@ -423,7 +424,7 @@ class Root : public MetaWithKeys { class Targets : public MetaWithKeys { public: explicit Targets(const Json::Value &json); - Targets(RepositoryType repo, Role role, const Json::Value &json, const std::shared_ptr &signer); + Targets(RepositoryType repo, const Role &role, const Json::Value &json, const std::shared_ptr &signer); Targets() = default; ~Targets() override = default; From e51c53282e5d201a43d670b75b0e431b78d292c7 Mon Sep 17 00:00:00 2001 From: Patrick Vacek Date: Wed, 30 Jan 2019 12:27:47 +0100 Subject: [PATCH 19/19] Fix review comments. * Simplify SQL logic for delegations. * Reduce if/for nesting for target searching. Signed-off-by: Patrick Vacek --- src/libaktualizr/storage/sqlstorage.cc | 17 +++----- src/libaktualizr/uptane/imagesrepository.cc | 48 ++++++++++----------- 2 files changed, 29 insertions(+), 36 deletions(-) diff --git a/src/libaktualizr/storage/sqlstorage.cc b/src/libaktualizr/storage/sqlstorage.cc index 7d9fe848f5..a143a059fe 100644 --- a/src/libaktualizr/storage/sqlstorage.cc +++ b/src/libaktualizr/storage/sqlstorage.cc @@ -522,17 +522,10 @@ void SQLStorage::storeDelegation(const std::string& data, const Uptane::Role rol return; } - auto del_statement = db.prepareStatement("DELETE FROM delegations WHERE role_name=?;", role.ToString()); + auto statement = db.prepareStatement("INSERT OR REPLACE INTO delegations VALUES (?, ?);", + SQLBlob(data), role.ToString()); - if (del_statement.step() != SQLITE_DONE) { - LOG_ERROR << "Can't clear delegation metadata: " << db.errmsg(); - return; - } - - auto ins_statement = db.prepareStatement("INSERT INTO delegations VALUES (?, ?);", - SQLBlob(data), role.ToString()); - - if (ins_statement.step() != SQLITE_DONE) { + if (statement.step() != SQLITE_DONE) { LOG_ERROR << "Can't add delegation metadata: " << db.errmsg(); return; } @@ -543,8 +536,8 @@ void SQLStorage::storeDelegation(const std::string& data, const Uptane::Role rol bool SQLStorage::loadDelegation(std::string* data, const Uptane::Role role) { SQLite3Guard db = dbConnection(); - auto statement = db.prepareStatement( - "SELECT meta FROM delegations WHERE role_name=? ORDER BY role_name DESC LIMIT 1;", role.ToString()); + auto statement = + db.prepareStatement("SELECT meta FROM delegations WHERE role_name=? LIMIT 1;", role.ToString()); int result = statement.step(); if (result == SQLITE_DONE) { diff --git a/src/libaktualizr/uptane/imagesrepository.cc b/src/libaktualizr/uptane/imagesrepository.cc index 2b743c74e4..cd96414812 100644 --- a/src/libaktualizr/uptane/imagesrepository.cc +++ b/src/libaktualizr/uptane/imagesrepository.cc @@ -127,33 +127,33 @@ std::unique_ptr ImagesRepository::getTarget(const Uptane::Target // Search for the target in the top-level targets metadata. Uptane::Targets toplevel = targets["targets"]; auto it = std::find(toplevel.targets.cbegin(), toplevel.targets.cend(), director_target); - if (it == toplevel.targets.cend()) { - // Check if the target matches any of the delegation paths. - for (const auto& delegate_name : toplevel.delegated_role_names_) { - Role delegate_role = Role::Delegation(delegate_name); - std::vector patterns = toplevel.paths_for_role_[delegate_role]; - for (const auto& pattern : patterns) { - if (fnmatch(pattern.c_str(), director_target.filename().c_str(), 0) == 0) { - // Match! Check delegation. - Uptane::Targets delegate = targets[delegate_name]; - auto d_it = std::find(delegate.targets.cbegin(), delegate.targets.cend(), director_target); - if (d_it == delegate.targets.cend()) { - // TODO: recurse here if the delegation is non-terminating. For now, - // assume that all delegations are terminating. - if (!toplevel.terminating_role_[delegate_role]) { - LOG_ERROR << "Nested delegations are not currently supported."; - } - return std::unique_ptr(nullptr); - } else { - return std_::make_unique(*d_it); - } - } + if (it != toplevel.targets.cend()) { + return std_::make_unique(*it); + } + + // Check if the target matches any of the delegation paths. + for (const auto& delegate_name : toplevel.delegated_role_names_) { + Role delegate_role = Role::Delegation(delegate_name); + std::vector patterns = toplevel.paths_for_role_[delegate_role]; + for (const auto& pattern : patterns) { + if (fnmatch(pattern.c_str(), director_target.filename().c_str(), 0) != 0) { + continue; + } + // Match! Check delegation. + Uptane::Targets delegate = targets[delegate_name]; + auto d_it = std::find(delegate.targets.cbegin(), delegate.targets.cend(), director_target); + if (d_it != delegate.targets.cend()) { + return std_::make_unique(*d_it); } + // TODO: recurse here if the delegation is non-terminating. For now, + // assume that all delegations are terminating. + if (!toplevel.terminating_role_[delegate_role]) { + LOG_ERROR << "Nested delegations are not currently supported."; + } + return std::unique_ptr(nullptr); } - return std::unique_ptr(nullptr); - } else { - return std_::make_unique(*it); } + return std::unique_ptr(nullptr); } } // namespace Uptane