Bug 1344453 - Part 1: Allow a special all paths rule in the Windows process sandbox when using semantics FILES_ALLOW_READONLY. r=jimm, a=lizzard
authorBob Owen <bobowencode@gmail.com>
Tue, 28 Mar 2017 08:36:16 +0100
changeset 379371 cfb4a743d65715d56dcb16ddf009397e2a9644e8
parent 379370 5cecb3ae97986d625a95b6db04cc704439aedbef
child 379372 85c59b19c241b3072441003583daff98655fd933
push id1419
push userjlund@mozilla.com
push dateMon, 10 Apr 2017 20:44:07 +0000
treeherdermozilla-release@5e6801b73ef6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimm, lizzard
bugs1344453
milestone53.0
Bug 1344453 - Part 1: Allow a special all paths rule in the Windows process sandbox when using semantics FILES_ALLOW_READONLY. r=jimm, a=lizzard This also changes the read only related status checks in filesystem_interception.cc to include STATUS_NETWORK_OPEN_RESTRICTION (0xC0000201), which gets returned in some cases and fails because we never ask the broker.
security/sandbox/chromium/sandbox/win/src/filesystem_interception.cc
security/sandbox/chromium/sandbox/win/src/filesystem_policy.cc
security/sandbox/modifications-to-chromium-to-reapply-after-upstream-merge.txt
--- a/security/sandbox/chromium/sandbox/win/src/filesystem_interception.cc
+++ b/security/sandbox/chromium/sandbox/win/src/filesystem_interception.cc
@@ -11,32 +11,37 @@
 #include "sandbox/win/src/policy_params.h"
 #include "sandbox/win/src/policy_target.h"
 #include "sandbox/win/src/sandbox_factory.h"
 #include "sandbox/win/src/sandbox_nt_util.h"
 #include "sandbox/win/src/sharedmem_ipc_client.h"
 #include "sandbox/win/src/target_services.h"
 #include "mozilla/sandboxing/sandboxLogging.h"
 
