Skip to content

Commit

Permalink
Fix case-sensitivity problems in extension content verification
Browse files Browse the repository at this point in the history
On case-insensitive filesystems, extensions can generate requests
(script src tags, XHR's, etc.) to their own resources using a
relative path with incorrect case and have those requests
work (see crbug.com/29941 for some history). However, for
extension content verification, we were looking up the expected
file content hashes using the relative path given in the request,
not the actual filename, which meant that any difference in case
would be treated as "no hashes for this file".

This patch switches to using case-insensitive lookups, but uses a
multimap so that case-sensitive filesystems should not experience
problems.

BUG=412693

TEST=Install the test extension at http://goo.gl/rOpGDu, and turn
on content verification to Enforce mode in about:flags. Without
this patch, the extension will get force disabled on
windows/mac. With the patch, this should be fixed.

Review URL: https://codereview.chromium.org/585583003

Cr-Commit-Position: refs/heads/master@{#297032}
  • Loading branch information
asargent authored and Commit bot committed Sep 26, 2014
1 parent aaaae22 commit 49264e0
Show file tree
Hide file tree
Showing 8 changed files with 127 additions and 54 deletions.
6 changes: 2 additions & 4 deletions extensions/browser/content_hash_fetcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -379,9 +379,7 @@ bool ContentHashFetcherJob::CreateHashes(const base::FilePath& hashes_file) {
extension_path_.AppendRelativePath(full_path, &relative_path);
relative_path = relative_path.NormalizePathSeparatorsTo('/');

const std::string* expected_root =
verified_contents_->GetTreeHashRoot(relative_path);
if (!expected_root)
if (!verified_contents_->HasTreeHashRoot(relative_path))
continue;

std::string contents;
Expand All @@ -396,7 +394,7 @@ bool ContentHashFetcherJob::CreateHashes(const base::FilePath& hashes_file) {
ComputedHashes::ComputeHashesForContent(contents, block_size_, &hashes);
std::string root =
ComputeTreeHashRoot(hashes, block_size_ / crypto::kSHA256Length);
if (expected_root && *expected_root != root) {
if (!verified_contents_->TreeHashRootEquals(relative_path, root)) {
VLOG(1) << "content mismatch for " << relative_path.AsUTF8Unsafe();
hash_mismatch_paths_.insert(relative_path);
continue;
Expand Down
7 changes: 1 addition & 6 deletions extensions/browser/content_hash_reader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,9 @@ bool ContentHashReader::Init() {
block_size_ % crypto::kSHA256Length != 0)
return false;

const std::string* expected_root =
verified_contents_->GetTreeHashRoot(relative_path_);
if (!expected_root)
return false;

std::string root =
ComputeTreeHashRoot(hashes_, block_size_ / crypto::kSHA256Length);
if (*expected_root != root)
if (!verified_contents_->TreeHashRootEquals(relative_path_, root))
return false;

status_ = SUCCESS;
Expand Down
30 changes: 21 additions & 9 deletions extensions/browser/verified_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -186,22 +186,34 @@ bool VerifiedContents::InitFrom(const base::FilePath& path,
return false;
base::FilePath file_path =
base::FilePath::FromUTF8Unsafe(file_path_string);
root_hashes_[file_path] = std::string();
root_hashes_[file_path].swap(root_hash);
RootHashes::iterator i = root_hashes_.insert(std::make_pair(
base::StringToLowerASCII(file_path.value()), std::string()));
i->second.swap(root_hash);
}

break;
}
return true;
}

const std::string* VerifiedContents::GetTreeHashRoot(
const base::FilePath& relative_path) {
std::map<base::FilePath, std::string>::const_iterator i =
root_hashes_.find(relative_path.NormalizePathSeparatorsTo('/'));
if (i == root_hashes_.end())
return NULL;
return &i->second;
bool VerifiedContents::HasTreeHashRoot(
const base::FilePath& relative_path) const {
base::FilePath::StringType path = base::StringToLowerASCII(
relative_path.NormalizePathSeparatorsTo('/').value());
return root_hashes_.find(path) != root_hashes_.end();
}

bool VerifiedContents::TreeHashRootEquals(const base::FilePath& relative_path,
const std::string& expected) const {
base::FilePath::StringType path = base::StringToLowerASCII(
relative_path.NormalizePathSeparatorsTo('/').value());
for (RootHashes::const_iterator i = root_hashes_.find(path);
i != root_hashes_.end();
++i) {
if (expected == i->second)
return true;
}
return false;
}

// We're loosely following the "JSON Web Signature" draft spec for signing
Expand Down
20 changes: 15 additions & 5 deletions extensions/browser/verified_contents.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,10 @@ class VerifiedContents {
const std::string& extension_id() const { return extension_id_; }
const base::Version& version() const { return version_; }

// This returns a pointer to the binary form of an expected sha256 root hash
// for |relative_path| computing using a tree hash algorithm.
const std::string* GetTreeHashRoot(const base::FilePath& relative_path);
bool HasTreeHashRoot(const base::FilePath& relative_path) const;

bool TreeHashRootEquals(const base::FilePath& relative_path,
const std::string& expected) const;

// If InitFrom has not been called yet, or was used in "ignore invalid
// signature" mode, this can return false.
Expand Down Expand Up @@ -83,8 +84,17 @@ class VerifiedContents {
std::string extension_id_;
base::Version version_;

// The expected treehash root hashes for each file.
std::map<base::FilePath, std::string> root_hashes_;
// The expected treehash root hashes for each file, lower cased so we can do
// case-insensitive lookups.
//
// We use a multi-map here so that we can do fast lookups of paths from
// requests on case-insensitive systems (windows, mac) where the request path
// might not have the exact right capitalization, but not break
// case-sensitive systems (linux, chromeos). TODO(asargent) - we should give
// developers client-side warnings in each of those cases, and have the
// webstore reject the cases they can statically detect. See crbug.com/29941
typedef std::multimap<base::FilePath::StringType, std::string> RootHashes;
RootHashes root_hashes_;

DISALLOW_COPY_AND_ASSIGN(VerifiedContents);
};
Expand Down
90 changes: 64 additions & 26 deletions extensions/browser/verified_contents_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,14 @@ namespace extensions {

namespace {

bool Base64UrlStringEquals(std::string input, const std::string* bytes) {
if (!bytes)
return false;
if (!VerifiedContents::FixupBase64Encoding(&input))
return false;
std::string DecodeBase64Url(const std::string& encoded) {
std::string fixed_up_base64 = encoded;
if (!VerifiedContents::FixupBase64Encoding(&fixed_up_base64))
return std::string();
std::string decoded;
if (!base::Base64Decode(input, &decoded))
return false;
if (decoded.size() != bytes->size())
return false;

if (bytes->empty())
return true;

return decoded == *bytes;
if (!base::Base64Decode(fixed_up_base64, &decoded))
return std::string();
return decoded;
}

bool GetPublicKey(const base::FilePath& path, std::string* public_key) {
Expand Down Expand Up @@ -68,24 +61,69 @@ TEST(VerifiedContents, Simple) {
EXPECT_EQ(contents.extension_id(), "abcdefghijklmnopabcdefghijklmnop");
EXPECT_EQ("1.2.3", contents.version().GetString());

EXPECT_TRUE(Base64UrlStringEquals(
"-vyyIIn7iSCzg7X3ICUI5wZa3tG7w7vyiCckxZdJGfs",
contents.GetTreeHashRoot(
base::FilePath::FromUTF8Unsafe("manifest.json"))));
EXPECT_TRUE(Base64UrlStringEquals(
"txHiG5KQvNoPOSH5FbQo9Zb5gJ23j3oFB0Ru9DOnziw",
contents.GetTreeHashRoot(
base::FilePath::FromUTF8Unsafe("background.js"))));
EXPECT_TRUE(contents.TreeHashRootEquals(
base::FilePath::FromUTF8Unsafe("manifest.json"),
DecodeBase64Url("-vyyIIn7iSCzg7X3ICUI5wZa3tG7w7vyiCckxZdJGfs")));

EXPECT_TRUE(contents.TreeHashRootEquals(
base::FilePath::FromUTF8Unsafe("background.js"),
DecodeBase64Url("txHiG5KQvNoPOSH5FbQo9Zb5gJ23j3oFB0Ru9DOnziw")));

base::FilePath foo_bar_html =
base::FilePath(FILE_PATH_LITERAL("foo")).AppendASCII("bar.html");
EXPECT_FALSE(foo_bar_html.IsAbsolute());
EXPECT_TRUE(
Base64UrlStringEquals("L37LFbT_hmtxRL7AfGZN9YTpW6yoz_ZiQ1opLJn1NZU",
contents.GetTreeHashRoot(foo_bar_html)));
EXPECT_TRUE(contents.TreeHashRootEquals(
foo_bar_html,
DecodeBase64Url("L37LFbT_hmtxRL7AfGZN9YTpW6yoz_ZiQ1opLJn1NZU")));

base::FilePath nonexistent = base::FilePath::FromUTF8Unsafe("nonexistent");
EXPECT_TRUE(contents.GetTreeHashRoot(nonexistent) == NULL);
EXPECT_FALSE(contents.HasTreeHashRoot(nonexistent));

EXPECT_TRUE(contents.TreeHashRootEquals(
base::FilePath::FromUTF8Unsafe("lowercase.html"),
DecodeBase64Url("HpLotLGCmmOdKYvGQmD3OkXMKGs458dbanY4WcfAZI0")));
EXPECT_TRUE(contents.TreeHashRootEquals(
base::FilePath::FromUTF8Unsafe("Lowercase.Html"),
DecodeBase64Url("HpLotLGCmmOdKYvGQmD3OkXMKGs458dbanY4WcfAZI0")));
EXPECT_TRUE(contents.TreeHashRootEquals(
base::FilePath::FromUTF8Unsafe("LOWERCASE.HTML"),
DecodeBase64Url("HpLotLGCmmOdKYvGQmD3OkXMKGs458dbanY4WcfAZI0")));

EXPECT_TRUE(contents.TreeHashRootEquals(
base::FilePath::FromUTF8Unsafe("ALLCAPS.HTML"),
DecodeBase64Url("bl-eV8ENowvtw6P14D4X1EP0mlcMoG-_aOx5o9C1364")));
EXPECT_TRUE(contents.TreeHashRootEquals(
base::FilePath::FromUTF8Unsafe("AllCaps.Html"),
DecodeBase64Url("bl-eV8ENowvtw6P14D4X1EP0mlcMoG-_aOx5o9C1364")));
EXPECT_TRUE(contents.TreeHashRootEquals(
base::FilePath::FromUTF8Unsafe("allcaps.html"),
DecodeBase64Url("bl-eV8ENowvtw6P14D4X1EP0mlcMoG-_aOx5o9C1364")));

EXPECT_TRUE(contents.TreeHashRootEquals(
base::FilePath::FromUTF8Unsafe("MixedCase.Html"),
DecodeBase64Url("zEAO9FwciigMNy3NtU2XNb-dS5TQMmVNx0T9h7WvXbQ")));
EXPECT_TRUE(contents.TreeHashRootEquals(
base::FilePath::FromUTF8Unsafe("MIXEDCASE.HTML"),
DecodeBase64Url("zEAO9FwciigMNy3NtU2XNb-dS5TQMmVNx0T9h7WvXbQ")));
EXPECT_TRUE(contents.TreeHashRootEquals(
base::FilePath::FromUTF8Unsafe("mixedcase.html"),
DecodeBase64Url("zEAO9FwciigMNy3NtU2XNb-dS5TQMmVNx0T9h7WvXbQ")));
EXPECT_TRUE(contents.TreeHashRootEquals(
base::FilePath::FromUTF8Unsafe("mIxedcAse.Html"),
DecodeBase64Url("zEAO9FwciigMNy3NtU2XNb-dS5TQMmVNx0T9h7WvXbQ")));

EXPECT_TRUE(contents.TreeHashRootEquals(
base::FilePath::FromUTF8Unsafe("mIxedcAse.Html"),
DecodeBase64Url("nKRqUcJg1_QZWAeCb4uFd5ouC0McuGavKp8TFDRqBgg")));
EXPECT_TRUE(contents.TreeHashRootEquals(
base::FilePath::FromUTF8Unsafe("MIXEDCASE.HTML"),
DecodeBase64Url("nKRqUcJg1_QZWAeCb4uFd5ouC0McuGavKp8TFDRqBgg")));
EXPECT_TRUE(contents.TreeHashRootEquals(
base::FilePath::FromUTF8Unsafe("mixedcase.html"),
DecodeBase64Url("nKRqUcJg1_QZWAeCb4uFd5ouC0McuGavKp8TFDRqBgg")));
EXPECT_TRUE(contents.TreeHashRootEquals(
base::FilePath::FromUTF8Unsafe("MixedCase.Html"),
DecodeBase64Url("nKRqUcJg1_QZWAeCb4uFd5ouC0McuGavKp8TFDRqBgg")));
}

} // namespace extensions
8 changes: 6 additions & 2 deletions extensions/test/data/content_verifier/README
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ openssl genrsa -out private_key.pem 2048
openssl rsa -in private_key.pem -pubout -out public_key.pem


The payload.json file contains randomly generated base64url encoded sha256
hashes for a set of file paths.


The signature was generated by:

1) Take the contents of payload.json and base64url encode them:
Expand All @@ -13,8 +17,8 @@ cat payload.json | tr -d \\n | base64 -w0 | tr / _ | tr + \- | tr -d '=' > paylo
2) Put the contents of payload_encoded.txt into the "payload" field of
verified_contents.json.

3) Copy the contents of the "protected" field from verified_contents.json into
protected.txt.
3) Copy the contents of the "protected" field (the one with {"kid":
"webstore"}) from verified_contents.json into protected.txt.

4) Concatenate the "protected" and "payload" fields with a '.' separator.

Expand Down
16 changes: 16 additions & 0 deletions extensions/test/data/content_verifier/payload.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,22 @@
{
"path": "foo/bar.html",
"root_hash": "L37LFbT_hmtxRL7AfGZN9YTpW6yoz_ZiQ1opLJn1NZU"
},
{
"path": "lowercase.html",
"root_hash": "HpLotLGCmmOdKYvGQmD3OkXMKGs458dbanY4WcfAZI0"
},
{
"path": "ALLCAPS.HTML",
"root_hash": "bl-eV8ENowvtw6P14D4X1EP0mlcMoG-_aOx5o9C1364"
},
{
"path": "MixedCase.Html",
"root_hash": "zEAO9FwciigMNy3NtU2XNb-dS5TQMmVNx0T9h7WvXbQ"
},
{
"path": "mIxedcAse.Html",
"root_hash": "nKRqUcJg1_QZWAeCb4uFd5ouC0McuGavKp8TFDRqBgg"
}
]
}
Expand Down
4 changes: 2 additions & 2 deletions extensions/test/data/content_verifier/verified_contents.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
{
"description": "treehash per file",
"signed_content": {
"payload": "eyAgImNvbnRlbnRfaGFzaGVzIjogWyAgICB7ICAgICAgImJsb2NrX3NpemUiOiA0MDk2LCAgICAgICJoYXNoX2Jsb2NrX3NpemUiOiA0MDk2LCAgICAgICJmb3JtYXQiOiAidHJlZWhhc2giLCAgICAgICJmaWxlcyI6IFsgICAgICAgIHsgICAgICAgICAgInBhdGgiOiAibWFuaWZlc3QuanNvbiIsICAgICAgICAgICJyb290X2hhc2giOiAiLXZ5eUlJbjdpU0N6ZzdYM0lDVUk1d1phM3RHN3c3dnlpQ2NreFpkSkdmcyIgICAgICAgIH0sICAgICAgICB7ICAgICAgICAgICJwYXRoIjogImJhY2tncm91bmQuanMiLCAgICAgICAgICAicm9vdF9oYXNoIjogInR4SGlHNUtRdk5vUE9TSDVGYlFvOVpiNWdKMjNqM29GQjBSdTlET256aXciICAgICAgICB9LCAgICAgICAgeyAgICAgICAgICAicGF0aCI6ICJmb28vYmFyLmh0bWwiLCAgICAgICAgICAicm9vdF9oYXNoIjogIkwzN0xGYlRfaG10eFJMN0FmR1pOOVlUcFc2eW96X1ppUTFvcExKbjFOWlUiICAgICAgICB9ICAgICAgXSAgICB9ICBdLCAgIml0ZW1faWQiOiAiYWJjZGVmZ2hpamtsbW5vcGFiY2RlZmdoaWprbG1ub3AiLCAgIml0ZW1fdmVyc2lvbiI6ICIxLjIuMyJ9",
"payload": "eyAgImNvbnRlbnRfaGFzaGVzIjogWyAgICB7ICAgICAgImJsb2NrX3NpemUiOiA0MDk2LCAgICAgICJoYXNoX2Jsb2NrX3NpemUiOiA0MDk2LCAgICAgICJmb3JtYXQiOiAidHJlZWhhc2giLCAgICAgICJmaWxlcyI6IFsgICAgICAgIHsgICAgICAgICAgInBhdGgiOiAibWFuaWZlc3QuanNvbiIsICAgICAgICAgICJyb290X2hhc2giOiAiLXZ5eUlJbjdpU0N6ZzdYM0lDVUk1d1phM3RHN3c3dnlpQ2NreFpkSkdmcyIgICAgICAgIH0sICAgICAgICB7ICAgICAgICAgICJwYXRoIjogImJhY2tncm91bmQuanMiLCAgICAgICAgICAicm9vdF9oYXNoIjogInR4SGlHNUtRdk5vUE9TSDVGYlFvOVpiNWdKMjNqM29GQjBSdTlET256aXciICAgICAgICB9LCAgICAgICAgeyAgICAgICAgICAicGF0aCI6ICJmb28vYmFyLmh0bWwiLCAgICAgICAgICAicm9vdF9oYXNoIjogIkwzN0xGYlRfaG10eFJMN0FmR1pOOVlUcFc2eW96X1ppUTFvcExKbjFOWlUiICAgICAgICB9LCAgICAgICAgeyAgICAgICAgICAicGF0aCI6ICJsb3dlcmNhc2UuaHRtbCIsICAgICAgICAgICJyb290X2hhc2giOiAiSHBMb3RMR0NtbU9kS1l2R1FtRDNPa1hNS0dzNDU4ZGJhblk0V2NmQVpJMCIgICAgICAgIH0sICAgICAgICB7ICAgICAgICAgICJwYXRoIjogIkFMTENBUFMuSFRNTCIsICAgICAgICAgICJyb290X2hhc2giOiAiYmwtZVY4RU5vd3Z0dzZQMTRENFgxRVAwbWxjTW9HLV9hT3g1bzlDMTM2NCIgICAgICAgIH0sICAgICAgICB7ICAgICAgICAgICJwYXRoIjogIk1peGVkQ2FzZS5IdG1sIiwgICAgICAgICAgInJvb3RfaGFzaCI6ICJ6RUFPOUZ3Y2lpZ01OeTNOdFUyWE5iLWRTNVRRTW1WTngwVDloN1d2WGJRIiAgICAgICAgfSwgICAgICAgIHsgICAgICAgICAgInBhdGgiOiAibUl4ZWRjQXNlLkh0bWwiLCAgICAgICAgICAicm9vdF9oYXNoIjogIm5LUnFVY0pnMV9RWldBZUNiNHVGZDVvdUMwTWN1R2F2S3A4VEZEUnFCZ2ciICAgICAgICB9ICAgICAgXSAgICB9ICBdLCAgIml0ZW1faWQiOiAiYWJjZGVmZ2hpamtsbW5vcGFiY2RlZmdoaWprbG1ub3AiLCAgIml0ZW1fdmVyc2lvbiI6ICIxLjIuMyJ9",
"signatures": [
{
"header": {"kid": "publisher"},
Expand All @@ -12,7 +12,7 @@
{
"header": {"kid": "webstore"},
"protected": "eyJhbGciOiJSUzI1NiJ9",
"signature": "q1r09EzaZk3H0VqynXbsUv4CXffIQrP6iSORDNUIRO4k9GvpMTD93JaaUsh4RYt0sHNpX-C4Z2koggnJtA7jbytxRq6M822me_oIxvwYj_jZIqZpEmEBMkuwC4vEFpQdUG-_k94yph_HAZK_FmLyCGuqC-yEaMTCRxXYLuc3ek_EmRReRQCfWcQHFnA_XuyFU7zjjcaCxEXOWaF3Pp4hytWjyWz06L4ITsblUrKboNZNb-ivo0Ub1A3ik17l0F8kvhKlCVAfOyLIsL43bgmOoqFL1QxLPvlZ2Fc__IHIcP4038Qgj6zy30__maKnX5AoOBJ3YzZIrU4V7WYqU9L_mQ"
"signature": "SEHL2Dlere3Dyus1mom5Q1eqn0wUUlReAZzSwlYGy9Y1MJyGzkTc2fY8OGq4pWUMhxXda6knyZmK9GsNWl55ikCqVS_fmG9QPXdh2rx6Pt3HbDy_nlWhYN7c-Cjx0NCWr3pzze_4bWy64HTb_BTK8zaEqYLp-JBJkb_xp8_1kHFHgeDiDBxyPsZey8Z59PD2en5MQyLZi8VEiqfXbztVClBjau0S9GGn0IpKIj6FpgeIkR3z2YxkUg22BBk7-Mqxy_2bHtncu0c57tcg2OkCrGKCR45W6r89FXqO_cHf0cYi_NHcMjgqul-hFrJwO6i09C16lS9J5-OWc5bR-TxJnA"
}
]
}
Expand Down

0 comments on commit 49264e0

Please sign in to comment.