Add support for CRD user-session to operate setuid

remoting/host/linux/remoting_user_session.cc

Index: remoting/host/linux/remoting_user_session.cc
diff --git a/remoting/host/linux/remoting_user_session.cc b/remoting/host/linux/remoting_user_session.cc
index 06b678c551e28dd7de99fe90a94bcc5696824abe..46bc868506e6322b48305111785a5abe6f5ea5bf 100644
--- a/remoting/host/linux/remoting_user_session.cc
+++ b/remoting/host/linux/remoting_user_session.cc
@@ -245,9 +245,12 @@ class PamHandle {
 
 // Runs the me2me script in a PAM session. Exits the program on failure.
 // If chown_log is true, the owner and group of the file associated with stdout
-// will be changed to the target user.
-void ExecuteSession(std::string user, base::StringPiece script_path,
-                    bool chown_log) {
+// will be changed to the target user. If match_uid is not nullopt, this
+// function will fail if the final user id does not match the one provided.
+void ExecuteSession(std::string user,
+                    base::StringPiece script_path,
+                    bool chown_log,
+                    base::Optional<uid_t> match_uid) {
   //////////////////////////////////////////////////////////////////////////////
   // Set up the PAM session
   //////////////////////////////////////////////////////////////////////////////
@@ -315,6 +318,10 @@ void ExecuteSession(std::string user, base::StringPiece script_path,
   pwinfo = getpwnam(user.c_str());
   PCHECK(pwinfo != nullptr) << "getpwnam failed";
 
+  if (match_uid && pwinfo->pw_uid != *match_uid) {
+    LOG(FATAL) << "PAM remapped username to one with a different user ID.";
+  }
+
   if (chown_log) {
     int result = fchown(STDOUT_FILENO, pwinfo->pw_uid, pwinfo->pw_gid);
     PLOG_IF(WARNING, result != 0) << "Failed to change log file owner";
@@ -398,6 +405,31 @@ int OpenLogFile() {
   return fd;
 }
 
+// Find the username for the current user. If either USER or LOGNAME is set to
+// a user matching our real user id, we return that. Otherwise, we use getpwuid
+// to attempt a reverse lookup. Note: It's possible to more than user with the
+// same user id but different group membership, home directories, et cetera,
+// which is why we check USER and LOGNAME first.
+std::string FindCurrentUsername() {
+  uid_t real_id = getuid();
+  struct passwd* pwinfo;
+  for (const char* var : {"USER", "LOGNAME"}) {
+    const char* value = getenv(var);
+    if (value) {
+      pwinfo = getpwnam(value);
+      // USER and LOGNAME can be overridden, so make sure the value is valid
+      // and matches the UID of the invoking user.
+      if (pwinfo && pwinfo->pw_uid == real_id) {
+        return pwinfo->pw_name;
+      }
+    }
+  }
+  errno = 0;
+  pwinfo = getpwuid(real_id);
+  PCHECK(pwinfo) << "getpwuid failed";
+  return pwinfo->pw_name;
+}
+
 // Daemonizes the process. Output is redirected to a file. Exits the program on
 // failure.
 //
@@ -474,14 +506,24 @@ int main(int argc, char** argv) {
     std::exit(EXIT_FAILURE);
   }
 
-  if (user.empty()) {
-    std::fputs("The target user must be specified.\n", stderr);
+  uid_t real_uid = getuid();

[Jamie, 2017/06/16 19:24:23]

Please be consistent with the naming of this; real_uid or real_id are both fine,
but pick one :)

[rkjnsn, 2017/06/16 20:28:31]

Good catch.

+
+  if (real_uid == 0 && user.empty()) {
+    std::fputs("Target user must be specified when run as root.\n", stderr);
+    std::exit(EXIT_FAILURE);
+  } else if (real_uid != 0 && !user.empty()) {
+    std::fputs("Target user may not be specified by non-root users.\n", stderr);
     std::exit(EXIT_FAILURE);

[Jamie, 2017/06/16 19:24:23]

Since you already check LOGNAME and USER and use them if they are valid,
couldn't you allow the user to be specified for non-root, but subject it to the
same check? Prohibiting it doesn't buy you anything because a user can set one
of the variables in lieu of passing the command-line flag.

If you allow the username to be specified on the command-line, you could even
get rid of the special-casing for LOGNAME and USER if you think it's
sufficiently rare that it will be needed, since users who do need it would have
a workaround.

[rkjnsn, 2017/06/16 20:28:31]

user-session isn't really intended to be called directly by the user. Instead,
it is expected that the user will continue to call
`/etc/init.d/chrome-remote-desktop start` (or possibly
`/opt/google/chrome-remote-desktop/chrome-remote-desktop --start`) as they
currently do, and user_session will be used transparently. In this context, it
really only makes sense to try to automatically obtain the correct username.

While we certainly *could* accept --user when run as non-root for only a few
additional lines of code, I don't think it would buy us anything. Even with it,
I imagine a user would be much more apt to run `sudo -u other_username
/etc/init.d/chrome-remote-desktop start` than
`/opt/google/chrome-remote-desktop/user-session --user=other_username
--me2me-script=/opt/google/chrome-remote-desktop/chrome-remote-desktop`. By
checking the USER environment variable, we ensure that the former does the right
thing.

   }
 
+  if (user.empty()) {
+    user = FindCurrentUsername();
+  }
+
   if (!foreground) {
     Daemonize();
   }
 
-  ExecuteSession(std::move(user), script_path.value(), !foreground);
+  ExecuteSession(std::move(user), script_path.value(), !foreground,
+                 real_uid != 0 ? base::make_optional(real_uid) : base::nullopt);
 }