Skip to content
This repository has been archived by the owner on May 21, 2024. It is now read-only.

OTA-3969: Do the full verification of metadata on the IP Secondary #1449

Merged
merged 2 commits into from
Nov 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/aktualizr_secondary/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ set(AKTUALIZR_SECONDARY_LIB_SRC
aktualizr_secondary.cc
aktualizr_secondary_config.cc
aktualizr_secondary_common.cc
aktualizr_secondary_metadata.cc
socket_server.cc
)

Expand Down Expand Up @@ -40,6 +41,7 @@ set(ALL_AKTUALIZR_SECONDARY_HEADERS
aktualizr_secondary_interface.h
aktualizr_secondary_config.h
aktualizr_secondary_common.h
aktualizr_secondary_metadata.h
socket_server.h
)

Expand All @@ -51,6 +53,11 @@ list(INSERT TEST_LIBS 0 aktualizr_secondary_static_lib)
add_aktualizr_test(NAME aktualizr_secondary_config
SOURCES aktualizr_secondary_config_test.cc PROJECT_WORKING_DIRECTORY)

add_aktualizr_test(NAME aktualizr_secondary_uptane_verification
SOURCES uptane_verification_test.cc
ARGS "$<TARGET_FILE:uptane-generator>"
PROJECT_WORKING_DIRECTORY)

