Bug 200182 - Allow more syscalls in the WebContent process' sandbox profile
Summary: Allow more syscalls in the WebContent process' sandbox profile
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-07-26 16:27 PDT by Chris Dumez
Modified: 2019-07-27 06:41 PDT (History)
6 users (show)

See Also:


Attachments
Patch (2.19 KB, patch)
2019-07-26 16:28 PDT, Chris Dumez
ggaren: review+
ap: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2019-07-26 16:27:10 PDT
Allow more syscalls in the WebContent process' sandbox profile to avoid getting killed.
Comment 1 Chris Dumez 2019-07-26 16:27:25 PDT
<rdar://problem/53594973>
Comment 2 Chris Dumez 2019-07-26 16:28:52 PDT
Created attachment 374998 [details]
Patch
Comment 3 Alexey Proskuryakov 2019-07-26 16:33:55 PDT
Comment on attachment 374998 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=374998&action=review

> Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:945
> +        (syscall-number SYS_recvmsg)

This one looks suspicious. Looking at the logs, it seems like there is actual networking happening in WebContent, which is not allowed. So we may need a higher level fix here.
Comment 4 Chris Dumez 2019-07-26 16:36:50 PDT
Comment on attachment 374998 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=374998&action=review

>> Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:945
>> +        (syscall-number SYS_recvmsg)
> 
> This one looks suspicious. Looking at the logs, it seems like there is actual networking happening in WebContent, which is not allowed. So we may need a higher level fix here.

I am no sandboxing expert so I followed the existing pattern. We already allow SYS_recvfrom / SYS_recvfrom_nocancel / SYS_sendto / SYS_sendmsg_nocancel / SYS_sendto_nocancel.
Comment 5 Chris Dumez 2019-07-26 16:37:48 PDT
Comment on attachment 374998 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=374998&action=review

>>> Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:945
>>> +        (syscall-number SYS_recvmsg)
>> 
>> This one looks suspicious. Looking at the logs, it seems like there is actual networking happening in WebContent, which is not allowed. So we may need a higher level fix here.
> 
> I am no sandboxing expert so I followed the existing pattern. We already allow SYS_recvfrom / SYS_recvfrom_nocancel / SYS_sendto / SYS_sendmsg_nocancel / SYS_sendto_nocancel.

Couldn't it be a socket used to talk to a daemon, as opposed to actual networking?
Comment 6 Alexey Proskuryakov 2019-07-26 17:05:19 PDT
> Couldn't it be a socket used to talk to a daemon, as opposed to actual networking?

I have no definitive proof, but I doubt it - there's always com.apple.NSURLConnectionLoader thread in those processes, and usually people who use sockets use them through low level API.

> I am no sandboxing expert so I followed the existing pattern. We already allow SYS_recvfrom / SYS_recvfrom_nocancel / SYS_sendto / SYS_sendmsg_nocancel / SYS_sendto_nocancel.

Maybe all of those are wrong :-O
Comment 7 Geoffrey Garen 2019-07-26 17:24:12 PDT
Comment on attachment 374998 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=374998&action=review

r=me

>>>> Source/WebKit/WebProcess/com.apple.WebProcess.sb.in:945
>>>> +        (syscall-number SYS_recvmsg)
>>> 
>>> This one looks suspicious. Looking at the logs, it seems like there is actual networking happening in WebContent, which is not allowed. So we may need a higher level fix here.
>> 
>> I am no sandboxing expert so I followed the existing pattern. We already allow SYS_recvfrom / SYS_recvfrom_nocancel / SYS_sendto / SYS_sendmsg_nocancel / SYS_sendto_nocancel.
> 
> Couldn't it be a socket used to talk to a daemon, as opposed to actual networking?

FWIW, in the traces I saw, each call to recvmsg was triggered by a network connection failure. Seems like someone might have tried to do some networking, but didn't necessarily succeed.

Given that these similar sys calls are already allowed, I'm inclined to allow this one too, so folks can stop crashing while we figure out how to generally remove access to this set of syscalls.
Comment 8 Alexey Proskuryakov 2019-07-26 17:30:40 PDT
Comment on attachment 374998 [details]
Patch

Let's not be in such a crazy hurry.
Comment 9 Alexey Proskuryakov 2019-07-26 23:45:20 PDT
> SYS_sendto 

Used by ASL logging facility, so definitely needed.

> SYS_recvfrom_nocancel 
> SYS_sendmsg_nocancel 
> SYS_sendto_nocancel

Very few tests hit these, http/tests/workers/service/serviceworker-websocket.https.html being one of those.

> SYS_recvfrom 

I didn't see this one hit on layout tests, but I did see SYS_recvmsg, the same test reproduces it. Maybe the internal implementation in libnetwork switched from one to another after Per Arne's initial testing.

So WebSockets in service workers try to use networking in WebContent. This appears related to bug 200161 and bug 199906. The former has a fix posted today, so we should re-test and hopefully remove the unnecessary syscalls.

I suggest landing this patch with only SYS_chmod_extended and SYS_lstat_extended added for now. We also need to wire service worker processes to WebKitTestRunner's crash reporting - it's quite unfortunate that these crashes are currently silent during tests.
Comment 10 Chris Dumez 2019-07-27 06:41:59 PDT
Committed r247890: <https://trac.webkit.org/changeset/247890>