Skip to content

Commit

Permalink
iwa: Add explicit signature and verification step during installation
Browse files Browse the repository at this point in the history
As a new, initial installation step, this CL uses the new
`IsolatedWebAppResponseReaderFactory` to read the Web Bundle as if
it wanted to read a file from the Web Bundle. This makes sure that
integrity block, signatures, and metadata are valid, and that the
Web Bundle's public key is trusted.

Bug: 1366309
Change-Id: I0e90a6d8b836c00d4ded426e03aa3e3e747ba966
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4173793
Commit-Queue: Christian Flach <cmfcmf@chromium.org>
Reviewed-by: Sonja Laurila <laurila@google.com>
Reviewed-by: Robbie McElrath <rmcelrath@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1097386}
  • Loading branch information
cmfcmf authored and Chromium LUCI CQ committed Jan 26, 2023
1 parent 4d53cfe commit 314b418
Show file tree
Hide file tree
Showing 6 changed files with 282 additions and 37 deletions.
Expand Up @@ -11,9 +11,11 @@

#include "base/check.h"
#include "base/containers/flat_set.h"
#include "base/files/file_path.h"
#include "base/functional/bind.h"
#include "base/functional/callback.h"
#include "base/functional/callback_helpers.h"
#include "base/functional/overloaded.h"
#include "base/memory/ptr_util.h"
#include "base/sequence_checker.h"
#include "base/strings/strcat.h"
Expand All @@ -22,6 +24,10 @@
#include "base/types/expected.h"
#include "base/values.h"
#include "chrome/browser/web_applications/commands/web_app_command.h"
#include "chrome/browser/web_applications/isolated_web_apps/isolated_web_app_response_reader.h"
#include "chrome/browser/web_applications/isolated_web_apps/isolated_web_app_response_reader_factory.h"
#include "chrome/browser/web_applications/isolated_web_apps/isolated_web_app_trust_checker.h"
#include "chrome/browser/web_applications/isolated_web_apps/isolated_web_app_validator.h"
#include "chrome/browser/web_applications/isolated_web_apps/pending_install_info.h"
#include "chrome/browser/web_applications/isolation_data.h"
#include "chrome/browser/web_applications/locks/app_lock.h"
Expand All @@ -34,12 +40,15 @@
#include "chrome/browser/web_applications/web_app_install_utils.h"
#include "chrome/browser/web_applications/web_app_url_loader.h"
#include "chrome/browser/web_applications/web_app_utils.h"
#include "components/prefs/pref_service.h"
#include "components/web_package/signed_web_bundles/signed_web_bundle_id.h"
#include "components/webapps/browser/install_result_code.h"
#include "components/webapps/browser/installable/installable_logging.h"
#include "components/webapps/browser/installable/installable_metrics.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/web_contents.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "third_party/abseil-cpp/absl/types/variant.h"
#include "third_party/blink/public/common/manifest/manifest_util.h"
#include "third_party/blink/public/mojom/manifest/manifest.mojom.h"
#include "url/gurl.h"
Expand All @@ -63,6 +72,20 @@ absl::optional<std::string> UTF16ToUTF8(base::StringPiece16 src) {
return dest;
}

std::unique_ptr<IsolatedWebAppResponseReaderFactory>
CreateDefaultResponseReaderFactory(content::BrowserContext& browser_context) {
Profile& profile = *Profile::FromBrowserContext(&browser_context);
PrefService& pref_service = *profile.GetPrefs();

auto trust_checker =
std::make_unique<IsolatedWebAppTrustChecker>(pref_service);
auto validator =
std::make_unique<IsolatedWebAppValidator>(std::move(trust_checker));

return std::make_unique<IsolatedWebAppResponseReaderFactory>(
std::move(validator));
}

} // namespace

