Chrome Frame: Add explicit object security attributes to the Chrome Frame version beacon. This will allow low integrity processes to access shared memory segment and lock and make shared memory segment read only after creation.

Also use lock names that include the hosting process.

BUG=61609
TEST=Start medium integrity Chrome Frame host running CF version X. Update CF to version Y > X. Start low integrity Chrome Frame host, observe that version X is loaded. 

Review URL: http://codereview.chromium.org/5012001

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

5 files changed, 230 insertions, 22 deletions

diff --git a/base/shared_memory.h b/base/shared_memory.h
index 719eb69..a088682 100644
--- a/base/shared_memory.h
+++ b/base/shared_memory.h
@@ -173,9 +173,11 @@ class SharedMemory {
   void Lock();
 
 #if defined(OS_WIN)
-  // A Lock() implementation with a timeout. Returns true if the Lock() has
-  // been acquired, false if the timeout was reached.
-  bool Lock(uint32 timeout_ms);
+  // A Lock() implementation with a timeout that also allows setting
+  // security attributes on the mutex. sec_attr may be NULL.
+  // Returns true if the Lock() has been acquired, false if the timeout was
+  // reached.
+  bool Lock(uint32 timeout_ms, SECURITY_ATTRIBUTES* sec_attr);
 #endif
 
   // Releases the shared memory lock.
diff --git a/base/shared_memory_win.cc b/base/shared_memory_win.cc
index 5f293fc..15c61dd 100644
--- a/base/shared_memory_win.cc
+++ b/base/shared_memory_win.cc
@@ -197,17 +197,16 @@ void SharedMemory::Close() {
 }
 
 void SharedMemory::Lock() {
-  Lock(INFINITE);
+  Lock(INFINITE, NULL);
 }
 
-bool SharedMemory::Lock(uint32 timeout_ms) {
+bool SharedMemory::Lock(uint32 timeout_ms, SECURITY_ATTRIBUTES* sec_attr) {
   if (lock_ == NULL) {
     std::wstring name = name_;
     name.append(L"lock");
-    lock_ = CreateMutex(NULL, FALSE, name.c_str());
-    DCHECK(lock_ != NULL);
+    lock_ = CreateMutex(sec_attr, FALSE, name.c_str());
     if (lock_ == NULL) {
-      DLOG(ERROR) << "Could not create mutex" << GetLastError();
+      PLOG(ERROR) << "Could not create mutex.";
       return false;  // there is nothing good we can do here.
     }
   }
diff --git a/chrome_frame/module_utils.cc b/chrome_frame/module_utils.cc
index cca852a..8b80cab 100644
--- a/chrome_frame/module_utils.cc
+++ b/chrome_frame/module_utils.cc
@@ -4,30 +4,35 @@
 
 #include "chrome_frame/module_utils.h"
 
+#include <aclapi.h>
 #include <atlbase.h>
+#include <atlsecurity.h>
+#include <sddl.h>
+
 #include "base/file_path.h"
 #include "base/file_version_info.h"
 #include "base/logging.h"
 #include "base/path_service.h"
 #include "base/shared_memory.h"
+#include "base/sys_info.h"
 #include "base/utf_string_conversions.h"
 #include "base/version.h"
+#include "chrome_frame/utils.h"
 
-const char kSharedMemoryName[] = "ChromeFrameVersionBeacon";
+const wchar_t kSharedMemoryName[] = L"ChromeFrameVersionBeacon_";
 const uint32 kSharedMemorySize = 128;
 const uint32 kSharedMemoryLockTimeoutMs = 1000;
 
 // static
 DllRedirector::DllRedirector() : first_module_handle_(NULL) {
-  // TODO(robertshield): Correctly construct the profile name here. Also allow
-  // for overrides to be taken from the environment.
-  shared_memory_.reset(new base::SharedMemory(ASCIIToWide(kSharedMemoryName)));
+  // TODO(robertshield): Allow for overrides to be taken from the environment.
+  std::wstring beacon_name(kSharedMemoryName);
+  beacon_name += GetHostProcessName(false);
+  shared_memory_.reset(new base::SharedMemory(beacon_name));
 }
 
 DllRedirector::DllRedirector(const char* shared_memory_name)
     : shared_memory_name_(shared_memory_name), first_module_handle_(NULL) {
-  // TODO(robertshield): Correctly construct the profile name here. Also allow
-  // for overrides to be taken from the environment.
   shared_memory_.reset(new base::SharedMemory(ASCIIToWide(shared_memory_name)));
 }
 
@@ -41,6 +46,73 @@ DllRedirector::~DllRedirector() {
   UnregisterAsFirstCFModule();
 }
 
+bool DllRedirector::BuildSecurityAttributesForLock(
+    CSecurityAttributes* sec_attr) {
+  DCHECK(sec_attr);
+  int32 major_version, minor_version, fix_version;
+  base::SysInfo::OperatingSystemVersionNumbers(&major_version,
+                                               &minor_version,
+                                               &fix_version);
+  if (major_version < 6) {
+    // Don't bother with changing ACLs on pre-vista.
+    return false;
+  }
+
+  bool success = false;
+
+  // Fill out the rest of the security descriptor from the process token.
+  CAccessToken token;
+  if (token.GetProcessToken(TOKEN_QUERY)) {
+    CSecurityDesc security_desc;
+    // Set the SACL from an SDDL string that allows access to low-integrity
+    // processes. See http://msdn.microsoft.com/en-us/library/bb625958.aspx.
+    if (security_desc.FromString(L"S:(ML;;NW;;;LW)")) {
+      CSid sid_owner;
+      if (token.GetOwner(&sid_owner)) {
+        security_desc.SetOwner(sid_owner);
+      } else {
+        NOTREACHED() << "Could not get owner.";
+      }
+      CSid sid_group;
+      if (token.GetPrimaryGroup(&sid_group)) {
+        security_desc.SetGroup(sid_group);
+      } else {
+        NOTREACHED() << "Could not get group.";
+      }
+      CDacl dacl;
+      if (token.GetDefaultDacl(&dacl)) {
+        // Add an access control entry mask for the current user.
+        // This is what grants this user access from lower integrity levels.
+        CSid sid_user;
+        if (token.GetUser(&sid_user)) {
+          success = dacl.AddAllowedAce(sid_user, MUTEX_ALL_ACCESS);
+          security_desc.SetDacl(dacl);
+          sec_attr->Set(security_desc);
+        }
+      }
+    }
+  }
+
+  return success;
+}
+
+bool DllRedirector::SetFileMappingToReadOnly(base::SharedMemoryHandle mapping) {
+  bool success = false;
+
+  CAccessToken token;
+  if (token.GetProcessToken(TOKEN_QUERY)) {
+    CSid sid_user;
+    if (token.GetUser(&sid_user)) {
+      CDacl dacl;
+      dacl.AddAllowedAce(sid_user, STANDARD_RIGHTS_READ | FILE_MAP_READ);
+      success = AtlSetDacl(mapping, SE_KERNEL_OBJECT, dacl);
+    }
+  }
+
+  return success;
+}
+
+
 bool DllRedirector::RegisterAsFirstCFModule() {
   DCHECK(first_module_handle_ == NULL);
 
@@ -50,7 +122,14 @@ bool DllRedirector::RegisterAsFirstCFModule() {
   // We sadly can't use the autolock here since we want to have a timeout.
   // Be careful not to return while holding the lock. Also, attempt to do as
   // little as possible while under this lock.
-  bool lock_acquired = shared_memory_->Lock(kSharedMemoryLockTimeoutMs);
+
+  bool lock_acquired = false;
+  CSecurityAttributes sec_attr;
+  if (BuildSecurityAttributesForLock(&sec_attr)) {
+    lock_acquired = shared_memory_->Lock(kSharedMemoryLockTimeoutMs, &sec_attr);
+  } else {
+    lock_acquired = shared_memory_->Lock(kSharedMemoryLockTimeoutMs, NULL);
+  }
 
   if (!lock_acquired) {
     // We couldn't get the lock in a reasonable amount of time, so fall
@@ -66,7 +145,13 @@ bool DllRedirector::RegisterAsFirstCFModule() {
                                             false,  // open_existing
                                             kSharedMemorySize);
 
-  if (!result) {
+  if (result) {
+    // We created the beacon, now we need to mutate the security attributes
+    // on the shared memory to allow read-only access and let low-integrity
+    // processes open it.
+    bool acls_set = SetFileMappingToReadOnly(shared_memory_->handle());
+    DCHECK(acls_set);
+  } else {
     created_beacon = false;
 
     // We failed to create the shared memory segment, suggesting it may already
@@ -123,7 +208,7 @@ bool DllRedirector::RegisterAsFirstCFModule() {
 
 void DllRedirector::UnregisterAsFirstCFModule() {
   if (base::SharedMemory::IsHandleValid(shared_memory_->handle())) {
-    bool lock_acquired = shared_memory_->Lock(kSharedMemoryLockTimeoutMs);
+    bool lock_acquired = shared_memory_->Lock(kSharedMemoryLockTimeoutMs, NULL);
     if (lock_acquired) {
       // Free our handles. The last closed handle SHOULD result in it being
       // deleted.
@@ -139,9 +224,8 @@ LPFNGETCLASSOBJECT DllRedirector::GetDllGetClassObjectPtr() {
   LPFNGETCLASSOBJECT proc_ptr = reinterpret_cast<LPFNGETCLASSOBJECT>(
       GetProcAddress(first_module_handle, "DllGetClassObject"));
   if (!proc_ptr) {
-    DLOG(ERROR) << "DllRedirector: Could get address of DllGetClassObject "
-                   "from first loaded module, GLE: "
-                << GetLastError();
+    DPLOG(ERROR) << "DllRedirector: Could not get address of DllGetClassObject "
+                    "from first loaded module.";
     // Oh boink, the first module we loaded was somehow bogus, make ourselves
     // the first module again.
     first_module_handle = reinterpret_cast<HMODULE>(&__ImageBase);
@@ -192,8 +276,8 @@ HMODULE DllRedirector::LoadVersionedModule(Version* version) {
 
   HMODULE hmodule = LoadLibrary(module_path.value().c_str());
   if (hmodule == NULL) {
-    DLOG(ERROR) << "Could not load reported module version "
-                << version->GetString();
+    DPLOG(ERROR) << "Could not load reported module version "
+                 << version->GetString();
   }
 
   return hmodule;
diff --git a/chrome_frame/module_utils.h b/chrome_frame/module_utils.h
index 0da5472..be8a89b 100644
--- a/chrome_frame/module_utils.h
+++ b/chrome_frame/module_utils.h
@@ -14,6 +14,9 @@
 #include "base/singleton.h"
 
 // Forward
+namespace ATL {
+class CSecurityAttributes;
+}
 class Version;
 
 // A singleton class that provides a facility to register the version of the
@@ -67,6 +70,15 @@ class DllRedirector {
   // actually have a valid version and not e.g. ..\..\..\..\MyEvilFolder\.
   virtual HMODULE LoadVersionedModule(Version* version);
 
+  // Builds the necessary SECURITY_ATTRIBUTES to allow low integrity access
+  // to an object. Returns true on success, false otherwise.
+  virtual bool BuildSecurityAttributesForLock(
+      ATL::CSecurityAttributes* sec_attr);
+
+  // Attempts to change the permissions on the given file mapping to read only.
+  // Returns true on success, false otherwise.
+  virtual bool SetFileMappingToReadOnly(base::SharedMemoryHandle mapping);
+
   // Shared memory segment that contains the version beacon.
   scoped_ptr<base::SharedMemory> shared_memory_;
 
diff --git a/chrome_frame/test/module_utils_test.cc b/chrome_frame/test/module_utils_test.cc
index 4bb16ce..26a0ac7 100644
--- a/chrome_frame/test/module_utils_test.cc
+++ b/chrome_frame/test/module_utils_test.cc
@@ -6,8 +6,10 @@
 
 #include "base/scoped_handle.h"
 #include "base/shared_memory.h"
+#include "base/sys_info.h"
 #include "base/utf_string_conversions.h"
 #include "base/version.h"
+#include "chrome_frame/test/chrome_frame_test_utils.h"
 #include "gtest/gtest.h"
 
 extern "C" IMAGE_DOS_HEADER __ImageBase;
@@ -68,6 +70,21 @@ class MockDllRedirector2 : public MockDllRedirector {
   }
 };
 
+class MockDllRedirectorNoPermissions : public MockDllRedirector {
+ public:
+  explicit MockDllRedirectorNoPermissions(const char* beacon_name)
+    : MockDllRedirector(beacon_name) {}
+
+  virtual bool BuildSecurityAttributesForLock(
+      ATL::CSecurityAttributes* sec_attr) {
+    return false;
+  }
+
+  virtual bool SetFileMappingToReadOnly(base::SharedMemoryHandle mapping) {
+    return true;
+  }
+};
+
 class DllRedirectorTest : public testing::Test {
  public:
   virtual void SetUp() {
@@ -290,3 +307,97 @@ TEST_F(DllRedirectorTest, BadVersionNumber) {
   EXPECT_EQ(reinterpret_cast<HMODULE>(&__ImageBase), first_module);
 }
 
+// TODO(robertshield): These tests rely on simulating access checks from a low
+// integrity process using impersonation. This may not be exactly identical to
+// actually having a separate low integrity process.
+TEST_F(DllRedirectorTest, LowIntegrityAccess) {
+  scoped_ptr<MockDllRedirector> first_redirector(
+      new MockDllRedirector(kTestVersionBeaconName));
+  EXPECT_TRUE(first_redirector->RegisterAsFirstCFModule());
+
+  // Ensure that we can acquire the mutex from medium integrity:
+  {
+    base::SharedMemory shared_memory(ASCIIToWide(kTestVersionBeaconName));
+    bool mutex_locked = shared_memory.Lock(kWaitTestTimeout, NULL);
+    EXPECT_TRUE(mutex_locked);
+
+    // Ensure that the shared memory is read-only:
+    EXPECT_FALSE(shared_memory.Open(kTestVersionBeaconName, false));
+    shared_memory.Close();
+    EXPECT_TRUE(shared_memory.Open(kTestVersionBeaconName, true));
+    shared_memory.Close();
+
+    if (mutex_locked)
+      shared_memory.Unlock();
+  }
+
+  int32 major_version, minor_version, fix_version;
+  base::SysInfo::OperatingSystemVersionNumbers(&major_version,
+                                               &minor_version,
+                                               &fix_version);
+  if (major_version >= 6) {
+    // Now move to low integrity
+    chrome_frame_test::LowIntegrityToken low_integrity_token;
+    ASSERT_TRUE(low_integrity_token.Impersonate());
+
+    // Ensure that we can also acquire the mutex from low integrity.
+    base::SharedMemory shared_memory(ASCIIToWide(kTestVersionBeaconName));
+    bool mutex_locked = shared_memory.Lock(kWaitTestTimeout, NULL);
+    EXPECT_TRUE(mutex_locked);
+
+    // Ensure that the shared memory is read-only:
+    EXPECT_FALSE(shared_memory.Open(kTestVersionBeaconName, false));
+    shared_memory.Close();
+    EXPECT_TRUE(shared_memory.Open(kTestVersionBeaconName, true));
+    shared_memory.Close();
+
+    if (mutex_locked)
+      shared_memory.Unlock();
+  }
+}
+
+TEST_F(DllRedirectorTest, LowIntegrityAccessDenied) {
+  // Run this test with a mock DllRedirector that doesn't set permissions
+  // on the shared memory.
+  scoped_ptr<MockDllRedirectorNoPermissions> first_redirector(
+      new MockDllRedirectorNoPermissions(kTestVersionBeaconName));
+  EXPECT_TRUE(first_redirector->RegisterAsFirstCFModule());
+
+  // Ensure that we can acquire the mutex from medium integrity:
+  {
+    base::SharedMemory shared_memory(ASCIIToWide(kTestVersionBeaconName));
+    bool mutex_locked = shared_memory.Lock(kWaitTestTimeout, NULL);
+    EXPECT_TRUE(mutex_locked);
+
+    // We should be able to open the memory as read/write.
+    EXPECT_TRUE(shared_memory.Open(kTestVersionBeaconName, false));
+    shared_memory.Close();
+
+    if (mutex_locked)
+      shared_memory.Unlock();
+  }
+
+  int32 major_version, minor_version, fix_version;
+  base::SysInfo::OperatingSystemVersionNumbers(&major_version,
+                                               &minor_version,
+                                               &fix_version);
+  if (major_version >= 6) {
+    // Now move to low integrity
+    chrome_frame_test::LowIntegrityToken low_integrity_token;
+    low_integrity_token.Impersonate();
+
+    // Ensure that we can't acquire the mutex without having set the
+    // Low Integrity ACE in the SACL.
+    base::SharedMemory shared_memory(ASCIIToWide(kTestVersionBeaconName));
+    bool mutex_locked = shared_memory.Lock(kWaitTestTimeout, NULL);
+    EXPECT_FALSE(mutex_locked);
+
+    // We shouldn't be able to open the memory.
+    EXPECT_FALSE(shared_memory.Open(kTestVersionBeaconName, false));
+    shared_memory.Close();
+
+    if (mutex_locked)
+      shared_memory.Unlock();
+  }
+}
+