Implement DaemonController Start(), Stop() and GetStatus() for Mac.

remoting/host/plugin/daemon_controller_mac.cc

Index: remoting/host/plugin/daemon_controller_mac.cc
diff --git a/remoting/host/plugin/daemon_controller_mac.cc b/remoting/host/plugin/daemon_controller_mac.cc
index 0aa7f956ad8b50fd0c74a250ff789e06747f2170..9c5cf26e981dcd8a51c12640f09f7bfcb2d0f0b6 100644
--- a/remoting/host/plugin/daemon_controller_mac.cc
+++ b/remoting/host/plugin/daemon_controller_mac.cc
@@ -4,17 +4,53 @@
 
 #include "remoting/host/plugin/daemon_controller.h"
 
+#include <launch.h>
+#include <sys/types.h>
+
 #include "base/basictypes.h"
+#include "base/bind.h"
 #include "base/compiler_specific.h"
+#include "base/file_path.h"
+#include "base/file_util.h"
+#include "base/json/json_writer.h"
 #include "base/logging.h"
+#include "base/mac/authorization_util.h"
+#include "base/mac/launchd.h"
+#include "base/mac/mac_logging.h"
+#include "base/mac/scoped_authorizationref.h"
+#include "base/mac/scoped_launch_data.h"
+#include "base/synchronization/lock.h"
+#include "base/threading/thread.h"
+#include "base/values.h"
+#include "remoting/host/json_host_config.h"
 
 namespace remoting {
 
 namespace {
 
+// The name of the Remoting Host service that is registered with launchd.
+#define kServiceName "org.chromium.chromoting"
+#define kConfigDir "/Library/PrivilegedHelperTools/"

[dmac, 2012/03/29 14:52:41]

normally we wouldn't hardcode a path like this. There isn't a easy helper for
PrivilegedHelperTools that I can find, but we would normally get the Library
directory using NSSearchPathForDirectoriesInDomains.

+
+// This helper script is executed as root.  It is passed a command-line option
+// (--enable or --disable), which causes it to create or remove a trigger file.
+// The trigger file (defined in the service's plist file) informs launchd
+// whether the Host service should be running.  Creating the trigger file causes
+// launchd to immediately start the service.  Deleting the trigger file has no
+// immediate effect, but it prevents the service from being restarted if it
+// becomes stopped.
+const char kStartStopTool[] = kConfigDir kServiceName ".me2me.sh";
+
+// Use a single configuration file, instead of separate "auth" and "host" files.
+// This is because the SetConfigAndStart() API only provides a single
+// dictionary, and splitting this into two dictionaries would require
+// knowledge of which keys belong in which files.
+const char kHostConfigFile[] = kConfigDir kServiceName ".json";
+
 class DaemonControllerMac : public remoting::DaemonController {
  public:
   DaemonControllerMac();
+  virtual ~DaemonControllerMac();
 
   virtual State GetState() OVERRIDE;
   virtual void GetConfig(const GetConfigCallback& callback) OVERRIDE;
@@ -24,23 +60,78 @@ class DaemonControllerMac : public remoting::DaemonController {
   virtual void Stop() OVERRIDE;
 
  private:
+  void DoGetConfig(const GetConfigCallback& callback);
+  void DoSetConfigAndStart(scoped_ptr<base::DictionaryValue> config);
+  void DoStop();
+
+  bool RunToolScriptAsRoot(const char* command);
+  bool StopService();
+
+  // The API for gaining root privileges is blocking (it prompts the user for
+  // a password).  Since Start() and Stop() must not block the main thread, they
+  // need to post their tasks to a separate thread.
+  base::Thread auth_thread_;
+
+  // This distinguishes between STATE_START_FAILED and STATE_STOPPED in
+  // GetState().
+  bool start_failed_;
+  base::Lock start_failed_lock_;
+
   DISALLOW_COPY_AND_ASSIGN(DaemonControllerMac);
 };
 
-DaemonControllerMac::DaemonControllerMac() {
+DaemonControllerMac::DaemonControllerMac()
+    : auth_thread_("Auth thread"),
+      start_failed_(false) {
+  auth_thread_.Start();
+}
+
+DaemonControllerMac::~DaemonControllerMac() {
+  // This will block if the thread is waiting on a root password prompt.  There
+  // doesn't seem to be an easy solution for this, other than to spawn a
+  // separate process to do the root elevation.
+
+  // TODO(lambroslambrou): Improve this, either by finding a way to terminate
+  // the thread, or by moving to a separate process.
+  auth_thread_.Stop();
 }
 
 DaemonController::State DaemonControllerMac::GetState() {
-  return DaemonController::STATE_NOT_IMPLEMENTED;
+  pid_t job_pid = base::mac::PIDForJob(kServiceName);
+  if (job_pid < 0) {
+    // TODO(lambroslambrou): Change this to STATE_NOT_INSTALLED when the
+    // installation process is implemented.
+    return remoting::DaemonController::STATE_NOT_IMPLEMENTED;

[Jamie, 2012/03/29 05:20:29]

Nit: You're already in namespace remoting. No need to specify it here I don't
think.

[Lambros, 2012/03/29 18:20:44]

Done.

+  } else if (job_pid == 0) {
+    // Service is stopped, or a start attempt failed.
+    base::AutoLock lock(start_failed_lock_);
+    return start_failed_ ? remoting::DaemonController::STATE_START_FAILED :
+        remoting::DaemonController::STATE_STOPPED;
+  } else {
+    return remoting::DaemonController::STATE_STARTED;
+  }
 }
 
 void DaemonControllerMac::GetConfig(const GetConfigCallback& callback) {
-  NOTIMPLEMENTED();
+  // base::Unretained() is safe, since this object owns the thread and therefore
+  // outlives it.
+  auth_thread_.message_loop_proxy()->PostTask(
+      FROM_HERE,
+      base::Bind(&DaemonControllerMac::DoGetConfig, base::Unretained(this),
+                 callback));
 }
 
 void DaemonControllerMac::SetConfigAndStart(
     scoped_ptr<base::DictionaryValue> config) {
-  NOTIMPLEMENTED();
+  {
+    base::AutoLock lock(start_failed_lock_);
+    start_failed_ = false;
+  }
+
+  auth_thread_.message_loop_proxy()->PostTask(
+      FROM_HERE,
+      base::Bind(&DaemonControllerMac::DoSetConfigAndStart,
+                 base::Unretained(this), base::Passed(&config)));
 }
 
 void DaemonControllerMac::SetPin(const std::string& pin) {
@@ -48,7 +139,115 @@ void DaemonControllerMac::SetPin(const std::string& pin) {
 }
 
 void DaemonControllerMac::Stop() {
-  NOTIMPLEMENTED();
+  auth_thread_.message_loop_proxy()->PostTask(
+      FROM_HERE,
+      base::Bind(&DaemonControllerMac::DoStop, base::Unretained(this)));
+}
+
+void DaemonControllerMac::DoGetConfig(const GetConfigCallback& callback) {
+  JsonHostConfig host_config(FilePath(kHostConfigFile),
+                             base::MessageLoopProxy::current());
+  host_config.Read();
+
+  scoped_ptr<base::DictionaryValue> config(new base::DictionaryValue());
+
+  const char* key = "host_id";
+  std::string value;
+  if (host_config.GetString(key, &value))
+    config.get()->SetString(key, value);
+  key = "xmpp_login";
+  if (host_config.GetString(key, &value))
+    config.get()->SetString(key, value);
+
+  callback.Run(config.Pass());
+}
+
+void DaemonControllerMac::DoSetConfigAndStart(
+    scoped_ptr<base::DictionaryValue> config) {
+  // JsonHostConfig doesn't provide a way to save on the current thread, wait
+  // for completion, and know whether the save succeeded.  Instead, use
+  // base::JSONWriter directly.
+
+  // TODO(lambroslambrou): Improve the JsonHostConfig interface.
+  std::string file_content;
+  base::JSONWriter::WriteWithOptions(config.get(),
+                                     base::JSONWriter::OPTIONS_PRETTY_PRINT,

[Jamie, 2012/03/29 05:20:29]

Optional: Pretty-printing doesn't seem necessary. If there's a simpler API that
doesn't take an options parameter, it might be worth using that instead.

[Lambros, 2012/03/29 18:20:44]

Done.

+                                     &file_content);
+  if (file_util::WriteFile(FilePath(kHostConfigFile), file_content.c_str(),
+                           file_content.size()) !=
+      static_cast<int>(file_content.size())) {
+    LOG(ERROR) << "Failed to write config file: " << kHostConfigFile;
+    base::AutoLock lock(start_failed_lock_);
+    start_failed_ = true;
+    return;
+  }
+
+  // Creating the trigger file causes launchd to start the service, so the
+  // extra step performed in DoStop() is not necessary here.
+  if (!RunToolScriptAsRoot("--enable")) {
+    base::AutoLock lock(start_failed_lock_);
+    start_failed_ = true;
+  }
+}
+
+void DaemonControllerMac::DoStop() {
+  if (!RunToolScriptAsRoot("--disable"))
+    return;
+
+  // Deleting the trigger file does not cause launchd to stop the service.
+  // Since the service is running for the local user's desktop (not as root),
+  // it has to be stopped for that user.  This cannot easily be done in the
+  // shell-script running as root, so it is done here instead.
+  StopService();
+}
+
+bool DaemonControllerMac::RunToolScriptAsRoot(const char* command) {
+  base::mac::ScopedAuthorizationRef authorization(
+      base::mac::AuthorizationCreateToRunAsRoot(CFSTR("")));

[dmac, 2012/03/29 14:52:41]

No prompt?

[Lambros, 2012/03/29 18:20:44]

Added TODO.

+  if (!authorization) {
+    LOG(ERROR) << "Failed to get root privileges.";
+    return false;
+  }
+
+  const char* arguments[] = { command, NULL };
+  int exit_status;
+  OSStatus status = base::mac::ExecuteWithPrivilegesAndWait(

[dmac, 2012/03/29 14:52:41]

Doing it this way is somewhat deprecated on Lion. Please see
http://atnan.com/blog/2012/02/29/modern-privileged-helper-tools-using-smjobbl...
for some details.

[Lambros, 2012/03/29 18:20:44]

This is non-trivial, unfortunately.  The SM framework is only available in sdk
10.6 and we still use 10.5.  Even then, the proposed solutions are quite
elaborate.

+      authorization.get(),
+      kStartStopTool,

[dmac, 2012/03/29 14:52:41]

instead of running kStartStopTool directly with full privileges, would it be
better to run it via sandbox-exec and control it's potential for evil?

Also, possibly check to make sure that it's owner and chmod values are correct
before firing it off? 

[Lambros, 2012/03/29 18:20:44]

Raised bug-report and added TODO for this.

On 2012/03/29 14:52:41, dmac wrote:

[dmac, 2012/03/29 23:46:25]

OK... I would consider this to be of high priority from a security standpoint.

+      kAuthorizationFlagDefaults,
+      arguments,
+      NULL,
+      &exit_status);
+  if (status != errAuthorizationSuccess) {
+    OSSTATUS_LOG(ERROR, status) << "AuthorizationExecuteWithPrivileges";
+    return false;
+  }
+  if (exit_status != 0) {
+    LOG(ERROR) << kStartStopTool << " failed with exit status " << exit_status;
+    return false;
+  }
+
+  return true;
+}
+
+bool DaemonControllerMac::StopService() {
+  base::mac::ScopedLaunchData response(
+      base::mac::MessageForJob(kServiceName, LAUNCH_KEY_STOPJOB));
+  if (!response) {
+    LOG(ERROR) << "Failed to send message to launchd";
+    return false;
+  }
+
+  // Got a response, so check if launchd sent a non-zero error code, otherwise
+  // assume the command was successful.
+  if (launch_data_get_type(response.get()) == LAUNCH_DATA_ERRNO) {
+    int error = launch_data_get_errno(response.get());
+    if (error) {
+      LOG(ERROR) << "launchd returned error " << error;
+      return false;
+    }
+  }
+  return true;
 }
 
 }  // namespace