Enable sandboxing for the database process on ios
Created attachment 231530 [details] Patch
Comment on attachment 231530 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=231530&action=review > Source/WebKit2/DatabaseProcess/ios/DatabaseProcessIOS.mm:34 > #import <WebCore/LocalizedStrings.h> > +#import <WebCore/FileSystem.h> sorting > Source/WebKit2/DatabaseProcess/ios/DatabaseProcessIOS.mm:54 > + NSBundle *webkit2Bundle = [NSBundle bundleForClass:NSClassFromString(@"WKView")]; WKView is not long for this world, doesn’t seem like a great choice. > Source/WebKit2/DatabaseProcess/ios/com.apple.WebKit.DatabasesIOS.sb.in:28 > +(allow file-read* file-write* (extension âcom.apple.YourProject.database_container_dirâ)) smart quotes? YourProject?
Comment on attachment 231530 [details] Patch Hmmm, messed up merge/refactoring to tidy.
Comment on attachment 231530 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=231530&action=review r- because of hardcoded directory path, and because it doesn't look like this code is suitable in production builds. I expected that you will need to modify WebKit.xcconfig to omit the profile from other platforms (and to fix my mistake with "com.apple.WebKit.DatabaseProcess.sb"). > Source/WebKit2/DatabaseProcess/ios/DatabaseProcessIOS.mm:58 > + NSArray *paths = NSSearchPathForDirectoriesInDomains(NSLibraryDirectory, NSUserDomainMask, YES); > + NSString *libraryDirectory = [paths objectAtIndex:0]; > + String databasePath = [libraryDirectory stringByAppendingPathComponent:@"WebKit/Databases"]; The database directory is not hardcoded to be this one - there is even a user default. There is already code in WebContext::ensureDatabaseProcess() that creates an extension for this directory, which simply needs to be consumed when initialization message is received. The only change may be that we need to change extension class for iOS. > Source/WebKit2/DatabaseProcess/ios/com.apple.WebKit.DatabasesIOS.sb.in:26 > +(deny default (with partial-symbolication)) > +(allow system-audit file-read-metadata) You didn't include common.sb or removed-dev-nodes.sb, how did you determine that they are not needed? > Source/WebKit2/Shared/ios/ChildProcessIOS.mm:35 > +#import <WebCore/SystemVersionMac.h> Is this used in the iOS implementation? > Source/WebKit2/Shared/ios/ChildProcessIOS.mm:55 > +extern "C" OSStatus SetApplicationIsDaemon(Boolean isDaemon); This is not used on iOS (likely doesn't exist either). > Source/WebKit2/Shared/ios/ChildProcessIOS.mm:70 > +void ChildProcess::initializeSandbox(const ChildProcessInitializationParameters& parameters, SandboxInitializationParameters& sandboxParameters) I don't see how code in this function is necessary, given that launchd will initialize sandbox for us. This is only for local testing, isn't it? > Source/WebKit2/Shared/ios/ChildProcessIOS.mm:72 > + NSBundle *webkit2Bundle = [NSBundle bundleForClass:NSClassFromString(@"WKView")]; What Tim said.
Created attachment 231727 [details] Patch
Created attachment 231790 [details] Patch
Comment on attachment 231790 [details] Patch Patch didn’t include the new file, ChildProcessIOS.mm
Created attachment 231805 [details] Patch
Comment on attachment 231805 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=231805&action=review > Source/WebKit2/DatabaseProcess/com.apple.WebKit.Databases.sb.in:25 > +#include "ios/com.apple.WebKit.DatabasesIOS.sb.in" I think that the database process profile needs to go directly to where we want it to be on iOS, which is a different place than on OS X. So, merging them like this will be problematic. > Source/WebKit2/DatabaseProcess/ios/DatabaseProcessIOS.mm:54 > + // Need to overide the default, because service has a different bundle ID. > + NSBundle *webkit2Bundle = [NSBundle bundleForClass:NSClassFromString(@"WKView")]; > + sandboxParameters.setOverrideSandboxProfilePath([webkit2Bundle pathForResource:@"com.apple.WebKit.Databases" ofType:@"sb"]); I still don't understand why this is needed, I thought we agreed that we didn't need any code to enter the sandbox.
DerivedSources.make has a rule to process the three profiles for Mac from .sb.in. We can put iOS versions in a directory(directories?) that is not in VPATH, if there is a name conflict - there is no need to preprocess them, as they won't need any ifdefs. Then we need to install the profiles into /usr/local/share/sandbox/embedded/profiles/builtin (will need to discuss whether we need a build alias as suggested). I don't know how that's done, but Xcode must have a way. For local testing, the profiles can be put in another special place, and edited even without rebuilding or installing roots.
Created attachment 231835 [details] Patch
Comment on attachment 231835 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=231835&action=review > Source/WebKit2/Shared/ios/ChildProcessIOS.mm:43 > +#define ENABLE_MANUAL_SANDBOXING 0 Can any of the above includes be under this ifdef?
Comment on attachment 231835 [details] Patch Clearing flags on attachment: 231835 Committed r169176: <http://trac.webkit.org/changeset/169176>
All reviewed patches have been landed. Closing bug.