Bug 163710

Summary:

Keychain Access in WebKit should be limited to a single process (macOS)

Product:

WebKit

Reporter:

Pranjal Jumde <pranjal.jumde>

Component:

WebKit Misc.

Assignee:

Pranjal Jumde <pranjal.jumde>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

aestes, bfulgham, commit-queue, dbates, ddkilzer

Priority:

Keywords:

InRadar

Version:

Safari 10

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://bugs.webkit.org/show_bug.cgi?id=170074

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none

Pranjal Jumde

Reported 2016-10-19 17:05:21 PDT

With the upcoming changes in the networking stack, we can remove Keychain access from the Networking and the WebContent process.

Attachments
Patch (2.62 KB, patch) 2016-10-19 17:20 PDT, Pranjal Jumde	no flags	Details Formatted Diff Diff
Patch (4.76 KB, patch) 2016-10-19 18:35 PDT, Pranjal Jumde	no flags	Details Formatted Diff Diff
Patch (4.84 KB, patch) 2016-10-20 11:32 PDT, Pranjal Jumde	no flags	Details Formatted Diff Diff
Patch (3.77 KB, patch) 2016-10-24 15:53 PDT, Pranjal Jumde	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Pranjal Jumde

Comment 1 2016-10-19 17:20:04 PDT

Created attachment 292132 [details] Patch

Brent Fulgham

Comment 2 2016-10-19 17:23:24 PDT

Comment on attachment 292132 [details] Patch Looks good, but we have to make sure we don't apply these changes to builds that target older operating systems.

Pranjal Jumde

Comment 3 2016-10-19 18:35:45 PDT

Created attachment 292141 [details] Patch

Sam Weinig

Comment 4 2016-10-19 23:12:33 PDT

Comment on attachment 292141 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=292141&action=review > Source/WebKit2/ChangeLog:10 > + * NetworkProcess/mac/com.apple.WebKit.NetworkProcess.sb.in: > + * WebProcess/com.apple.WebProcess.sb.in: Please add more information in this change log. > Source/WebKit2/NetworkProcess/mac/com.apple.WebKit.NetworkProcess.sb.in:141 > +(allow file-read* > + (subpath "/private/var/db/mds") > + (literal "/private/var/db/DetachedSignatures") > + (literal "/Library/Preferences/com.apple.crypto.plist") > + (literal "/Library/Preferences/com.apple.security.plist") > + (literal "/Library/Preferences/com.apple.security.common.plist") > + (literal "/Library/Preferences/com.apple.security.revocation.plist") > + (home-literal "/Library/Application Support/SyncServices/Local/ClientsWithChanges/com.apple.Keychain") > + (home-literal "/Library/Preferences/com.apple.security.plist") > + (home-literal "/Library/Preferences/com.apple.security.revocation.plist")) Is this defining some of these rules twice if __MAC_OS_X_VERSION_MIN_REQUIRED < 101200 is true?

Pranjal Jumde

Comment 5 2016-10-20 11:32:25 PDT

Created attachment 292229 [details] Patch

Brent Fulgham

Comment 6 2016-10-21 09:16:14 PDT

Comment on attachment 292229 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=292229&action=review > Source/WebKit2/NetworkProcess/mac/com.apple.WebKit.NetworkProcess.sb.in:142 > + This seems like a lot of duplication. Can't you just have the #ifdef check just by about the one line related to "/Library/Keychains" ? > Source/WebKit2/WebProcess/com.apple.WebProcess.sb.in:245 > +#endif Ditto the above comment.

Pranjal Jumde

Comment 7 2016-10-24 15:53:26 PDT

Created attachment 292672 [details] Patch

WebKit Commit Bot

Comment 8 2016-11-14 13:05:29 PST

Comment on attachment 292672 [details] Patch Clearing flags on attachment: 292672 Committed r208702: <http://trac.webkit.org/changeset/208702>

WebKit Commit Bot

Comment 9 2016-11-14 13:05:34 PST

All reviewed patches have been landed. Closing bug.

Brent Fulgham

Comment 10 2016-11-14 13:41:58 PST

Follow up: Bumped version to avoid breaking STP and nightly users. Committed r208707: <http://trac.webkit.org/changeset/208707>

Note You need to log in before you can comment on or make changes to this bug.