Bug 1301034: Log when non-static file policy AddRule calls fail in Windows SandboxBroker. draft
authorBob Owen <bobowencode@gmail.com>
Mon, 12 Sep 2016 11:11:22 +0100
changeset 412582 a53bae0d2ad27d74d9ea67486228d8d7c62713d9
parent 410066 8c9c4e816e86f903c1d820f3f29715dc070a5a4a
child 531020 65df0dd692b8e41d5a7343d1e4bf1b552daf8467
push id29207
push userbobowencode@gmail.com
push dateMon, 12 Sep 2016 10:22:05 +0000
bugs1301034
milestone51.0a1
Bug 1301034: Log when non-static file policy AddRule calls fail in Windows SandboxBroker. MozReview-Commit-ID: DA5NizLfFfA
security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp
--- a/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp
+++ b/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp
@@ -3,24 +3,29 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "sandboxBroker.h"
 
 #include "base/win/windows_version.h"
 #include "mozilla/Assertions.h"
+#include "mozilla/Logging.h"
 #include "sandbox/win/src/sandbox.h"
 #include "sandbox/win/src/security_level.h"
 
 namespace mozilla
 {
 
 sandbox::BrokerServices *SandboxBroker::sBrokerService = nullptr;
 
+static LazyLogModule sSandboxBrokerLog("SandboxBroker");
+
+#define LOG_E(...) MOZ_LOG(sSandboxBrokerLog, LogLevel::Error, (__VA_ARGS__))
+
 /* static */
 void
 SandboxBroker::Initialize(sandbox::BrokerServices* aBrokerServices)
 {
   sBrokerService = aBrokerServices;
 }
 
 SandboxBroker::SandboxBroker()
@@ -117,17 +122,17 @@ SandboxBroker::SetSecurityLevelForConten
     delayedIntegrityLevel = sandbox::INTEGRITY_LEVEL_LOW;
   } else if (aSandboxLevel == 1) {
     jobLevel = sandbox::JOB_NONE;
     accessTokenLevel = sandbox::USER_NON_ADMIN;
     initialIntegrityLevel = sandbox::INTEGRITY_LEVEL_LOW;
     delayedIntegrityLevel = sandbox::INTEGRITY_LEVEL_LOW;
   }
 
-  sandbox::ResultCode result = mPolicy->SetJobLevel(jobLevel,
+  sandbox::ResultCode result = mPolicy->SetJobLevel(jobLevel,
                                                     0 /* ui_exceptions */);
   MOZ_RELEASE_ASSERT(sandbox::SBOX_ALL_OK == result,
                      "Setting job level failed, have you set memory limit when jobLevel == JOB_NONE?");
 
   result = mPolicy->SetTokenLevel(sandbox::USER_RESTRICTED_SAME_ACCESS,
                                   accessTokenLevel);
   MOZ_RELEASE_ASSERT(sandbox::SBOX_ALL_OK == result,
                      "Lockdown level cannot be USER_UNPROTECTED or USER_LAST if initial level was USER_RESTRICTED_SAME_ACCESS");
@@ -453,45 +458,61 @@ SandboxBroker::AllowReadFile(wchar_t con
   if (!mPolicy) {
     return false;
   }
 
   auto result =
     mPolicy->AddRule(sandbox::TargetPolicy::SUBSYS_FILES,
                      sandbox::TargetPolicy::FILES_ALLOW_READONLY,
                      file);
-  return (sandbox::SBOX_ALL_OK == result);
+  if (sandbox::SBOX_ALL_OK != result) {
+    LOG_E("Failed (ResultCode %d) to add read access to: %S", result, file);
+    return false;
+  }
+
+  return true;
 }
 
 bool
 SandboxBroker::AllowReadWriteFile(wchar_t const *file)
 {
   if (!mPolicy) {
     return false;
   }
 
   auto result =
     mPolicy->AddRule(sandbox::TargetPolicy::SUBSYS_FILES,
                      sandbox::TargetPolicy::FILES_ALLOW_ANY,
                      file);
-  return (sandbox::SBOX_ALL_OK == result);
+  if (sandbox::SBOX_ALL_OK != result) {
+    LOG_E("Failed (ResultCode %d) to add read/write access to: %S",
+          result, file);
+    return false;
+  }
+
+  return true;
 }
 
 bool
 SandboxBroker::AllowDirectory(wchar_t const *dir)
 {
   if (!mPolicy) {
     return false;
   }
 
   auto result =
     mPolicy->AddRule(sandbox::TargetPolicy::SUBSYS_FILES,
                      sandbox::TargetPolicy::FILES_ALLOW_DIR_ANY,
                      dir);
-  return (sandbox::SBOX_ALL_OK == result);
+  if (sandbox::SBOX_ALL_OK != result) {
+    LOG_E("Failed (ResultCode %d) to add directory access to: %S", result, dir);
+    return false;
+  }
+
+  return true;
 }
 
 bool
 SandboxBroker::AddTargetPeer(HANDLE aPeerProcess)
 {
   if (!sBrokerService) {
     return false;
   }