Commit 096a9e60 authored by rdsmith@chromium.org's avatar rdsmith@chromium.org

Revert 182279 due to failure in CloudPolicyTest.FetchPolicyWithRotatedKey on ChromeOS Asan bots.

> Verify the signature on user cloud policy downloads.
> 
> The signature on the user cloud policy blob is already verified by the session
> manager on ChromeOS, but should also be verified by Chrome before storing new
> policy, and after loading policy from the cache.
> 
> BUG=174015
> 
> 
> Review URL: https://chromiumcodereview.appspot.com/12183017

TBR=joaodasilva@chromium.org
Review URL: https://codereview.chromium.org/12255017

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@182302 0039d316-1c4b-4281-b951-d872f2087c98
parent 2bc90940
...@@ -256,8 +256,6 @@ void BrowserPolicyConnector::InitializeUserPolicy( ...@@ -256,8 +256,6 @@ void BrowserPolicyConnector::InitializeUserPolicy(
const base::FilePath policy_dir = profile_dir.Append(kPolicyDir); const base::FilePath policy_dir = profile_dir.Append(kPolicyDir);
const base::FilePath policy_cache_file = policy_dir.Append(kPolicyCacheFile); const base::FilePath policy_cache_file = policy_dir.Append(kPolicyCacheFile);
const base::FilePath token_cache_file = policy_dir.Append(kTokenCacheFile); const base::FilePath token_cache_file = policy_dir.Append(kTokenCacheFile);
FilePath policy_key_dir;
PathService::Get(chrome::DIR_USER_POLICY_KEYS, &policy_key_dir);
if (wait_for_policy_fetch) if (wait_for_policy_fetch)
device_management_service_->ScheduleInitialization(0); device_management_service_->ScheduleInitialization(0);
...@@ -272,9 +270,8 @@ void BrowserPolicyConnector::InitializeUserPolicy( ...@@ -272,9 +270,8 @@ void BrowserPolicyConnector::InitializeUserPolicy(
} else if (!IsNonEnterpriseUser(user_name)) { } else if (!IsNonEnterpriseUser(user_name)) {
scoped_ptr<CloudPolicyStore> store( scoped_ptr<CloudPolicyStore> store(
new UserCloudPolicyStoreChromeOS( new UserCloudPolicyStoreChromeOS(
chromeos::DBusThreadManager::Get()->GetCryptohomeClient(),
chromeos::DBusThreadManager::Get()->GetSessionManagerClient(), chromeos::DBusThreadManager::Get()->GetSessionManagerClient(),
user_name, policy_key_dir, token_cache_file, policy_cache_file)); user_name, token_cache_file, policy_cache_file));
user_cloud_policy_manager_.reset( user_cloud_policy_manager_.reset(
new UserCloudPolicyManagerChromeOS(store.Pass(), new UserCloudPolicyManagerChromeOS(store.Pass(),
wait_for_policy_fetch)); wait_for_policy_fetch));
......
...@@ -6,7 +6,6 @@ ...@@ -6,7 +6,6 @@
#define CHROME_BROWSER_POLICY_USER_CLOUD_POLICY_STORE_CHROMEOS_H_ #define CHROME_BROWSER_POLICY_USER_CLOUD_POLICY_STORE_CHROMEOS_H_
#include <string> #include <string>
#include <vector>
#include "base/basictypes.h" #include "base/basictypes.h"
#include "base/compiler_specific.h" #include "base/compiler_specific.h"
...@@ -15,10 +14,8 @@ ...@@ -15,10 +14,8 @@
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "chrome/browser/policy/cloud_policy_validator.h" #include "chrome/browser/policy/cloud_policy_validator.h"
#include "chrome/browser/policy/user_cloud_policy_store_base.h" #include "chrome/browser/policy/user_cloud_policy_store_base.h"
#include "chromeos/dbus/dbus_method_call_status.h"
namespace chromeos { namespace chromeos {
class CryptohomeClient;
class SessionManagerClient; class SessionManagerClient;
} }
...@@ -36,10 +33,8 @@ class LegacyPolicyCacheLoader; ...@@ -36,10 +33,8 @@ class LegacyPolicyCacheLoader;
class UserCloudPolicyStoreChromeOS : public UserCloudPolicyStoreBase { class UserCloudPolicyStoreChromeOS : public UserCloudPolicyStoreBase {
public: public:
UserCloudPolicyStoreChromeOS( UserCloudPolicyStoreChromeOS(
chromeos::CryptohomeClient* cryptohome_client,
chromeos::SessionManagerClient* session_manager_client, chromeos::SessionManagerClient* session_manager_client,
const std::string& username, const std::string& username,
const base::FilePath& user_policy_key_dir,
const base::FilePath& legacy_token_cache_file, const base::FilePath& legacy_token_cache_file,
const base::FilePath& legacy_policy_cache_file); const base::FilePath& legacy_policy_cache_file);
virtual ~UserCloudPolicyStoreChromeOS(); virtual ~UserCloudPolicyStoreChromeOS();
...@@ -50,28 +45,25 @@ class UserCloudPolicyStoreChromeOS : public UserCloudPolicyStoreBase { ...@@ -50,28 +45,25 @@ class UserCloudPolicyStoreChromeOS : public UserCloudPolicyStoreBase {
virtual void Load() OVERRIDE; virtual void Load() OVERRIDE;
private: private:
// Starts validation of |policy| before storing it.
void ValidatePolicyForStore(
scoped_ptr<enterprise_management::PolicyFetchResponse> policy);
// Completion handler for policy validation on the Store() path.
// Starts a store operation if the validation succeeded.
void OnPolicyToStoreValidated(UserCloudPolicyValidator* validator);
// Called back from SessionManagerClient for policy store operations.
void OnPolicyStored(bool);
// Called back from SessionManagerClient for policy load operations. // Called back from SessionManagerClient for policy load operations.
void OnPolicyRetrieved(const std::string& policy_blob); void OnPolicyRetrieved(const std::string& policy_blob);
// Starts validation of the loaded |policy| before installing it.
void ValidateRetrievedPolicy(
scoped_ptr<enterprise_management::PolicyFetchResponse> policy);
// Completion handler for policy validation on the Load() path. Installs the // Completion handler for policy validation on the Load() path. Installs the
// policy and publishes it if validation succeeded. // policy and publishes it if validation succeeded.
void OnRetrievedPolicyValidated(UserCloudPolicyValidator* validator); void OnRetrievedPolicyValidated(UserCloudPolicyValidator* validator);
// Completion handler for policy validation on the Load() path. Starts a store
// operation if the validation succeeded.
void OnPolicyToStoreValidated(UserCloudPolicyValidator* validator);
// Called back from SessionManagerClient for policy store operations.
void OnPolicyStored(bool);
// Starts policy blob validation.
void Validate(
scoped_ptr<enterprise_management::PolicyFetchResponse> policy,
const UserCloudPolicyValidator::CompletionCallback& callback);
// Callback for loading legacy caches. // Callback for loading legacy caches.
void OnLegacyLoadFinished( void OnLegacyLoadFinished(
const std::string& dm_token, const std::string& dm_token,
...@@ -91,29 +83,8 @@ class UserCloudPolicyStoreChromeOS : public UserCloudPolicyStoreBase { ...@@ -91,29 +83,8 @@ class UserCloudPolicyStoreChromeOS : public UserCloudPolicyStoreBase {
// Removes the passed-in legacy cache directory. // Removes the passed-in legacy cache directory.
static void RemoveLegacyCacheDir(const base::FilePath& dir); static void RemoveLegacyCacheDir(const base::FilePath& dir);
// Invokes |callback| after reloading |policy_key_|.
void ReloadPolicyKey(const base::Closure& callback);
// Reads the contents of |path| into |key|.
static void LoadPolicyKey(const FilePath& path, std::vector<uint8>* key);
// Callback for the key reloading.
void OnPolicyKeyReloaded(std::vector<uint8>* key,
const base::Closure& callback);
// Invokes |callback| after creating |policy_key_|, if it hasn't been created
// yet; otherwise invokes |callback| immediately.
void EnsurePolicyKeyLoaded(const base::Closure& callback);
// Callback for getting the sanitized username from |cryptohome_client_|.
void OnGetSanitizedUsername(const base::Closure& callback,
chromeos::DBusMethodCallStatus call_status,
const std::string& sanitized_username);
chromeos::CryptohomeClient* cryptohome_client_;
chromeos::SessionManagerClient* session_manager_client_; chromeos::SessionManagerClient* session_manager_client_;
const std::string username_; const std::string username_;
base::FilePath user_policy_key_dir_;
base::WeakPtrFactory<UserCloudPolicyStoreChromeOS> weak_factory_; base::WeakPtrFactory<UserCloudPolicyStoreChromeOS> weak_factory_;
...@@ -123,10 +94,6 @@ class UserCloudPolicyStoreChromeOS : public UserCloudPolicyStoreBase { ...@@ -123,10 +94,6 @@ class UserCloudPolicyStoreChromeOS : public UserCloudPolicyStoreBase {
scoped_ptr<LegacyPolicyCacheLoader> legacy_loader_; scoped_ptr<LegacyPolicyCacheLoader> legacy_loader_;
bool legacy_caches_loaded_; bool legacy_caches_loaded_;
bool policy_key_loaded_;
FilePath policy_key_path_;
std::vector<uint8> policy_key_;
DISALLOW_COPY_AND_ASSIGN(UserCloudPolicyStoreChromeOS); DISALLOW_COPY_AND_ASSIGN(UserCloudPolicyStoreChromeOS);
}; };
......
...@@ -93,17 +93,12 @@ const char kFilepathSinglePrefExtensions[] = ...@@ -93,17 +93,12 @@ const char kFilepathSinglePrefExtensions[] =
#endif // defined(OS_LINUX) #endif // defined(OS_LINUX)
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
const char kDefaultAppOrderFileName[] = const char kDefaultAppOrderFileName[] =
#if defined(GOOGLE_CHROME_BUILD) #if defined(GOOGLE_CHROME_BUILD)
FILE_PATH_LITERAL("/usr/share/google-chrome/default_app_order.json"); FILE_PATH_LITERAL("/usr/share/google-chrome/default_app_order.json");
#else #else
FILE_PATH_LITERAL("/usr/share/chromium/default_app_order.json"); FILE_PATH_LITERAL("/usr/share/chromium/default_app_order.json");
#endif // defined(GOOGLE_CHROME_BUILD) #endif // defined(GOOGLE_CHROME_BUILD)
const FilePath::CharType kDefaultUserPolicyKeysDir[] =
FILE_PATH_LITERAL("/var/run/user_policy");
#endif // defined(OS_CHROMEOS) #endif // defined(OS_CHROMEOS)
} // namespace } // namespace
...@@ -410,9 +405,6 @@ bool PathProvider(int key, base::FilePath* result) { ...@@ -410,9 +405,6 @@ bool PathProvider(int key, base::FilePath* result) {
case chrome::FILE_DEFAULT_APP_ORDER: case chrome::FILE_DEFAULT_APP_ORDER:
cur = base::FilePath(FILE_PATH_LITERAL(kDefaultAppOrderFileName)); cur = base::FilePath(FILE_PATH_LITERAL(kDefaultAppOrderFileName));
break; break;
case chrome::DIR_USER_POLICY_KEYS:
cur = FilePath(kDefaultUserPolicyKeysDir);
break;
#endif #endif
// The following are only valid in the development environment, and // The following are only valid in the development environment, and
// will fail if executed from an installed executable (because the // will fail if executed from an installed executable (because the
......
...@@ -104,8 +104,6 @@ enum { ...@@ -104,8 +104,6 @@ enum {
// reside. // reside.
FILE_DEFAULT_APP_ORDER, // Full path to the json file that defines the FILE_DEFAULT_APP_ORDER, // Full path to the json file that defines the
// default app order. // default app order.
DIR_USER_POLICY_KEYS, // Directory where the session_manager stores
// the user policy keys.
#endif #endif
// Valid only in development environment; TODO(darin): move these // Valid only in development environment; TODO(darin): move these
......
...@@ -45,8 +45,7 @@ Example: ...@@ -45,8 +45,7 @@ Example:
}, },
"managed_users" : [ "managed_users" : [
"secret123456" "secret123456"
], ]
"current_key_index": 0
} }
""" """
...@@ -456,17 +455,18 @@ class RequestHandler(object): ...@@ -456,17 +455,18 @@ class RequestHandler(object):
settings = dp.ChromeDeviceSettingsProto() settings = dp.ChromeDeviceSettingsProto()
self.GatherDevicePolicySettings(settings, policy.get(policy_key, {})) self.GatherDevicePolicySettings(settings, policy.get(policy_key, {}))
# Sign with 'current_key_index', defaulting to key 0. # Figure out the key we want to use. If multiple keys are configured, the
# server will rotate through them in a round-robin fashion.
signing_key = None signing_key = None
req_key = None req_key = None
current_key_index = policy.get('current_key_index', 0) key_version = 1
nkeys = len(self._server.keys) nkeys = len(self._server.keys)
if (msg.signature_type == dm.PolicyFetchRequest.SHA1_RSA and if msg.signature_type == dm.PolicyFetchRequest.SHA1_RSA and nkeys > 0:
current_key_index in range(nkeys)):
signing_key = self._server.keys[current_key_index]
if msg.public_key_version in range(1, nkeys + 1): if msg.public_key_version in range(1, nkeys + 1):
# requested key exists, use for signing and rotate. # requested key exists, use for signing and rotate.
req_key = self._server.keys[msg.public_key_version - 1]['private_key'] req_key = self._server.keys[msg.public_key_version - 1]['private_key']
key_version = (msg.public_key_version % nkeys) + 1
signing_key = self._server.keys[key_version - 1]
# Fill the policy data protobuf. # Fill the policy data protobuf.
policy_data = dm.PolicyData() policy_data = dm.PolicyData()
...@@ -480,7 +480,7 @@ class RequestHandler(object): ...@@ -480,7 +480,7 @@ class RequestHandler(object):
policy_data.settings_entity_id = msg.settings_entity_id policy_data.settings_entity_id = msg.settings_entity_id
if signing_key: if signing_key:
policy_data.public_key_version = current_key_index + 1 policy_data.public_key_version = key_version
if msg.policy_type == 'google/chromeos/publicaccount': if msg.policy_type == 'google/chromeos/publicaccount':
policy_data.username = msg.settings_entity_id policy_data.username = msg.settings_entity_id
else: else:
...@@ -498,7 +498,7 @@ class RequestHandler(object): ...@@ -498,7 +498,7 @@ class RequestHandler(object):
if signing_key: if signing_key:
fetch_response.policy_data_signature = ( fetch_response.policy_data_signature = (
signing_key['private_key'].hashAndSign(signed_data).tostring()) signing_key['private_key'].hashAndSign(signed_data).tostring())
if msg.public_key_version != current_key_index + 1: if msg.public_key_version != key_version:
fetch_response.new_public_key = signing_key['public_key'] fetch_response.new_public_key = signing_key['public_key']
if req_key: if req_key:
fetch_response.new_public_key_signature = ( fetch_response.new_public_key_signature = (
...@@ -572,13 +572,12 @@ class TestServer(object): ...@@ -572,13 +572,12 @@ class TestServer(object):
assert key is not None assert key is not None
self.keys.append({ 'private_key' : key }) self.keys.append({ 'private_key' : key })
else: else:
# Generate 2 private keys if none were passed from the command line. # Generate a key if none were specified.
for i in range(2): key = tlslite.api.generateRSAKey(1024)
key = tlslite.api.generateRSAKey(512) assert key is not None
assert key is not None self.keys.append({ 'private_key' : key })
self.keys.append({ 'private_key' : key })
# Derive the public keys from the private keys. # Derive the public keys from the loaded private keys.
for entry in self.keys: for entry in self.keys:
key = entry['private_key'] key = entry['private_key']
......
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