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

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Pranjal Jumde]

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

[Pranjal Jumde]

Created attachment 292132 [details]
Patch

[Brent Fulgham]

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]

Created attachment 292141 [details]
Patch

[Sam Weinig]

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]

Created attachment 292229 [details]
Patch

[Brent Fulgham]

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]

Created attachment 292672 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 292672 [details]
Patch

Clearing flags on attachment: 292672

Committed r208702: <http://trac.webkit.org/changeset/208702>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Brent Fulgham]

Follow up:

Bumped version to avoid breaking STP and nightly users.

Committed r208707: <http://trac.webkit.org/changeset/208707>