+// This status occurs when trying to access a network share on the machine from
+// which it is shared.
+#define STATUS_NETWORK_OPEN_RESTRICTION ((NTSTATUS)0xC0000201L)
+
 namespace sandbox {
 
 NTSTATUS WINAPI TargetNtCreateFile(NtCreateFileFunction orig_CreateFile,
                                    PHANDLE file, ACCESS_MASK desired_access,
                                    POBJECT_ATTRIBUTES object_attributes,
                                    PIO_STATUS_BLOCK io_status,
                                    PLARGE_INTEGER allocation_size,
                                    ULONG file_attributes, ULONG sharing,
                                    ULONG disposition, ULONG options,
                                    PVOID ea_buffer, ULONG ea_length) {
   // Check if the process can open it first.
   NTSTATUS status = orig_CreateFile(file, desired_access, object_attributes,
                                     io_status, allocation_size,
                                     file_attributes, sharing, disposition,
                                     options, ea_buffer, ea_length);
-  if (STATUS_ACCESS_DENIED != status)
+  if (STATUS_ACCESS_DENIED != status &&
+      STATUS_NETWORK_OPEN_RESTRICTION != status)
     return status;
 
   mozilla::sandboxing::LogBlocked("NtCreateFile",
                                   object_attributes->ObjectName->Buffer,
                                   object_attributes->ObjectName->Length);
 
   // We don't trust that the IPC can work this early.
   if (!SandboxFactory::GetTargetServices()->GetState()->InitCalled())
@@ -109,17 +114,18 @@ NTSTATUS WINAPI TargetNtCreateFile(NtCre
 NTSTATUS WINAPI TargetNtOpenFile(NtOpenFileFunction orig_OpenFile, PHANDLE file,
                                  ACCESS_MASK desired_access,
                                  POBJECT_ATTRIBUTES object_attributes,
                                  PIO_STATUS_BLOCK io_status, ULONG sharing,
                                  ULONG options) {
   // Check if the process can open it first.
   NTSTATUS status = orig_OpenFile(file, desired_access, object_attributes,
                                   io_status, sharing, options);
-  if (STATUS_ACCESS_DENIED != status)
+  if (STATUS_ACCESS_DENIED != status &&
+      STATUS_NETWORK_OPEN_RESTRICTION != status)
     return status;
 
   mozilla::sandboxing::LogBlocked("NtOpenFile",
                                   object_attributes->ObjectName->Buffer,
                                   object_attributes->ObjectName->Length);
 
   // We don't trust that the IPC can work this early.
   if (!SandboxFactory::GetTargetServices()->GetState()->InitCalled())
@@ -188,17 +194,18 @@ NTSTATUS WINAPI TargetNtOpenFile(NtOpenF
 }
 
 NTSTATUS WINAPI TargetNtQueryAttributesFile(
     NtQueryAttributesFileFunction orig_QueryAttributes,
     POBJECT_ATTRIBUTES object_attributes,
     PFILE_BASIC_INFORMATION file_attributes) {
   // Check if the process can query it first.
   NTSTATUS status = orig_QueryAttributes(object_attributes, file_attributes);
-  if (STATUS_ACCESS_DENIED != status)
+  if (STATUS_ACCESS_DENIED != status &&
+      STATUS_NETWORK_OPEN_RESTRICTION != status)
     return status;
 
   mozilla::sandboxing::LogBlocked("NtQueryAttributesFile",
                                   object_attributes->ObjectName->Buffer,
                                   object_attributes->ObjectName->Length);
 
   // We don't trust that the IPC can work this early.
   if (!SandboxFactory::GetTargetServices()->GetState()->InitCalled())
@@ -253,17 +260,18 @@ NTSTATUS WINAPI TargetNtQueryAttributesF
 
 NTSTATUS WINAPI TargetNtQueryFullAttributesFile(
     NtQueryFullAttributesFileFunction orig_QueryFullAttributes,
     POBJECT_ATTRIBUTES object_attributes,
     PFILE_NETWORK_OPEN_INFORMATION file_attributes) {
   // Check if the process can query it first.
   NTSTATUS status = orig_QueryFullAttributes(object_attributes,
                                              file_attributes);
-  if (STATUS_ACCESS_DENIED != status)
+  if (STATUS_ACCESS_DENIED != status &&
+      STATUS_NETWORK_OPEN_RESTRICTION != status)
     return status;
 
   mozilla::sandboxing::LogBlocked("NtQueryFullAttributesFile",
                                   object_attributes->ObjectName->Buffer,
                                   object_attributes->ObjectName->Length);
 
   // We don't trust that the IPC can work this early.
   if (!SandboxFactory::GetTargetServices()->GetState()->InitCalled())
--- a/security/sandbox/chromium/sandbox/win/src/filesystem_policy.cc
+++ b/security/sandbox/chromium/sandbox/win/src/filesystem_policy.cc
@@ -77,17 +77,21 @@ namespace sandbox {
 bool FileSystemPolicy::GenerateRules(const wchar_t* name,
                                      TargetPolicy::Semantics semantics,
                                      LowLevelPolicy* policy) {
   base::string16 mod_name(name);
   if (mod_name.empty()) {
     return false;
   }
 
-  if (!PreProcessName(&mod_name)) {
+  // Don't pre-process the path name and check for reparse points if it is the
+  // special case of allowing read access to all paths.
+  if (!(semantics == TargetPolicy::FILES_ALLOW_READONLY
+        && mod_name.compare(L"*") == 0)
+      && !PreProcessName(&mod_name)) {
     // The path to be added might contain a reparse point.
     NOTREACHED();
     return false;
   }
 
   // TODO(cpu) bug 32224: This prefix add is a hack because we don't have the
   // infrastructure to normalize names. In any case we need to escape the
   // question marks.
--- a/security/sandbox/modifications-to-chromium-to-reapply-after-upstream-merge.txt
+++ b/security/sandbox/modifications-to-chromium-to-reapply-after-upstream-merge.txt
@@ -1,9 +1,10 @@
 Please add a link to the bugzilla bug and patch name that should be re-applied.
 Also, please update any existing links to their actual mozilla-central changeset.
 
 https://hg.mozilla.org/mozilla-central/rev/a05726163a79
 https://hg.mozilla.org/mozilla-central/rev/7df8d6639971
 https://hg.mozilla.org/mozilla-central/rev/e834e810a3fa
 https://hg.mozilla.org/mozilla-central/rev/c70d06fa5302
 https://hg.mozilla.org/mozilla-central/rev/d24db55deb85
-https://bugzilla.mozilla.org/show_bug.cgi?id=1321724 bug1321724.patch
+https://hg.mozilla.org/mozilla-central/rev/0e6bf137521e
+https://bugzilla.mozilla.org/show_bug.cgi?id=1344453 bug1344453part1.patch