203419 2019-10-25 09:04:48 -0700 [iOS] Fix sandbox violations seen while running layout tests 2019-10-28 16:58:52 -0700 1 1 1 Unclassified WebKit WebKit Misc. WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 pvollan pvollan ap bfulgham commit-queue webkit-bug-importer oldest_to_newest 1583844 0 pvollan 2019-10-25 09:04:48 -0700 Deny mach lookup to 'com.apple.logd' and 'com.apple.logd.events' and suppress logs, since these are believed to be unneeded in the WebContent process. Allow sysctl write to 'vm.footprint_suspend' internally. Deny mach lookup to 'com.apple.system.notification_center' and suppress logs, since allowing this is not believed to be needed in the WebContent process. 1583850 1 381930 pvollan 2019-10-25 09:09:06 -0700 Created attachment 381930 Patch 1583864 2 381930 bfulgham 2019-10-25 09:41:36 -0700 Comment on attachment 381930 Patch r=me 1583870 3 pvollan 2019-10-25 09:47:51 -0700 (In reply to Brent Fulgham from comment #2) > Comment on attachment 381930 [details] > Patch > > r=me Thanks for reviewing! 1583944 4 commit-queue 2019-10-25 12:03:08 -0700 The commit-queue encountered the following flaky tests while processing attachment 381930: imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/module/integrity.html bug 203394 (author: ysuzuki@apple.com) The commit-queue is continuing to process your patch. 1583945 5 commit-queue 2019-10-25 12:03:09 -0700 The commit-queue encountered the following flaky tests while processing attachment 381930: The commit-queue is continuing to process your patch. 1584097 6 commit-queue 2019-10-25 16:59:54 -0700 The commit-queue encountered the following flaky tests while processing attachment 381930: imported/w3c/web-platform-tests/html/semantics/scripting-1/the-script-element/module/integrity.html bug 203394 (author: ysuzuki@apple.com) The commit-queue is continuing to process your patch. 1584098 7 381930 commit-queue 2019-10-25 17:00:38 -0700 Comment on attachment 381930 Patch Clearing flags on attachment: 381930 Committed r251612: <https://trac.webkit.org/changeset/251612> 1584099 8 commit-queue 2019-10-25 17:00:39 -0700 All reviewed patches have been landed. Closing bug. 1584102 9 webkit-bug-importer 2019-10-25 17:01:16 -0700 <rdar://problem/56637619> 1584360 10 381930 ap 2019-10-27 18:43:13 -0700 Comment on attachment 381930 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=381930&action=review > Source/WebKit/ChangeLog:11 > + Deny mach lookup to 'com.apple.logd' and 'com.apple.logd.events' and suppress logs, since these are > + believed to be unneeded in the WebContent process. Allow sysctl write to 'vm.footprint_suspend'. > + Deny mach lookup to 'com.apple.system.notification_center' and suppress logs, since allowing this > + is not believed to be needed in the WebContent process. Where can stack traces for these violations be seen? It seems quite counter-intuitive that we do not need these connections. Can you document these decisions in the associated radar, if they cannot be documented here? Right now, there is nearly zero paper trail for why these dangerous changes were made. > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb:482 > +(with-filter (system-attribute apple-internal) > + (allow sysctl-read sysctl-write > + (sysctl-name "vm.footprint_suspend"))) "system-attribute apple-internal" is not what it seems. This change allows vm.footprint_suspend on some AppleInternal installs, but not on others, and it's unlikely that this is what you were after. You can see <rdar://problem/55853605> for some discussion of the differences. But also, footprint is a publicly shipping tool. What specifically about vm.footprint_suspend makes it AppleInternal only? 1584753 11 pvollan 2019-10-28 16:58:52 -0700 (In reply to Alexey Proskuryakov from comment #10) > Comment on attachment 381930 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=381930&action=review > > > Source/WebKit/ChangeLog:11 > > + Deny mach lookup to 'com.apple.logd' and 'com.apple.logd.events' and suppress logs, since these are > > + believed to be unneeded in the WebContent process. Allow sysctl write to 'vm.footprint_suspend'. > > + Deny mach lookup to 'com.apple.system.notification_center' and suppress logs, since allowing this > > + is not believed to be needed in the WebContent process. > > Where can stack traces for these violations be seen? It seems quite > counter-intuitive that we do not need these connections. > > Can you document these decisions in the associated radar, if they cannot be > documented here? Right now, there is nearly zero paper trail for why these > dangerous changes were made. > You are absolutely right. I have reverted most of these changes in https://bugs.webkit.org/show_bug.cgi?id=203505, except for access to 'com.apple.logd.events', which is not being looked up during layout tests. > > Source/WebKit/Resources/SandboxProfiles/ios/com.apple.WebKit.WebContent.sb:482 > > +(with-filter (system-attribute apple-internal) > > + (allow sysctl-read sysctl-write > > + (sysctl-name "vm.footprint_suspend"))) > > "system-attribute apple-internal" is not what it seems. This change allows > vm.footprint_suspend on some AppleInternal installs, but not on others, and > it's unlikely that this is what you were after. You can see > <rdar://problem/55853605> for some discussion of the differences. > > But also, footprint is a publicly shipping tool. What specifically about > vm.footprint_suspend makes it AppleInternal only? See comment in <rdar://problem/56637619>. Thanks for reviewing! 381930 2019-10-25 09:09:06 -0700 2019-10-25 17:00:38 -0700 Patch bug-203419-20191025090905.patch text/plain 3045 pvollan SW5kZXg6IFNvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJL aXQvQ2hhbmdlTG9nCShyZXZpc2lvbiAyNTE1OTIpCisrKyBTb3VyY2UvV2ViS2l0L0NoYW5nZUxv Zwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE5IEBACisyMDE5LTEwLTI1ICBQZXIgQXJuZSBW b2xsYW4gIDxwdm9sbGFuQGFwcGxlLmNvbT4KKworICAgICAgICBbaU9TXSBGaXggc2FuZGJveCB2 aW9sYXRpb25zIHNlZW4gd2hpbGUgcnVubmluZyBsYXlvdXQgdGVzdHMKKyAgICAgICAgaHR0cHM6 Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIwMzQxOQorCisgICAgICAgIFJldmll d2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIERlbnkgbWFjaCBsb29rdXAgdG8gJ2Nv bS5hcHBsZS5sb2dkJyBhbmQgJ2NvbS5hcHBsZS5sb2dkLmV2ZW50cycgYW5kIHN1cHByZXNzIGxv Z3MsIHNpbmNlIHRoZXNlIGFyZQorICAgICAgICBiZWxpZXZlZCB0byBiZSB1bm5lZWRlZCBpbiB0 aGUgV2ViQ29udGVudCBwcm9jZXNzLiBBbGxvdyBzeXNjdGwgd3JpdGUgdG8gJ3ZtLmZvb3Rwcmlu dF9zdXNwZW5kJy4KKyAgICAgICAgRGVueSBtYWNoIGxvb2t1cCB0byAnY29tLmFwcGxlLnN5c3Rl bS5ub3RpZmljYXRpb25fY2VudGVyJyBhbmQgc3VwcHJlc3MgbG9ncywgc2luY2UgYWxsb3dpbmcg dGhpcworICAgICAgICBpcyBub3QgYmVsaWV2ZWQgdG8gYmUgbmVlZGVkIGluIHRoZSBXZWJDb250 ZW50IHByb2Nlc3MuCisKKyAgICAgICAgTm8gbmV3IHRlc3RzLCBjb3ZlcmVkIGJ5IGV4aXN0aW5n IHRlc3RzLgorCisgICAgICAgICogUmVzb3VyY2VzL1NhbmRib3hQcm9maWxlcy9pb3MvY29tLmFw cGxlLldlYktpdC5XZWJDb250ZW50LnNiOgorCiAyMDE5LTEwLTI1ICBDaHJpcyBEdW1leiAgPGNk dW1lekBhcHBsZS5jb20+CiAKICAgICAgICAgW2lPU10gUmVncmVzc2lvbihyMjUxMDY3KSBXZWJQ cm9jZXNzZXMgd2l0aCBzZXJ2aWNlIHdvcmtlcnMgbm8gbG9uZ2VyIGtlZXAgdGhlaXIgbmV0d29y ayBwcm9jZXNzIGFsaXZlCkluZGV4OiBTb3VyY2UvV2ViS2l0L1Jlc291cmNlcy9TYW5kYm94UHJv ZmlsZXMvaW9zL2NvbS5hcHBsZS5XZWJLaXQuV2ViQ29udGVudC5zYgo9PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBT b3VyY2UvV2ViS2l0L1Jlc291cmNlcy9TYW5kYm94UHJvZmlsZXMvaW9zL2NvbS5hcHBsZS5XZWJL aXQuV2ViQ29udGVudC5zYgkocmV2aXNpb24gMjUxNTcyKQorKysgU291cmNlL1dlYktpdC9SZXNv dXJjZXMvU2FuZGJveFByb2ZpbGVzL2lvcy9jb20uYXBwbGUuV2ViS2l0LldlYkNvbnRlbnQuc2IJ KHdvcmtpbmcgY29weSkKQEAgLTQ1MSwxMSArNDUxLDkgQEAKICAgICAgICAgKGV4dGVuc2lvbi1j bGFzcyAiY29tLmFwcGxlLmFwcC1zYW5kYm94LnJlYWQtd3JpdGUiICJjb20uYXBwbGUuYXBwLXNh bmRib3gucmVhZCIpCiAgICAgICAgIChleHRlbnNpb24gImNvbS5hcHBsZS5maWxlcHJvdmlkZXIu cmVhZC13cml0ZSIpKSkKIAotKHdoZW4gKGRlZmluZWQ/ICdyZXN0cmljdGl2ZS1leHRlbnNpb24p Ci0gICAgICAod2l0aC1maWx0ZXIgKHJlcXVpcmUtbm90IChyZXF1aXJlLWVudGl0bGVtZW50ICJn ZXQtdGFzay1hbGxvdyIpKQotICAgICAgICAgIChkZW55IG1hY2gtbG9va3VwICh3aXRoIG5vLXJl cG9ydCkKLSAgICAgICAgICAgICAgICAoZ2xvYmFsLW5hbWUgImNvbS5hcHBsZS5sb2dkIikKLSAg ICAgICAgICAgICAgICAoZ2xvYmFsLW5hbWUgImNvbS5hcHBsZS5sb2dkLmV2ZW50cyIpKSkpCiso ZGVueSBtYWNoLWxvb2t1cCAod2l0aCBuby1yZXBvcnQpCisgICAgKGdsb2JhbC1uYW1lICJjb20u YXBwbGUubG9nZCIpCisgICAgKGdsb2JhbC1uYW1lICJjb20uYXBwbGUubG9nZC5ldmVudHMiKSkK IAogKGFsbG93IGlwYy1wb3NpeC1zaG0tcmVhZCoKICAgICAgICAoaXBjLXBvc2l4LW5hbWUtcHJl Zml4ICJhcHBsZS5jZnByZWZzLiIpKQpAQCAtNDc5LDE5ICs0NzcsMTcgQEAKIChkZW55IHN5c2N0 bC1yZWFkICh3aXRoIG5vLXJlcG9ydCkKICAgICAgIChzeXNjdGwtbmFtZSAic3lzY3RsLnByb2Nf bmF0aXZlIikpCiAKKyh3aXRoLWZpbHRlciAoc3lzdGVtLWF0dHJpYnV0ZSBhcHBsZS1pbnRlcm5h bCkKKyAgICAoYWxsb3cgc3lzY3RsLXJlYWQgc3lzY3RsLXdyaXRlCisgICAgICAgICAgIChzeXNj dGwtbmFtZSAidm0uZm9vdHByaW50X3N1c3BlbmQiKSkpCisKIChhbGxvdyBmaWxlLXJlYWQtbWV0 YWRhdGEgbmV0d29yay1vdXRib3VuZAogICAgICAgIChsaXRlcmFsICIvcHJpdmF0ZS92YXIvcnVu L3N5c2xvZyIpKQogCi0oaWYgKGRlZmluZWQ/ICdyZXN0cmljdGl2ZS1leHRlbnNpb24pCi0gICAg KGJlZ2luCi0gICAgICAgIChkZW55IG1hY2gtbG9va3VwICh3aXRoIG5vLXJlcG9ydCkKLSAgICAg ICAgICAgICAgIChnbG9iYWwtbmFtZSAiY29tLmFwcGxlLnN5c3RlbS5ub3RpZmljYXRpb25fY2Vu dGVyIikpCi0gICAgICAgIChkZW55IGlwYy1wb3NpeC1zaG0tcmVhZCogKHdpdGggbm8tcmVwb3J0 KQotICAgICAgICAgICAgICAgKGlwYy1wb3NpeC1uYW1lICJhcHBsZS5zaG0ubm90aWZpY2F0aW9u X2NlbnRlciIpKSkKLTsgZWxzZQotICAgIChiZWdpbgotICAgICAgICAoYWxsb3cgaXBjLXBvc2l4 LXNobS1yZWFkKgotICAgICAgICAgICAgICAgKGlwYy1wb3NpeC1uYW1lICJhcHBsZS5zaG0ubm90 aWZpY2F0aW9uX2NlbnRlciIpKSkpCisoZGVueSBtYWNoLWxvb2t1cCAod2l0aCBuby1yZXBvcnQp CisgICAgICAgKGdsb2JhbC1uYW1lICJjb20uYXBwbGUuc3lzdGVtLm5vdGlmaWNhdGlvbl9jZW50 ZXIiKSkKKyhkZW55IGlwYy1wb3NpeC1zaG0tcmVhZCogKHdpdGggbm8tcmVwb3J0KQorICAgICAg IChpcGMtcG9zaXgtbmFtZSAiYXBwbGUuc2htLm5vdGlmaWNhdGlvbl9jZW50ZXIiKSkKIAogKGxv Z2QtZGlhZ25vc3RpYy1jbGllbnQpCiAK