From 49264e03b28ad3813382bef032839eddf893fa7e Mon Sep 17 00:00:00 2001 From: asargent Date: Fri, 26 Sep 2014 14:15:25 -0700 Subject: [PATCH] Fix case-sensitivity problems in extension content verification 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} --- extensions/browser/content_hash_fetcher.cc | 6 +- extensions/browser/content_hash_reader.cc | 7 +- extensions/browser/verified_contents.cc | 30 +++++-- extensions/browser/verified_contents.h | 20 +++-- .../browser/verified_contents_unittest.cc | 90 +++++++++++++------ extensions/test/data/content_verifier/README | 8 +- .../test/data/content_verifier/payload.json | 16 ++++ .../content_verifier/verified_contents.json | 4 +- 8 files changed, 127 insertions(+), 54 deletions(-) diff --git a/extensions/browser/content_hash_fetcher.cc b/extensions/browser/content_hash_fetcher.cc index aa3475e94e1c44..3ffdcc01465144 100644 --- a/extensions/browser/content_hash_fetcher.cc +++ b/extensions/browser/content_hash_fetcher.cc @@ -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; @@ -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; diff --git a/extensions/browser/content_hash_reader.cc b/extensions/browser/content_hash_reader.cc index 76fb3e2941b08d..76c7e606126e5d 100644 --- a/extensions/browser/content_hash_reader.cc +++ b/extensions/browser/content_hash_reader.cc @@ -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; diff --git a/extensions/browser/verified_contents.cc b/extensions/browser/verified_contents.cc index 47c94662a166ca..d9ff75c2abd5a7 100644 --- a/extensions/browser/verified_contents.cc +++ b/extensions/browser/verified_contents.cc @@ -186,8 +186,9 @@ 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; @@ -195,13 +196,24 @@ bool VerifiedContents::InitFrom(const base::FilePath& path, return true; } -const std::string* VerifiedContents::GetTreeHashRoot( - const base::FilePath& relative_path) { - std::map::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 diff --git a/extensions/browser/verified_contents.h b/extensions/browser/verified_contents.h index 1cf0bdb16f8675..7e7b7f95bca4e3 100644 --- a/extensions/browser/verified_contents.h +++ b/extensions/browser/verified_contents.h @@ -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. @@ -83,8 +84,17 @@ class VerifiedContents { std::string extension_id_; base::Version version_; - // The expected treehash root hashes for each file. - std::map 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 RootHashes; + RootHashes root_hashes_; DISALLOW_COPY_AND_ASSIGN(VerifiedContents); }; diff --git a/extensions/browser/verified_contents_unittest.cc b/extensions/browser/verified_contents_unittest.cc index 80f1f813a30f97..865129c1093aac 100644 --- a/extensions/browser/verified_contents_unittest.cc +++ b/extensions/browser/verified_contents_unittest.cc @@ -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) { @@ -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 diff --git a/extensions/test/data/content_verifier/README b/extensions/test/data/content_verifier/README index 8379a83c6f325f..b9a8bb618f359b 100644 --- a/extensions/test/data/content_verifier/README +++ b/extensions/test/data/content_verifier/README @@ -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: @@ -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. diff --git a/extensions/test/data/content_verifier/payload.json b/extensions/test/data/content_verifier/payload.json index 3b67981a329fdf..d9d03cf25d33f0 100644 --- a/extensions/test/data/content_verifier/payload.json +++ b/extensions/test/data/content_verifier/payload.json @@ -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" } ] } diff --git a/extensions/test/data/content_verifier/verified_contents.json b/extensions/test/data/content_verifier/verified_contents.json index 594b39a2dbda92..d9b759b4a677f3 100644 --- a/extensions/test/data/content_verifier/verified_contents.json +++ b/extensions/test/data/content_verifier/verified_contents.json @@ -2,7 +2,7 @@ { "description": "treehash per file", "signed_content": { - "payload": "eyAgImNvbnRlbnRfaGFzaGVzIjogWyAgICB7ICAgICAgImJsb2NrX3NpemUiOiA0MDk2LCAgICAgICJoYXNoX2Jsb2NrX3NpemUiOiA0MDk2LCAgICAgICJmb3JtYXQiOiAidHJlZWhhc2giLCAgICAgICJmaWxlcyI6IFsgICAgICAgIHsgICAgICAgICAgInBhdGgiOiAibWFuaWZlc3QuanNvbiIsICAgICAgICAgICJyb290X2hhc2giOiAiLXZ5eUlJbjdpU0N6ZzdYM0lDVUk1d1phM3RHN3c3dnlpQ2NreFpkSkdmcyIgICAgICAgIH0sICAgICAgICB7ICAgICAgICAgICJwYXRoIjogImJhY2tncm91bmQuanMiLCAgICAgICAgICAicm9vdF9oYXNoIjogInR4SGlHNUtRdk5vUE9TSDVGYlFvOVpiNWdKMjNqM29GQjBSdTlET256aXciICAgICAgICB9LCAgICAgICAgeyAgICAgICAgICAicGF0aCI6ICJmb28vYmFyLmh0bWwiLCAgICAgICAgICAicm9vdF9oYXNoIjogIkwzN0xGYlRfaG10eFJMN0FmR1pOOVlUcFc2eW96X1ppUTFvcExKbjFOWlUiICAgICAgICB9ICAgICAgXSAgICB9ICBdLCAgIml0ZW1faWQiOiAiYWJjZGVmZ2hpamtsbW5vcGFiY2RlZmdoaWprbG1ub3AiLCAgIml0ZW1fdmVyc2lvbiI6ICIxLjIuMyJ9", + "payload": "eyAgImNvbnRlbnRfaGFzaGVzIjogWyAgICB7ICAgICAgImJsb2NrX3NpemUiOiA0MDk2LCAgICAgICJoYXNoX2Jsb2NrX3NpemUiOiA0MDk2LCAgICAgICJmb3JtYXQiOiAidHJlZWhhc2giLCAgICAgICJmaWxlcyI6IFsgICAgICAgIHsgICAgICAgICAgInBhdGgiOiAibWFuaWZlc3QuanNvbiIsICAgICAgICAgICJyb290X2hhc2giOiAiLXZ5eUlJbjdpU0N6ZzdYM0lDVUk1d1phM3RHN3c3dnlpQ2NreFpkSkdmcyIgICAgICAgIH0sICAgICAgICB7ICAgICAgICAgICJwYXRoIjogImJhY2tncm91bmQuanMiLCAgICAgICAgICAicm9vdF9oYXNoIjogInR4SGlHNUtRdk5vUE9TSDVGYlFvOVpiNWdKMjNqM29GQjBSdTlET256aXciICAgICAgICB9LCAgICAgICAgeyAgICAgICAgICAicGF0aCI6ICJmb28vYmFyLmh0bWwiLCAgICAgICAgICAicm9vdF9oYXNoIjogIkwzN0xGYlRfaG10eFJMN0FmR1pOOVlUcFc2eW96X1ppUTFvcExKbjFOWlUiICAgICAgICB9LCAgICAgICAgeyAgICAgICAgICAicGF0aCI6ICJsb3dlcmNhc2UuaHRtbCIsICAgICAgICAgICJyb290X2hhc2giOiAiSHBMb3RMR0NtbU9kS1l2R1FtRDNPa1hNS0dzNDU4ZGJhblk0V2NmQVpJMCIgICAgICAgIH0sICAgICAgICB7ICAgICAgICAgICJwYXRoIjogIkFMTENBUFMuSFRNTCIsICAgICAgICAgICJyb290X2hhc2giOiAiYmwtZVY4RU5vd3Z0dzZQMTRENFgxRVAwbWxjTW9HLV9hT3g1bzlDMTM2NCIgICAgICAgIH0sICAgICAgICB7ICAgICAgICAgICJwYXRoIjogIk1peGVkQ2FzZS5IdG1sIiwgICAgICAgICAgInJvb3RfaGFzaCI6ICJ6RUFPOUZ3Y2lpZ01OeTNOdFUyWE5iLWRTNVRRTW1WTngwVDloN1d2WGJRIiAgICAgICAgfSwgICAgICAgIHsgICAgICAgICAgInBhdGgiOiAibUl4ZWRjQXNlLkh0bWwiLCAgICAgICAgICAicm9vdF9oYXNoIjogIm5LUnFVY0pnMV9RWldBZUNiNHVGZDVvdUMwTWN1R2F2S3A4VEZEUnFCZ2ciICAgICAgICB9ICAgICAgXSAgICB9ICBdLCAgIml0ZW1faWQiOiAiYWJjZGVmZ2hpamtsbW5vcGFiY2RlZmdoaWprbG1ub3AiLCAgIml0ZW1fdmVyc2lvbiI6ICIxLjIuMyJ9", "signatures": [ { "header": {"kid": "publisher"}, @@ -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" } ] }