InstallIsolatedWebAppCommand::InstallIsolatedWebAppCommand(
Expand All @@ -74,11 +97,32 @@ InstallIsolatedWebAppCommand::InstallIsolatedWebAppCommand(
base::OnceCallback<void(base::expected<InstallIsolatedWebAppCommandSuccess,
InstallIsolatedWebAppCommandError>)>
callback)
: InstallIsolatedWebAppCommand(
isolation_info,
isolation_data,
std::move(web_contents),
std::move(url_loader),
browser_context,
std::move(callback),
CreateDefaultResponseReaderFactory(browser_context)) {}

InstallIsolatedWebAppCommand::InstallIsolatedWebAppCommand(
const IsolatedWebAppUrlInfo& isolation_info,
const IsolationData& isolation_data,
std::unique_ptr<content::WebContents> web_contents,
std::unique_ptr<WebAppUrlLoader> url_loader,
content::BrowserContext& browser_context,
base::OnceCallback<void(base::expected<InstallIsolatedWebAppCommandSuccess,
InstallIsolatedWebAppCommandError>)>
callback,
std::unique_ptr<IsolatedWebAppResponseReaderFactory>
response_reader_factory)
: WebAppCommandTemplate<AppLock>("InstallIsolatedWebAppCommand"),
lock_description_(
std::make_unique<AppLockDescription>(isolation_info.app_id())),
isolation_info_(isolation_info),
isolation_data_(isolation_data),
response_reader_factory_(std::move(response_reader_factory)),
web_contents_(std::move(web_contents)),
url_loader_(std::move(url_loader)),
browser_context_(browser_context),
Expand Down Expand Up @@ -125,6 +169,73 @@ void InstallIsolatedWebAppCommand::StartWithLock(
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
lock_ = std::move(lock);

CheckTrustAndSignatures();
}

void InstallIsolatedWebAppCommand::CheckTrustAndSignatures() {
absl::visit(
base::Overloaded{
[&](const IsolationData::InstalledBundle& content) {
DCHECK_EQ(isolation_info_.web_bundle_id().type(),
web_package::SignedWebBundleId::Type::kEd25519PublicKey);
CheckTrustAndSignaturesOfBundle(content.path);
},
[&](const IsolationData::DevModeBundle& content) {
DCHECK_EQ(isolation_info_.web_bundle_id().type(),
web_package::SignedWebBundleId::Type::kEd25519PublicKey);
CheckTrustAndSignaturesOfBundle(content.path);
},
[&](const IsolationData::DevModeProxy& content) {
DCHECK_EQ(isolation_info_.web_bundle_id().type(),
web_package::SignedWebBundleId::Type::kDevelopment);
// Dev mode proxy mode does not use Web Bundles, hence there is no
// bundle to validate / trust and no signatures to check.
OnTrustAndSignaturesChecked(absl::nullopt);
}},
isolation_data_.content);
}

void InstallIsolatedWebAppCommand::CheckTrustAndSignaturesOfBundle(
const base::FilePath& path) {
// To check whether the bundle is valid and trusted, we attempt to create a
// `IsolatedWebAppResponseReader`. If a response reader is created
// successfully, then this means that the Signed Web Bundle...
// - ...is well formatted and uses a supported Web Bundle version.
// - ...contains a valid integrity block with a trusted public key.
// - ...has signatures that were verified successfully (as long as
// `skip_signature_verification` below is set to `false`).
// - ...contains valid metadata / no invalid URLs.
response_reader_factory_->CreateResponseReader(
path, isolation_info_.web_bundle_id(),
// During installation, we always want to verify signatures, regardless
// of the OS.
/*skip_signature_verification=*/false,
base::BindOnce(
[](base::expected<std::unique_ptr<IsolatedWebAppResponseReader>,
IsolatedWebAppResponseReaderFactory::Error> reader)
-> absl::optional<IsolatedWebAppResponseReaderFactory::Error> {
// Convert expected<Reader,Error> into optional<Error> to match the
// signature of `OnTrustAndSignaturesChecked`. This is necessary for
// compatibility with the dev mode proxy case, where
// `OnTrustAndSignaturesChecked` is called with `absl::nullopt` to
// indicate success.
if (!reader.has_value()) {
return std::move(reader.error());
}
return absl::nullopt;
})
.Then(base::BindOnce(
&InstallIsolatedWebAppCommand::OnTrustAndSignaturesChecked,
weak_factory_.GetWeakPtr())));
}

void InstallIsolatedWebAppCommand::OnTrustAndSignaturesChecked(
absl::optional<IsolatedWebAppResponseReaderFactory::Error> error) {
if (error) {
ReportFailure(IsolatedWebAppResponseReaderFactory::ErrorToString(*error));
return;
}

CreateStoragePartition();
LoadUrl();
}
Expand Down
Expand Up @@ -17,6 +17,7 @@
#include "base/types/expected.h"
#include "base/values.h"
#include "chrome/browser/web_applications/commands/web_app_command.h"
#include "chrome/browser/web_applications/isolated_web_apps/isolated_web_app_response_reader_factory.h"
#include "chrome/browser/web_applications/isolated_web_apps/isolated_web_app_url_info.h"
#include "chrome/browser/web_applications/os_integration/os_integration_manager.h"
#include "chrome/browser/web_applications/web_app_id.h"
Expand Down Expand Up @@ -83,6 +84,20 @@ class InstallIsolatedWebAppCommand : public WebAppCommandTemplate<AppLock> {
void(base::expected<InstallIsolatedWebAppCommandSuccess,
InstallIsolatedWebAppCommandError>)> callback);

// Same constructor as above, but additionally exposes the
// `response_reader_factory` for providing a mock factory in testing.
explicit InstallIsolatedWebAppCommand(
const IsolatedWebAppUrlInfo& isolation_info,
const IsolationData& isolation_data,
std::unique_ptr<content::WebContents> web_contents,
std::unique_ptr<WebAppUrlLoader> url_loader,
content::BrowserContext& browser_context,
base::OnceCallback<
void(base::expected<InstallIsolatedWebAppCommandSuccess,
InstallIsolatedWebAppCommandError>)> callback,
std::unique_ptr<IsolatedWebAppResponseReaderFactory>
response_reader_factory);

InstallIsolatedWebAppCommand(const InstallIsolatedWebAppCommand&) = delete;
InstallIsolatedWebAppCommand& operator=(const InstallIsolatedWebAppCommand&) =
delete;
Expand Down Expand Up @@ -113,6 +128,11 @@ class InstallIsolatedWebAppCommand : public WebAppCommandTemplate<AppLock> {
std::map<GURL, std::vector<SkBitmap>> icons_map,
std::map<GURL, int /*http_status_code*/> icons_http_results);

void CheckTrustAndSignatures();
void CheckTrustAndSignaturesOfBundle(const base::FilePath& path);
void OnTrustAndSignaturesChecked(
absl::optional<IsolatedWebAppResponseReaderFactory::Error> error);

void CreateStoragePartition();

void LoadUrl();
Expand Down Expand Up @@ -140,6 +160,8 @@ class InstallIsolatedWebAppCommand : public WebAppCommandTemplate<AppLock> {
IsolatedWebAppUrlInfo isolation_info_;
IsolationData isolation_data_;

std::unique_ptr<IsolatedWebAppResponseReaderFactory> response_reader_factory_;

std::unique_ptr<content::WebContents> web_contents_;

std::unique_ptr<WebAppUrlLoader> url_loader_;
Expand Down

0 comments on commit 314b418

Please sign in to comment.