Commit 4313f778 authored by Polina Bondarenko's avatar Polina Bondarenko Committed by Commit Bot

arc: add requiredKeyPairs policy

Add requiredKeyPairs ARC policy that contains certificate names for the
smart card certificates, available to ARC.

This policy is needed to be able to remove smart cards from the list
once the reader is detached.

The list is updated after the ArcSmartCardManagerBridge::Refresh is called.

Bug: b:119914122
Test: ./out/Debug/unit_tests --gtest_filter=ArcSmartCardManagerBridgeTest*
Test: ./out/Debug/unit_tests --gtest_filter=ArcPolicyBridgeRequiredKeyPairTest*
Test: ./out/Debug/browser_tests --gtest_filter=Arc*
Test: detach a smart card reader from the local device, make sure that there is no \
      certificate shown in a client certificate user dialog launched from the test \
      Android app.

Change-Id: I524929ac25d67edaaf54eecdd61448a060393c00
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1724083
Commit-Queue: Polina Bondarenko <pbond@chromium.org>
Auto-Submit: Polina Bondarenko <pbond@chromium.org>
Reviewed-by: default avatarBartosz Fabianowski <bartfab@chromium.org>
Reviewed-by: default avatarEdman Anjos <edman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#695977}
parent c77b9979
...@@ -40,7 +40,7 @@ ArcCertInstaller::~ArcCertInstaller() { ...@@ -40,7 +40,7 @@ ArcCertInstaller::~ArcCertInstaller() {
queue_->RemoveObserver(this); queue_->RemoveObserver(this);
} }
void ArcCertInstaller::InstallArcCerts( std::set<std::string> ArcCertInstaller::InstallArcCerts(
const std::vector<net::ScopedCERTCertificate>& certificates, const std::vector<net::ScopedCERTCertificate>& certificates,
InstallArcCertsCallback callback) { InstallArcCertsCallback callback) {
VLOG(1) << "ArcCertInstaller::InstallArcCerts"; VLOG(1) << "ArcCertInstaller::InstallArcCerts";
...@@ -52,7 +52,7 @@ void ArcCertInstaller::InstallArcCerts( ...@@ -52,7 +52,7 @@ void ArcCertInstaller::InstallArcCerts(
pending_status_ = true; pending_status_ = true;
} }
required_cert_names_.clear(); std::set<std::string> required_cert_names;
callback_ = std::move(callback); callback_ = std::move(callback);
for (const auto& nss_cert : certificates) { for (const auto& nss_cert : certificates) {
...@@ -64,14 +64,15 @@ void ArcCertInstaller::InstallArcCerts( ...@@ -64,14 +64,15 @@ void ArcCertInstaller::InstallArcCerts(
std::string cert_name = std::string cert_name =
x509_certificate_model::GetCertNameOrNickname(nss_cert.get()); x509_certificate_model::GetCertNameOrNickname(nss_cert.get());
required_cert_names.insert(cert_name);
InstallArcCert(cert_name, nss_cert); InstallArcCert(cert_name, nss_cert);
} }
// Cleanup |known_cert_names_| according to |required_cert_names_|. // Cleanup |known_cert_names_| according to |required_cert_names|.
for (auto it = known_cert_names_.begin(); it != known_cert_names_.end();) { for (auto it = known_cert_names_.begin(); it != known_cert_names_.end();) {
auto cert_name = it++; auto cert_name = it++;
if (!required_cert_names_.count(*cert_name)) if (!required_cert_names.count(*cert_name))
known_cert_names_.erase(cert_name); known_cert_names_.erase(cert_name);
} }
...@@ -79,13 +80,14 @@ void ArcCertInstaller::InstallArcCerts( ...@@ -79,13 +80,14 @@ void ArcCertInstaller::InstallArcCerts(
std::move(callback_).Run(pending_status_); std::move(callback_).Run(pending_status_);
pending_status_ = true; pending_status_ = true;
} }
return required_cert_names;
} }
void ArcCertInstaller::InstallArcCert( void ArcCertInstaller::InstallArcCert(
const std::string& name, const std::string& name,
const net::ScopedCERTCertificate& nss_cert) { const net::ScopedCERTCertificate& nss_cert) {
VLOG(1) << "ArcCertInstaller::InstallArcCert " << name; VLOG(1) << "ArcCertInstaller::InstallArcCert " << name;
required_cert_names_.insert(name);
// Do not install certificate if already exists. // Do not install certificate if already exists.
if (known_cert_names_.count(name)) if (known_cert_names_.count(name))
...@@ -134,8 +136,8 @@ void ArcCertInstaller::OnJobFinished(policy::RemoteCommandJob* command) { ...@@ -134,8 +136,8 @@ void ArcCertInstaller::OnJobFinished(policy::RemoteCommandJob* command) {
} }
// If the cert installation is failed, save the status and remove from the // If the cert installation is failed, save the status and remove from the
// |known_cert_names_| to be able to re-try the installation when the // |known_cert_names_|. Use the |pending_status_| to notify clients should
// |Refresh| is called again. // re-try installation.
if (command->status() != policy::RemoteCommandJob::Status::SUCCEEDED) { if (command->status() != policy::RemoteCommandJob::Status::SUCCEEDED) {
LOG(ERROR) << "Failed to install certificate " LOG(ERROR) << "Failed to install certificate "
<< pending_commands_[command->unique_id()]; << pending_commands_[command->unique_id()];
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include <memory> #include <memory>
#include <set> #include <set>
#include <string> #include <string>
#include <vector>
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "components/policy/core/common/remote_commands/remote_command_job.h" #include "components/policy/core/common/remote_commands/remote_command_job.h"
...@@ -40,9 +41,11 @@ class ArcCertInstaller : public policy::RemoteCommandsQueue::Observer { ...@@ -40,9 +41,11 @@ class ArcCertInstaller : public policy::RemoteCommandsQueue::Observer {
using InstallArcCertsCallback = base::OnceCallback<void(bool result)>; using InstallArcCertsCallback = base::OnceCallback<void(bool result)>;
// Install missing certificates via ARC remote commands. // Install missing certificates via ARC remote commands.
//
// Return set of the names of certificates required being installed on ARC.
// Return false via |callback| in case of any error, and true otherwise. // Return false via |callback| in case of any error, and true otherwise.
// Made virtual for override in test. // Made virtual for override in test.
virtual void InstallArcCerts( virtual std::set<std::string> InstallArcCerts(
const std::vector<net::ScopedCERTCertificate>& certs, const std::vector<net::ScopedCERTCertificate>& certs,
InstallArcCertsCallback callback); InstallArcCertsCallback callback);
...@@ -67,9 +70,6 @@ class ArcCertInstaller : public policy::RemoteCommandsQueue::Observer { ...@@ -67,9 +70,6 @@ class ArcCertInstaller : public policy::RemoteCommandsQueue::Observer {
// The |pending_status_| is returned via |callback_|. // The |pending_status_| is returned via |callback_|.
bool pending_status_ = true; bool pending_status_ = true;
// Names of certificates required to be installed on ARC.
std::set<std::string> required_cert_names_;
// Names of certificates installed or pending to be installed on ARC. // Names of certificates installed or pending to be installed on ARC.
std::set<std::string> known_cert_names_; std::set<std::string> known_cert_names_;
......
...@@ -10,9 +10,12 @@ ...@@ -10,9 +10,12 @@
#include "base/callback.h" #include "base/callback.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/memory/singleton.h" #include "base/memory/singleton.h"
#include "chrome/browser/chromeos/arc/policy/arc_policy_bridge.h"
#include "chrome/browser/chromeos/certificate_provider/certificate_provider_service.h" #include "chrome/browser/chromeos/certificate_provider/certificate_provider_service.h"
#include "chrome/browser/chromeos/certificate_provider/certificate_provider_service_factory.h" #include "chrome/browser/chromeos/certificate_provider/certificate_provider_service_factory.h"
#include "components/arc/arc_browser_context_keyed_service_factory_base.h" #include "components/arc/arc_browser_context_keyed_service_factory_base.h"
#include "components/policy/core/common/policy_map.h"
#include "components/policy/core/common/policy_namespace.h"
#include "net/cert/x509_util_nss.h" #include "net/cert/x509_util_nss.h"
namespace arc { namespace arc {
...@@ -49,10 +52,16 @@ ArcSmartCardManagerBridge* ArcSmartCardManagerBridge::GetForBrowserContext( ...@@ -49,10 +52,16 @@ ArcSmartCardManagerBridge* ArcSmartCardManagerBridge::GetForBrowserContext(
return ArcSmartCardManagerBridgeFactory::GetForBrowserContext(context); return ArcSmartCardManagerBridgeFactory::GetForBrowserContext(context);
} }
// static
BrowserContextKeyedServiceFactory* ArcSmartCardManagerBridge::GetFactory() {
return ArcSmartCardManagerBridgeFactory::GetInstance();
}
ArcSmartCardManagerBridge::ArcSmartCardManagerBridge( ArcSmartCardManagerBridge::ArcSmartCardManagerBridge(
content::BrowserContext* context, content::BrowserContext* context,
ArcBridgeService* bridge_service) ArcBridgeService* bridge_service)
: ArcSmartCardManagerBridge( : ArcSmartCardManagerBridge(
context,
bridge_service, bridge_service,
chromeos::CertificateProviderServiceFactory::GetForBrowserContext( chromeos::CertificateProviderServiceFactory::GetForBrowserContext(
context) context)
...@@ -60,10 +69,12 @@ ArcSmartCardManagerBridge::ArcSmartCardManagerBridge( ...@@ -60,10 +69,12 @@ ArcSmartCardManagerBridge::ArcSmartCardManagerBridge(
std::make_unique<ArcCertInstaller>(context)) {} std::make_unique<ArcCertInstaller>(context)) {}
ArcSmartCardManagerBridge::ArcSmartCardManagerBridge( ArcSmartCardManagerBridge::ArcSmartCardManagerBridge(
content::BrowserContext* context,
ArcBridgeService* bridge_service, ArcBridgeService* bridge_service,
std::unique_ptr<chromeos::CertificateProvider> certificate_provider, std::unique_ptr<chromeos::CertificateProvider> certificate_provider,
std::unique_ptr<ArcCertInstaller> installer) std::unique_ptr<ArcCertInstaller> installer)
: arc_bridge_service_(bridge_service), : context_(context),
arc_bridge_service_(bridge_service),
certificate_provider_(std::move(certificate_provider)), certificate_provider_(std::move(certificate_provider)),
installer_(std::move(installer)), installer_(std::move(installer)),
weak_ptr_factory_(this) { weak_ptr_factory_(this) {
...@@ -103,7 +114,17 @@ void ArcSmartCardManagerBridge::DidGetCerts( ...@@ -103,7 +114,17 @@ void ArcSmartCardManagerBridge::DidGetCerts(
certificates.push_back(std::move(nss_cert)); certificates.push_back(std::move(nss_cert));
} }
installer_->InstallArcCerts(std::move(certificates), std::move(callback)); std::set<std::string> new_required_cert_names =
installer_->InstallArcCerts(std::move(certificates), std::move(callback));
if (required_cert_names_ != new_required_cert_names) {
required_cert_names_ = new_required_cert_names;
ArcPolicyBridge* const policy_bridge =
ArcPolicyBridge::GetForBrowserContext(context_);
if (policy_bridge) {
policy_bridge->OnPolicyUpdated(policy::PolicyNamespace(),
policy::PolicyMap(), policy::PolicyMap());
}
}
} }
} // namespace arc } // namespace arc
...@@ -6,6 +6,8 @@ ...@@ -6,6 +6,8 @@
#define CHROME_BROWSER_CHROMEOS_ARC_ENTERPRISE_CERT_STORE_ARC_SMART_CARD_MANAGER_BRIDGE_H_ #define CHROME_BROWSER_CHROMEOS_ARC_ENTERPRISE_CERT_STORE_ARC_SMART_CARD_MANAGER_BRIDGE_H_
#include <memory> #include <memory>
#include <set>
#include <vector>
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "chrome/browser/chromeos/arc/enterprise/cert_store/arc_cert_installer.h" #include "chrome/browser/chromeos/arc/enterprise/cert_store/arc_cert_installer.h"
...@@ -15,6 +17,8 @@ ...@@ -15,6 +17,8 @@
#include "components/keyed_service/core/keyed_service.h" #include "components/keyed_service/core/keyed_service.h"
#include "net/cert/scoped_nss_types.h" #include "net/cert/scoped_nss_types.h"
class BrowserContextKeyedServiceFactory;
namespace content { namespace content {
class BrowserContext; class BrowserContext;
...@@ -33,11 +37,15 @@ class ArcSmartCardManagerBridge : public KeyedService, ...@@ -33,11 +37,15 @@ class ArcSmartCardManagerBridge : public KeyedService,
static ArcSmartCardManagerBridge* GetForBrowserContext( static ArcSmartCardManagerBridge* GetForBrowserContext(
content::BrowserContext* context); content::BrowserContext* context);
// Return the factory instance for this class.
static BrowserContextKeyedServiceFactory* GetFactory();
ArcSmartCardManagerBridge(content::BrowserContext* context, ArcSmartCardManagerBridge(content::BrowserContext* context,
ArcBridgeService* bridge_service); ArcBridgeService* bridge_service);
// This constructor is public only for testing. // This constructor is public only for testing.
ArcSmartCardManagerBridge( ArcSmartCardManagerBridge(
content::BrowserContext* context,
ArcBridgeService* bridge_service, ArcBridgeService* bridge_service,
std::unique_ptr<chromeos::CertificateProvider> certificate_provider, std::unique_ptr<chromeos::CertificateProvider> certificate_provider,
std::unique_ptr<ArcCertInstaller> installer); std::unique_ptr<ArcCertInstaller> installer);
...@@ -47,15 +55,29 @@ class ArcSmartCardManagerBridge : public KeyedService, ...@@ -47,15 +55,29 @@ class ArcSmartCardManagerBridge : public KeyedService,
// SmartCardManagerHost overrides. // SmartCardManagerHost overrides.
void Refresh(RefreshCallback callback) override; void Refresh(RefreshCallback callback) override;
std::vector<std::string> get_required_cert_names() const {
return std::vector<std::string>(required_cert_names_.begin(),
required_cert_names_.end());
}
void set_required_cert_names_for_testing(
const std::vector<std::string>& cert_names) {
required_cert_names_ =
std::set<std::string>(cert_names.begin(), cert_names.end());
}
private: private:
void DidGetCerts(RefreshCallback callback, void DidGetCerts(RefreshCallback callback,
net::ClientCertIdentityList cert_identities); net::ClientCertIdentityList cert_identities);
content::BrowserContext* const context_;
ArcBridgeService* const arc_bridge_service_; // Owned by ArcServiceManager. ArcBridgeService* const arc_bridge_service_; // Owned by ArcServiceManager.
std::unique_ptr<chromeos::CertificateProvider> certificate_provider_; std::unique_ptr<chromeos::CertificateProvider> certificate_provider_;
std::unique_ptr<ArcCertInstaller> installer_; std::unique_ptr<ArcCertInstaller> installer_;
std::set<std::string> required_cert_names_;
base::WeakPtrFactory<ArcSmartCardManagerBridge> weak_ptr_factory_; base::WeakPtrFactory<ArcSmartCardManagerBridge> weak_ptr_factory_;
DISALLOW_COPY_AND_ASSIGN(ArcSmartCardManagerBridge); DISALLOW_COPY_AND_ASSIGN(ArcSmartCardManagerBridge);
......
...@@ -2,17 +2,22 @@ ...@@ -2,17 +2,22 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
#include <set>
#include <string> #include <string>
#include <vector> #include <vector>
#include "base/bind.h"
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
#include "base/threading/sequenced_task_runner_handle.h" #include "base/threading/sequenced_task_runner_handle.h"
#include "chrome/browser/chromeos/arc/enterprise/cert_store/arc_cert_installer.h" #include "chrome/browser/chromeos/arc/enterprise/cert_store/arc_cert_installer.h"
#include "chrome/browser/chromeos/arc/enterprise/cert_store/arc_smart_card_manager_bridge.h" #include "chrome/browser/chromeos/arc/enterprise/cert_store/arc_smart_card_manager_bridge.h"
#include "chrome/browser/chromeos/arc/policy/arc_policy_bridge.h"
#include "chrome/browser/chromeos/certificate_provider/certificate_provider.h" #include "chrome/browser/chromeos/certificate_provider/certificate_provider.h"
#include "chrome/common/net/x509_certificate_model_nss.h" #include "chrome/common/net/x509_certificate_model_nss.h"
#include "chrome/test/base/testing_profile.h" #include "chrome/test/base/testing_profile.h"
#include "components/arc/session/arc_bridge_service.h" #include "components/arc/session/arc_bridge_service.h"
#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
#include "components/keyed_service/core/keyed_service.h"
#include "components/policy/core/common/remote_commands/remote_commands_queue.h" #include "components/policy/core/common/remote_commands/remote_commands_queue.h"
#include "content/public/test/browser_task_environment.h" #include "content/public/test/browser_task_environment.h"
#include "crypto/rsa_private_key.h" #include "crypto/rsa_private_key.h"
...@@ -108,10 +113,30 @@ class MockArcCertInstaller : public ArcCertInstaller { ...@@ -108,10 +113,30 @@ class MockArcCertInstaller : public ArcCertInstaller {
std::unique_ptr<policy::RemoteCommandsQueue> queue) std::unique_ptr<policy::RemoteCommandsQueue> queue)
: ArcCertInstaller(profile, std::move(queue)) {} : ArcCertInstaller(profile, std::move(queue)) {}
MOCK_METHOD2(InstallArcCerts, MOCK_METHOD2(InstallArcCerts,
void(const std::vector<net::ScopedCERTCertificate>& certs, std::set<std::string>(
InstallArcCertsCallback callback)); const std::vector<net::ScopedCERTCertificate>& certs,
InstallArcCertsCallback callback));
}; };
class MockArcPolicyBridge : public ArcPolicyBridge {
public:
MockArcPolicyBridge(content::BrowserContext* context,
ArcBridgeService* bridge_service,
policy::PolicyService* policy_service)
: ArcPolicyBridge(context, bridge_service, policy_service) {}
MOCK_METHOD3(OnPolicyUpdated,
void(const policy::PolicyNamespace& ns,
const policy::PolicyMap& previous,
const policy::PolicyMap& current));
};
std::unique_ptr<KeyedService> BuildPolicyBridge(
ArcBridgeService* bridge_service,
content::BrowserContext* profile) {
return std::make_unique<MockArcPolicyBridge>(profile, bridge_service,
nullptr);
}
} // namespace } // namespace
class ArcSmartCardManagerBridgeTest : public testing::Test { class ArcSmartCardManagerBridgeTest : public testing::Test {
...@@ -123,8 +148,12 @@ class ArcSmartCardManagerBridgeTest : public testing::Test { ...@@ -123,8 +148,12 @@ class ArcSmartCardManagerBridgeTest : public testing::Test {
provider_ = new FakeCertificateProvider(); provider_ = new FakeCertificateProvider();
installer_ = new StrictMock<MockArcCertInstaller>( installer_ = new StrictMock<MockArcCertInstaller>(
&profile_, std::make_unique<policy::RemoteCommandsQueue>()); &profile_, std::make_unique<policy::RemoteCommandsQueue>());
policy_bridge_ = static_cast<MockArcPolicyBridge*>(
ArcPolicyBridge::GetFactory()->SetTestingFactoryAndUse(
&profile_,
base::BindRepeating(&BuildPolicyBridge, bridge_service_.get())));
bridge_ = std::make_unique<ArcSmartCardManagerBridge>( bridge_ = std::make_unique<ArcSmartCardManagerBridge>(
bridge_service_.get(), base::WrapUnique(provider_), &profile_, bridge_service_.get(), base::WrapUnique(provider_),
base::WrapUnique(installer_)); base::WrapUnique(installer_));
} }
...@@ -136,16 +165,18 @@ class ArcSmartCardManagerBridgeTest : public testing::Test { ...@@ -136,16 +165,18 @@ class ArcSmartCardManagerBridgeTest : public testing::Test {
FakeCertificateProvider* provider() { return provider_; } FakeCertificateProvider* provider() { return provider_; }
MockArcCertInstaller* installer() { return installer_; } MockArcCertInstaller* installer() { return installer_; }
MockArcPolicyBridge* policy_bridge() { return policy_bridge_; }
ArcSmartCardManagerBridge* bridge() { return bridge_.get(); } ArcSmartCardManagerBridge* bridge() { return bridge_.get(); }
private: private:
content::BrowserTaskEnvironment browser_task_environment_; content::BrowserTaskEnvironment browser_task_environment_;
TestingProfile profile_;
std::unique_ptr<ArcBridgeService> bridge_service_; std::unique_ptr<ArcBridgeService> bridge_service_;
TestingProfile profile_;
FakeCertificateProvider* provider_; // Owned by |bridge_|. FakeCertificateProvider* provider_; // Owned by |bridge_|.
MockArcCertInstaller* installer_; // Owned by |bridge_|. MockArcCertInstaller* installer_; // Owned by |bridge_|.
MockArcPolicyBridge* policy_bridge_; // Not owned.
std::unique_ptr<ArcSmartCardManagerBridge> bridge_; std::unique_ptr<ArcSmartCardManagerBridge> bridge_;
...@@ -162,6 +193,7 @@ TEST_F(ArcSmartCardManagerBridgeTest, NoSmartCardTest) { ...@@ -162,6 +193,7 @@ TEST_F(ArcSmartCardManagerBridgeTest, NoSmartCardTest) {
.WillOnce( .WillOnce(
WithArg<1>(Invoke([](base::OnceCallback<void(bool result)> callback) { WithArg<1>(Invoke([](base::OnceCallback<void(bool result)> callback) {
std::move(callback).Run(true); std::move(callback).Run(true);
return std::set<std::string>();
}))); })));
bridge()->Refresh(base::BindOnce([](bool result) { EXPECT_TRUE(result); })); bridge()->Refresh(base::BindOnce([](bool result) { EXPECT_TRUE(result); }));
} }
...@@ -174,10 +206,12 @@ TEST_F(ArcSmartCardManagerBridgeTest, BasicSmartCardTest) { ...@@ -174,10 +206,12 @@ TEST_F(ArcSmartCardManagerBridgeTest, BasicSmartCardTest) {
ASSERT_TRUE(provider()->SetCertificates(cert_names)); ASSERT_TRUE(provider()->SetCertificates(cert_names));
EXPECT_CALL(*installer(), EXPECT_CALL(*installer(),
InstallArcCerts(EqualsClientCertIdentityList(cert_names), _)) InstallArcCerts(EqualsClientCertIdentityList(cert_names), _))
.WillOnce( .WillOnce(WithArg<1>(
WithArg<1>(Invoke([](base::OnceCallback<void(bool result)> callback) { Invoke([&cert_names](base::OnceCallback<void(bool result)> callback) {
std::move(callback).Run(true); std::move(callback).Run(true);
return std::set<std::string>(cert_names.begin(), cert_names.end());
}))); })));
EXPECT_CALL(*policy_bridge(), OnPolicyUpdated(_, _, _));
bridge()->Refresh(base::BindOnce([](bool result) { EXPECT_TRUE(result); })); bridge()->Refresh(base::BindOnce([](bool result) { EXPECT_TRUE(result); }));
} }
......
...@@ -18,6 +18,7 @@ ...@@ -18,6 +18,7 @@
#include "base/strings/string_util.h" #include "base/strings/string_util.h"
#include "base/values.h" #include "base/values.h"
#include "chrome/browser/chromeos/arc/arc_session_manager.h" #include "chrome/browser/chromeos/arc/arc_session_manager.h"
#include "chrome/browser/chromeos/arc/enterprise/cert_store/arc_smart_card_manager_bridge.h"
#include "chrome/browser/chromeos/profiles/profile_helper.h" #include "chrome/browser/chromeos/profiles/profile_helper.h"
#include "chrome/browser/policy/developer_tools_policy_handler.h" #include "chrome/browser/policy/developer_tools_policy_handler.h"
#include "chrome/browser/policy/profile_policy_connector.h" #include "chrome/browser/policy/profile_policy_connector.h"
...@@ -44,6 +45,7 @@ namespace { ...@@ -44,6 +45,7 @@ namespace {
constexpr char kArcCaCerts[] = "caCerts"; constexpr char kArcCaCerts[] = "caCerts";
constexpr char kPolicyCompliantJson[] = "{ \"policyCompliant\": true }"; constexpr char kPolicyCompliantJson[] = "{ \"policyCompliant\": true }";
constexpr char kArcRequiredKeyPairs[] = "requiredKeyPairs";
// invert_bool_value: If the Chrome policy and the ARC policy with boolean value // invert_bool_value: If the Chrome policy and the ARC policy with boolean value
// have opposite semantics, set this to true so the bool is inverted before // have opposite semantics, set this to true so the bool is inverted before
...@@ -214,10 +216,24 @@ void AddOncCaCertsToPolicies(const policy::PolicyMap& policy_map, ...@@ -214,10 +216,24 @@ void AddOncCaCertsToPolicies(const policy::PolicyMap& policy_map,
filtered_policies->Set(kArcCaCerts, std::move(ca_certs)); filtered_policies->Set(kArcCaCerts, std::move(ca_certs));
} }
std::string GetFilteredJSONPolicies(const policy::PolicyMap& policy_map, void AddRequiredKeyPairs(const ArcSmartCardManagerBridge* smart_card_manager,
const PrefService* profile_prefs, base::DictionaryValue* filtered_policies) {
const std::string& guid, if (!smart_card_manager)
bool is_affiliated) { return;
std::unique_ptr<base::ListValue> cert_names(
std::make_unique<base::ListValue>());
for (const auto& name : smart_card_manager->get_required_cert_names()) {
cert_names->GetList().emplace_back(name);
}
filtered_policies->Set(kArcRequiredKeyPairs, std::move(cert_names));
}
std::string GetFilteredJSONPolicies(
const policy::PolicyMap& policy_map,
const PrefService* profile_prefs,
const std::string& guid,
bool is_affiliated,
const ArcSmartCardManagerBridge* smart_card_manager) {
base::DictionaryValue filtered_policies; base::DictionaryValue filtered_policies;
// Parse ArcPolicy as JSON string before adding other policies to the // Parse ArcPolicy as JSON string before adding other policies to the
// dictionary. // dictionary.
...@@ -275,6 +291,8 @@ std::string GetFilteredJSONPolicies(const policy::PolicyMap& policy_map, ...@@ -275,6 +291,8 @@ std::string GetFilteredJSONPolicies(const policy::PolicyMap& policy_map,
filtered_policies.SetString("guid", guid); filtered_policies.SetString("guid", guid);
AddRequiredKeyPairs(smart_card_manager, &filtered_policies);
std::string policy_json; std::string policy_json;
JSONStringValueSerializer serializer(&policy_json); JSONStringValueSerializer serializer(&policy_json);
serializer.Serialize(filtered_policies); serializer.Serialize(filtered_policies);
...@@ -354,6 +372,11 @@ ArcPolicyBridge* ArcPolicyBridge::GetForBrowserContextForTesting( ...@@ -354,6 +372,11 @@ ArcPolicyBridge* ArcPolicyBridge::GetForBrowserContextForTesting(
return ArcPolicyBridgeFactory::GetForBrowserContextForTesting(context); return ArcPolicyBridgeFactory::GetForBrowserContextForTesting(context);
} }
// static
BrowserContextKeyedServiceFactory* ArcPolicyBridge::GetFactory() {
return ArcPolicyBridgeFactory::GetInstance();
}
base::WeakPtr<ArcPolicyBridge> ArcPolicyBridge::GetWeakPtr() { base::WeakPtr<ArcPolicyBridge> ArcPolicyBridge::GetWeakPtr() {
return weak_ptr_factory_.GetWeakPtr(); return weak_ptr_factory_.GetWeakPtr();
} }
...@@ -542,9 +565,12 @@ std::string ArcPolicyBridge::GetCurrentJSONPolicies() const { ...@@ -542,9 +565,12 @@ std::string ArcPolicyBridge::GetCurrentJSONPolicies() const {
const Profile* const profile = Profile::FromBrowserContext(context_); const Profile* const profile = Profile::FromBrowserContext(context_);
const user_manager::User* const user = const user_manager::User* const user =
chromeos::ProfileHelper::Get()->GetUserByProfile(profile); chromeos::ProfileHelper::Get()->GetUserByProfile(profile);
const ArcSmartCardManagerBridge* smart_card_manager =
ArcSmartCardManagerBridge::GetForBrowserContext(context_);
return GetFilteredJSONPolicies(policy_map, profile->GetPrefs(), return GetFilteredJSONPolicies(policy_map, profile->GetPrefs(),
instance_guid_, user->IsAffiliated()); instance_guid_, user->IsAffiliated(),
smart_card_manager);
} }
void ArcPolicyBridge::OnReportComplianceParseSuccess( void ArcPolicyBridge::OnReportComplianceParseSuccess(
......
...@@ -23,6 +23,8 @@ ...@@ -23,6 +23,8 @@
#include "components/policy/core/common/policy_namespace.h" #include "components/policy/core/common/policy_namespace.h"
#include "components/policy/core/common/policy_service.h" #include "components/policy/core/common/policy_service.h"
class BrowserContextKeyedServiceFactory;
namespace base { namespace base {
class Value; class Value;
} }
...@@ -107,6 +109,9 @@ class ArcPolicyBridge : public KeyedService, ...@@ -107,6 +109,9 @@ class ArcPolicyBridge : public KeyedService,
static ArcPolicyBridge* GetForBrowserContextForTesting( static ArcPolicyBridge* GetForBrowserContextForTesting(
content::BrowserContext* context); content::BrowserContext* context);
// Return the factory instance for this class.
static BrowserContextKeyedServiceFactory* GetFactory();
base::WeakPtr<ArcPolicyBridge> GetWeakPtr(); base::WeakPtr<ArcPolicyBridge> GetWeakPtr();
ArcPolicyBridge(content::BrowserContext* context, ArcPolicyBridge(content::BrowserContext* context,
...@@ -173,6 +178,7 @@ class ArcPolicyBridge : public KeyedService, ...@@ -173,6 +178,7 @@ class ArcPolicyBridge : public KeyedService,
ArcBridgeService* const arc_bridge_service_; // Owned by ArcServiceManager. ArcBridgeService* const arc_bridge_service_; // Owned by ArcServiceManager.
policy::PolicyService* policy_service_ = nullptr; policy::PolicyService* policy_service_ = nullptr;
bool is_managed_ = false; bool is_managed_ = false;
// HACK(b/73762796): A GUID that is regenerated whenever |this| is created, // HACK(b/73762796): A GUID that is regenerated whenever |this| is created,
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/values.h" #include "base/values.h"
#include "chrome/browser/chromeos/arc/enterprise/cert_store/arc_smart_card_manager_bridge.h"
#include "chrome/browser/chromeos/arc/policy/arc_policy_bridge.h" #include "chrome/browser/chromeos/arc/policy/arc_policy_bridge.h"
#include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h" #include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h"
#include "chrome/browser/policy/developer_tools_policy_handler.h" #include "chrome/browser/policy/developer_tools_policy_handler.h"
...@@ -22,10 +23,13 @@ ...@@ -22,10 +23,13 @@
#include "components/arc/session/arc_bridge_service.h" #include "components/arc/session/arc_bridge_service.h"
#include "components/arc/test/connection_holder_util.h" #include "components/arc/test/connection_holder_util.h"
#include "components/arc/test/fake_policy_instance.h" #include "components/arc/test/fake_policy_instance.h"
#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
#include "components/keyed_service/core/keyed_service.h"
#include "components/policy/core/common/mock_policy_service.h" #include "components/policy/core/common/mock_policy_service.h"
#include "components/policy/core/common/policy_map.h" #include "components/policy/core/common/policy_map.h"
#include "components/policy/core/common/policy_namespace.h" #include "components/policy/core/common/policy_namespace.h"
#include "components/policy/core/common/policy_types.h" #include "components/policy/core/common/policy_types.h"
#include "components/policy/core/common/remote_commands/remote_commands_queue.h"
#include "components/policy/policy_constants.h" #include "components/policy/policy_constants.h"
#include "components/sync_preferences/testing_pref_service_syncable.h" #include "components/sync_preferences/testing_pref_service_syncable.h"
#include "components/user_manager/scoped_user_manager.h" #include "components/user_manager/scoped_user_manager.h"
...@@ -73,6 +77,9 @@ constexpr char kFakeONC[] = ...@@ -73,6 +77,9 @@ constexpr char kFakeONC[] =
constexpr char kPolicyCompliantResponse[] = "{ \"policyCompliant\": true }"; constexpr char kPolicyCompliantResponse[] = "{ \"policyCompliant\": true }";
constexpr char kFakeCertName[] = "cert_name";
constexpr char kRequiredKeyPairFormat[] = "\"requiredKeyPairs\":[%s%s%s]";
MATCHER_P(ValueEquals, expected, "value matches") { MATCHER_P(ValueEquals, expected, "value matches") {
return *expected == *arg; return *expected == *arg;
} }
...@@ -168,6 +175,8 @@ class ArcPolicyBridgeTestBase { ...@@ -168,6 +175,8 @@ class ArcPolicyBridgeTestBase {
profile_ = testing_profile_manager_->CreateTestingProfile("user@gmail.com"); profile_ = testing_profile_manager_->CreateTestingProfile("user@gmail.com");
ASSERT_TRUE(profile_); ASSERT_TRUE(profile_);
smart_card_manager_ = GetArcSmartCardManager();
// TODO(hidehiko): Use Singleton instance tied to BrowserContext. // TODO(hidehiko): Use Singleton instance tied to BrowserContext.
policy_bridge_ = std::make_unique<ArcPolicyBridge>( policy_bridge_ = std::make_unique<ArcPolicyBridge>(
profile_, bridge_service_.get(), &policy_service_); profile_, bridge_service_.get(), &policy_service_);
...@@ -184,6 +193,7 @@ class ArcPolicyBridgeTestBase { ...@@ -184,6 +193,7 @@ class ArcPolicyBridgeTestBase {
bridge_service_->policy()->CloseInstance(policy_instance_.get()); bridge_service_->policy()->CloseInstance(policy_instance_.get());
policy_instance_.reset(); policy_instance_.reset();
policy_bridge_->RemoveObserver(&observer_); policy_bridge_->RemoveObserver(&observer_);
testing_profile_manager_.reset();
} }
protected: protected:
...@@ -212,12 +222,29 @@ class ArcPolicyBridgeTestBase { ...@@ -212,12 +222,29 @@ class ArcPolicyBridgeTestBase {
Mock::VerifyAndClearExpectations(&observer_); Mock::VerifyAndClearExpectations(&observer_);
} }
// Specifies a testing factory for ArcSmartCardManagerBridge and returns
// instance.
// Returns nullptr by default.
// Override if the test wants to use a real smart card manager.
virtual ArcSmartCardManagerBridge* GetArcSmartCardManager() {
return static_cast<ArcSmartCardManagerBridge*>(
ArcSmartCardManagerBridge::GetFactory()->SetTestingFactoryAndUse(
profile(),
base::BindRepeating(
[](content::BrowserContext* profile)
-> std::unique_ptr<KeyedService> { return nullptr; })));
}
ArcPolicyBridge* policy_bridge() { return policy_bridge_.get(); } ArcPolicyBridge* policy_bridge() { return policy_bridge_.get(); }
const std::string& instance_guid() { return instance_guid_; } const std::string& instance_guid() { return instance_guid_; }
FakePolicyInstance* policy_instance() { return policy_instance_.get(); } FakePolicyInstance* policy_instance() { return policy_instance_.get(); }
policy::PolicyMap& policy_map() { return policy_map_; } policy::PolicyMap& policy_map() { return policy_map_; }
base::RunLoop& run_loop() { return run_loop_; } base::RunLoop& run_loop() { return run_loop_; }
TestingProfile* profile() { return profile_; } TestingProfile* profile() { return profile_; }
ArcBridgeService* bridge_service() { return bridge_service_.get(); }
ArcSmartCardManagerBridge* smart_card_manager() {
return smart_card_manager_;
}
private: private:
content::BrowserTaskEnvironment task_environment_; content::BrowserTaskEnvironment task_environment_;
...@@ -227,8 +254,9 @@ class ArcPolicyBridgeTestBase { ...@@ -227,8 +254,9 @@ class ArcPolicyBridgeTestBase {
std::unique_ptr<TestingProfileManager> testing_profile_manager_; std::unique_ptr<TestingProfileManager> testing_profile_manager_;
base::RunLoop run_loop_; base::RunLoop run_loop_;
TestingProfile* profile_; TestingProfile* profile_;
std::unique_ptr<ArcBridgeService> bridge_service_; std::unique_ptr<ArcBridgeService> bridge_service_;
ArcSmartCardManagerBridge* smart_card_manager_; // Not owned.
std::unique_ptr<ArcPolicyBridge> policy_bridge_; std::unique_ptr<ArcPolicyBridge> policy_bridge_;
std::string instance_guid_; std::string instance_guid_;
MockArcPolicyBridgeObserver observer_; MockArcPolicyBridgeObserver observer_;
...@@ -262,6 +290,23 @@ class ArcPolicyBridgeAffiliatedTest : public ArcPolicyBridgeTestBase, ...@@ -262,6 +290,23 @@ class ArcPolicyBridgeAffiliatedTest : public ArcPolicyBridgeTestBase,
const bool is_affiliated_; const bool is_affiliated_;
}; };
// Tests required key pair policy.
class ArcPolicyBridgeRequiredKeyPairTest : public ArcPolicyBridgeTest {
protected:
ArcSmartCardManagerBridge* GetArcSmartCardManager() override {
return static_cast<ArcSmartCardManagerBridge*>(
ArcSmartCardManagerBridge::GetFactory()->SetTestingFactoryAndUse(
profile(), base::BindRepeating(
[](ArcBridgeService* bridge_service,
content::BrowserContext* profile)
-> std::unique_ptr<KeyedService> {
return std::make_unique<ArcSmartCardManagerBridge>(
profile, bridge_service, nullptr, nullptr);
},
bridge_service())));
}
};
TEST_F(ArcPolicyBridgeTest, UnmanagedTest) { TEST_F(ArcPolicyBridgeTest, UnmanagedTest) {
policy_bridge()->OverrideIsManagedForTesting(false); policy_bridge()->OverrideIsManagedForTesting(false);
GetPoliciesAndVerifyResult(""); GetPoliciesAndVerifyResult("");
...@@ -568,4 +613,25 @@ INSTANTIATE_TEST_SUITE_P(ArcPolicyBridgeAffiliatedTestInstance, ...@@ -568,4 +613,25 @@ INSTANTIATE_TEST_SUITE_P(ArcPolicyBridgeAffiliatedTestInstance,
ArcPolicyBridgeAffiliatedTest, ArcPolicyBridgeAffiliatedTest,
testing::Bool()); testing::Bool());
// Tests that if smart card manager is non-null, the required key pair policy is
// set to the required certificate list.
TEST_F(ArcPolicyBridgeRequiredKeyPairTest, RequiredKeyPairsBasicTest) {
EXPECT_TRUE(smart_card_manager());
// One certificate is required to be installed.
smart_card_manager()->set_required_cert_names_for_testing(
std::vector<std::string>({kFakeCertName}));
GetPoliciesAndVerifyResult(
"{\"guid\":\"" + instance_guid() + "\"," +
base::StringPrintf(kRequiredKeyPairFormat, "\"", kFakeCertName, "\"") +
"}");
// An empty list is required to be installed.
smart_card_manager()->set_required_cert_names_for_testing(
std::vector<std::string>());
GetPoliciesAndVerifyResult(
"{\"guid\":\"" + instance_guid() + "\"," +
base::StringPrintf(kRequiredKeyPairFormat, "", "", "") + "}");
}
} // namespace arc } // namespace arc
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