Added logging in PolicyBase::AddRule.

Should help to investigate sandbox failures on user side.
Logs from the attached bug have records about failed start of utility process, but without clues about cause.

BUG=408184

Review URL: https://codereview.chromium.org/546903002

Cr-Commit-Position: refs/heads/master@{#293647}

2 files changed, 93 insertions, 78 deletions

diff --git a/sandbox/win/src/sandbox_policy_base.cc b/sandbox/win/src/sandbox_policy_base.cc
index 7b9262b..4604bfd 100644
--- a/sandbox/win/src/sandbox_policy_base.cc
+++ b/sandbox/win/src/sandbox_policy_base.cc
@@ -373,85 +373,16 @@ ResultCode PolicyBase::SetStderrHandle(HANDLE handle) {
   return SBOX_ALL_OK;
 }
 
-ResultCode PolicyBase::AddRule(SubSystem subsystem, Semantics semantics,
+ResultCode PolicyBase::AddRule(SubSystem subsystem,
+                               Semantics semantics,
                                const wchar_t* pattern) {
-  if (NULL == policy_) {
-    policy_ = MakeBrokerPolicyMemory();
-    DCHECK(policy_);
-    policy_maker_ = new LowLevelPolicy(policy_);
-    DCHECK(policy_maker_);
-  }
-
-  switch (subsystem) {
-    case SUBSYS_FILES: {
-      if (!file_system_init_) {
-        if (!FileSystemPolicy::SetInitialRules(policy_maker_))
-          return SBOX_ERROR_BAD_PARAMS;
-        file_system_init_ = true;
-      }
-      if (!FileSystemPolicy::GenerateRules(pattern, semantics, policy_maker_)) {
-        NOTREACHED();
-        return SBOX_ERROR_BAD_PARAMS;
-      }
-      break;
-    }
-    case SUBSYS_SYNC: {
-      if (!SyncPolicy::GenerateRules(pattern, semantics, policy_maker_)) {
-        NOTREACHED();
-        return SBOX_ERROR_BAD_PARAMS;
-      }
-      break;
-    }
-    case SUBSYS_PROCESS: {
-      if (lockdown_level_  < USER_INTERACTIVE &&
-          TargetPolicy::PROCESS_ALL_EXEC == semantics) {
-        // This is unsupported. This is a huge security risk to give full access
-        // to a process handle.
-        return SBOX_ERROR_UNSUPPORTED;
-      }
-      if (!ProcessPolicy::GenerateRules(pattern, semantics, policy_maker_)) {
-        NOTREACHED();
-        return SBOX_ERROR_BAD_PARAMS;
-      }
-      break;
-    }
-    case SUBSYS_NAMED_PIPES: {
-      if (!NamedPipePolicy::GenerateRules(pattern, semantics, policy_maker_)) {
-        NOTREACHED();
-        return SBOX_ERROR_BAD_PARAMS;
-      }
-      break;
-    }
-    case SUBSYS_REGISTRY: {
-      if (!RegistryPolicy::GenerateRules(pattern, semantics, policy_maker_)) {
-        NOTREACHED();
-        return SBOX_ERROR_BAD_PARAMS;
-      }
-      break;
-    }
-    case SUBSYS_HANDLES: {
-      if (!HandlePolicy::GenerateRules(pattern, semantics, policy_maker_)) {
-        NOTREACHED();
-        return SBOX_ERROR_BAD_PARAMS;
-      }
-      break;
-    }
-
-    case SUBSYS_WIN32K_LOCKDOWN: {
-      if (!ProcessMitigationsWin32KLockdownPolicy::GenerateRules(
-              pattern, semantics,policy_maker_)) {
-        NOTREACHED();
-        return SBOX_ERROR_BAD_PARAMS;
-      }
-      break;
-    }
-
-    default: {
-      return SBOX_ERROR_UNSUPPORTED;
-    }
-  }
-
-  return SBOX_ALL_OK;
+  ResultCode result = AddRuleInternal(subsystem, semantics, pattern);
+  LOG_IF(ERROR, result != SBOX_ALL_OK) << "Failed to add sandbox rule."
+                                       << " error = " << result
+                                       << ", subsystem = " << subsystem
+                                       << ", semantics = " << semantics
+                                       << ", pattern = '" << pattern << "'";
+  return result;
 }
 
 ResultCode PolicyBase::AddDllToUnload(const wchar_t* dll_name) {
@@ -735,4 +666,84 @@ bool PolicyBase::SetupHandleCloser(TargetProcess* target) {
   return handle_closer_.InitializeTargetHandles(target);
 }
 
+ResultCode PolicyBase::AddRuleInternal(SubSystem subsystem,
+                                       Semantics semantics,
+                                       const wchar_t* pattern) {
+  if (NULL == policy_) {
+    policy_ = MakeBrokerPolicyMemory();
+    DCHECK(policy_);
+    policy_maker_ = new LowLevelPolicy(policy_);
+    DCHECK(policy_maker_);
+  }
+
+  switch (subsystem) {
+    case SUBSYS_FILES: {
+      if (!file_system_init_) {
+        if (!FileSystemPolicy::SetInitialRules(policy_maker_))
+          return SBOX_ERROR_BAD_PARAMS;
+        file_system_init_ = true;
+      }
+      if (!FileSystemPolicy::GenerateRules(pattern, semantics, policy_maker_)) {
+        NOTREACHED();
+        return SBOX_ERROR_BAD_PARAMS;
+      }
+      break;
+    }
+    case SUBSYS_SYNC: {
+      if (!SyncPolicy::GenerateRules(pattern, semantics, policy_maker_)) {
+        NOTREACHED();
+        return SBOX_ERROR_BAD_PARAMS;
+      }
+      break;
+    }
+    case SUBSYS_PROCESS: {
+      if (lockdown_level_ < USER_INTERACTIVE &&
+          TargetPolicy::PROCESS_ALL_EXEC == semantics) {
+        // This is unsupported. This is a huge security risk to give full access
+        // to a process handle.
+        return SBOX_ERROR_UNSUPPORTED;
+      }
+      if (!ProcessPolicy::GenerateRules(pattern, semantics, policy_maker_)) {
+        NOTREACHED();
+        return SBOX_ERROR_BAD_PARAMS;
+      }
+      break;
+    }
+    case SUBSYS_NAMED_PIPES: {
+      if (!NamedPipePolicy::GenerateRules(pattern, semantics, policy_maker_)) {
+        NOTREACHED();
+        return SBOX_ERROR_BAD_PARAMS;
+      }
+      break;
+    }
+    case SUBSYS_REGISTRY: {
+      if (!RegistryPolicy::GenerateRules(pattern, semantics, policy_maker_)) {
+        NOTREACHED();
+        return SBOX_ERROR_BAD_PARAMS;
+      }
+      break;
+    }
+    case SUBSYS_HANDLES: {
+      if (!HandlePolicy::GenerateRules(pattern, semantics, policy_maker_)) {
+        NOTREACHED();
+        return SBOX_ERROR_BAD_PARAMS;
+      }
+      break;
+    }
+
+    case SUBSYS_WIN32K_LOCKDOWN: {
+      if (!ProcessMitigationsWin32KLockdownPolicy::GenerateRules(
+              pattern, semantics, policy_maker_)) {
+        NOTREACHED();
+        return SBOX_ERROR_BAD_PARAMS;
+      }
+      break;
+    }
+
+    default: { return SBOX_ERROR_UNSUPPORTED; }
+  }
+
+  return SBOX_ALL_OK;
+}
+
 }  // namespace sandbox
diff --git a/sandbox/win/src/sandbox_policy_base.h b/sandbox/win/src/sandbox_policy_base.h
index 952df0d..9ba8d27 100644
--- a/sandbox/win/src/sandbox_policy_base.h
+++ b/sandbox/win/src/sandbox_policy_base.h
@@ -115,6 +115,10 @@ class PolicyBase : public Dispatcher, public TargetPolicy {
   // Sets up the handle closer for a new target.
   bool SetupHandleCloser(TargetProcess* target);
 
+  ResultCode AddRuleInternal(SubSystem subsystem,
+                             Semantics semantics,
+                             const wchar_t* pattern);
+
   // This lock synchronizes operations on the targets_ collection.
   CRITICAL_SECTION lock_;
   // Maintains the list of target process associated with this policy.