add_aktualizr_test(NAME aktualizr_secondary_update
SOURCES update_test.cc
ARGS ${PROJECT_BINARY_DIR}/ostree_repo PROJECT_WORKING_DIRECTORY)
Expand Down
116 changes: 78 additions & 38 deletions src/aktualizr_secondary/aktualizr_secondary.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ class SecondaryAdapter : public Uptane::SecondaryInterface {
Uptane::HardwareIdentifier getHwId() override { return secondary.getHwIdResp(); }
PublicKey getPublicKey() override { return secondary.getPublicKeyResp(); }
Json::Value getManifest() override { return secondary.getManifestResp(); }
bool putMetadata(const Uptane::RawMetaPack& meta_pack) override { return secondary.putMetadataResp(meta_pack); }
bool putMetadata(const Uptane::RawMetaPack& meta_pack) override {
return secondary.putMetadataResp(Metadata(meta_pack));
}
int32_t getRootVersion(bool director) override { return secondary.getRootVersionResp(director); }
bool putRoot(const std::string& root, bool director) override { return secondary.putRootResp(root, director); }
bool sendFirmware(const std::shared_ptr<std::string>& data) override {
Expand Down Expand Up @@ -63,36 +65,7 @@ Json::Value AktualizrSecondary::getManifestResp() const {
return keys_.signTuf(manifest);
}

bool AktualizrSecondary::putMetadataResp(const Uptane::RawMetaPack& meta_pack) {
TimeStamp now(TimeStamp::Now());
detected_attack_.clear();

// TODO: proper partial verification
root_ = Uptane::Root(Uptane::RepositoryType::Director(), Utils::parseJSON(meta_pack.director_root), root_);
Uptane::Targets targets(Uptane::RepositoryType::Director(), Uptane::Role::Targets(),
Utils::parseJSON(meta_pack.director_targets), std::make_shared<Uptane::Root>(root_));
if (meta_targets_.version() > targets.version()) {
detected_attack_ = "Rollback attack detected";
return true;
}
meta_targets_ = targets;
std::vector<Uptane::Target>::const_iterator it;
bool target_found = false;
for (it = meta_targets_.targets.begin(); it != meta_targets_.targets.end(); ++it) {
if (it->IsForSecondary(getSerialResp())) {
if (target_found) {
detected_attack_ = "Duplicate entry for this ECU";
break;
}
target_found = true;
target_ = std_::make_unique<Uptane::Target>(*it);
}
}
storage_->storeRoot(meta_pack.director_root, Uptane::RepositoryType::Director(), Uptane::Version(root_.version()));
storage_->storeNonRoot(meta_pack.director_targets, Uptane::RepositoryType::Director(), Uptane::Role::Targets());

return true;
}
bool AktualizrSecondary::putMetadataResp(const Metadata& metadata) { return doFullVerification(metadata); }

int32_t AktualizrSecondary::getRootVersionResp(bool director) const {
std::string root_meta;
Expand All @@ -113,14 +86,17 @@ bool AktualizrSecondary::putRootResp(const std::string& root, bool director) {
}

bool AktualizrSecondary::sendFirmwareResp(const std::shared_ptr<std::string>& firmware) {
if (target_ == nullptr) {
LOG_ERROR << "No valid installation target found";
auto targetsForThisEcu = director_repo_.getTargets(getSerial());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be rechecking the metadata (without fetching) like we do on the Primary before installation? It seems redundant, but it's a protection against unexpected changes in the time since the metadata was received.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree, we should check at least an image file size (binary case) and its hash (for both ostree and binary).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's also worth rechecking the whole metadata chain from the metadata, but yeah, checking the object's hash/size is even more important.


if (targetsForThisEcu.size() != 1) {
LOG_ERROR << "Invalid number of targets (should be one): " << targetsForThisEcu.size();
return false;
}
auto targetToApply = targetsForThisEcu[0];

std::string treehub_server;

if (target_->IsOstree()) {
if (targetToApply.IsOstree()) {
// this is the ostree specific case
try {
std::string ca, cert, pkey, server_url;
Expand All @@ -137,9 +113,9 @@ bool AktualizrSecondary::sendFirmwareResp(const std::shared_ptr<std::string>& fi

data::InstallationResult install_res;

if (target_->IsOstree()) {
if (targetToApply.IsOstree()) {
#ifdef BUILD_OSTREE
install_res = OstreeManager::pull(config_.pacman.sysroot, treehub_server, keys_, *target_);
install_res = OstreeManager::pull(config_.pacman.sysroot, treehub_server, keys_, targetToApply);

if (install_res.result_code.num_code != data::ResultCode::Numeric::kOk) {
LOG_ERROR << "Could not pull from OSTree (" << install_res.result_code.toString()
Expand All @@ -156,12 +132,19 @@ bool AktualizrSecondary::sendFirmwareResp(const std::shared_ptr<std::string>& fi
return false;
}

install_res = pacman->install(*target_);
// This check is not applicable for the ostree case, just for the binary file case(s)
if (targetToApply.length() != firmware->length()) {
LOG_ERROR << "The target image size specified in metadata " << targetToApply.length()
<< " does not match actual size " << firmware->length();
return false;
}

install_res = pacman->install(targetToApply);
if (install_res.result_code.num_code != data::ResultCode::Numeric::kOk) {
LOG_ERROR << "Could not install target (" << install_res.result_code.toString() << "): " << install_res.description;
return false;
}
storage_->saveInstalledVersion(getSerialResp().ToString(), *target_, InstalledVersionUpdateMode::kCurrent);
storage_->saveInstalledVersion(getSerialResp().ToString(), targetToApply, InstalledVersionUpdateMode::kCurrent);
return true;
}

Expand Down Expand Up @@ -200,3 +183,60 @@ void AktualizrSecondary::connectToPrimary() {
LOG_INFO << "Failed to connect to Primary";
}
}

bool AktualizrSecondary::doFullVerification(const Metadata& metadata) {
// 5.4.4.2. Full verification https://uptane.github.io/uptane-standard/uptane-standard.html#metadata_verification

// 1. Load and verify the current time or the most recent securely attested time.
//
// We trust the time that the given system/OS/ECU provides, In ECU We Trust :)
TimeStamp now(TimeStamp::Now());

// 2. Download and check the Root metadata file from the Director repository, following the procedure in
// Section 5.4.4.3. DirectorRepository::updateMeta() method implements this verification step, certain steps are
// missing though. see the method source code for details

// 3. NOT SUPPORTED: Download and check the Timestamp metadata file from the Director repository, following the
// procedure in Section 5.4.4.4.
// 4. NOT SUPPORTED: Download and check the Snapshot metadata file from the Director repository, following the
// procedure in Section 5.4.4.5.
//
// 5. Download and check the Targets metadata file from the Director repository, following the procedure in
// Section 5.4.4.6. DirectorRepository::updateMeta() method implements this verification step
//
// The followin steps of the Director's target metadata verification are missing in DirectorRepository::updateMeta()
// 6. If checking Targets metadata from the Director repository, verify that there are no delegations.
// 7. If checking Targets metadata from the Director repository, check that no ECU identifier is represented more
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make a tech debt ticket for this and put it in the robustness epic?

It looks like you've already implemented the check for the secondary; is your proposal to move that check into the Uptane code directly so it is applied universally as early as is reasonable?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I'll do it.

I did #7, not #6, and yes my proposal to do apply it universally.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood. We implicitly do step 6 in that we ignore the concept of delegations from the Director, but it's not a bad idea to check for them and throw a warning if found. Since we don't use the Snapshot role on the Director, though, all we can do is just check for them in the Targets metadata.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// than once.
if (!director_repo_.updateMeta(*storage_, metadata)) {
LOG_ERROR << "Failed to update director metadata: " << director_repo_.getLastException().what();
return false;
}

auto targetsForThisEcu = director_repo_.getTargets(getSerial());

if (targetsForThisEcu.size() != 1) {
LOG_ERROR << "Invalid number of targets (should be one): " << targetsForThisEcu.size();
return false;
}

// 6. Download and check the Root metadata file from the Image repository, following the procedure in Section 5.4.4.3.
// 7. Download and check the Timestamp metadata file from the Image repository, following the procedure in
// Section 5.4.4.4.
// 8. Download and check the Snapshot metadata file from the Image repository, following the procedure in
// Section 5.4.4.5.
// 9. Download and check the top-level Targets metadata file from the Image repository, following the procedure in
// Section 5.4.4.6.
if (!image_repo_.updateMeta(*storage_, metadata)) {
LOG_ERROR << "Failed to update image metadata: " << image_repo_.getLastException().what();
return false;
}

// 10. Verify that Targets metadata from the Director and Image repositories match.
if (!(director_repo_.getTargets() == *image_repo_.getTargets())) {
LOG_ERROR << "Targets metadata from the Director and Image repositories DOES NOT match ";
return false;
}

return true;
}
12 changes: 10 additions & 2 deletions src/aktualizr_secondary/aktualizr_secondary.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,15 @@
#include "crypto/keymanager.h"
#include "socket_server.h"
#include "storage/invstorage.h"
#include "uptane/tuf.h"
#include "utilities/types.h"
#include "utilities/utils.h"

#include "uptane/directorrepository.h"
#include "uptane/imagesrepository.h"
#include "uptane/tuf.h"

#include "aktualizr_secondary_metadata.h"

class AktualizrSecondary : public AktualizrSecondaryInterface, private AktualizrSecondaryCommon {
public:
AktualizrSecondary(const AktualizrSecondaryConfig& config, const std::shared_ptr<INvStorage>& storage);
Expand All @@ -24,7 +29,7 @@ class AktualizrSecondary : public AktualizrSecondaryInterface, private Aktualizr
Uptane::HardwareIdentifier getHwIdResp() const;
PublicKey getPublicKeyResp() const;
Json::Value getManifestResp() const;
bool putMetadataResp(const Uptane::RawMetaPack& meta_pack);
bool putMetadataResp(const Metadata& metadata);
int32_t getRootVersionResp(bool director) const;
bool putRootResp(const std::string& root, bool director);
bool sendFirmwareResp(const std::shared_ptr<std::string>& firmware);
Expand All @@ -34,9 +39,12 @@ class AktualizrSecondary : public AktualizrSecondaryInterface, private Aktualizr

private:
void connectToPrimary();
bool doFullVerification(const Metadata& metadata);

private:
SocketServer socket_server_;
Uptane::DirectorRepository director_repo_;
Uptane::ImagesRepository image_repo_;
};

#endif // AKTUALIZR_SECONDARY_H
7 changes: 3 additions & 4 deletions src/aktualizr_secondary/aktualizr_secondary_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,16 @@ class AktualizrSecondaryCommon {

bool uptaneInitialize();

AktualizrSecondaryConfig config_;
const Uptane::EcuSerial& getSerial() const { return ecu_serial_; }

protected:
AktualizrSecondaryConfig config_;
std::shared_ptr<INvStorage> storage_;
KeyManager keys_;
Uptane::EcuSerial ecu_serial_;
Uptane::HardwareIdentifier hardware_id_;
std::shared_ptr<PackageManagerInterface> pacman;
Uptane::Root root_;
Uptane::Targets meta_targets_;
std::string detected_attack_;
std::unique_ptr<Uptane::Target> target_;
};

#endif // AKTUALIZR_SECONDARY_COMMON_H_
50 changes: 50 additions & 0 deletions src/aktualizr_secondary/aktualizr_secondary_metadata.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
#include "aktualizr_secondary_metadata.h"

Metadata::Metadata(const Uptane::RawMetaPack& meta_pack)
: _director_metadata{{Uptane::Role::ROOT, meta_pack.director_root},
{Uptane::Role::TARGETS, meta_pack.director_targets}},
_image_metadata{
{Uptane::Role::ROOT, meta_pack.image_root},
{Uptane::Role::TIMESTAMP, meta_pack.image_timestamp},
{Uptane::Role::SNAPSHOT, meta_pack.image_snapshot},
{Uptane::Role::TARGETS, meta_pack.image_targets},
} {}

bool Metadata::fetchRole(std::string* result, int64_t maxsize, Uptane::RepositoryType repo, const Uptane::Role& role,
Uptane::Version version) const {
(void)maxsize;
(void)version;

return getRoleMetadata(result, repo, role);
}

bool Metadata::fetchLatestRole(std::string* result, int64_t maxsize, Uptane::RepositoryType repo,
const Uptane::Role& role) const {
(void)maxsize;
return getRoleMetadata(result, repo, role);
}

bool Metadata::getRoleMetadata(std::string* result, const Uptane::RepositoryType& repo,
const Uptane::Role& role) const {
const std::unordered_map<std::string, std::string>* metadata_map = nullptr;

if (repo == Uptane::RepositoryType::Director()) {
metadata_map = &_director_metadata;
} else if (repo == Uptane::RepositoryType::Image()) {
metadata_map = &_image_metadata;
}

if (metadata_map == nullptr) {
LOG_ERROR << "There are no any metadata for the given type of repository: " << repo.toString();
return false;
}

auto found_meta_it = metadata_map->find(role.ToString());
if (found_meta_it == metadata_map->end()) {
LOG_ERROR << "There are no any metadata for the given type of role: " << repo.toString() << ": " << role.ToString();
return false;
}

*result = found_meta_it->second;
return true;
}
25 changes: 25 additions & 0 deletions src/aktualizr_secondary/aktualizr_secondary_metadata.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#ifndef AKTUALIZR_SECONDARY_METADATA_H_
#define AKTUALIZR_SECONDARY_METADATA_H_

#include <unordered_map>

#include "uptane/fetcher.h"

class Metadata : public Uptane::IMetadataFetcher {
public:
Metadata(const Uptane::RawMetaPack& meta_pack);

bool fetchRole(std::string* result, int64_t maxsize, Uptane::RepositoryType repo, const Uptane::Role& role,
Uptane::Version version) const override;
bool fetchLatestRole(std::string* result, int64_t maxsize, Uptane::RepositoryType repo,
const Uptane::Role& role) const override;

private:
bool getRoleMetadata(std::string* result, const Uptane::RepositoryType& repo, const Uptane::Role& role) const;

private:
const std::unordered_map<std::string, std::string> _director_metadata;
const std::unordered_map<std::string, std::string> _image_metadata;
};

#endif // AKTUALIZR_SECONDARY_METADATA_H_
Loading