Commit ed319ed9 authored by Istiaque Ahmed's avatar Istiaque Ahmed Committed by Commit Bot

Introduce canonical paths as StrongType: CanonicalRelativePath

Currently these paths in content verification is
base::FilePath::StringType, which can be unintentionally converted
from any base::FilePath. This CL make the concept strong typed,
using util::StrongType. This CL also names it with a hint that
this is a relative path and not a full path.

No behavior changes are expected with this CL.

Bug: 1051401
Change-Id: Ib4117e4d2f83e58ae3b294a22b6d7e2720ed5fb4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2059572
Commit-Queue: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: default avatarOleg Davydov <burunduk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#744334}
parent 0322c90b
...@@ -20,7 +20,6 @@ ...@@ -20,7 +20,6 @@
#include "build/build_config.h" #include "build/build_config.h"
#include "crypto/secure_hash.h" #include "crypto/secure_hash.h"
#include "crypto/sha2.h" #include "crypto/sha2.h"
#include "extensions/browser/content_verifier/content_verifier_utils.h"
#include "extensions/browser/content_verifier/scoped_uma_recorder.h" #include "extensions/browser/content_verifier/scoped_uma_recorder.h"
namespace extensions { namespace extensions {
...@@ -66,30 +65,29 @@ ComputedHashes::Data::HashInfo& ComputedHashes::Data::HashInfo::operator=( ...@@ -66,30 +65,29 @@ ComputedHashes::Data::HashInfo& ComputedHashes::Data::HashInfo::operator=(
const ComputedHashes::Data::HashInfo* ComputedHashes::Data::GetItem( const ComputedHashes::Data::HashInfo* ComputedHashes::Data::GetItem(
const base::FilePath& relative_path) const { const base::FilePath& relative_path) const {
base::FilePath::StringType canonicalized_path = CanonicalRelativePath canonical_path =
content_verifier_utils::CanonicalizeRelativePath(relative_path); content_verifier_utils::CanonicalizeRelativePath(relative_path);
auto iter = items_.find(canonicalized_path); auto iter = items_.find(canonical_path);
return iter == items_.end() ? nullptr : &iter->second; return iter == items_.end() ? nullptr : &iter->second;
} }
void ComputedHashes::Data::Add(const base::FilePath& relative_path, void ComputedHashes::Data::Add(const base::FilePath& relative_path,
int block_size, int block_size,
std::vector<std::string> hashes) { std::vector<std::string> hashes) {
base::FilePath::StringType canonicalized_path = CanonicalRelativePath canonical_path =
content_verifier_utils::CanonicalizeRelativePath(relative_path); content_verifier_utils::CanonicalizeRelativePath(relative_path);
items_.insert( items_.insert(std::make_pair(
std::make_pair(canonicalized_path, canonical_path, HashInfo(block_size, std::move(hashes),
HashInfo(block_size, std::move(hashes),
relative_path.NormalizePathSeparatorsTo('/')))); relative_path.NormalizePathSeparatorsTo('/'))));
} }
void ComputedHashes::Data::Remove(const base::FilePath& relative_path) { void ComputedHashes::Data::Remove(const base::FilePath& relative_path) {
base::FilePath::StringType canonicalized_path = CanonicalRelativePath canonical_path =
content_verifier_utils::CanonicalizeRelativePath(relative_path); content_verifier_utils::CanonicalizeRelativePath(relative_path);
items_.erase(canonicalized_path); items_.erase(canonical_path);
} }
const std::map<base::FilePath::StringType, ComputedHashes::Data::HashInfo>& const std::map<CanonicalRelativePath, ComputedHashes::Data::HashInfo>&
ComputedHashes::Data::items() const { ComputedHashes::Data::items() const {
return items_; return items_;
} }
......
...@@ -15,12 +15,14 @@ ...@@ -15,12 +15,14 @@
#include "base/callback.h" #include "base/callback.h"
#include "base/files/file_path.h" #include "base/files/file_path.h"
#include "base/optional.h" #include "base/optional.h"
#include "extensions/browser/content_verifier/content_verifier_utils.h"
namespace extensions { namespace extensions {
using IsCancelledCallback = base::RepeatingCallback<bool(void)>; using IsCancelledCallback = base::RepeatingCallback<bool(void)>;
using ShouldComputeHashesCallback = using ShouldComputeHashesCallback =
base::RepeatingCallback<bool(const base::FilePath& relative_path)>; base::RepeatingCallback<bool(const base::FilePath& relative_path)>;
using CanonicalRelativePath = content_verifier_utils::CanonicalRelativePath;
// A class for storage and serialization of a set of SHA256 block hashes // A class for storage and serialization of a set of SHA256 block hashes
// computed over the files inside an extension. // computed over the files inside an extension.
...@@ -51,7 +53,7 @@ class ComputedHashes { ...@@ -51,7 +53,7 @@ class ComputedHashes {
HashInfo(HashInfo&&); HashInfo(HashInfo&&);
HashInfo& operator=(HashInfo&&); HashInfo& operator=(HashInfo&&);
}; };
using Items = std::map<base::FilePath::StringType, HashInfo>; using Items = std::map<CanonicalRelativePath, HashInfo>;
Data(); Data();
~Data(); ~Data();
......
...@@ -49,9 +49,10 @@ class Extension; ...@@ -49,9 +49,10 @@ class Extension;
// 2. Relative unix path: Some underlying parts of content-verification // 2. Relative unix path: Some underlying parts of content-verification
// require uniform separator, we use '/' as separator so it is effectively // require uniform separator, we use '/' as separator so it is effectively
// unix style. Note that this is a reversible transformation. // unix style. Note that this is a reversible transformation.
// 3. Canonicalized relative_path: Canonicalized paths are used as keys of // 3. content_verifier_utils::CanonicalRelativePath:
// maps within VerifiedContents and ComputedHashes. This takes care of OS // Canonicalized relative paths are used as keys of maps within
// specific file access issues: // VerifiedContents and ComputedHashes. This takes care of OS specific file
// access issues:
// - windows/mac is case insensitive while accessing files. // - windows/mac is case insensitive while accessing files.
// - windows ignores (.| )+ suffixes in filename while accessing a file. // - windows ignores (.| )+ suffixes in filename while accessing a file.
// Canonicalization consists of normalizing the separators, lower casing // Canonicalization consists of normalizing the separators, lower casing
......
...@@ -22,15 +22,15 @@ bool TrimDotSpaceSuffix(const base::FilePath::StringType& path, ...@@ -22,15 +22,15 @@ bool TrimDotSpaceSuffix(const base::FilePath::StringType& path,
return true; return true;
} }
base::FilePath::StringType CanonicalizeRelativePath( CanonicalRelativePath CanonicalizeRelativePath(
const base::FilePath& relative_path) { const base::FilePath& relative_path) {
base::FilePath::StringType canonicalized_path = base::FilePath::StringType canonical_path =
relative_path.NormalizePathSeparatorsTo('/').value(); relative_path.NormalizePathSeparatorsTo('/').value();
if (!IsFileAccessCaseSensitive()) if (!IsFileAccessCaseSensitive())
canonicalized_path = base::ToLowerASCII(canonicalized_path); canonical_path = base::ToLowerASCII(canonical_path);
if (IsDotSpaceFilenameSuffixIgnored()) if (IsDotSpaceFilenameSuffixIgnored())
TrimDotSpaceSuffix(canonicalized_path, &canonicalized_path); TrimDotSpaceSuffix(canonical_path, &canonical_path);
return canonicalized_path; return CanonicalRelativePath(std::move(canonical_path));
} }
} // namespace content_verifier_utils } // namespace content_verifier_utils
......
...@@ -5,11 +5,24 @@ ...@@ -5,11 +5,24 @@
#define EXTENSIONS_BROWSER_CONTENT_VERIFIER_CONTENT_VERIFIER_UTILS_H_ #define EXTENSIONS_BROWSER_CONTENT_VERIFIER_CONTENT_VERIFIER_UTILS_H_
#include "base/files/file_path.h" #include "base/files/file_path.h"
#include "base/util/type_safety/strong_alias.h"
#include "build/build_config.h" #include "build/build_config.h"
namespace extensions { namespace extensions {
namespace content_verifier_utils { namespace content_verifier_utils {
// Extension relative FilePath's canonical version for content verification
// system. Canonicalization consists of:
// - Normalizing path separators to '/'.
// This is done because GURLs generally use '/' separators (that is passed
// to content verifier via extension_protocols) and manifest.json paths
// also specify '/' separators.
// - In case-insensitive OS, lower casing path.
// - In Windows, trimming "dot-space" suffix in path.
using CanonicalRelativePath =
::util::StrongAlias<class CanonicalRelativePathTag,
base::FilePath::StringType>;
// Returns true if |path| ends with (.| )+. // Returns true if |path| ends with (.| )+.
// |out_path| will contain "." and/or " " suffix removed from |path|. // |out_path| will contain "." and/or " " suffix removed from |path|.
bool TrimDotSpaceSuffix(const base::FilePath::StringType& path, bool TrimDotSpaceSuffix(const base::FilePath::StringType& path,
...@@ -39,7 +52,7 @@ constexpr bool IsDotSpaceFilenameSuffixIgnored() { ...@@ -39,7 +52,7 @@ constexpr bool IsDotSpaceFilenameSuffixIgnored() {
// Returns platform specific canonicalized version of |relative_path| for // Returns platform specific canonicalized version of |relative_path| for
// content verification system. // content verification system.
base::FilePath::StringType CanonicalizeRelativePath( CanonicalRelativePath CanonicalizeRelativePath(
const base::FilePath& relative_path); const base::FilePath& relative_path);
} // namespace content_verifier_utils } // namespace content_verifier_utils
......
...@@ -171,11 +171,11 @@ std::unique_ptr<VerifiedContents> VerifiedContents::Create( ...@@ -171,11 +171,11 @@ std::unique_ptr<VerifiedContents> VerifiedContents::Create(
return nullptr; return nullptr;
} }
base::FilePath::StringType canonicalized_path = content_verifier_utils::CanonicalRelativePath canonical_path =
content_verifier_utils::CanonicalizeRelativePath( content_verifier_utils::CanonicalizeRelativePath(
base::FilePath::FromUTF8Unsafe(*file_path_string)); base::FilePath::FromUTF8Unsafe(*file_path_string));
auto i = verified_contents->root_hashes_.insert( auto i = verified_contents->root_hashes_.insert(
std::make_pair(canonicalized_path, std::string())); std::make_pair(canonical_path, std::string()));
i->second.swap(root_hash); i->second.swap(root_hash);
} }
...@@ -334,10 +334,11 @@ bool VerifiedContents::VerifySignature(const std::string& protected_value, ...@@ -334,10 +334,11 @@ bool VerifiedContents::VerifySignature(const std::string& protected_value,
} }
bool VerifiedContents::TreeHashRootEqualsImpl( bool VerifiedContents::TreeHashRootEqualsImpl(
const base::FilePath::StringType& normalized_relative_path, const content_verifier_utils::CanonicalRelativePath&
canonical_relative_path,
const std::string& expected) const { const std::string& expected) const {
std::pair<RootHashes::const_iterator, RootHashes::const_iterator> hashes = std::pair<RootHashes::const_iterator, RootHashes::const_iterator> hashes =
root_hashes_.equal_range(normalized_relative_path); root_hashes_.equal_range(canonical_relative_path);
for (auto iter = hashes.first; iter != hashes.second; ++iter) { for (auto iter = hashes.first; iter != hashes.second; ++iter) {
if (expected == iter->second) if (expected == iter->second)
return true; return true;
......
...@@ -16,9 +16,12 @@ ...@@ -16,9 +16,12 @@
#include "base/files/file_path.h" #include "base/files/file_path.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/version.h" #include "base/version.h"
#include "extensions/browser/content_verifier/content_verifier_utils.h"
namespace extensions { namespace extensions {
using CanonicalRelativePath = content_verifier_utils::CanonicalRelativePath;
// This class encapsulates the data in a "verified_contents.json" file // This class encapsulates the data in a "verified_contents.json" file
// generated by the webstore for a .crx file. That data includes a set of // generated by the webstore for a .crx file. That data includes a set of
// signed expected hashes of file content which can be used to check for // signed expected hashes of file content which can be used to check for
...@@ -66,7 +69,7 @@ class VerifiedContents { ...@@ -66,7 +69,7 @@ class VerifiedContents {
const std::string& signature_bytes); const std::string& signature_bytes);
bool TreeHashRootEqualsImpl( bool TreeHashRootEqualsImpl(
const base::FilePath::StringType& normalized_relative_path, const CanonicalRelativePath& canonical_relative_path,
const std::string& expected) const; const std::string& expected) const;
// The public key we should use for signature verification. // The public key we should use for signature verification.
...@@ -95,7 +98,7 @@ class VerifiedContents { ...@@ -95,7 +98,7 @@ class VerifiedContents {
// TODO(crbug.com/29941) - we should give developers client-side warnings in // TODO(crbug.com/29941) - we should give developers client-side warnings in
// each of those cases, and have the webstore reject the cases they can // each of those cases, and have the webstore reject the cases they can
// statically detect. // statically detect.
typedef std::multimap<base::FilePath::StringType, std::string> RootHashes; typedef std::multimap<CanonicalRelativePath, std::string> RootHashes;
RootHashes root_hashes_; RootHashes root_hashes_;
DISALLOW_COPY_AND_ASSIGN(VerifiedContents); DISALLOW_COPY_AND_ASSIGN(VerifiedContents);
......
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