203618 2019-10-30 10:51:22 -0700 It should be possible to create a mach sandbox extension for the WebContent process before the audit token is known 2019-10-30 17:33:58 -0700 1 1 1 Unclassified WebKit WebKit Misc. WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 203629 1 pvollan pvollan benjamin bfulgham cdumez cmarcelo commit-queue dbates ews-watchlist webkit-bug-importer oldest_to_newest 1585418 0 pvollan 2019-10-30 10:51:22 -0700 Currently, we are only able to create a mach sandbox extension for the WebContent process if we know its audit token. It should be possible to create a mach extension without the audit token, since this is needed when we want to create extensions before the PID or audit token is known. These extensions are typically sent in the WebProcess creation parameters. 1585419 1 webkit-bug-importer 2019-10-30 10:51:45 -0700 <rdar://problem/56750384> 1585429 2 382332 pvollan 2019-10-30 11:05:59 -0700 Created attachment 382332 Patch 1585479 3 382332 ews-watchlist 2019-10-30 13:26:27 -0700 Comment on attachment 382332 Patch Attachment 382332 did not pass jsc-ews (mac): Output: https://webkit-queues.webkit.org/results/13192015 New failing tests: mozilla-tests.yaml/js1_5/Array/regress-101964.js.mozilla-ftl-eager-no-cjit-validate-phases 1585594 4 382332 bfulgham 2019-10-30 15:40:42 -0700 Comment on attachment 382332 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=382332&action=review > Source/WebKit/ChangeLog:11 > + typically sent in the WebProcess creation parameters. Is there a way to ensure we only call this no-audit-token code in cases where we don't have a connection yet? Maybe have the code take a ProcessProxy object and get its connection internally so a future developer can't accidentally misuse the API? > Source/WebKit/UIProcess/ios/WebProcessProxyIOS.mm:58 > + if (!SandboxExtension::createHandleForMachLookup("com.apple.iphone.axserver-systemwide", connection() ? connection()->getAuditToken() : WTF::nullopt, handle)) What if we passed the {Web|Network|ProcessProxy here (instead of the audit token pointer) so we could always confirm we were only passing a nullptr audit token when a connection was missing. 1585663 5 pvollan 2019-10-30 16:38:10 -0700 (In reply to Brent Fulgham from comment #4) > Comment on attachment 382332 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=382332&action=review > > > Source/WebKit/ChangeLog:11 > > + typically sent in the WebProcess creation parameters. > > Is there a way to ensure we only call this no-audit-token code in cases > where we don't have a connection yet? Maybe have the code take a > ProcessProxy object and get its connection internally so a future developer > can't accidentally misuse the API? > > > Source/WebKit/UIProcess/ios/WebProcessProxyIOS.mm:58 > > + if (!SandboxExtension::createHandleForMachLookup("com.apple.iphone.axserver-systemwide", connection() ? connection()->getAuditToken() : WTF::nullopt, handle)) > > What if we passed the {Web|Network|ProcessProxy here (instead of the audit > token pointer) so we could always confirm we were only passing a nullptr > audit token when a connection was missing. I think this is an excellent idea to enforce providing the audit token when available. However, I see that we normally don't include things from WebKit (e.g. #include <WebKit/WebProcessProxy.h>) from files in the WebKit Shared folder. Given this, should I still make the change? Thanks for reviewing! 1585669 6 382332 bfulgham 2019-10-30 16:43:50 -0700 Comment on attachment 382332 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=382332&action=review >>> Source/WebKit/ChangeLog:11 >>> + typically sent in the WebProcess creation parameters. >> >> Is there a way to ensure we only call this no-audit-token code in cases where we don't have a connection yet? Maybe have the code take a ProcessProxy object and get its connection internally so a future developer can't accidentally misuse the API? > > I think this is an excellent idea to enforce providing the audit token when available. However, I see that we normally don't include things from WebKit (e.g. #include <WebKit/WebProcessProxy.h>) from files in the WebKit Shared folder. Given this, should I still make the change? > > Thanks for reviewing! I see -- since this is used in UIProcess and {Web|Network}Process, this would be a layering violation. Okay -- let's leave it as-is. 1585674 7 382332 pvollan 2019-10-30 16:53:38 -0700 Comment on attachment 382332 Patch Thanks! 1585688 8 382332 commit-queue 2019-10-30 17:33:56 -0700 Comment on attachment 382332 Patch Clearing flags on attachment: 382332 Committed r251825: <https://trac.webkit.org/changeset/251825> 1585689 9 commit-queue 2019-10-30 17:33:58 -0700 All reviewed patches have been landed. Closing bug. 382332 2019-10-30 11:05:59 -0700 2019-10-30 17:33:56 -0700 Patch bug-203618-20191030110558.patch text/plain 6236 pvollan SW5kZXg6IFNvdXJjZS9XVEYvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XVEYvQ2hh bmdlTG9nCShyZXZpc2lvbiAyNTE3MTEpCisrKyBTb3VyY2UvV1RGL0NoYW5nZUxvZwkod29ya2lu ZyBjb3B5KQpAQCAtMSwzICsxLDE0IEBACisyMDE5LTEwLTMwICBQZXIgQXJuZSBWb2xsYW4gIDxw dm9sbGFuQGFwcGxlLmNvbT4KKworICAgICAgICBJdCBzaG91bGQgYmUgcG9zc2libGUgdG8gY3Jl YXRlIGEgbWFjaCBzYW5kYm94IGV4dGVuc2lvbiBmb3IgdGhlIFdlYkNvbnRlbnQgcHJvY2VzcyBi ZWZvcmUgdGhlIGF1ZGl0IHRva2VuIGlzIGtub3duCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJr aXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMDM2MTgKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JP RFkgKE9PUFMhKS4KKworICAgICAgICBBZGRlZCBTUEkgdG8gY3JlYXRlIG1hY2ggZXh0ZW5zaW9u IHdpdGhvdXQgUElEIG9yIGF1ZGl0IHRva2VuLgorCisgICAgICAgICogd3RmL3NwaS9kYXJ3aW4v U2FuZGJveFNQSS5oOgorCiAyMDE5LTEwLTI2ICBDaHJpcyBMb3JkICA8Y2xvcmRAaWdhbGlhLmNv bT4KIAogICAgICAgICBQdXQgT2Zmc2NyZWVuQ2FudmFzIGJlaGluZCBhIGJ1aWxkIGZsYWcKSW5k ZXg6IFNvdXJjZS9XVEYvd3RmL3NwaS9kYXJ3aW4vU2FuZGJveFNQSS5oCj09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0t IFNvdXJjZS9XVEYvd3RmL3NwaS9kYXJ3aW4vU2FuZGJveFNQSS5oCShyZXZpc2lvbiAyNTE3MTEp CisrKyBTb3VyY2UvV1RGL3d0Zi9zcGkvZGFyd2luL1NhbmRib3hTUEkuaAkod29ya2luZyBjb3B5 KQpAQCAtNjYsNiArNjYsNyBAQCBjaGFyICpzYW5kYm94X2V4dGVuc2lvbl9pc3N1ZV9maWxlX3Rv X3ByCiAjaWYgSEFWRShTQU5EQk9YX0lTU1VFX01BQ0hfRVhURU5TSU9OX1RPX1BST0NFU1NfQllf QVVESVRfVE9LRU4pCiBjaGFyICpzYW5kYm94X2V4dGVuc2lvbl9pc3N1ZV9tYWNoX3RvX3Byb2Nl c3MoY29uc3QgY2hhciAqZXh0ZW5zaW9uX2NsYXNzLCBjb25zdCBjaGFyICpuYW1lLCB1aW50MzJf dCBmbGFncywgYXVkaXRfdG9rZW5fdCk7CiAjZW5kaWYKK2NoYXIgKnNhbmRib3hfZXh0ZW5zaW9u X2lzc3VlX21hY2goY29uc3QgY2hhciAqZXh0ZW5zaW9uX2NsYXNzLCBjb25zdCBjaGFyICpuYW1l LCB1aW50MzJfdCBmbGFncyk7CiBpbnQgc2FuZGJveF9jaGVjayhwaWRfdCwgY29uc3QgY2hhciAq b3BlcmF0aW9uLCBlbnVtIHNhbmRib3hfZmlsdGVyX3R5cGUsIC4uLik7CiBpbnQgc2FuZGJveF9j aGVja19ieV9hdWRpdF90b2tlbihhdWRpdF90b2tlbl90LCBjb25zdCBjaGFyICpvcGVyYXRpb24s IGVudW0gc2FuZGJveF9maWx0ZXJfdHlwZSwgLi4uKTsKIGludCBzYW5kYm94X2NvbnRhaW5lcl9w YXRoX2Zvcl9waWQocGlkX3QsIGNoYXIgKmJ1ZmZlciwgc2l6ZV90IGJ1ZnNpemUpOwpJbmRleDog U291cmNlL1dlYktpdC9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYktpdC9DaGFu Z2VMb2cJKHJldmlzaW9uIDI1MTc4MykKKysrIFNvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCSh3b3Jr aW5nIGNvcHkpCkBAIC0xLDMgKzEsMjUgQEAKKzIwMTktMTAtMzAgIFBlciBBcm5lIFZvbGxhbiAg PHB2b2xsYW5AYXBwbGUuY29tPgorCisgICAgICAgIEl0IHNob3VsZCBiZSBwb3NzaWJsZSB0byBj cmVhdGUgYSBtYWNoIHNhbmRib3ggZXh0ZW5zaW9uIGZvciB0aGUgV2ViQ29udGVudCBwcm9jZXNz IGJlZm9yZSB0aGUgYXVkaXQgdG9rZW4gaXMga25vd24KKyAgICAgICAgaHR0cHM6Ly9idWdzLndl YmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIwMzYxOAorCisgICAgICAgIFJldmlld2VkIGJ5IE5P Qk9EWSAoT09QUyEpLgorCisgICAgICAgIEN1cnJlbnRseSwgd2UgYXJlIG9ubHkgYWJsZSB0byBj cmVhdGUgYSBtYWNoIHNhbmRib3ggZXh0ZW5zaW9uIGZvciB0aGUgV2ViQ29udGVudCBwcm9jZXNz IGlmIHdlIGtub3cgaXRzCisgICAgICAgIGF1ZGl0IHRva2VuLiBJdCBzaG91bGQgYmUgcG9zc2li bGUgdG8gY3JlYXRlIGEgbWFjaCBleHRlbnNpb24gd2l0aG91dCB0aGUgYXVkaXQgdG9rZW4sIHNp bmNlIHRoaXMgaXMKKyAgICAgICAgbmVlZGVkIHdoZW4gd2Ugd2FudCB0byBjcmVhdGUgZXh0ZW5z aW9ucyBiZWZvcmUgdGhlIFBJRCBvciBhdWRpdCB0b2tlbiBpcyBrbm93bi4gVGhlc2UgZXh0ZW5z aW9ucyBhcmUKKyAgICAgICAgdHlwaWNhbGx5IHNlbnQgaW4gdGhlIFdlYlByb2Nlc3MgY3JlYXRp b24gcGFyYW1ldGVycy4KKyAgICAgICAgCisgICAgICAgIE5vIG5ldyB0ZXN0cywgdGhpcyBpcyBu b3QgYSBiZWhhdmlvciBjaGFuZ2UsIGJ1dCBhIHBhdGNoIGluIHByZXBhcmF0aW9uIGZvciBmdXR1 cmUgcGF0Y2hlcy4KKworICAgICAgICAqIFNoYXJlZC9Db2NvYS9TYW5kYm94RXh0ZW5zaW9uQ29j b2EubW06CisgICAgICAgIChXZWJLaXQ6OlNhbmRib3hFeHRlbnNpb25JbXBsOjpzYW5kYm94RXh0 ZW5zaW9uRm9yVHlwZSk6CisgICAgICAgIChXZWJLaXQ6OlNhbmRib3hFeHRlbnNpb246OmNyZWF0 ZUhhbmRsZUZvck1hY2hMb29rdXApOgorICAgICAgICAoV2ViS2l0OjpTYW5kYm94RXh0ZW5zaW9u OjpjcmVhdGVIYW5kbGVGb3JNYWNoTG9va3VwQnlBdWRpdFRva2VuKTogRGVsZXRlZC4KKyAgICAg ICAgKiBTaGFyZWQvU2FuZGJveEV4dGVuc2lvbi5oOgorICAgICAgICAqIFVJUHJvY2Vzcy9pb3Mv V2ViUHJvY2Vzc1Byb3h5SU9TLm1tOgorICAgICAgICAoV2ViS2l0OjpXZWJQcm9jZXNzUHJveHk6 OnVuYmxvY2tBY2Nlc3NpYmlsaXR5U2VydmVySWZOZWVkZWQpOgorCiAyMDE5LTEwLTMwICBDaHJp cyBEdW1leiAgPGNkdW1lekBhcHBsZS5jb20+CiAKICAgICAgICAgW2lPU10gW1dLMl0gSW1wcm92 ZSBwcm9jZXNzIGFzc2VydGlvbi1yZWxhdGVkIGxvZ2dpbmcgdG8gaGVscCBpZGVudGlmeSBjYXVz ZXMgb2YgbGVha2VkIGFzc2VydGlvbnMKSW5kZXg6IFNvdXJjZS9XZWJLaXQvU2hhcmVkL1NhbmRi b3hFeHRlbnNpb24uaAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViS2l0L1NoYXJlZC9TYW5kYm94 RXh0ZW5zaW9uLmgJKHJldmlzaW9uIDI1MTcxMSkKKysrIFNvdXJjZS9XZWJLaXQvU2hhcmVkL1Nh bmRib3hFeHRlbnNpb24uaAkod29ya2luZyBjb3B5KQpAQCAtMTA1LDcgKzEwNSw3IEBAIHB1Ymxp YzoKICAgICBzdGF0aWMgU3RyaW5nIGNyZWF0ZUhhbmRsZUZvclRlbXBvcmFyeUZpbGUoY29uc3Qg U3RyaW5nJiBwcmVmaXgsIFR5cGUsIEhhbmRsZSYpOwogICAgIHN0YXRpYyBib29sIGNyZWF0ZUhh bmRsZUZvckdlbmVyaWNFeHRlbnNpb24oY29uc3QgU3RyaW5nJiBleHRlbnNpb25DbGFzcywgSGFu ZGxlJik7CiAjaWYgSEFWRShBVURJVF9UT0tFTikKLSAgICBzdGF0aWMgYm9vbCBjcmVhdGVIYW5k bGVGb3JNYWNoTG9va3VwQnlBdWRpdFRva2VuKGNvbnN0IFN0cmluZyYgc2VydmljZSwgYXVkaXRf dG9rZW5fdCwgSGFuZGxlJik7CisgICAgc3RhdGljIGJvb2wgY3JlYXRlSGFuZGxlRm9yTWFjaExv b2t1cChjb25zdCBTdHJpbmcmIHNlcnZpY2UsIE9wdGlvbmFsPGF1ZGl0X3Rva2VuX3Q+LCBIYW5k bGUmKTsKICAgICBzdGF0aWMgYm9vbCBjcmVhdGVIYW5kbGVGb3JSZWFkQnlBdWRpdFRva2VuKGNv bnN0IFN0cmluZyYgcGF0aCwgYXVkaXRfdG9rZW5fdCwgSGFuZGxlJik7CiAjZW5kaWYKICAgICB+ U2FuZGJveEV4dGVuc2lvbigpOwpJbmRleDogU291cmNlL1dlYktpdC9TaGFyZWQvQ29jb2EvU2Fu ZGJveEV4dGVuc2lvbkNvY29hLm1tCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJLaXQvU2hhcmVk L0NvY29hL1NhbmRib3hFeHRlbnNpb25Db2NvYS5tbQkocmV2aXNpb24gMjUxNzExKQorKysgU291 cmNlL1dlYktpdC9TaGFyZWQvQ29jb2EvU2FuZGJveEV4dGVuc2lvbkNvY29hLm1tCSh3b3JraW5n IGNvcHkpCkBAIC05Miw5ICs5Miw5IEBAIHByaXZhdGU6CiAgICAgICAgIGNhc2UgU2FuZGJveEV4 dGVuc2lvbjo6VHlwZTo6UmVhZFdyaXRlOgogICAgICAgICAgICAgcmV0dXJuIHNhbmRib3hfZXh0 ZW5zaW9uX2lzc3VlX2ZpbGUoQVBQX1NBTkRCT1hfUkVBRF9XUklURSwgcGF0aCwgMCk7CiAgICAg ICAgIGNhc2UgU2FuZGJveEV4dGVuc2lvbjo6VHlwZTo6TWFjaDoKLSNpZiBIQVZFKFNBTkRCT1hf SVNTVUVfTUFDSF9FWFRFTlNJT05fVE9fUFJPQ0VTU19CWV9BVURJVF9UT0tFTikKICAgICAgICAg ICAgIGlmICghYXVkaXRUb2tlbikKLSAgICAgICAgICAgICAgICByZXR1cm4gbnVsbHB0cjsKKyAg ICAgICAgICAgICAgICByZXR1cm4gc2FuZGJveF9leHRlbnNpb25faXNzdWVfbWFjaCgiY29tLmFw cGxlLndlYmtpdC5leHRlbnNpb24ubWFjaCJfcywgcGF0aCwgMCk7CisjaWYgSEFWRShTQU5EQk9Y X0lTU1VFX01BQ0hfRVhURU5TSU9OX1RPX1BST0NFU1NfQllfQVVESVRfVE9LRU4pCiAgICAgICAg ICAgICByZXR1cm4gc2FuZGJveF9leHRlbnNpb25faXNzdWVfbWFjaF90b19wcm9jZXNzKCJjb20u YXBwbGUud2Via2l0LmV4dGVuc2lvbi5tYWNoIl9zLCBwYXRoLCAwLCAqYXVkaXRUb2tlbik7CiAj ZWxzZQogICAgICAgICAgICAgVU5VU0VEX1BBUkFNKGF1ZGl0VG9rZW4pOwpAQCAtMzM2LDcgKzMz Niw3IEBAIGJvb2wgU2FuZGJveEV4dGVuc2lvbjo6Y3JlYXRlSGFuZGxlRm9yR2UKICAgICByZXR1 cm4gdHJ1ZTsKIH0KIAotYm9vbCBTYW5kYm94RXh0ZW5zaW9uOjpjcmVhdGVIYW5kbGVGb3JNYWNo TG9va3VwQnlBdWRpdFRva2VuKGNvbnN0IFN0cmluZyYgc2VydmljZSwgYXVkaXRfdG9rZW5fdCBh dWRpdFRva2VuLCBIYW5kbGUmIGhhbmRsZSkKK2Jvb2wgU2FuZGJveEV4dGVuc2lvbjo6Y3JlYXRl SGFuZGxlRm9yTWFjaExvb2t1cChjb25zdCBTdHJpbmcmIHNlcnZpY2UsIE9wdGlvbmFsPGF1ZGl0 X3Rva2VuX3Q+IGF1ZGl0VG9rZW4sIEhhbmRsZSYgaGFuZGxlKQogewogICAgIEFTU0VSVCghaGFu ZGxlLm1fc2FuZGJveEV4dGVuc2lvbik7CiAgICAgCkluZGV4OiBTb3VyY2UvV2ViS2l0L1VJUHJv Y2Vzcy9pb3MvV2ViUHJvY2Vzc1Byb3h5SU9TLm1tCj09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJL aXQvVUlQcm9jZXNzL2lvcy9XZWJQcm9jZXNzUHJveHlJT1MubW0JKHJldmlzaW9uIDI1MTcxMSkK KysrIFNvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL2lvcy9XZWJQcm9jZXNzUHJveHlJT1MubW0JKHdv cmtpbmcgY29weSkKQEAgLTU0LDE0ICs1NCw4IEBAIHZvaWQgV2ViUHJvY2Vzc1Byb3h5Ojp1bmJs b2NrQWNjZXNzaWJpbGkKICAgICBpZiAoIWNhblNlbmRNZXNzYWdlKCkpCiAgICAgICAgIHJldHVy bjsKIAotICAgIEFTU0VSVChjb25uZWN0aW9uKCkgJiYgY29ubmVjdGlvbigpLT5nZXRBdWRpdFRv a2VuKCkpOwotICAgIGlmICghY29ubmVjdGlvbigpIHx8ICFjb25uZWN0aW9uKCktPmdldEF1ZGl0 VG9rZW4oKSkgewotICAgICAgICBXVEZMb2dBbHdheXMoIlVuYWJsZSB0byBnZXQgYXVkaXQgdG9r ZW4uIik7Ci0gICAgICAgIHJldHVybjsKLSAgICB9Ci0gICAgCiAgICAgU2FuZGJveEV4dGVuc2lv bjo6SGFuZGxlIGhhbmRsZTsKLSAgICBpZiAoIVNhbmRib3hFeHRlbnNpb246OmNyZWF0ZUhhbmRs ZUZvck1hY2hMb29rdXBCeUF1ZGl0VG9rZW4oImNvbS5hcHBsZS5pcGhvbmUuYXhzZXJ2ZXItc3lz dGVtd2lkZSIsICooY29ubmVjdGlvbigpLT5nZXRBdWRpdFRva2VuKCkpLCBoYW5kbGUpKQorICAg IGlmICghU2FuZGJveEV4dGVuc2lvbjo6Y3JlYXRlSGFuZGxlRm9yTWFjaExvb2t1cCgiY29tLmFw cGxlLmlwaG9uZS5heHNlcnZlci1zeXN0ZW13aWRlIiwgY29ubmVjdGlvbigpID8gY29ubmVjdGlv bigpLT5nZXRBdWRpdFRva2VuKCkgOiBXVEY6Om51bGxvcHQsIGhhbmRsZSkpCiAgICAgICAgIHJl dHVybjsKIAogICAgIHNlbmQoTWVzc2FnZXM6OldlYlByb2Nlc3M6OlVuYmxvY2tBY2Nlc3NpYmls aXR5U2VydmVyKGhhbmRsZSksIDApOwo=