Use audit_token_t instead of pid_t for checking sandbox of other processes
Created attachment 306534 [details] Patch
Comment on attachment 306534 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=306534&action=review > Source/WTF/wtf/spi/darwin/SandboxSPI.h:47 > +int sandbox_check_by_audit_token(audit_token_t, const char *operation, enum sandbox_filter_type, ...); For some reason I seem to recall that this SPI was not available on some version of macOS that we cared to support at least six months ago. Is this SPI available on all the versions of macOS that we care to support?
Comment on attachment 306534 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=306534&action=review >> Source/WTF/wtf/spi/darwin/SandboxSPI.h:47 >> +int sandbox_check_by_audit_token(audit_token_t, const char *operation, enum sandbox_filter_type, ...); > > For some reason I seem to recall that this SPI was not available on some version of macOS that we cared to support at least six months ago. Is this SPI available on all the versions of macOS that we care to support? I find its existence all the way back to Yosemite.
Created attachment 306542 [details] Patch
Created attachment 306543 [details] Patch
(In reply to Alex Christensen from comment #3) > Comment on attachment 306534 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=306534&action=review > > >> Source/WTF/wtf/spi/darwin/SandboxSPI.h:47 > >> +int sandbox_check_by_audit_token(audit_token_t, const char *operation, enum sandbox_filter_type, ...); > > > > For some reason I seem to recall that this SPI was not available on some version of macOS that we cared to support at least six months ago. Is this SPI available on all the versions of macOS that we care to support? > > I find its existence all the way back to Yosemite. Cool. We do not support Yosemite anymore.
Comment on attachment 306543 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=306543&action=review > Source/WebKit2/PluginProcess/mac/PluginProcessMac.mm:625 > + RELEASE_ASSERT(currentProcessIsSandboxed()); We will never hit this assertion. Should we still ensure our parent process is sandboxed?
(In reply to Daniel Bates from comment #7) > Comment on attachment 306543 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=306543&action=review > > > Source/WebKit2/PluginProcess/mac/PluginProcessMac.mm:625 > > + RELEASE_ASSERT(currentProcessIsSandboxed()); > > We will never hit this assertion. I mean, this assertion is always guaranteed to be satisfied because we only execute it when currentProcessIsSandboxed() evaluates to true.
Comment on attachment 306543 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=306543&action=review >>> Source/WebKit2/PluginProcess/mac/PluginProcessMac.mm:625 >>> + RELEASE_ASSERT(currentProcessIsSandboxed()); >> >> We will never hit this assertion. Should we still ensure our parent process is sandboxed? > > I mean, this assertion is always guaranteed to be satisfied because we only execute it when currentProcessIsSandboxed() evaluates to true. oh, the assertion used to be asserting that the parent process was sandboxed with getppid, not the current process with getpid. Gotta be careful! I'll fix this.
There's no connection in this case. I'm going to just remove the assertion since there's no good way to assert about the parent process without a connection and without using a possibly-reused pid.
Created attachment 306547 [details] Patch
<rdar://problem/31158189>
Comment on attachment 306547 [details] Patch Clearing flags on attachment: 306547 Committed r215132: <http://trac.webkit.org/changeset/215132>
All reviewed patches have been landed. Closing bug.