VCD: Authenticate all CameraHalClient's

Authenticates all CameraHalClient's and disable
CameraHalDispather::RegisterClient().

Now that we finish migrating CameraHalServer and CameraHalClient
registration, we have a stabilized interface and behavior for
TokenManager. Here we add token_manager_unittest and a series of
unittests to make sure things work as expected.

Bug: b/170075468
Test: Verify that all CameraHalClient's are correctly recognized and
authenticated. \
   (hatch-arc-r) $ capture_unittests --gtest_filter='*TokenManager*' \
                   capture_unittests --gtest_filter='*Dispatcher*'

Change-Id: I701cc4de99b34a1599c36b57d63a5a2429d6ef9a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2576228
Reviewed-by: Guido Urdaneta <guidou@chromium.org>
Reviewed-by: Wei Lee <wtlee@chromium.org>
Commit-Queue: Jasmine Chen <lnishan@google.com>
Cr-Commit-Position: refs/heads/master@{#837520}

media/capture/BUILD.gn

@@ -536,6 +536,7 @@ test("capture_unittests") {
       "video/chromeos/camera_hal_delegate_unittest.cc",
       "video/chromeos/camera_hal_dispatcher_impl_unittest.cc",
       "video/chromeos/request_manager_unittest.cc",
+      "video/chromeos/token_manager_unittest.cc",
     ]
     deps += [
       "//build/config/linux/libdrm",

media/capture/video/chromeos/camera_hal_dispatcher_impl.cc

@@ -15,6 +15,7 @@
 #include "base/files/file.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
+#include "base/notreached.h"
 #include "base/posix/eintr_wrapper.h"
 #include "base/rand_util.h"
 #include "base/single_thread_task_runner.h"
@@ -101,6 +102,16 @@ class MojoCameraClientObserver : public CameraClientObserver {
 
 CameraClientObserver::~CameraClientObserver() = default;
 
+bool CameraClientObserver::Authenticate(TokenManager* token_manager) {
+  auto authenticated_type =
+      token_manager->AuthenticateClient(type_, auth_token_);
+  if (!authenticated_type) {
+    return false;
+  }
+  type_ = authenticated_type.value();
+  return true;
+}
+
 FailedCameraHalServerCallbacks::FailedCameraHalServerCallbacks()
     : callbacks_(this) {}
 FailedCameraHalServerCallbacks::~FailedCameraHalServerCallbacks() = default;
@@ -300,14 +311,7 @@ void CameraHalDispatcherImpl::RegisterServerWithToken(
 
 void CameraHalDispatcherImpl::RegisterClient(
     mojo::PendingRemote<cros::mojom::CameraHalClient> client) {
-  // RegisterClient can be called locally by ArcCameraBridge, so it's not
-  // necessarily called on |proxy_thread_|.
-
-  // TODO(b/170075468): Reject this call once we've migrated all camera clients.
-  auto temporary_token = base::UnguessableToken::Create();
-  RegisterClientWithToken(std::move(client),
-                          cros::mojom::CameraClientType::UNKNOWN,
-                          temporary_token, base::DoNothing());
+  NOTREACHED() << "RegisterClient() is disabled";
 }
 
 void CameraHalDispatcherImpl::RegisterClientWithToken(
@@ -551,8 +555,7 @@ void CameraHalDispatcherImpl::AddClientObserverOnProxyThread(
     std::unique_ptr<CameraClientObserver> observer,
     base::OnceCallback<void(int32_t)> result_callback) {
   DCHECK(proxy_task_runner_->BelongsToCurrentThread());
-  if (!token_manager_.AuthenticateClient(observer->GetType(),
-                                         observer->GetAuthToken())) {
+  if (!observer->Authenticate(&token_manager_)) {
     LOG(ERROR) << "Failed to authenticate camera client observer";
     std::move(result_callback).Run(-EPERM);
     return;

media/capture/video/chromeos/camera_hal_dispatcher_impl.h

@@ -58,6 +58,8 @@ class CAPTURE_EXPORT CameraClientObserver {
   cros::mojom::CameraClientType GetType() { return type_; }
   const base::UnguessableToken GetAuthToken() { return auth_token_; }
 
+  bool Authenticate(TokenManager* token_manager);
+
  private:
   cros::mojom::CameraClientType type_;
   base::UnguessableToken auth_token_;

media/capture/video/chromeos/camera_hal_dispatcher_impl_unittest.cc

@@ -11,6 +11,7 @@
 #include "base/posix/safe_strerror.h"
 #include "base/run_loop.h"
 #include "base/single_thread_task_runner.h"
+#include "base/synchronization/waitable_event.h"
 #include "base/test/task_environment.h"
 #include "media/capture/video/chromeos/mojom/camera_common.mojom.h"
 #include "media/capture/video/chromeos/mojom/cros_camera_service.mojom.h"
@@ -90,7 +91,8 @@ class MockCameraActiveClientObserver : public CameraActiveClientObserver {
 
 class CameraHalDispatcherImplTest : public ::testing::Test {
  public:
-  CameraHalDispatcherImplTest() = default;
+  CameraHalDispatcherImplTest()
+      : register_client_event_(base::WaitableEvent::ResetPolicy::AUTOMATIC) {}
 
   ~CameraHalDispatcherImplTest() override = default;
 
@@ -127,12 +129,15 @@ class CameraHalDispatcherImplTest : public ::testing::Test {
                                         std::move(callback));
   }
 
-  static void RegisterClient(
+  static void RegisterClientWithToken(
       CameraHalDispatcherImpl* dispatcher,
-      mojo::PendingRemote<cros::mojom::CameraHalClient> client) {
-    // TODO(b/170075468): Migrate to RegisterClientWithToken once the migration
-    // is done.
-    dispatcher->RegisterClient(std::move(client));
+      mojo::PendingRemote<cros::mojom::CameraHalClient> client,
+      cros::mojom::CameraClientType type,
+      const base::UnguessableToken& token,
+      cros::mojom::CameraHalDispatcher::RegisterClientWithTokenCallback
+          callback) {
+    dispatcher->RegisterClientWithToken(std::move(client), type, token,
+                                        std::move(callback));
   }
 
   void OnRegisteredServer(
@@ -145,10 +150,22 @@ class CameraHalDispatcherImplTest : public ::testing::Test {
     }
   }
 
+  void OnRegisteredClient(int32_t result) {
+    last_register_client_result_ = result;
+    if (result != 0) {
+      // If registration fails, CameraHalClient::SetUpChannel() will not be
+      // called, and we need to quit the run loop here.
+      QuitRunLoop();
+    }
+    register_client_event_.Signal();
+  }
+
  protected:
   // We can't use std::unique_ptr here because the constructor and destructor of
   // CameraHalDispatcherImpl are private.
   CameraHalDispatcherImpl* dispatcher_;
+  base::WaitableEvent register_client_event_;
+  int32_t last_register_client_result_;
 
  private:
   base::test::TaskEnvironment task_environment_;
@@ -179,14 +196,25 @@ TEST_F(CameraHalDispatcherImplTest, ServerConnectionError) {
           base::BindOnce(&CameraHalDispatcherImplTest::OnRegisteredServer,
                          base::Unretained(this))));
   auto client = mock_client->GetPendingRemote();
+  auto type = cros::mojom::CameraClientType::TESTING;
   GetProxyTaskRunner()->PostTask(
       FROM_HERE,
-      base::BindOnce(&CameraHalDispatcherImplTest::RegisterClient,
-                     base::Unretained(dispatcher_), std::move(client)));
+      base::BindOnce(
+          &CameraHalDispatcherImplTest::RegisterClientWithToken,
+          base::Unretained(dispatcher_), std::move(client), type,
+          dispatcher_->GetTokenForTrustedClient(type),
+          base::BindOnce(&CameraHalDispatcherImplTest::OnRegisteredClient,
+                         base::Unretained(this))));
 
   // Wait until the client gets the established Mojo channel.
   DoLoop();
 
+  // The client registration callback may be called after
+  // CameraHalClient::SetUpChannel(). Use a waitable event to make sure we have
+  // the result.
+  register_client_event_.Wait();
+  ASSERT_EQ(last_register_client_result_, 0);
+
   // Re-create a new server to simulate a server crash.
   mock_server = std::make_unique<MockCameraHalServer>();
 
@@ -234,15 +262,26 @@ TEST_F(CameraHalDispatcherImplTest, ClientConnectionError) {
           base::BindOnce(&CameraHalDispatcherImplTest::OnRegisteredServer,
                          base::Unretained(this))));
   auto client = mock_client->GetPendingRemote();
+  auto type = cros::mojom::CameraClientType::TESTING;
   GetProxyTaskRunner()->PostTask(
       FROM_HERE,
-      base::BindOnce(&CameraHalDispatcherImplTest::RegisterClient,
-                     base::Unretained(dispatcher_), std::move(client)));
+      base::BindOnce(
+          &CameraHalDispatcherImplTest::RegisterClientWithToken,
+          base::Unretained(dispatcher_), std::move(client), type,
+          dispatcher_->GetTokenForTrustedClient(type),
+          base::BindOnce(&CameraHalDispatcherImplTest::OnRegisteredClient,
+                         base::Unretained(this))));
 
   // Wait until the client gets the established Mojo channel.
   DoLoop();
 
-  // Re-create a new server to simulate a server crash.
+  // The client registration callback may be called after
+  // CameraHalClient::SetUpChannel(). Use a waitable event to make sure we have
+  // the result.
+  register_client_event_.Wait();
+  ASSERT_EQ(last_register_client_result_, 0);
+
+  // Re-create a new client to simulate a client crash.
   mock_client = std::make_unique<MockCameraHalClient>();
 
   // Make sure we re-create the Mojo channel from the same server to the new
@@ -254,13 +293,103 @@ TEST_F(CameraHalDispatcherImplTest, ClientConnectionError) {
           InvokeWithoutArgs(this, &CameraHalDispatcherImplTest::QuitRunLoop));
 
   client = mock_client->GetPendingRemote();
+  type = cros::mojom::CameraClientType::TESTING;
   GetProxyTaskRunner()->PostTask(
       FROM_HERE,
-      base::BindOnce(&CameraHalDispatcherImplTest::RegisterClient,
-                     base::Unretained(dispatcher_), std::move(client)));
+      base::BindOnce(
+          &CameraHalDispatcherImplTest::RegisterClientWithToken,
+          base::Unretained(dispatcher_), std::move(client), type,
+          dispatcher_->GetTokenForTrustedClient(type),
+          base::BindOnce(&CameraHalDispatcherImplTest::OnRegisteredClient,
+                         base::Unretained(this))));
 
   // Wait until the clients gets the newly established Mojo channel.
   DoLoop();
+
+  // Make sure the client is still successfully registered.
+  register_client_event_.Wait();
+  ASSERT_EQ(last_register_client_result_, 0);
+}
+
+// Test that trusted camera HAL clients (e.g., Chrome, Android, Testing) can be
+// registered successfully.
+TEST_F(CameraHalDispatcherImplTest, RegisterClientSuccess) {
+  // First verify that a the CameraHalDispatcherImpl establishes a Mojo channel
+  // between the server and the client.
+  auto mock_server = std::make_unique<MockCameraHalServer>();
+
+  auto server = mock_server->GetPendingRemote();
+  GetProxyTaskRunner()->PostTask(
+      FROM_HERE,
+      base::BindOnce(
+          &CameraHalDispatcherImplTest::RegisterServer,
+          base::Unretained(dispatcher_), std::move(server),
+          base::BindOnce(&CameraHalDispatcherImplTest::OnRegisteredServer,
+                         base::Unretained(this))));
+
+  for (auto type : TokenManager::kTrustedClientTypes) {
+    auto mock_client = std::make_unique<MockCameraHalClient>();
+    EXPECT_CALL(*mock_server, DoCreateChannel(_, _)).Times(1);
+    EXPECT_CALL(*mock_client, DoSetUpChannel(_))
+        .Times(1)
+        .WillOnce(
+            InvokeWithoutArgs(this, &CameraHalDispatcherImplTest::QuitRunLoop));
+
+    auto client = mock_client->GetPendingRemote();
+    GetProxyTaskRunner()->PostTask(
+        FROM_HERE,
+        base::BindOnce(
+            &CameraHalDispatcherImplTest::RegisterClientWithToken,
+            base::Unretained(dispatcher_), std::move(client), type,
+            dispatcher_->GetTokenForTrustedClient(type),
+            base::BindOnce(&CameraHalDispatcherImplTest::OnRegisteredClient,
+                           base::Unretained(this))));
+
+    // Wait until the client gets the established Mojo channel.
+    DoLoop();
+
+    // The client registration callback may be called after
+    // CameraHalClient::SetUpChannel(). Use a waitable event to make sure we
+    // have the result.
+    register_client_event_.Wait();
+    ASSERT_EQ(last_register_client_result_, 0);
+  }
+}
+
+// Test that CameraHalClient registration fails when a wrong (empty) token is
+// provided.
+TEST_F(CameraHalDispatcherImplTest, RegisterClientFail) {
+  // First verify that a the CameraHalDispatcherImpl establishes a Mojo channel
+  // between the server and the client.
+  auto mock_server = std::make_unique<MockCameraHalServer>();
+  auto mock_client = std::make_unique<MockCameraHalClient>();
+
+  auto server = mock_server->GetPendingRemote();
+  GetProxyTaskRunner()->PostTask(
+      FROM_HERE,
+      base::BindOnce(
+          &CameraHalDispatcherImplTest::RegisterServer,
+          base::Unretained(dispatcher_), std::move(server),
+          base::BindOnce(&CameraHalDispatcherImplTest::OnRegisteredServer,
+                         base::Unretained(this))));
+
+  // Use an empty token to make sure authentication fails.
+  base::UnguessableToken empty_token;
+  auto client = mock_client->GetPendingRemote();
+  GetProxyTaskRunner()->PostTask(
+      FROM_HERE,
+      base::BindOnce(
+          &CameraHalDispatcherImplTest::RegisterClientWithToken,
+          base::Unretained(dispatcher_), std::move(client),
+          cros::mojom::CameraClientType::TESTING, empty_token,
+          base::BindOnce(&CameraHalDispatcherImplTest::OnRegisteredClient,
+                         base::Unretained(this))));
+
+  // We do not need to enter a run loop here because
+  // CameraHalClient::SetUpChannel() isn't expected to called, and we only need
+  // to wait for the callback from CameraHalDispatcher::RegisterClientWithToken.
+  register_client_event_.Wait();
+  ASSERT_EQ(last_register_client_result_, -EPERM);
 }
 
 // Test that CameraHalDispatcherImpl correctly fires CameraActiveClientObserver

media/capture/video/chromeos/token_manager.cc

@@ -65,20 +65,20 @@ bool WriteTokenToFile(const base::FilePath& token_path,
 
 namespace media {
 
+constexpr char TokenManager::kServerTokenPath[];
+constexpr char TokenManager::kTestClientTokenPath[];
+constexpr std::array<cros::mojom::CameraClientType, 3>
+    TokenManager::kTrustedClientTypes;
+
 TokenManager::TokenManager() = default;
 TokenManager::~TokenManager() = default;
 
 bool TokenManager::GenerateServerToken() {
-  static constexpr char kServerTokenPath[] = "/run/camera_tokens/server/token";
-
   server_token_ = base::UnguessableToken::Create();
   return WriteTokenToFile(base::FilePath(kServerTokenPath), server_token_);
 }
 
 bool TokenManager::GenerateTestClientToken() {
-  static constexpr char kTestClientTokenPath[] =
-      "/run/camera_tokens/testing/token";
-
   return WriteTokenToFile(
       base::FilePath(kTestClientTokenPath),
       GetTokenForTrustedClient(cros::mojom::CameraClientType::TESTING));
@@ -87,8 +87,10 @@ bool TokenManager::GenerateTestClientToken() {
 base::UnguessableToken TokenManager::GetTokenForTrustedClient(
     cros::mojom::CameraClientType type) {
   base::AutoLock l(client_token_map_lock_);
-  // pluginvm's token should be generated by vm_permission_service.
-  CHECK_NE(type, cros::mojom::CameraClientType::PLUGINVM);
+  if (std::find(kTrustedClientTypes.begin(), kTrustedClientTypes.end(), type) ==
+      kTrustedClientTypes.end()) {
+    return base::UnguessableToken();
+  }
   auto& token_set = client_token_map_[type];
   if (token_set.empty()) {
     token_set.insert(base::UnguessableToken::Create());
@@ -120,21 +122,36 @@ bool TokenManager::AuthenticateServer(const base::UnguessableToken& token) {
   return server_token_ == token;
 }
 
-bool TokenManager::AuthenticateClient(cros::mojom::CameraClientType type,
-                                      const base::UnguessableToken& token) {
+base::Optional<cros::mojom::CameraClientType> TokenManager::AuthenticateClient(
+    cros::mojom::CameraClientType type,
+    const base::UnguessableToken& token) {
   base::AutoLock l(client_token_map_lock_);
-  // TODO(b/170075468): Check other clients after they have been migrated to
-  // CameraHalDispatcher::RegisterClientWithToken.
-  if (type != cros::mojom::CameraClientType::CHROME) {
-    return true;
+  if (type == cros::mojom::CameraClientType::UNKNOWN) {
+    for (const auto& client_token_map_pair : client_token_map_) {
+      const auto& token_set = client_token_map_pair.second;
+      if (token_set.find(token) != token_set.end()) {
+        return client_token_map_pair.first;
+      }
+    }
+    return base::nullopt;
   }
   auto& token_set = client_token_map_[type];
-  return token_set.find(token) != token_set.end();
+  if (token_set.find(token) == token_set.end()) {
+    return base::nullopt;
+  }
+  return type;
 }
 
 void TokenManager::AssignServerTokenForTesting(
     const base::UnguessableToken& token) {
   server_token_ = token;
 }
 
+void TokenManager::AssignClientTokenForTesting(
+    cros::mojom::CameraClientType type,
+    const base::UnguessableToken& token) {
+  base::AutoLock l(client_token_map_lock_);
+  client_token_map_[type].insert(token);
+}
+
 }  // namespace media