mac: Let gcapi install chrome as the CGSession owner

chrome/installer/gcapi_mac/gcapi.mm

Index: chrome/installer/gcapi_mac/gcapi.mm
diff --git a/chrome/installer/gcapi_mac/gcapi.mm b/chrome/installer/gcapi_mac/gcapi.mm
index ab561e7977c9c67ca94a980c710e23da0971b09a..5099e26a451d56cd73fcf24311dca798ae7e86ea 100644
--- a/chrome/installer/gcapi_mac/gcapi.mm
+++ b/chrome/installer/gcapi_mac/gcapi.mm
@@ -5,30 +5,33 @@
 #include "chrome/installer/gcapi_mac/gcapi.h"
 
 #import <Cocoa/Cocoa.h>
+#include <pwd.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/utsname.h>
 
 namespace {
 
+// The "~~" prefixes are replaced with the home directory of the
+// console owner (i.e. not the home directory of the euid).
 NSString* const kChromeInstallPath = @"/Applications/Google Chrome.app";
 
 NSString* const kBrandKey = @"KSBrandID";
 NSString* const kSystemBrandPath = @"/Library/Google/Google Chrome Brand.plist";

[Mark Mentovai, 2012/08/13 13:14:42]

Provided we’re just letting Chrome set up a user ticket, we no longer need to
bother with system brand file locations at all.

[Nico, 2012/08/13 14:52:57]

Done.

-NSString* const kUserBrandPath = @"~/Library/Google/Google Chrome Brand.plist";
+NSString* const kUserBrandPath = @"~~/Library/Google/Google Chrome Brand.plist";
 
 NSString* const kSystemKsadminPath =
     @"/Library/Google/GoogleSoftwareUpdate/GoogleSoftwareUpdate.bundle/"
      "Contents/MacOS/ksadmin";
 
 NSString* const kUserKsadminPath =
-    @"~/Library/Google/GoogleSoftwareUpdate/GoogleSoftwareUpdate.bundle/"
+    @"~~/Library/Google/GoogleSoftwareUpdate/GoogleSoftwareUpdate.bundle/"
      "Contents/MacOS/ksadmin";
 
 NSString* const kSystemMasterPrefsPath =
     @"/Library/Google/Google Chrome Master Preferences";
 NSString* const kUserMasterPrefsPath =
-    @"~/Library/Application Support/Google/Google Chrome Master Preferences";
+    @"~~/Library/Application Support/Google/Google Chrome Master Preferences";
 
 NSString* const kChannelKey = @"KSChannelID";
 NSString* const kVersionKey = @"KSVersion";
@@ -60,16 +63,44 @@ bool IsOSXVersionSupported() {
   return mac_os_x_minor_version >= 6 && mac_os_x_minor_version <= 8;
 }
 
+// Returns the pid/gid of the logged-in user, even if getuid() claims that the
+// current user is root.
+// Returns NULL on error.
+passwd* GetRealUserId() {
+  CFDictionaryRef sessionInfoDict = CGSessionCopyCurrentDictionary();
+  if (!sessionInfoDict)
+    return NULL;  // Possibly no screen plugged in.
+
+  CFNumberRef userUID = (CFNumberRef)CFDictionaryGetValue(sessionInfoDict,

[Mark Mentovai, 2012/08/13 13:14:42]

Can you use something like CFCast here?

[Nico, 2012/08/13 15:42:15]

Doneish.

+                                                          kCGSessionUserIDKey);
+  uid_t uid;
+  BOOL success = CFNumberGetValue(userUID, kCFNumberSInt32Type, &uid);
+  CFRelease(sessionInfoDict);
+  if (!success)
+    return NULL;
+
+  return getpwuid(uid);
+}
+
 enum TicketKind {
   kSystemTicket, kUserTicket
 };
 
-BOOL HasChromeTicket(TicketKind kind) {
+// Replaces "~~" with |homedir|.
+NSString* AdjustHomedir(NSString* s, const char* homedir) {
+  NSString* homeDir = [NSString stringWithUTF8String:homedir];

[Mark Mentovai, 2012/08/13 13:14:42]

Naming nit: home_dir

[Nico, 2012/08/13 14:52:57]

Done.

+  return [s stringByReplacingOccurrencesOfString:@"~~"

[Mark Mentovai, 2012/08/13 13:14:42]

I’d be a little more comfortable if this only operated on ~~ at the beginning of
the string.

[Nico, 2012/08/13 14:52:57]

Done.

+                                      withString:homeDir];
+}
+
+BOOL HasChromeTicket(TicketKind kind, const char* homedir) {
   // Don't use Objective-C 2 loop syntax, in case an installer runs on 10.4.
   NSMutableArray* keystonePaths =

[Mark Mentovai, 2012/08/13 13:14:42]

Naming nit: keystone_paths. Line 109 too.

[Nico, 2012/08/13 14:52:57]

Done.

       [NSMutableArray arrayWithObject:kSystemKsadminPath];
-  if (kind == kUserTicket && geteuid() != 0)
-    [keystonePaths insertObject:kUserKsadminPath atIndex:0];
+  if (kind == kUserTicket != 0) {
+    [keystonePaths insertObject:AdjustHomedir(kUserKsadminPath, homedir)
+                        atIndex:0];
+  }
   NSEnumerator* e = [keystonePaths objectEnumerator];
   id ksPath;
   while ((ksPath = [e nextObject])) {
@@ -78,6 +109,8 @@ BOOL HasChromeTicket(TicketKind kind) {
     bool ksadminRanSuccessfully = false;
 
     @try {
+      // XXX does this need to run as other user? or can admin read the user store?

[Mark Mentovai, 2012/08/13 13:14:42]

What do you mean? That you want to check the console user’s Keystone tickets
from some other UID?

[Nico, 2012/08/13 14:13:33]

Right, I imagine this code will usually run with euid 0, but I want to check for
the console user's user tickets, not for root's user tickets.

[Mark Mentovai, 2012/08/13 15:04:34]

Nico wrote:
Root will be able to read a user store from a filesystem permission perspective
(except for corner cases like FileVault 1), BUT:

How will ksadmin know which user store to read? If you run ksadmin as root, it
shows you the system store (even if you tell it to do --user-store). To check
the user store, you want this to check the console user’s store, don’t you?

[Nico, 2012/08/13 15:42:16]

Done.

+      // it should be able to, so I think this is fine as is?
       task = [[NSTask alloc] init];
       [task setLaunchPath:ksPath];
 
@@ -120,7 +153,7 @@ mode_t Permissions() {
   return 0755;
 }
 
-BOOL CreatePathToFile(NSString* path) {
+BOOL CreatePathToFile(NSString* path, const passwd* user) {
   path = [path stringByDeletingLastPathComponent];
 
   // Default owner, group, permissions:
@@ -128,14 +161,22 @@ BOOL CreatePathToFile(NSString* path) {
   //   more information, see umask.
   // * The owner ID is set to the effective user ID of the process.
   // * The group ID is set to that of the parent directory.
-  // The default group ID is fine. Owner ID is fine too, since user directory
-  // paths won't be created if euid is 0. Do set permissions explicitly; for
-  // admin paths all admins can write, for user paths just the owner may.
+  // The default group ID is fine. Owner ID is fine if creating a system path,
+  // but when creating a user path explicitly set the owner in case euid is 0.
+  // Do set permissions explicitly; for admin paths all admins can write, for

[Mark Mentovai, 2012/08/13 13:14:42]

You haven’t made this so that all admins can write to admin paths. Not clear if
you intended to based on this comment now.

[Nico, 2012/08/13 14:52:57]

For admins, the group is set to wheel and permissions are 755, so this should
work (?)

[Mark Mentovai, 2012/08/13 15:04:34]

Nico wrote:
How do you figure? Admins aren’t a member of group wheel and in any case, you
haven’t set the group write bit.

[Nico, 2012/08/13 15:42:16]

Done. (?)

+  // user paths just the owner may.
   NSMutableDictionary* attributes = [NSMutableDictionary
       dictionaryWithObject:[NSNumber numberWithShort:Permissions()]
                     forKey:NSFilePosixPermissions];
-  if (geteuid() == 0)
+  if (user) {
+    [attributes setObject:[NSNumber numberWithInt:user->pw_uid]
+                   forKey:NSFileOwnerAccountID];
+    // XXX do this? not needed i think?
+    //[attributes setObject:[NSNumber numberWithInt:user->pw_gid]
+                   //forKey:NSFileGroupOwnerAccountID];
+  } else {
     [attributes setObject:@"wheel" forKey:NSFileGroupOwnerAccountName];
+  }
 
   NSFileManager* manager = [NSFileManager defaultManager];
   return [manager createDirectoryAtPath:path
@@ -144,11 +185,16 @@ BOOL CreatePathToFile(NSString* path) {
                                   error:nil];
 }
 
-// Tries to write |data| at |system_path| or if that fails and geteuid() is not
-// 0 at |user_path|. Returns the path where it wrote, or nil on failure.
-NSString* WriteData(NSData* data, NSString* system_path, NSString* user_path) {
+// Tries to write |data| at |system_path| or if that fails at |user_path|.
+// Returns the path where it wrote, or nil on failure.
+NSString* WriteData(NSData* data,
+                    NSString* system_path,
+                    NSString* user_path,
+                    const passwd* user) {
+  // XXX do we still want to try to write this to a system path? probably?

[Mark Mentovai, 2012/08/13 13:14:42]

Brand file, no. The brand file should only be written where the ticket is
written.

Master prefs file, sure, you can write that to a system path.

[Nico, 2012/08/13 14:52:57]

Done.

+
   // Try system first.
-  if (CreatePathToFile(system_path) &&
+  if (CreatePathToFile(system_path, NULL) &&
       [data writeToFile:system_path atomically:YES]) {
     // files are created with group of parent dir (good), owner of euid (good).
     chmod([system_path fileSystemRepresentation], Permissions() & ~0111);
@@ -156,21 +202,17 @@ NSString* WriteData(NSData* data, NSString* system_path, NSString* user_path) {
   }
 
   // Failed, try user.
-  // -stringByExpandingTildeInPath returns root's home directory if this is run
-  // setuid root, and in that case the kSystemBrandPath path above should have
-  // worked anyway. So only try user if geteuid() isn't root.
-  if (geteuid() != 0) {
-    user_path = [user_path stringByExpandingTildeInPath];
-    if (CreatePathToFile(user_path) &&
-        [data writeToFile:user_path atomically:YES]) {
-      chmod([user_path fileSystemRepresentation], Permissions() & ~0111);
-      return user_path;
-    }
+  user_path = AdjustHomedir(user_path, user->pw_dir);
+  if (CreatePathToFile(user_path, user) &&
+      [data writeToFile:user_path atomically:YES]) {
+    chmod([user_path fileSystemRepresentation], Permissions() & ~0111);
+    chown([user_path fileSystemRepresentation], user->pw_uid, user->pw_gid);
+    return user_path;
   }
   return nil;
 }
 
-NSString* WriteBrandCode(const char* brand_code) {
+NSString* WriteBrandCode(const char* brand_code, const passwd* user) {
   NSDictionary* brand_dict = @{
       kBrandKey: [NSString stringWithUTF8String:brand_code],
   };
@@ -179,15 +221,16 @@ NSString* WriteBrandCode(const char* brand_code) {
                     format:NSPropertyListBinaryFormat_v1_0
           errorDescription:nil];
 
-  return WriteData(contents, kSystemBrandPath, kUserBrandPath);
+  return WriteData(contents, kSystemBrandPath, kUserBrandPath, user);
 }
 
 BOOL WriteMasterPrefs(const char* master_prefs_contents,
-                      size_t master_prefs_contents_size) {
+                      size_t master_prefs_contents_size,
+                      const passwd* user) {
   NSData* contents = [NSData dataWithBytes:master_prefs_contents
                                     length:master_prefs_contents_size];
-  return
-      WriteData(contents, kSystemMasterPrefsPath, kUserMasterPrefsPath) != nil;
+  return WriteData(
+      contents, kSystemMasterPrefsPath, kUserMasterPrefsPath, user) != nil;
 }
 
 NSString* PathToFramework(NSString* app_path, NSDictionary* info_plist) {
@@ -213,6 +256,8 @@ bool isbrandchar(int c) {
 }  // namespace
 
 int GoogleChromeCompatibilityCheck(unsigned* reasons) {
+  passwd* user = GetRealUserId();
+
   unsigned local_reasons = 0;
 
   NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init];
@@ -220,15 +265,20 @@ int GoogleChromeCompatibilityCheck(unsigned* reasons) {
   if (!IsOSXVersionSupported())
     local_reasons |= GCCC_ERROR_OSNOTSUPPORTED;
 
-  if (HasChromeTicket(kSystemTicket))
+  if (HasChromeTicket(kSystemTicket, user->pw_dir))
     local_reasons |= GCCC_ERROR_SYSTEMLEVELALREADYPRESENT;
 
-  if (geteuid() != 0 && HasChromeTicket(kUserTicket))
+  if (HasChromeTicket(kUserTicket, user->pw_dir))
     local_reasons |= GCCC_ERROR_USERLEVELALREADYPRESENT;
 
   if (![[NSFileManager defaultManager] isWritableFileAtPath:@"/Applications"])
     local_reasons |= GCCC_ERROR_ACCESSDENIED;
 
+  if (!local_reasons && [[NSFileManager defaultManager]
+          fileExistsAtPath:@"/Applications/Google Chrome.app"]) {
+    local_reasons |= GCCC_ERROR_ACCESSDENIED;

[Mark Mentovai, 2012/08/13 13:14:42]

Is this the right “reason” or should you add a new “reason” to cover this?

[Nico, 2012/08/13 14:52:57]

How would the new reason be called?

Or should there be just a generic ALREADYPRESENT that's returned for
user/system/this case?

[Mark Mentovai, 2012/08/13 15:04:34]

Nico wrote:
Yeah, that’s probably best.

ACCESSDENIED doesn’t necessarily fit this case at all, absent any test to see if
it’s writeable—that’s overkilll, since we don’t want to overwrite it if it’s
present.

[Nico, 2012/08/13 15:42:16]

Done.

+  }
+
   [pool drain];
 
   // Done. Copy/return results.
@@ -245,6 +295,8 @@ int InstallGoogleChrome(const char* source_path,
   if (!GoogleChromeCompatibilityCheck(NULL))
     return 0;
 
+  passwd* user = GetRealUserId();
+
   @autoreleasepool {
     NSString* app_path = [NSString stringWithUTF8String:source_path];
     NSString* info_plist_path =
@@ -260,9 +312,13 @@ int InstallGoogleChrome(const char* source_path,
     }
 
     @try {
+      // install.sh tries to make the installed app admin-writable, but
+      // only when it's not run as root.
+      NSString* run_as = [NSString stringWithUTF8String:user->pw_name];
       NSTask* task = [[[NSTask alloc] init] autorelease];
-      [task setLaunchPath:install_script];
-      [task setArguments:@[app_path, kChromeInstallPath]];
+      [task setLaunchPath:@"/usr/bin/sudo"];
+      [task setArguments:
+          @[@"-u", run_as, install_script, app_path, kChromeInstallPath]];
       [task launch];
       [task waitUntilExit];
       if ([task terminationStatus] != 0) {
@@ -285,20 +341,23 @@ int InstallGoogleChrome(const char* source_path,
 
     NSString* brand_path = nil;
     if (valid_brand_code)
-      brand_path = WriteBrandCode(brand_code);
+      brand_path = WriteBrandCode(brand_code, user);
 
     // Write master prefs.
     if (master_prefs_contents)
-      WriteMasterPrefs(master_prefs_contents, master_prefs_contents_size);
+      WriteMasterPrefs(master_prefs_contents, master_prefs_contents_size, user);
 
-    // TODO Set default browser if requested. Will be tricky when running as
-    // root.
+    // TODO Set default browser if requested.
   }
   return 1;
 }
 
 int LaunchGoogleChrome() {
+  //passwd* user = GetRealUserId();
+
   @autoreleasepool {
+    // NSWorkspace launches processes as the current console owner,
+    // even when running with euid of 0.
     return [[NSWorkspace sharedWorkspace] launchApplication:kChromeInstallPath];
   }
 }