mac: Make the (10.6-only) sandbox hole for the components build smaller

Part 2/2 of the changes requested by jeremy at
https://chromiumcodereview.appspot.com/10389047

Previously, the components build added (allow file-read-metadata) to
the sandbox on 10.6. With this patch, only all parent directories of the
bundle executable get this exception.

BUG=127465
TEST=component build still runs

Review URL: https://chromiumcodereview.appspot.com/10539009

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@141020 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 58 insertions, 41 deletions

diff --git a/content/common/common.sb b/content/common/common.sb
index bdce888..958bbb6 100644
--- a/content/common/common.sb
+++ b/content/common/common.sb
@@ -35,5 +35,7 @@
 ; Needed for IPC on 10.6
 ;10.6_OR_ABOVE (allow ipc-posix-shm)
 
+; Component build workaround for a dyld bug, used on OS X <= 10.6.
+; Enables reading file metadata for the Chrome bundle and its parent paths.
 ; http://crbug.com/127465
 @COMPONENT_BUILD_WORKAROUND@
diff --git a/content/common/sandbox_mac.h b/content/common/sandbox_mac.h
index 60c94ed..225df07 100644
--- a/content/common/sandbox_mac.h
+++ b/content/common/sandbox_mac.h
@@ -125,6 +125,10 @@ class CONTENT_EXPORT Sandbox {
                   std::string *final_sandbox_profile_str);
 
  private:
+  // Returns an (allow file-read-metadata) rule for |allowed_path| and all its
+  // parent directories.
+  static NSString* AllowMetadataForPath(const FilePath& allowed_path);
+
   // Escape |src_utf8| for use in a plain string variable in a sandbox
   // configuraton file.  On return |dst| is set to the quoted output.
   // Returns: true on success, false otherwise.
