Commit 735b92d3 authored by Joshua Pawlicki's avatar Joshua Pawlicki Committed by Commit Bot

Record and pass the differential fingerprint for extensions.

Bug: 1007393
Change-Id: I49a1269a45ccbe3f5da67db2ec8adfc03ddd1d47
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1893496Reviewed-by: default avatarDevlin <rdevlin.cronin@chromium.org>
Reviewed-by: default avatarSorin Jianu <sorin@chromium.org>
Commit-Queue: Joshua Pawlicki <waffles@chromium.org>
Auto-Submit: Joshua Pawlicki <waffles@chromium.org>
Cr-Commit-Position: refs/heads/master@{#715083}
parent 3f74842b
......@@ -84,6 +84,8 @@ void RequestSender::SendInternal() {
url = BuildUpdateUrl(url, request_query_string);
}
DVLOG(2) << "Sending Omaha request: " << request_body_;
network_fetcher_ = config_->GetNetworkFetcherFactory()->Create();
if (!network_fetcher_) {
base::ThreadTaskRunnerHandle::Get()->PostTask(
......
......@@ -945,6 +945,8 @@ void SandboxedUnpacker::ReportSuccess(
base::DictionaryValue* SandboxedUnpacker::RewriteManifestFile(
const base::DictionaryValue& manifest) {
constexpr int64_t kMaxFingerprintSize = 1024;
// Add the public key extracted earlier to the parsed manifest and overwrite
// the original manifest. We do this to ensure the manifest doesn't contain an
// exploitable bug that could be used to compromise the browser.
......@@ -953,6 +955,16 @@ base::DictionaryValue* SandboxedUnpacker::RewriteManifestFile(
manifest.CreateDeepCopy();
final_manifest->SetString(manifest_keys::kPublicKey, public_key_);
{
std::string differential_fingerprint;
if (base::ReadFileToStringWithMaxSize(
extension_root_.Append(kDifferentialFingerprintFilename),
&differential_fingerprint, kMaxFingerprintSize)) {
final_manifest->SetStringKey(manifest_keys::kDifferentialFingerprint,
std::move(differential_fingerprint));
}
}
std::string manifest_json;
JSONStringValueSerializer serializer(&manifest_json);
serializer.set_pretty_print(true);
......
......@@ -30,6 +30,7 @@
#include "extensions/common/extension_paths.h"
#include "extensions/common/manifest_constants.h"
#include "extensions/common/switches.h"
#include "extensions/common/value_builder.h"
#include "extensions/common/verifier_formats.h"
#include "extensions/strings/grit/extensions_strings.h"
#include "extensions/test/test_extensions_client.h"
......@@ -244,6 +245,19 @@ class SandboxedUnpackerTest : public ExtensionsTest {
EXPECT_TRUE(client_deleted);
}
void SetPublicKey(const std::string& key) {
sandboxed_unpacker_->public_key_ = key;
}
void SetExtensionRoot(const base::FilePath& path) {
sandboxed_unpacker_->extension_root_ = path;
}
base::DictionaryValue* RewriteManifestFile(
const base::DictionaryValue& manifest) {
return sandboxed_unpacker_->RewriteManifestFile(manifest);
}
data_decoder::test::InProcessDataDecoder& in_process_data_decoder() {
return in_process_data_decoder_;
}
......@@ -408,6 +422,31 @@ TEST_F(SandboxedUnpackerTest, FailHashCheck) {
GetInstallErrorDetail());
}
TEST_F(SandboxedUnpackerTest, TestRewriteManifestInjections) {
constexpr char kTestKey[] = "test_key";
constexpr char kTestVersion[] = "1.2.3";
constexpr char kVersionStr[] = "version";
SetPublicKey(kTestKey);
SetExtensionRoot(extensions_dir_.GetPath());
std::string fingerprint = "1.0123456789abcdef";
base::WriteFile(extensions_dir_.GetPath().Append(
FILE_PATH_LITERAL("manifest.fingerprint")),
fingerprint.c_str(),
base::checked_cast<int>(fingerprint.size()));
std::unique_ptr<base::DictionaryValue> manifest(RewriteManifestFile(
*DictionaryBuilder().Set(kVersionStr, kTestVersion).Build()));
auto* key = manifest->FindStringKey("key");
auto* version = manifest->FindStringKey(kVersionStr);
auto* differential_fingerprint =
manifest->FindStringKey("differential_fingerprint");
ASSERT_NE(nullptr, key);
ASSERT_NE(nullptr, version);
ASSERT_NE(nullptr, differential_fingerprint);
EXPECT_EQ(kTestKey, *key);
EXPECT_EQ(kTestVersion, *version);
EXPECT_EQ(fingerprint, *differential_fingerprint);
}
TEST_F(SandboxedUnpackerTest, InvalidMessagesFile) {
SetupUnpackerWithDirectory("invalid_messages_file.crx");
// Check that there is no _locales folder.
......
......@@ -101,9 +101,12 @@ UpdateDataProvider::GetData(bool install_immediately,
crx_component->pk_hash.size());
crx_component->app_id =
update_client::GetCrxIdFromPublicKeyHash(crx_component->pk_hash);
crx_component->version = extension_data.is_corrupt_reinstall
? base::Version("0.0.0.0")
: extension->version();
if (extension_data.is_corrupt_reinstall) {
crx_component->version = base::Version("0.0.0.0");
} else {
crx_component->version = extension->version();
crx_component->fingerprint = extension->DifferentialFingerprint();
}
crx_component->allows_background_download = false;
crx_component->requires_network_encryption = true;
crx_component->crx_format_requirement =
......
......@@ -92,6 +92,15 @@ class UpdateDataProviderTest : public ExtensionsTest {
bool enabled,
int disable_reasons,
Manifest::Location location) {
AddExtension(extension_id, version, "", enabled, disable_reasons, location);
}
void AddExtension(const std::string& extension_id,
const std::string& version,
const std::string& fingerprint,
bool enabled,
int disable_reasons,
Manifest::Location location) {
base::ScopedTempDir temp_dir;
ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
ASSERT_TRUE(base::PathExists(temp_dir.GetPath()));
......@@ -104,11 +113,13 @@ class UpdateDataProviderTest : public ExtensionsTest {
ASSERT_TRUE(AddFileToDirectory(temp_dir.GetPath(), bar_html, "world"));
ExtensionBuilder builder;
builder.SetManifest(DictionaryBuilder()
.Set("name", "My First Extension")
.Set("version", version)
.Set("manifest_version", 2)
.Build());
DictionaryBuilder manifest_builder;
manifest_builder.Set("name", "My First Extension")
.Set("version", version)
.Set("manifest_version", 2);
if (!fingerprint.empty())
manifest_builder.Set("differential_fingerprint", fingerprint);
builder.SetManifest(manifest_builder.Build());
builder.SetID(extension_id);
builder.SetPath(temp_dir.GetPath());
builder.SetLocation(location);
......@@ -144,6 +155,32 @@ TEST_F(UpdateDataProviderTest, GetData_NoDataAdded) {
EXPECT_EQ(0UL, data.size());
}
TEST_F(UpdateDataProviderTest, GetData_Fingerprint) {
scoped_refptr<UpdateDataProvider> data_provider =
base::MakeRefCounted<UpdateDataProvider>(browser_context());
const std::string version = "0.1.2.3";
const std::string fingerprint = "1.0123456789abcdef";
AddExtension(kExtensionId1, version, true,
disable_reason::DisableReason::DISABLE_NONE, Manifest::INTERNAL);
AddExtension(kExtensionId2, version, fingerprint, true,
disable_reason::DisableReason::DISABLE_NONE, Manifest::INTERNAL);
ExtensionUpdateDataMap update_data;
update_data[kExtensionId1] = {};
update_data[kExtensionId2] = {};
const auto data =
data_provider->GetData(false /*install_immediately*/, update_data,
{kExtensionId1, kExtensionId2});
ASSERT_EQ(2UL, data.size());
EXPECT_EQ(version, data[0]->version.GetString());
EXPECT_EQ(version, data[1]->version.GetString());
EXPECT_EQ("2." + version, data[0]->fingerprint);
EXPECT_EQ(fingerprint, data[1]->fingerprint);
}
TEST_F(UpdateDataProviderTest, GetData_EnabledExtension) {
scoped_refptr<UpdateDataProvider> data_provider =
base::MakeRefCounted<UpdateDataProvider>(browser_context());
......
......@@ -437,6 +437,7 @@ if (enable_extensions) {
"manifest_handlers/oauth2_manifest_unittest.cc",
"manifest_handlers/replacement_apps_unittest.cc",
"manifest_handlers/shared_module_manifest_unittest.cc",
"manifest_unittest.cc",
"message_bundle_unittest.cc",
"permissions/api_permission_set_unittest.cc",
"permissions/api_permission_unittest.cc",
......
......@@ -130,6 +130,10 @@
"channel": "stable",
"extension_types": "all"
},
"differential_fingerprint": {
"channel": "stable",
"extension_types": "all"
},
"externally_connectable": {
"channel": "stable",
"extension_types": [
......
......@@ -13,6 +13,8 @@ const char kExtensionScheme[] = "chrome-extension";
const base::FilePath::CharType kManifestFilename[] =
FILE_PATH_LITERAL("manifest.json");
const base::FilePath::CharType kDifferentialFingerprintFilename[] =
FILE_PATH_LITERAL("manifest.fingerprint");
const base::FilePath::CharType kLocaleFolder[] =
FILE_PATH_LITERAL("_locales");
const base::FilePath::CharType kMessagesFilename[] =
......
......@@ -20,6 +20,9 @@ extern const char kExtensionScheme[];
// The name of the manifest inside an extension.
extern const base::FilePath::CharType kManifestFilename[];
// The name of the differential fingerprint file inside an extension.
extern const base::FilePath::CharType kDifferentialFingerprintFilename[];
// The name of locale folder inside an extension.
extern const base::FilePath::CharType kLocaleFolder[];
......
......@@ -421,6 +421,20 @@ const std::string Extension::VersionString() const {
return version_.GetString();
}
const std::string Extension::DifferentialFingerprint() const {
std::string fingerprint;
// We currently support two sources of differential fingerprints:
// server-provided and synthesized. Fingerprints are of the format V.FP, where
// V indicates the fingerprint type (1 for SHA256 hash, 2 for app version) and
// FP indicates the value. The hash-based FP from the server is more precise
// (a hash of the extension CRX), so use that when available, otherwise
// synthesize a 2.VERSION fingerprint for use. For more information, see
// https://github.com/google/omaha/blob/master/doc/ServerProtocolV3.md#packages--fingerprints
return manifest_->GetString(keys::kDifferentialFingerprint, &fingerprint)
? fingerprint
: "2." + VersionString();
}
const std::string Extension::GetVersionForDisplay() const {
if (version_name_.size() > 0)
return version_name_;
......
......@@ -261,6 +261,7 @@ class Extension : public base::RefCountedThreadSafe<Extension> {
const base::Version& version() const { return version_; }
const std::string& version_name() const { return version_name_; }
const std::string VersionString() const;
const std::string DifferentialFingerprint() const;
const std::string GetVersionForDisplay() const;
const std::string& name() const { return display_name_; }
const std::string& short_name() const { return short_name_; }
......
......@@ -218,6 +218,13 @@ bool Manifest::ValidateManifest(
it.key()));
}
}
if (IsUnpackedLocation(location_) &&
value_->FindPath(manifest_keys::kDifferentialFingerprint)) {
warnings->push_back(
InstallWarning(manifest_errors::kHasDifferentialFingerprint,
manifest_keys::kDifferentialFingerprint));
}
return true;
}
......
......@@ -50,6 +50,7 @@ const char kDeclarativeRuleResourcesKey[] = "rule_resources";
const char kDefaultLocale[] = "default_locale";
const char kDescription[] = "description";
const char kDevToolsPage[] = "devtools_page";
const char kDifferentialFingerprint[] = "differential_fingerprint";
const char kDisplayInLauncher[] = "display_in_launcher";
const char kDisplayInNewTabPage[] = "display_in_new_tab_page";
const char kEventName[] = "event_name";
......@@ -310,6 +311,9 @@ const char kDefaultStateShouldNotBeSet[] =
"keys.";
const char kExpectString[] = "Expect string value.";
const char kFileNotFound[] = "File not found: *.";
const char kHasDifferentialFingerprint[] =
"Manifest contains a differential_fingerprint key that will be overridden "
"on extension update.";
const char kInvalidAboutPage[] = "Invalid value for 'about_page'.";
const char kInvalidAboutPageExpectRelativePath[] =
"Invalid value for 'about_page'. Value must be a relative path.";
......
......@@ -51,6 +51,7 @@ extern const char kDeclarativeRuleResourcesKey[];
extern const char kDefaultLocale[];
extern const char kDescription[];
extern const char kDevToolsPage[];
extern const char kDifferentialFingerprint[];
extern const char kDisplayInLauncher[];
extern const char kDisplayInNewTabPage[];
extern const char kEventName[];
......@@ -278,6 +279,7 @@ extern const char kDefaultStateShouldNotBeSet[];
extern const char kDevToolsExperimental[];
extern const char kExpectString[];
extern const char kFileNotFound[];
extern const char kHasDifferentialFingerprint[];
extern const char kInvalidAboutPage[];
extern const char kInvalidAboutPageExpectRelativePath[];
extern const char kInvalidAction[];
......
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "extensions/common/manifest.h"
#include <memory>
#include <utility>
#include "extensions/common/install_warning.h"
#include "extensions/common/manifest_constants.h"
#include "extensions/common/value_builder.h"
#include "testing/gtest/include/gtest/gtest.h"
using ManifestTest = testing::Test;
namespace extensions {
TEST(ManifestTest, ValidateWarnsOnDiffFingerprintKeyUnpacked) {
std::string error;
std::vector<InstallWarning> warnings;
Manifest(Manifest::UNPACKED,
DictionaryBuilder()
.Set(manifest_keys::kDifferentialFingerprint, "")
.Build())
.ValidateManifest(&error, &warnings);
EXPECT_EQ("", error);
EXPECT_EQ(1uL, warnings.size());
EXPECT_EQ(manifest_errors::kHasDifferentialFingerprint, warnings[0].message);
}
TEST(ManifestTest, ValidateWarnsOnDiffFingerprintKeyCommandLine) {
std::string error;
std::vector<InstallWarning> warnings;
Manifest(Manifest::COMMAND_LINE,
DictionaryBuilder()
.Set(manifest_keys::kDifferentialFingerprint, "")
.Build())
.ValidateManifest(&error, &warnings);
EXPECT_EQ("", error);
EXPECT_EQ(1uL, warnings.size());
EXPECT_EQ(manifest_errors::kHasDifferentialFingerprint, warnings[0].message);
}
TEST(ManifestTest, ValidateSilentOnDiffFingerprintKeyInternal) {
std::string error;
std::vector<InstallWarning> warnings;
Manifest(Manifest::INTERNAL,
DictionaryBuilder()
.Set(manifest_keys::kDifferentialFingerprint, "")
.Build())
.ValidateManifest(&error, &warnings);
EXPECT_EQ("", error);
EXPECT_EQ(0uL, warnings.size());
}
TEST(ManifestTest, ValidateSilentOnNoDiffFingerprintKeyUnpacked) {
std::string error;
std::vector<InstallWarning> warnings;
Manifest(Manifest::UNPACKED, DictionaryBuilder().Build())
.ValidateManifest(&error, &warnings);
EXPECT_EQ("", error);
EXPECT_EQ(0uL, warnings.size());
}
TEST(ManifestTest, ValidateSilentOnNoDiffFingerprintKeyInternal) {
std::string error;
std::vector<InstallWarning> warnings;
Manifest(Manifest::INTERNAL, DictionaryBuilder().Build())
.ValidateManifest(&error, &warnings);
EXPECT_EQ("", error);
EXPECT_EQ(0uL, warnings.size());
}
} // namespace extensions
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment