pw_software_update: Fix issues
Gate all bundle accessor operations under successful verification. Also
update test cases for the changes and make them more closely mimic the
scenario of dev to prod rotation. Updated presubmit step dependencies so
that the unit tests will be run in CI/CQ.
Bug: 456
Change-Id: I88995ed8cd3ee2df9bcead7dabe58a1977c2a4f1
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/71880
Reviewed-by: Ali Zhang <alizhang@google.com>
Commit-Queue: Yecheng Zhao <zyecheng@google.com>
diff --git a/pw_presubmit/py/pw_presubmit/pigweed_presubmit.py b/pw_presubmit/py/pw_presubmit/pigweed_presubmit.py
index b1035d5..db8dbf4 100755
--- a/pw_presubmit/py/pw_presubmit/pigweed_presubmit.py
+++ b/pw_presubmit/py/pw_presubmit/pigweed_presubmit.py
@@ -220,13 +220,22 @@
def gn_software_update_build(ctx: PresubmitContext):
build.install_package(ctx.package_root, 'nanopb')
build.install_package(ctx.package_root, 'protobuf')
- build.gn_gen(ctx.root,
- ctx.output_dir,
- dir_pw_third_party_protobuf='"{}"'.format(ctx.package_root /
- 'protobuf'),
- dir_pw_third_party_nanopb='"{}"'.format(ctx.package_root /
- 'nanopb'),
- pw_software_update_LANDING_BUNDLE_VERIFICATION=False)
+ build.install_package(ctx.package_root, 'mbedtls')
+ build.install_package(ctx.package_root, 'micro-ecc')
+ build.gn_gen(
+ ctx.root,
+ ctx.output_dir,
+ dir_pw_third_party_protobuf='"{}"'.format(ctx.package_root /
+ 'protobuf'),
+ dir_pw_third_party_nanopb='"{}"'.format(ctx.package_root / 'nanopb'),
+ dir_pw_third_party_micro_ecc='"{}"'.format(ctx.package_root /
+ 'micro-ecc'),
+ pw_crypto_ECDSA_BACKEND='"{}"'.format(ctx.root /
+ 'pw_crypto:ecdsa_uecc'),
+ dir_pw_third_party_mbedtls='"{}"'.format(ctx.package_root / 'mbedtls'),
+ pw_crypto_SHA256_BACKEND='"{}"'.format(ctx.root /
+ 'pw_crypto:sha256_mbedtls'),
+ pw_software_update_LANDING_BUNDLE_VERIFICATION=False)
build.ninja(
ctx.output_dir,
*_at_all_optimization_levels('host_clang'),
diff --git a/pw_software_update/BUILD.gn b/pw_software_update/BUILD.gn
index 2ec95c7..bcdc0b1 100644
--- a/pw_software_update/BUILD.gn
+++ b/pw_software_update/BUILD.gn
@@ -182,14 +182,18 @@
"$dir_pw_kvs:fake_flash_test_key_value_store",
]
configs = [ ":generated_test_bundle_include" ]
+
+ defines = []
+ if (pw_software_update_LANDING_BUNDLE_VERIFICATION) {
+ defines += [ "PW_SOFTWARE_UPDATE_LANDING_BUNDLE_VERIFICATION" ]
+ }
}
pw_test_group("tests") {
- tests = [ ":bundled_update_service_test" ]
-
- if (!pw_software_update_LANDING_BUNDLE_VERIFICATION) {
- tests += [ ":update_bundle_test" ]
- }
+ tests = [
+ ":bundled_update_service_test",
+ ":update_bundle_test",
+ ]
}
pw_test("bundled_update_service_test") {
diff --git a/pw_software_update/public/pw_software_update/bundled_update_backend.h b/pw_software_update/public/pw_software_update/bundled_update_backend.h
index f071deb..4b0672b4 100644
--- a/pw_software_update/public/pw_software_update/bundled_update_backend.h
+++ b/pw_software_update/public/pw_software_update/bundled_update_backend.h
@@ -138,7 +138,7 @@
// TODO(pwbug/456): Investigate whether we should get a writer i.e.
// GetRootMetadataWriter() instead of passing a reader.
virtual Status SafelyPersistRootMetadata(
- [[maybe_unused]] stream::Reader& root_metadata) {
+ [[maybe_unused]] stream::IntervalReader root_metadata) {
return Status::Unimplemented();
};
};
diff --git a/pw_software_update/public/pw_software_update/update_bundle_accessor.h b/pw_software_update/public/pw_software_update/update_bundle_accessor.h
index b252291..aaf72bd 100644
--- a/pw_software_update/public/pw_software_update/update_bundle_accessor.h
+++ b/pw_software_update/public/pw_software_update/update_bundle_accessor.h
@@ -139,7 +139,12 @@
// TODO(pwbug/456): Figure out a way to propagate error.
stream::IntervalReader GetTargetPayload(std::string_view target_file);
- protobuf::Message GetDecoder() { return decoder_; }
+ // Returns a protobuf::Message representation of the update bundle.
+ //
+ // Returns:
+ // An instance of protobuf::Message of the udpate bundle.
+ // FAILED_PRECONDITION - Bundle is not open and verified.
+ protobuf::Message GetDecoder();
private:
blob_store::BlobStore& bundle_;
diff --git a/pw_software_update/py/pw_software_update/generate_test_bundle.py b/pw_software_update/py/pw_software_update/generate_test_bundle.py
index f3270e6..26ef2b1 100644
--- a/pw_software_update/py/pw_software_update/generate_test_bundle.py
+++ b/pw_software_update/py/pw_software_update/generate_test_bundle.py
@@ -58,12 +58,18 @@
8Ny2tXY+Qggzl77G7wvCNF5+koz7ecsV6sKjK+dFiAXOIdqlga7p2j0A
-----END PRIVATE KEY-----"""
-TEST_TARGETS_KEY = """-----BEGIN PRIVATE KEY-----
+TEST_TARGETS_DEV_KEY = """-----BEGIN PRIVATE KEY-----
MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQggRCrido5vZOnkULH
sxQDt9Qoe/TlEKoqa1bhO1HFbi6hRANCAASVwdXbGWM7+f/r+Z2W6Dbd7CQA0Cbb
pkBv5PnA+DZnCkFhLW2kTn89zQv8W1x4m9maoINp9QPXQ4/nXlrVHqDg
-----END PRIVATE KEY-----"""
+TEST_TARGETS_PROD_KEY = """-----BEGIN PRIVATE KEY-----
+MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgx2VdB2EsUKghuLMG
+RmxzqX2jnLTq5pxsFgO5Rrf5jlehRANCAASVijeDpemxVSlgZOOW0yvwE5QkXkq0
+geWonkusMP0+MXopnmN0QlpgaCnG40TSr/W+wFjRmNCklL4dXk01oCwD
+-----END PRIVATE KEY-----"""
+
TEST_ROOT_VERSION = 2
TEST_TARGETS_VERSION = 2
@@ -86,13 +92,20 @@
return byte_array_declaration(proto.SerializeToString(), name)
-def private_key_pem_bytes(key: ec.EllipticCurvePrivateKey) -> bytes:
- """Serializes a private key in PEM format"""
+def private_key_public_pem_bytes(key: ec.EllipticCurvePrivateKey) -> bytes:
+ """Serializes the public part of a private key in PEM format"""
return key.public_key().public_bytes(
serialization.Encoding.PEM,
serialization.PublicFormat.SubjectPublicKeyInfo)
+def private_key_private_pem_bytes(key: ec.EllipticCurvePrivateKey) -> bytes:
+ """Serializes the private part of a private key in PEM format"""
+ return key.private_bytes(encoding=serialization.Encoding.PEM,
+ format=serialization.PrivateFormat.PKCS8,
+ encryption_algorithm=serialization.NoEncryption())
+
+
class Bundle:
"""A helper for test UpdateBundle generation"""
def __init__(self):
@@ -100,8 +113,10 @@
TEST_DEV_KEY.encode(), None)
self._root_prod_key = serialization.load_pem_private_key(
TEST_PROD_KEY.encode(), None)
- self._targets_key = serialization.load_pem_private_key(
- TEST_TARGETS_KEY.encode(), None)
+ self._targets_dev_key = serialization.load_pem_private_key(
+ TEST_TARGETS_DEV_KEY.encode(), None)
+ self._targets_prod_key = serialization.load_pem_private_key(
+ TEST_TARGETS_PROD_KEY.encode(), None)
self._payloads: Dict[str, bytes] = {}
# Adds some update files.
for key, value in TARGET_FILES.items():
@@ -113,34 +128,39 @@
def generate_dev_root_metadata(self) -> RootMetadata:
"""Generates a root metadata with the dev key"""
+ # The dev root metadata contains both the prod and the dev public key,
+ # so that it can rotate to prod. But it will only use a dev targets
+ # key.
return root_metadata.gen_root_metadata(
- root_metadata.RootKeys([private_key_pem_bytes(self._root_dev_key)
- ]),
+ root_metadata.RootKeys([
+ private_key_public_pem_bytes(self._root_dev_key),
+ private_key_public_pem_bytes(self._root_prod_key),
+ ]),
root_metadata.TargetsKeys(
- [private_key_pem_bytes(self._targets_key)]), TEST_ROOT_VERSION)
+ [private_key_public_pem_bytes(self._targets_dev_key)]),
+ TEST_ROOT_VERSION)
def generate_prod_root_metadata(self) -> RootMetadata:
"""Generates a root metadata with the prod key"""
+ # The prod root metadta contains only the prod public key and uses the
+ # prod targets key
return root_metadata.gen_root_metadata(
root_metadata.RootKeys(
- [private_key_pem_bytes(self._root_prod_key)]),
+ [private_key_public_pem_bytes(self._root_prod_key)]),
root_metadata.TargetsKeys(
- [private_key_pem_bytes(self._targets_key)]), TEST_ROOT_VERSION)
+ [private_key_public_pem_bytes(self._targets_prod_key)]),
+ TEST_ROOT_VERSION)
def generate_dev_signed_root_metadata(self) -> SignedRootMetadata:
- """Generates a root metadata signed only by the dev key"""
+ """Generates a dev signed root metadata"""
signed_root = SignedRootMetadata()
root_metadata_proto = self.generate_dev_root_metadata()
signed_root.serialized_root_metadata = \
root_metadata_proto.SerializeToString()
return dev_sign.sign_root_metadata(
- signed_root,
- self._root_dev_key.private_bytes(
- encoding=serialization.Encoding.PEM,
- format=serialization.PrivateFormat.PKCS8,
- encryption_algorithm=serialization.NoEncryption()))
+ signed_root, private_key_private_pem_bytes(self._root_dev_key))
- def generate_rotation_prod_signed_root_metadata(
+ def generate_prod_signed_root_metadata(
self,
root_metadata_proto: RootMetadata = None) -> SignedRootMetadata:
"""Generates a root metadata signed by the prod key"""
@@ -150,16 +170,8 @@
signed_root = SignedRootMetadata(
serialized_root_metadata=root_metadata_proto.SerializeToString())
- for key in [self._root_dev_key, self._root_prod_key]:
- signature = keys.create_ecdsa_signature(
- signed_root.serialized_root_metadata,
- key.private_bytes(
- encoding=serialization.Encoding.PEM,
- format=serialization.PrivateFormat.PKCS8,
- encryption_algorithm=serialization.NoEncryption()))
- signed_root.signatures.append(signature)
-
- return signed_root
+ return dev_sign.sign_root_metadata(
+ signed_root, private_key_private_pem_bytes(self._root_prod_key))
def generate_targets_metadata(self) -> TargetsMetadata:
"""Generates the targets metadata"""
@@ -168,11 +180,11 @@
TEST_TARGETS_VERSION)
return targets
- def generate_bundle(
+ def generate_unsigned_bundle(
self,
targets_metadata: TargetsMetadata = None,
signed_root_metadata: SignedRootMetadata = None) -> UpdateBundle:
- """Generate an update bundle"""
+ """Generate an unsigned (targets metadata) update bundle"""
bundle = UpdateBundle()
if not targets_metadata:
@@ -185,18 +197,34 @@
SignedTargetsMetadata(serialized_targets_metadata=targets_metadata.
SerializeToString()))
- bundle = dev_sign.sign_update_bundle(
- bundle,
- self._targets_key.private_bytes(
- encoding=serialization.Encoding.PEM,
- format=serialization.PrivateFormat.PKCS8,
- encryption_algorithm=serialization.NoEncryption()))
-
for name, payload in self._payloads.items():
bundle.target_payloads[name] = payload
return bundle
+ def generate_dev_signed_bundle(
+ self,
+ targets_metadata_override: TargetsMetadata = None,
+ signed_root_metadata: SignedRootMetadata = None) -> UpdateBundle:
+ """Generate a dev signed update bundle"""
+ return dev_sign.sign_update_bundle(
+ self.generate_unsigned_bundle(targets_metadata_override,
+ signed_root_metadata),
+ private_key_private_pem_bytes(self._targets_dev_key))
+
+ def generate_prod_signed_bundle(
+ self,
+ targets_metadata_override: TargetsMetadata = None,
+ signed_root_metadata: SignedRootMetadata = None) -> UpdateBundle:
+ """Generate a prod signed update bundle"""
+ # The targets metadata in a prod signed bundle can only be verified
+ # by a prod signed root. Because it is signed by the prod targets key.
+ # The prod signed root however, can be verified by a dev root.
+ return dev_sign.sign_update_bundle(
+ self.generate_unsigned_bundle(targets_metadata_override,
+ signed_root_metadata),
+ private_key_private_pem_bytes(self._targets_prod_key))
+
def generate_manifest(self) -> Manifest:
"""Generates the manifest"""
manifest = Manifest()
@@ -223,45 +251,53 @@
test_bundle = Bundle()
dev_signed_root = test_bundle.generate_dev_signed_root_metadata()
- dev_signed_bundle = test_bundle.generate_bundle(None, dev_signed_root)
+ dev_signed_bundle = test_bundle.generate_dev_signed_bundle()
manifest_proto = test_bundle.generate_manifest()
prod_signed_root = \
- test_bundle.generate_rotation_prod_signed_root_metadata()
- prod_signed_bundle = test_bundle.generate_bundle(None, prod_signed_root)
+ test_bundle.generate_prod_signed_root_metadata()
+ prod_signed_bundle = test_bundle.generate_prod_signed_bundle(
+ None, prod_signed_root)
# Generates a prod root metadata that fails signature verification against
- # the dev root (i.e. it has a bad dev signature).
- bad_dev_signature = test_bundle.generate_prod_root_metadata()
- signed_bad_dev_signature = \
- test_bundle\
- .generate_rotation_prod_signed_root_metadata(bad_dev_signature)
- # Compromises the first signature, which is dev signed.
- signed_bad_dev_signature.signatures[0].sig = b'1' * 64
- signed_bad_dev_signature_bundle = test_bundle.generate_bundle(
- None, signed_bad_dev_signature)
-
- # Generates a prod root metadtata that fails to verify itself (bad prod
- # signature).
+ # the dev root (i.e. it has a bad prod signature). This is done by making
+ # a bad prod signature.
bad_prod_signature = test_bundle.generate_prod_root_metadata()
signed_bad_prod_signature = \
test_bundle\
- .generate_rotation_prod_signed_root_metadata(
+ .generate_prod_signed_root_metadata(
bad_prod_signature)
- # Compromises the second signature, which is prod signed.
- signed_bad_prod_signature.signatures[1].sig = b'1' * 64
- signed_bad_prod_signature_bundle = test_bundle.generate_bundle(
+ # Compromises the signature.
+ signed_bad_prod_signature.signatures[0].sig = b'1' * 64
+ signed_bad_prod_signature_bundle = test_bundle.generate_prod_signed_bundle(
None, signed_bad_prod_signature)
+ # Generates a prod root metadtata that fails to verify itself. Specifically,
+ # the prod signature cannot be verified by the key in the incoming root
+ # metadata. This is done by dev signing a prod root metadata.
+ signed_mismatched_root_key_and_signature = SignedRootMetadata(
+ serialized_root_metadata=test_bundle.generate_prod_root_metadata(
+ ).SerializeToString())
+ dev_root_key = serialization.load_pem_private_key(TEST_DEV_KEY.encode(),
+ None)
+ signature = keys.create_ecdsa_signature(
+ signed_mismatched_root_key_and_signature.serialized_root_metadata,
+ private_key_private_pem_bytes(dev_root_key)) # type: ignore
+ signed_mismatched_root_key_and_signature.signatures.append(signature)
+ mismatched_root_key_and_signature_bundle = test_bundle\
+ .generate_prod_signed_bundle(None,
+ signed_mismatched_root_key_and_signature)
+
# Generates a prod root metadata with rollback attempt.
root_rollback = test_bundle.generate_prod_root_metadata()
root_rollback.common_metadata.version = TEST_ROOT_VERSION - 1
signed_root_rollback = test_bundle.\
- generate_rotation_prod_signed_root_metadata(root_rollback)
- root_rollback_bundle = test_bundle.generate_bundle(None,
- signed_root_rollback)
+ generate_prod_signed_root_metadata(root_rollback)
+ root_rollback_bundle = test_bundle.generate_prod_signed_bundle(
+ None, signed_root_rollback)
# Generates a bundle with a bad target signature.
- bad_targets_siganture = test_bundle.generate_bundle()
+ bad_targets_siganture = test_bundle.generate_prod_signed_bundle(
+ None, prod_signed_root)
# Compromises the signature.
bad_targets_siganture.targets_metadata['targets'].signatures[
0].sig = b'1' * 64
@@ -269,7 +305,8 @@
# Generates a bundle with rollback attempt
targets_rollback = test_bundle.generate_targets_metadata()
targets_rollback.common_metadata.version = TEST_TARGETS_VERSION - 1
- targets_rollback_bundle = test_bundle.generate_bundle(targets_rollback)
+ targets_rollback_bundle = test_bundle.generate_prod_signed_bundle(
+ targets_rollback, prod_signed_root)
# Generate bundles with mismatched hash
mismatched_hash_targets_bundles = []
@@ -292,25 +329,28 @@
for idx, payload_file in enumerate(TARGET_FILES.items()):
mismatched_hash_targets = test_bundle.generate_targets_metadata()
mismatched_hash_targets.target_files[idx].hashes[0].hash = b'0' * 32
- mismatched_hash_targets_bundle = test_bundle.generate_bundle(
- mismatched_hash_targets)
+ mismatched_hash_targets_bundle = test_bundle\
+ .generate_prod_signed_bundle(
+ mismatched_hash_targets, prod_signed_root)
mismatched_hash_targets_bundles.append(mismatched_hash_targets_bundle)
mismatched_length_targets = test_bundle.generate_targets_metadata()
mismatched_length_targets.target_files[idx].length = 1
- mismatched_length_targets_bundle = test_bundle.generate_bundle(
- mismatched_length_targets)
+ mismatched_length_targets_bundle = test_bundle\
+ .generate_prod_signed_bundle(
+ mismatched_length_targets, prod_signed_root)
mismatched_length_targets_bundles.append(
mismatched_length_targets_bundle)
missing_hash_targets = test_bundle.generate_targets_metadata()
missing_hash_targets.target_files[idx].hashes.pop()
- missing_hash_targets_bundle = test_bundle.generate_bundle(
- missing_hash_targets)
+ missing_hash_targets_bundle = test_bundle.generate_prod_signed_bundle(
+ missing_hash_targets, prod_signed_root)
missing_hash_targets_bundles.append(missing_hash_targets_bundle)
file_name, _ = payload_file
- personalized_out_bundle = test_bundle.generate_bundle()
+ personalized_out_bundle = test_bundle.generate_prod_signed_bundle(
+ None, prod_signed_root)
personalized_out_bundle.target_payloads.pop(file_name)
personalized_out_bundles.append(personalized_out_bundle)
@@ -325,8 +365,8 @@
header.write(
proto_array_declaration(prod_signed_bundle, 'kTestProdBundle'))
header.write(
- proto_array_declaration(signed_bad_dev_signature_bundle,
- 'kTestBadDevSignatureBundle'))
+ proto_array_declaration(mismatched_root_key_and_signature_bundle,
+ 'kTestMismatchedRootKeyAndSignature'))
header.write(
proto_array_declaration(signed_bad_prod_signature_bundle,
'kTestBadProdSignature'))
diff --git a/pw_software_update/update_bundle_accessor.cc b/pw_software_update/update_bundle_accessor.cc
index 08fdcf3..e62316f 100644
--- a/pw_software_update/update_bundle_accessor.cc
+++ b/pw_software_update/update_bundle_accessor.cc
@@ -328,15 +328,18 @@
Status UpdateBundleAccessor::OpenAndVerify(const ManifestAccessor&) {
PW_TRY(DoOpen());
-#if !defined(PW_SOFTWARE_UPDATE_LANDING_BUNDLE_VERIFICATION)
PW_TRY(DoVerify());
-#endif
return OkStatus();
}
// Get the target element corresponding to `target_file`
stream::IntervalReader UpdateBundleAccessor::GetTargetPayload(
std::string_view target_file_name) {
+ if (!bundle_verified_) {
+ PW_LOG_DEBUG("Bundled has not passed verification yet");
+ return Status::FailedPrecondition();
+ }
+
protobuf::StringToBytesMap target_payloads =
decoder_.AsStringToBytesMap(static_cast<uint32_t>(
pw::software_update::UpdateBundle::Fields::TARGET_PAYLOADS));
@@ -346,8 +349,21 @@
return payload.GetBytesReader();
}
+protobuf::Message UpdateBundleAccessor::GetDecoder() {
+ if (!bundle_verified_) {
+ PW_LOG_DEBUG("Bundled has not passed verification yet");
+ return Status::FailedPrecondition();
+ }
+
+ return decoder_;
+}
+
Result<bool> UpdateBundleAccessor::IsTargetPayloadIncluded(
std::string_view target_file_name) {
+ if (!bundle_verified_) {
+ PW_LOG_DEBUG("Bundled has not passed verification yet");
+ return Status::FailedPrecondition();
+ }
// TODO(pwbug/456): Perform personalization check first. If the target
// is personalized out. Don't need to proceed.
@@ -427,9 +443,8 @@
}
Status UpdateBundleAccessor::Close() {
- // TODO(pwbug/456): To be implemented.
bundle_verified_ = false;
- return bundle_reader_.Close();
+ return bundle_reader_.IsOpen() ? bundle_reader_.Close() : OkStatus();
}
Status UpdateBundleAccessor::DoOpen() {
@@ -437,7 +452,6 @@
PW_TRY(bundle_reader_.Open());
decoder_ =
protobuf::Message(bundle_reader_, bundle_reader_.ConservativeReadLimit());
- (void)backend_;
return OkStatus();
}
@@ -448,7 +462,7 @@
bundle_verified_ = true;
return OkStatus();
}
-
+#if !defined(PW_SOFTWARE_UPDATE_LANDING_BUNDLE_VERIFICATION)
// Verify and upgrade the on-device trust to the incoming root metadata if
// one is included.
PW_TRY(UpgradeRoot());
@@ -463,9 +477,8 @@
// TODO(pwbug/456): Invoke the backend to do downstream verification of the
// bundle (e.g. compatibility and manifest completeness checks).
-
+#endif
bundle_verified_ = true;
-
return OkStatus();
}
@@ -548,7 +561,12 @@
return Status::Unauthenticated();
}
- // Persist the new root.
+ // Persist the root immediately after it is successfully verified. This is
+ // to make sure the trust anchor is up-to-date in storage as soon as
+ // we are confident. Although targets metadata and product-specific
+ // verification have not been done yet. They should be independent from and
+ // not gate the upgrade of root key. This allows timely revokation of
+ // compromise keys.
stream::IntervalReader new_root_reader = new_root.ToBytes().GetBytesReader();
PW_TRY(backend_.SafelyPersistRootMetadata(new_root_reader));
@@ -635,7 +653,7 @@
Result<stream::SeekableReader*> manifest_reader =
backend_.GetCurrentManifestReader();
PW_TRY(manifest_reader.status());
-
+ PW_CHECK_NOTNULL(manifest_reader.value());
protobuf::Message manifest(*manifest_reader.value(),
manifest_reader.value()->ConservativeReadLimit());
diff --git a/pw_software_update/update_bundle_test.cc b/pw_software_update/update_bundle_test.cc
index 208ea48..12dd0c5 100644
--- a/pw_software_update/update_bundle_test.cc
+++ b/pw_software_update/update_bundle_test.cc
@@ -38,7 +38,7 @@
class TestBundledUpdateBackend final : public BundledUpdateBackend {
public:
TestBundledUpdateBackend()
- : trusted_root_reader_({}), current_manifest_reader_({}) {}
+ : current_manifest_reader_({}), trusted_root_memory_reader_({}) {}
Status ApplyReboot() override { return Status::Unimplemented(); }
Status PostRebootFinalize() override { return OkStatus(); }
@@ -61,7 +61,11 @@
void DisableBundleTransferHandler() override {}
void SetTrustedRoot(ConstByteSpan trusted_root) {
- trusted_root_reader_ = stream::MemoryReader(trusted_root);
+ trusted_root_memory_reader_ = stream::MemoryReader(trusted_root);
+ trusted_root_reader_ = stream::IntervalReader(
+ trusted_root_memory_reader_,
+ 0,
+ trusted_root_memory_reader_.ConservativeReadLimit());
}
void SetCurrentManifest(ConstByteSpan current_manifest) {
@@ -77,8 +81,9 @@
}
virtual Status SafelyPersistRootMetadata(
- [[maybe_unused]] stream::Reader& root_metadata) override {
+ [[maybe_unused]] stream::IntervalReader root_metadata) override {
new_root_persisted_ = true;
+ trusted_root_reader_ = root_metadata;
return OkStatus();
};
@@ -91,11 +96,15 @@
}
private:
- stream::MemoryReader trusted_root_reader_;
+ stream::IntervalReader trusted_root_reader_;
stream::MemoryReader current_manifest_reader_;
bool new_root_persisted_ = false;
size_t backend_verified_files_ = 0;
Status verify_target_file_result_ = OkStatus();
+
+ // A memory reader for buffer passed by SetTrustedRoot(). This will be used
+ // to back `trusted_root_reader_`
+ stream::MemoryReader trusted_root_memory_reader_;
};
class UpdateBundleTest : public testing::Test {
@@ -124,6 +133,40 @@
ASSERT_OK(blob_writer.Close());
}
+ // A helper to verify that all bundle operations are disallowed because
+ // the bundle is not open or verified.
+ void VerifyAllBundleOperationsDisallowed(
+ UpdateBundleAccessor& update_bundle) {
+ // We need to check specificially that failure is due to rejecting
+ // unverified/unopen bundle, not anything else.
+ ASSERT_EQ(update_bundle.GetDecoder().status(),
+ Status::FailedPrecondition());
+ ASSERT_EQ(update_bundle.GetTargetPayload("any").status(),
+ Status::FailedPrecondition());
+ ASSERT_EQ(update_bundle.IsTargetPayloadIncluded("any").status(),
+ Status::FailedPrecondition());
+
+ std::byte manifest_buffer[sizeof(kTestBundleManifest)];
+ stream::MemoryWriter manifest_writer(manifest_buffer);
+ ASSERT_EQ(update_bundle.PersistManifest(manifest_writer),
+ Status::FailedPrecondition());
+ }
+
+ // A helper to verify that UpdateBundleAccessor::OpenAndVerify() fails and
+ // that all bundle operations are disallowed as a result. Also check whether
+ // root metadata should be expected to be persisted.
+ void CheckOpenAndVerifyFail(UpdateBundleAccessor& update_bundle,
+ bool expect_new_root_persisted) {
+ ASSERT_FALSE(backend().IsNewRootPersisted());
+ ManifestAccessor current_manifest;
+ ASSERT_NOT_OK(update_bundle.OpenAndVerify(current_manifest));
+ ASSERT_EQ(backend().IsNewRootPersisted(), expect_new_root_persisted);
+ VerifyAllBundleOperationsDisallowed(update_bundle);
+
+ ASSERT_OK(update_bundle.Close());
+ VerifyAllBundleOperationsDisallowed(update_bundle);
+ }
+
private:
kvs::FakeFlashMemoryBuffer<kSectorSize, kSectorCount> blob_flash_;
kvs::FlashPartition blob_partition_;
@@ -206,9 +249,10 @@
0);
}
+#if !defined(PW_SOFTWARE_UPDATE_LANDING_BUNDLE_VERIFICATION)
TEST_F(UpdateBundleTest, PersistManifestFailIfNotVerified) {
backend().SetTrustedRoot(kDevSignedRoot);
- StageTestBundle(kTestBadDevSignatureBundle);
+ StageTestBundle(kTestBadProdSignature);
UpdateBundleAccessor update_bundle(bundle_blob(), backend());
ManifestAccessor current_manifest;
@@ -221,7 +265,7 @@
TEST_F(UpdateBundleTest, BundleVerificationDisabled) {
backend().SetTrustedRoot(kDevSignedRoot);
- StageTestBundle(kTestBadDevSignatureBundle);
+ StageTestBundle(kTestBadProdSignature);
UpdateBundleAccessor update_bundle(bundle_blob(), backend(), true);
// Since bundle verification is disabled. The bad bundle should not report
@@ -253,18 +297,44 @@
// No file is personalized out in kTestProdBundle. Backend verification
// should not be invoked.
ASSERT_EQ(backend().NumFilesVerified(), static_cast<size_t>(0));
+
+ ASSERT_OK(update_bundle.Close());
+ VerifyAllBundleOperationsDisallowed(update_bundle);
}
-TEST_F(UpdateBundleTest, OpenAndVerifyFailsOnBadDevSignature) {
+TEST_F(UpdateBundleTest,
+ OpenAndVerifyWithoutIncomingRootSucceedsWithAllVerification) {
backend().SetTrustedRoot(kDevSignedRoot);
backend().SetCurrentManifest(kTestBundleManifest);
- StageTestBundle(kTestBadDevSignatureBundle);
+ // kTestDevBundle does not contain an incoming root. See
+ // pw_software_update/py/pw_software_update/generate_test_bundle.py for
+ // detail of generation.
+ StageTestBundle(kTestDevBundle);
UpdateBundleAccessor update_bundle(bundle_blob(), backend());
ManifestAccessor current_manifest;
ASSERT_FALSE(backend().IsNewRootPersisted());
- ASSERT_NOT_OK(update_bundle.OpenAndVerify(current_manifest));
+ ASSERT_OK(update_bundle.OpenAndVerify(current_manifest));
ASSERT_FALSE(backend().IsNewRootPersisted());
+
+ // No file is personalized out in kTestDevBundle. Backend verification
+ // should not be invoked.
+ ASSERT_EQ(backend().NumFilesVerified(), static_cast<size_t>(0));
+
+ ASSERT_OK(update_bundle.Close());
+ VerifyAllBundleOperationsDisallowed(update_bundle);
+}
+
+TEST_F(UpdateBundleTest, OpenAndVerifyFailsOnMismatchedRootKeyAndSignature) {
+ backend().SetTrustedRoot(kDevSignedRoot);
+ backend().SetCurrentManifest(kTestBundleManifest);
+ // kTestMismatchedRootKeyAndSignature has a dev root metadata that is
+ // prod signed. The root metadata will not be able to verify itself.
+ // See pw_software_update/py/pw_software_update/generate_test_bundle.py for
+ // detail of generation.
+ StageTestBundle(kTestMismatchedRootKeyAndSignature);
+ UpdateBundleAccessor update_bundle(bundle_blob(), backend());
+ CheckOpenAndVerifyFail(update_bundle, false);
}
TEST_F(UpdateBundleTest, OpenAndVerifyFailsOnBadProdSignature) {
@@ -272,11 +342,7 @@
backend().SetCurrentManifest(kTestBundleManifest);
StageTestBundle(kTestBadProdSignature);
UpdateBundleAccessor update_bundle(bundle_blob(), backend());
-
- ManifestAccessor current_manifest;
- ASSERT_FALSE(backend().IsNewRootPersisted());
- ASSERT_NOT_OK(update_bundle.OpenAndVerify(current_manifest));
- ASSERT_FALSE(backend().IsNewRootPersisted());
+ CheckOpenAndVerifyFail(update_bundle, false);
}
TEST_F(UpdateBundleTest, OpenAndVerifyFailsOnBadTargetsSignature) {
@@ -284,9 +350,7 @@
backend().SetCurrentManifest(kTestBundleManifest);
StageTestBundle(kTestBadTargetsSignature);
UpdateBundleAccessor update_bundle(bundle_blob(), backend());
-
- ManifestAccessor current_manifest;
- ASSERT_NOT_OK(update_bundle.OpenAndVerify(current_manifest));
+ CheckOpenAndVerifyFail(update_bundle, true);
}
TEST_F(UpdateBundleTest, OpenAndVerifyFailsOnBadTargetsRollBack) {
@@ -294,27 +358,26 @@
backend().SetCurrentManifest(kTestBundleManifest);
StageTestBundle(kTestTargetsRollback);
UpdateBundleAccessor update_bundle(bundle_blob(), backend());
-
- ManifestAccessor current_manifest;
- ASSERT_NOT_OK(update_bundle.OpenAndVerify(current_manifest));
+ CheckOpenAndVerifyFail(update_bundle, true);
}
-TEST_F(UpdateBundleTest, OpenAndVerifySucceedsWithMissingManifest) {
+TEST_F(UpdateBundleTest, OpenAndVerifySucceedsWithoutExistingManifest) {
backend().SetTrustedRoot(kDevSignedRoot);
StageTestBundle(kTestProdBundle);
UpdateBundleAccessor update_bundle(bundle_blob(), backend());
ManifestAccessor current_manifest;
+ ASSERT_FALSE(backend().IsNewRootPersisted());
ASSERT_OK(update_bundle.OpenAndVerify(current_manifest));
+ ASSERT_TRUE(backend().IsNewRootPersisted());
}
TEST_F(UpdateBundleTest, OpenAndVerifyFailsOnRootRollback) {
backend().SetTrustedRoot(kDevSignedRoot);
+ backend().SetCurrentManifest(kTestBundleManifest);
StageTestBundle(kTestRootRollback);
UpdateBundleAccessor update_bundle(bundle_blob(), backend());
-
- ManifestAccessor current_manifest;
- ASSERT_NOT_OK(update_bundle.OpenAndVerify(current_manifest));
+ CheckOpenAndVerifyFail(update_bundle, false);
}
TEST_F(UpdateBundleTest, OpenAndVerifyFailsOnMismatchedTargetHashFile0) {
@@ -325,9 +388,7 @@
// The hash value for file 0 in the targets metadata is made incorrect.
StageTestBundle(kTestBundleMismatchedTargetHashFile0);
UpdateBundleAccessor update_bundle(bundle_blob(), backend());
-
- ManifestAccessor current_manifest;
- ASSERT_NOT_OK(update_bundle.OpenAndVerify(current_manifest));
+ CheckOpenAndVerifyFail(update_bundle, true);
}
TEST_F(UpdateBundleTest, OpenAndVerifyFailsOnMismatchedTargetHashFile1) {
@@ -338,9 +399,7 @@
// The hash value for file 1 in the targets metadata is made incorrect.
StageTestBundle(kTestBundleMismatchedTargetHashFile1);
UpdateBundleAccessor update_bundle(bundle_blob(), backend());
-
- ManifestAccessor current_manifest;
- ASSERT_NOT_OK(update_bundle.OpenAndVerify(current_manifest));
+ CheckOpenAndVerifyFail(update_bundle, true);
}
TEST_F(UpdateBundleTest, OpenAndVerifyFailsOnMissingTargetHashFile0) {
@@ -351,9 +410,7 @@
// The hash value for file 0 is removed.
StageTestBundle(kTestBundleMissingTargetHashFile0);
UpdateBundleAccessor update_bundle(bundle_blob(), backend());
-
- ManifestAccessor current_manifest;
- ASSERT_NOT_OK(update_bundle.OpenAndVerify(current_manifest));
+ CheckOpenAndVerifyFail(update_bundle, true);
}
TEST_F(UpdateBundleTest, OpenAndVerifyFailsOnMissingTargetHashFile1) {
@@ -364,9 +421,7 @@
// The hash value for file 1 is removed.
StageTestBundle(kTestBundleMissingTargetHashFile1);
UpdateBundleAccessor update_bundle(bundle_blob(), backend());
-
- ManifestAccessor current_manifest;
- ASSERT_NOT_OK(update_bundle.OpenAndVerify(current_manifest));
+ CheckOpenAndVerifyFail(update_bundle, true);
}
TEST_F(UpdateBundleTest, OpenAndVerifyFailsOnMismatchedTargetLengthFile0) {
@@ -377,9 +432,7 @@
// The length value for file 0 in the targets metadata is made incorrect (1).
StageTestBundle(kTestBundleMismatchedTargetLengthFile0);
UpdateBundleAccessor update_bundle(bundle_blob(), backend());
-
- ManifestAccessor current_manifest;
- ASSERT_NOT_OK(update_bundle.OpenAndVerify(current_manifest));
+ CheckOpenAndVerifyFail(update_bundle, true);
}
TEST_F(UpdateBundleTest, OpenAndVerifyFailsOnMismatchedTargetLengthFile1) {
@@ -390,9 +443,7 @@
// The length value for file 0 in the targets metadata is made incorrect (1).
StageTestBundle(kTestBundleMismatchedTargetLengthFile1);
UpdateBundleAccessor update_bundle(bundle_blob(), backend());
-
- ManifestAccessor current_manifest;
- ASSERT_NOT_OK(update_bundle.OpenAndVerify(current_manifest));
+ CheckOpenAndVerifyFail(update_bundle, true);
}
TEST_F(UpdateBundleTest, OpenAndVerifySucceedsWithPersonalizedOutFile0) {
@@ -435,8 +486,7 @@
StageTestBundle(kTestBundlePersonalizedOutFile1);
UpdateBundleAccessor update_bundle(bundle_blob(), backend());
backend().SetVerifyTargetFileResult(Status::Internal());
- ManifestAccessor current_manifest;
- ASSERT_NOT_OK(update_bundle.OpenAndVerify(current_manifest));
+ CheckOpenAndVerifyFail(update_bundle, true);
}
-
+#endif
} // namespace pw::software_update
