170074 2017-03-24 13:39:55 -0700 Re-enable the network and web processes' keychain access to fix client certificate authentication 2017-03-25 14:20:00 -0700 1 1 1 Unclassified WebKit WebKit2 WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=163710 InRadar P2 Normal --- 1 wilander wilander achristensen bfulgham commit-queue webkit-bug-importer wilander oldest_to_newest 1290977 0 wilander 2017-03-24 13:39:55 -0700 https://bugs.webkit.org/show_bug.cgi?id=163710 caused a regression in client certificate authentication for macOS. 1290978 1 wilander 2017-03-24 13:41:38 -0700 rdar://problem/31095987 1290983 2 305321 wilander 2017-03-24 13:46:19 -0700 Created attachment 305321 Patch 1291069 3 305321 bfulgham 2017-03-24 16:40:40 -0700 Comment on attachment 305321 Patch This looks great to me. r=me. 1291070 4 305321 wilander 2017-03-24 16:42:44 -0700 Comment on attachment 305321 Patch Thanks, Brent! 1291077 5 305321 ap 2017-03-24 16:48:51 -0700 Comment on attachment 305321 Patch Shouldn't we remove the ifdef from WebProcess.sb too? There are still loads that occur in WebContent. 1291084 6 wilander 2017-03-24 16:57:30 -0700 I do not have a test case that requires a WebProcess.sb changes to pass. My failing test only needed the change in this patch. I'm happy to revisit. 1291087 7 bfulgham 2017-03-24 17:03:12 -0700 (In reply to Alexey Proskuryakov from comment #5) > Comment on attachment 305321 [details] > Patch > > Shouldn't we remove the ifdef from WebProcess.sb too? There are still loads > that occur in WebContent. We believe the loads in WebContent are doing the right thing using the updated APIs, and so we want to keep them. If we discover evidence that we need to weaken WebProcess, too, we can do so once we have data indicating it is actually needed. 1291091 8 305321 commit-queue 2017-03-24 17:12:58 -0700 Comment on attachment 305321 Patch Clearing flags on attachment: 305321 Committed r214389: <http://trac.webkit.org/changeset/214389> 1291092 9 commit-queue 2017-03-24 17:13:02 -0700 All reviewed patches have been landed. Closing bug. 1291119 10 ap 2017-03-24 19:07:44 -0700 There was no evidence that the original change was safe, and I don't see any evidence presented that it's safe to keep in WebProcess. What did you test to see client certificate authentication work in WebProcess? 1291120 11 ap 2017-03-24 19:11:21 -0700 Resource loading has always required identical sandbox rules in WebContent and Networking processes, so it's quite unlikely that this case is different. 1291130 12 ap 2017-03-24 19:45:02 -0700 The reason why I'm re-opening this bug and not just filing a new one is that I do believe that the above invariant is important, so we should either restore it right away, or positively prove that it's not required any more. 1291154 13 bfulgham 2017-03-24 22:40:50 -0700 That is certainly important information. It's a shame no one on the architecture team ever bothered to document this anywhere! Can you share which aspects of loading must be kept the same between the two processes? Certainly there are many more exceptions in WebProcess than NetworkProcess. Are any of them running afoul of this invariant,too? John: Can you please provide a supplemental patch to support this undocumented invariant, and please add a comment somewhere so that we remember to keep these in sync in the future? 1291218 14 305382 wilander 2017-03-25 10:31:04 -0700 Created attachment 305382 Patch 1291219 15 wilander 2017-03-25 10:33:32 -0700 My patch reverts the version check instead of doing an exact match with the network process sandbox. I don't want to change pre-existing sandbox behavior before we get client certificate authentication working again. 1291251 16 305382 bfulgham 2017-03-25 13:21:32 -0700 Comment on attachment 305382 Patch R=me 1291257 17 305382 commit-queue 2017-03-25 14:19:57 -0700 Comment on attachment 305382 Patch Clearing flags on attachment: 305382 Committed r214404: <http://trac.webkit.org/changeset/214404> 1291258 18 commit-queue 2017-03-25 14:20:00 -0700 All reviewed patches have been landed. Closing bug. 305321 2017-03-24 13:46:19 -0700 2017-03-25 10:31:01 -0700 Patch bug-170074-20170324134618.patch text/plain 1916 wilander SW5kZXg6IFNvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi S2l0Mi9DaGFuZ2VMb2cJKHJldmlzaW9uIDIxNDM2OSkKKysrIFNvdXJjZS9XZWJLaXQyL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSArMSwxNCBAQAorMjAxNy0wMy0yNCAgSm9obiBXaWxh bmRlciAgPHdpbGFuZGVyQGFwcGxlLmNvbT4KKworICAgICAgICBSZS1lbmFibGUgdGhlIG5ldHdv cmsgcHJvY2Vzcycga2V5Y2hhaW4gYWNjZXNzIHRvIGZpeCBjbGllbnQgY2VydGlmaWNhdGUgYXV0 aGVudGljYXRpb24KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dp P2lkPTE3MDA3NAorICAgICAgICA8cmRhcjovL3Byb2JsZW0vMzEwOTU5ODc+CisKKyAgICAgICAg UmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBOZXR3b3JrUHJvY2Vzcy9t YWMvY29tLmFwcGxlLldlYktpdC5OZXR3b3JrUHJvY2Vzcy5zYi5pbjoKKyAgICAgICAgICAgIFJl dmVydGVkIHJlbWFpbmluZyBjaGFuZ2UgZnJvbQorICAgICAgICAgICAgaHR0cHM6Ly90cmFjLndl YmtpdC5vcmcvY2hhbmdlc2V0LzIwODcwMi93ZWJraXQgYW5kCisgICAgICAgICAgICBodHRwczov L3RyYWMud2Via2l0Lm9yZy9jaGFuZ2VzZXQvMjA4NzA3L3dlYmtpdC4KKwogPT0gUm9sbGVkIG92 ZXIgdG8gQ2hhbmdlTG9nLTIwMTctMDMtMjMgPT0KSW5kZXg6IFNvdXJjZS9XZWJLaXQyL05ldHdv cmtQcm9jZXNzL21hYy9jb20uYXBwbGUuV2ViS2l0Lk5ldHdvcmtQcm9jZXNzLnNiLmluCj09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT0KLS0tIFNvdXJjZS9XZWJLaXQyL05ldHdvcmtQcm9jZXNzL21hYy9jb20uYXBwbGUuV2Vi S2l0Lk5ldHdvcmtQcm9jZXNzLnNiLmluCShyZXZpc2lvbiAyMTQyNzYpCisrKyBTb3VyY2UvV2Vi S2l0Mi9OZXR3b3JrUHJvY2Vzcy9tYWMvY29tLmFwcGxlLldlYktpdC5OZXR3b3JrUHJvY2Vzcy5z Yi5pbgkod29ya2luZyBjb3B5KQpAQCAtMTUyLDcgKzE1Miw2IEBACiAgICAgICAgKGdsb2JhbC1u YW1lICJjb20uYXBwbGUuQ29yZUF1dGhlbnRpY2F0aW9uLmFnZW50LmxpYnhwYyIpCiAgICAgICAg KGdsb2JhbC1uYW1lICJjb20uYXBwbGUuU2VjdXJpdHlTZXJ2ZXIiKSkKIAotI2lmIF9fTUFDX09T X1hfVkVSU0lPTl9NSU5fUkVRVUlSRUQgPCAxMDEyNDAKIDs7IEZJWE1FOiBUaGlzIHNob3VsZCBi ZSByZW1vdmVkIHdoZW4gPHJkYXI6Ly9wcm9ibGVtLzEwNDc5Njg1PiBpcyBmaXhlZC4KIDs7IFJl c3RyaWN0IEFwcFNhbmRib3hlZCBwcm9jZXNzZXMgZnJvbSBjcmVhdGluZyAvTGlicmFyeS9LZXlj aGFpbnMsIGJ1dCBhbGxvdyBhY2Nlc3MgdG8gdGhlIGNvbnRlbnRzIG9mIC9MaWJyYXJ5L0tleWNo YWluczoKIChhbGxvdyBmaWxlLXJlYWQtZGF0YSBmaWxlLXJlYWQtbWV0YWRhdGEgZmlsZS13cml0 ZS1kYXRhCkBAIC0xNjMsNyArMTYyLDYgQEAKIChkZW55IGZpbGUtcmVhZCogZmlsZS13cml0ZSoK ICAgICAocmVnZXggKHN0cmluZy1hcHBlbmQgIi9MaWJyYXJ5L0tleWNoYWlucy8iICh1dWlkLXJl Z2V4LXN0cmluZykgIigvfCQpIikpCiAgICAgKGhvbWUtcmVnZXggKHN0cmluZy1hcHBlbmQgIi9M aWJyYXJ5L0tleWNoYWlucy8iICh1dWlkLXJlZ2V4LXN0cmluZykgIigvfCQpIikpKQotI2VuZGlm CiAKIChhbGxvdyBmaWxlLXJlYWQqIGZpbGUtd3JpdGUqIChzdWJwYXRoICIvcHJpdmF0ZS92YXIv ZGIvbWRzL3N5c3RlbSIpKSA7OyBGSVhNRTogVGhpcyBzaG91bGQgYmUgcmVtb3ZlZCB3aGVuIDxy ZGFyOi8vcHJvYmxlbS85NTM4NDE0PiBpcyBmaXhlZC4KIAo= 305382 2017-03-25 10:31:04 -0700 2017-03-25 14:19:57 -0700 Patch bug-170074-20170325103104.patch text/plain 2215 wilander SW5kZXg6IFNvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi S2l0Mi9DaGFuZ2VMb2cJKHJldmlzaW9uIDIxNDM5OSkKKysrIFNvdXJjZS9XZWJLaXQyL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIyIEBACisyMDE3LTAzLTI1ICBKb2huIFdp bGFuZGVyICA8d2lsYW5kZXJAYXBwbGUuY29tPgorCisgICAgICAgIFJlLWVuYWJsZSB0aGUgd2Vi IHByb2Nlc3MnIGtleWNoYWluIGFjY2VzcyB0byBmaXggY2xpZW50IGNlcnRpZmljYXRlIGF1dGhl bnRpY2F0aW9uCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD0xNzAwNzQKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzMxMDk1OTg3PgorCisgICAgICAgIFJl dmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRoaXMgaXMgYSBmb2xsb3ctdXAg cGF0Y2ggdG8KKyAgICAgICAgaHR0cHM6Ly90cmFjLndlYmtpdC5vcmcvY2hhbmdlc2V0LzIxNDM4 OS93ZWJraXQKKyAgICAgICAgc2luY2UgYWNjb3JkaW5nIHRvIEFsZXhleSBQcm9za3VyeWFrb3Ys IHJlc291cmNlCisgICAgICAgIGxvYWRpbmcgaGFzIGFsd2F5cyByZXF1aXJlZCBpZGVudGljYWwg c2FuZGJveAorICAgICAgICBydWxlcyBpbiBXZWJDb250ZW50IGFuZCBOZXR3b3JraW5nIHByb2Nl c3Nlcy4KKworICAgICAgICAqIFdlYlByb2Nlc3MvY29tLmFwcGxlLldlYlByb2Nlc3Muc2IuaW46 CisgICAgICAgICAgICBSZXZlcnRlZCByZW1haW5pbmcgY2hhbmdlIGZyb20KKyAgICAgICAgICAg IGh0dHBzOi8vdHJhYy53ZWJraXQub3JnL2NoYW5nZXNldC8yMDg3MDIvd2Via2l0IGFuZAorICAg ICAgICAgICAgaHR0cHM6Ly90cmFjLndlYmtpdC5vcmcvY2hhbmdlc2V0LzIwODcwNy93ZWJraXQu CisKIDIwMTctMDMtMjQgIFNpbW9uIEZyYXNlciAgPHNpbW9uLmZyYXNlckBhcHBsZS5jb20+CiAK ICAgICAgICAgW2lPUyBXSzJdIE1vdmUgZnJvbSBhIHByZS1jb21taXQgaGFuZGxlciB0byBkaXNw YXRjaF9hc3luYyBmb3IgdmlzaWJsZSBjb250ZW50IHJlY3QgdXBkYXRlcwpJbmRleDogU291cmNl L1dlYktpdDIvV2ViUHJvY2Vzcy9jb20uYXBwbGUuV2ViUHJvY2Vzcy5zYi5pbgo9PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 Ci0tLSBTb3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNzL2NvbS5hcHBsZS5XZWJQcm9jZXNzLnNiLmlu CShyZXZpc2lvbiAyMTQzOTkpCisrKyBTb3VyY2UvV2ViS2l0Mi9XZWJQcm9jZXNzL2NvbS5hcHBs ZS5XZWJQcm9jZXNzLnNiLmluCSh3b3JraW5nIGNvcHkpCkBAIC0zMzYsNyArMzM2LDYgQEAKICAg ICAgICAoZ2xvYmFsLW5hbWUgImNvbS5hcHBsZS5Db3JlQXV0aGVudGljYXRpb24uYWdlbnQubGli eHBjIikKICAgICAgICAoZ2xvYmFsLW5hbWUgImNvbS5hcHBsZS5TZWN1cml0eVNlcnZlciIpKQog Ci0jaWYgX19NQUNfT1NfWF9WRVJTSU9OX01JTl9SRVFVSVJFRCA8IDEwMTI0MAogOzsgRklYTUU6 IFRoaXMgc2hvdWxkIGJlIHJlbW92ZWQgd2hlbiA8cmRhcjovL3Byb2JsZW0vMTA0Nzk2ODU+IGlz IGZpeGVkLgogOzsgUmVzdHJpY3QgQXBwU2FuZGJveGVkIHByb2Nlc3NlcyBmcm9tIGNyZWF0aW5n IC9MaWJyYXJ5L0tleWNoYWlucywgYnV0IGFsbG93IGFjY2VzcyB0byB0aGUgY29udGVudHMgb2Yg L0xpYnJhcnkvS2V5Y2hhaW5zOgogKGFsbG93IGZpbGUtcmVhZC1kYXRhIGZpbGUtcmVhZC1tZXRh ZGF0YSBmaWxlLXdyaXRlLWRhdGEKQEAgLTM1MCw3ICszNDksNiBAQAogKGRlbnkgZmlsZS1yZWFk KiBmaWxlLXdyaXRlKgogICAgIChyZWdleCAoc3RyaW5nLWFwcGVuZCAiL0xpYnJhcnkvS2V5Y2hh aW5zLyIgKHV1aWQtcmVnZXgtc3RyaW5nKSAiKC98JCkiKSkKICAgICAoaG9tZS1yZWdleCAoc3Ry aW5nLWFwcGVuZCAiL0xpYnJhcnkvS2V5Y2hhaW5zLyIgKHV1aWQtcmVnZXgtc3RyaW5nKSAiKC98 JCkiKSkpCi0jZW5kaWYKIAogKGFsbG93IGZpbGUtcmVhZCogZmlsZS13cml0ZSogKHN1YnBhdGgg Ii9wcml2YXRlL3Zhci9kYi9tZHMvc3lzdGVtIikpIDs7IEZJWE1FOiBUaGlzIHNob3VsZCBiZSBy ZW1vdmVkIHdoZW4gPHJkYXI6Ly9wcm9ibGVtLzk1Mzg0MTQ+IGlzIGZpeGVkLgogCg==