Bug 135121 - Provide networking process with access to its HSTS db
Summary: Provide networking process with access to its HSTS db
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Oliver Hunt
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-07-21 11:11 PDT by Oliver Hunt
Modified: 2014-07-22 12:59 PDT (History)
1 user (show)

See Also:


Attachments
Patch (7.74 KB, patch)
2014-07-21 11:14 PDT, Oliver Hunt
no flags Details | Formatted Diff | Diff
Patch (7.78 KB, patch)
2014-07-22 11:21 PDT, Oliver Hunt
no flags Details | Formatted Diff | Diff
Patch (7.57 KB, patch)
2014-07-22 11:42 PDT, Oliver Hunt
ap: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Hunt 2014-07-21 11:11:01 PDT
Provide networking process with access to its parent app relative cache directory
Comment 1 Oliver Hunt 2014-07-21 11:14:05 PDT
Created attachment 235230 [details]
Patch
Comment 2 Alexey Proskuryakov 2014-07-21 12:10:14 PDT
Comment on attachment 235230 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=235230&action=review

> Source/WebKit2/ChangeLog:14
> +        Long term we will probably want to restrict this somewhat, but we obviously
> +        can't control the exact files the CFNetwork may wish to use and create so
> +        I'm not sure how feasible this would be.

Having discussed this, hopefully we only need to allow HSTS.plist in this location.
Comment 3 Oliver Hunt 2014-07-22 11:00:46 PDT
Comment on attachment 235230 [details]
Patch

Where trying a much more restrictive approach
Comment 4 Oliver Hunt 2014-07-22 11:21:30 PDT
Created attachment 235300 [details]
Patch
Comment 5 Alexey Proskuryakov 2014-07-22 11:27:42 PDT
Comment on attachment 235300 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=235300&action=review

r=me conditional on adding a FIXME with radar number to make this unnecessary. Please don't land without one.

> Source/WebKit2/ChangeLog:10
> +        directory in the network process, as the network sandbox

s/network process/UI process/

> Source/WebKit2/NetworkProcess/cocoa/NetworkProcessCocoa.mm:64
> +    SandboxExtension::consumePermanently(parameters.hstsDatabasePathExtensionHandle);

Do we need to do this on OS X? I don't think that we do, so it's confusing to have this code run on both platforms. Confusion in security sensitive code is worse than #ifs.

> Source/WebKit2/Shared/Network/NetworkProcessCreationParameters.h:63
> +    SandboxExtension::Handle hstsDatabasePathExtensionHandle;

Can we have a FIXME here with a bug tracking making this unnecessary please?

> Source/WebKit2/UIProcess/WebContext.cpp:1218
> +    if (!m_overrideNetworkingHSTSDatabasePath.isEmpty())
> +        return m_overrideNetworkingHSTSDatabasePath;

There is no code anywhere to set m_overrideNetworkingHSTSDatabasePath. Please remove it.
Comment 6 Oliver Hunt 2014-07-22 11:42:34 PDT
Created attachment 235301 [details]
Patch
Comment 7 Alexey Proskuryakov 2014-07-22 12:02:06 PDT
Comment on attachment 235301 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=235301&action=review

Looks good to me, but still breaking builds.

> Source/WebKit2/Shared/Network/NetworkProcessCreationParameters.h:63
> +    // Remove this once <rdar://problem/17726660> is fixed.

"FIXME: "
Comment 8 Oliver Hunt 2014-07-22 12:59:28 PDT
Committed r171356: <http://trac.webkit.org/changeset/171356>