@@ -148,7 +152,7 @@ class CONTENT_EXPORT Sandbox {
   // Convert provided path into a "canonical" path matching what the Sandbox
   // expects i.e. one without symlinks.
   // This path is not necessarily unique e.g. in the face of hardlinks.
-  static void GetCanonicalSandboxPath(FilePath* path);
+  static FilePath GetCanonicalSandboxPath(const FilePath& path);
 
   FRIEND_TEST_ALL_PREFIXES(MacDirAccessSandboxTest, StringEscape);
   FRIEND_TEST_ALL_PREFIXES(MacDirAccessSandboxTest, RegexEscape);
diff --git a/content/common/sandbox_mac.mm b/content/common/sandbox_mac.mm
index 0b0d44c..26cad6e 100644
--- a/content/common/sandbox_mac.mm
+++ b/content/common/sandbox_mac.mm
@@ -16,6 +16,7 @@ extern "C" {
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/file_util.h"
+#include "base/mac/bundle_locations.h"
 #include "base/mac/mac_util.h"
 #include "base/mac/scoped_cftyperef.h"
 #include "base/mac/scoped_nsautorelease_pool.h"
@@ -105,6 +106,38 @@ NOINLINE void FatalStringQuoteException(const std::string& str) {
 
 namespace sandbox {
 
+// static
+NSString* Sandbox::AllowMetadataForPath(const FilePath& allowed_path) {
+  // Collect a list of all parent directories.
+  FilePath last_path = allowed_path;
+  std::vector<FilePath> subpaths;
+  for (FilePath path = allowed_path;
+       path.value() != last_path.value();
+       path = path.DirName()) {
+    subpaths.push_back(path);
+    last_path = path;
+  }
+
+  // Iterate through all parents and allow stat() on them explicitly.
+  NSString* sandbox_command = @"(allow file-read-metadata ";
+  for (std::vector<FilePath>::reverse_iterator i = subpaths.rbegin();
+       i != subpaths.rend();
+       ++i) {
+    std::string subdir_escaped;
+    if (!QuotePlainString(i->value(), &subdir_escaped)) {
+      FatalStringQuoteException(i->value());
+      return nil;
+    }
+
+    NSString* subdir_escaped_ns =
+        base::SysUTF8ToNSString(subdir_escaped.c_str());
+    sandbox_command =
+        [sandbox_command stringByAppendingFormat:@"(literal \"%@\")",
+            subdir_escaped_ns];
+  }
+
+  return [sandbox_command stringByAppendingString:@")"];
+}
 
 // static
 bool Sandbox::QuotePlainString(const std::string& src_utf8, std::string* dst) {
@@ -294,36 +327,11 @@ NSString* Sandbox::BuildAllowDirectoryAccessSandboxString(
   // needed so the caller doesn't need to worry about things like /var
   // being a link to /private/var (like in the paths CreateNewTempDirectory()
   // returns).
-  FilePath allowed_dir_canonical(allowed_dir);
-  GetCanonicalSandboxPath(&allowed_dir_canonical);
-
-  // Collect a list of all parent directories.
-  FilePath last_path = allowed_dir_canonical;
-  std::vector<FilePath> subpaths;
-  for (FilePath path = allowed_dir_canonical.DirName();
-       path.value() != last_path.value();
-       path = path.DirName()) {
-    subpaths.push_back(path);
-    last_path = path;
-  }
-
-  // Iterate through all parents and allow stat() on them explicitly.
-  NSString* sandbox_command = @"(allow file-read-metadata ";
-  for (std::vector<FilePath>::reverse_iterator i = subpaths.rbegin();
-       i != subpaths.rend();
-       ++i) {
-    std::string subdir_escaped;
-    if (!QuotePlainString(i->value(), &subdir_escaped)) {
-      FatalStringQuoteException(i->value());
-      return nil;
-    }
+  FilePath allowed_dir_canonical = GetCanonicalSandboxPath(allowed_dir);
 
-    NSString* subdir_escaped_ns =
-        base::SysUTF8ToNSString(subdir_escaped.c_str());
-    sandbox_command =
-        [sandbox_command stringByAppendingFormat:@"(literal \"%@\")",
-            subdir_escaped_ns];
-  }
+  NSString* sandbox_command = AllowMetadataForPath(allowed_dir_canonical);
+  sandbox_command = [sandbox_command
+      substringToIndex:[sandbox_command length] - 1];  // strip trailing ')'
 
   // Finally append the leaf directory.  Unlike its parents (for which only
   // stat() should be allowed), the leaf directory needs full access.
@@ -524,8 +532,7 @@ bool Sandbox::EnableSandbox(int sandbox_type,
   // (see renderer.sb for details).
   std::string home_dir = [NSHomeDirectory() fileSystemRepresentation];
 
-  FilePath home_dir_canonical(home_dir);
-  GetCanonicalSandboxPath(&home_dir_canonical);
+  FilePath home_dir_canonical = GetCanonicalSandboxPath(FilePath(home_dir));
 
   substitutions["USER_HOMEDIR_AS_LITERAL"] =
       SandboxSubstring(home_dir_canonical.value(),
@@ -550,8 +557,12 @@ bool Sandbox::EnableSandbox(int sandbox_type,
   // contains LC_RPATH load commands. The components build uses those.
   // See http://crbug.com/127465
   if (base::mac::IsOSSnowLeopardOrEarlier()) {
+    FilePath bundle_executable = base::mac::NSStringToFilePath(
+        [base::mac::MainBundle() executablePath]);
+    NSString* sandbox_command = AllowMetadataForPath(
+        GetCanonicalSandboxPath(bundle_executable));
     substitutions["COMPONENT_BUILD_WORKAROUND"] =
-        SandboxSubstring("(allow file-read-metadata)");
+        SandboxSubstring(base::SysNSStringToUTF8(sandbox_command));
   }
 #endif
 
@@ -576,23 +587,23 @@ bool Sandbox::EnableSandbox(int sandbox_type,
 }
 
 // static
-void Sandbox::GetCanonicalSandboxPath(FilePath* path) {
-  int fd = HANDLE_EINTR(open(path->value().c_str(), O_RDONLY));
+FilePath Sandbox::GetCanonicalSandboxPath(const FilePath& path) {
+  int fd = HANDLE_EINTR(open(path.value().c_str(), O_RDONLY));
   if (fd < 0) {
     DPLOG(FATAL) << "GetCanonicalSandboxPath() failed for: "
-                 << path->value();
-    return;
+                 << path.value();
+    return path;
   }
   file_util::ScopedFD file_closer(&fd);
 
   FilePath::CharType canonical_path[MAXPATHLEN];
   if (HANDLE_EINTR(fcntl(fd, F_GETPATH, canonical_path)) != 0) {
     DPLOG(FATAL) << "GetCanonicalSandboxPath() failed for: "
-                 << path->value();
-    return;
+                 << path.value();
+    return path;
   }
 
-  *path = FilePath(canonical_path);
+  return FilePath(canonical_path);
 }
 
 }  // namespace sandbox
diff --git a/content/common/sandbox_mac_diraccess_unittest.mm b/content/common/sandbox_mac_diraccess_unittest.mm
index 868bc9a..139b1b6 100644
--- a/content/common/sandbox_mac_diraccess_unittest.mm
+++ b/content/common/sandbox_mac_diraccess_unittest.mm
@@ -150,7 +150,7 @@ TEST_F(MacDirAccessSandboxTest, SandboxAccess) {
   // This step is important on OS X since the sandbox only understands "real"
   // paths and the paths CreateNewTempDirectory() returns are empirically in
   // /var which is a symlink to /private/var .
-  Sandbox::GetCanonicalSandboxPath(&tmp_dir);
+  tmp_dir = Sandbox::GetCanonicalSandboxPath(tmp_dir);
   ScopedDirectory cleanup(&tmp_dir);
 
   const char* sandbox_dir_cases[] = {