Enable web process sandbox for WPE. This is expected to break unexpected things, so the start of a new release cycle is the perfect time for it. It's on by default, but applications can opt-out. Originally we had expected not to provide an opt-out, but now you can effectively opt-out of the bind mounting by whitelisting / so there's no real benefit. Can change any of this if desired.
Created attachment 363228 [details] Patch
Created attachment 363229 [details] Patch
Created attachment 363230 [details] Patch
> but now you can effectively opt-out of the bind mounting by whitelisting / so there's no real benefit. I'm not sure that would actually work. Since user provided mounts are added after the webkit ones I think it will just error with conflicts? (untested)
If WPE maintainers want me to remove the opt-out, I can do so. Otherwise I'll leave it in.
BTW the WPE bots need new deps: bubblewrap and libseccomp
Comment on attachment 363230 [details] Patch Attachment 363230 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/11319346 New failing tests: imported/w3c/web-platform-tests/webrtc/simplecall.https.html
Created attachment 363242 [details] Archive of layout-test-results from ews126 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6
Comment on attachment 363230 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=363230&action=review > Source/cmake/OptionsWPE.cmake:93 > +if (CMAKE_SYSTEM_NAME MATCHES "Linux" AND NOT EXISTS "/.flatpak-info") > + WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_BUBBLEWRAP_SANDBOX PRIVATE ON) > +else () > + WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_BUBBLEWRAP_SANDBOX PRIVATE OFF) > +endif () Is the option private by default with a purpose? IMO it should be public, allowing deployers to disable it in case they are providing their own sandboxing solution. Also, what's the story with .flatpak-info? Is this only meant for flatpak builds?
(In reply to Zan Dobersek from comment #9) > Is the option private by default with a purpose? IMO it should be public, > allowing deployers to disable it in case they are providing their own > sandboxing solution. I picked private to make it mandatory, but I'll change it to public. > Also, what's the story with .flatpak-info? Is this only meant for flatpak > builds? It's to disable the setting for flatpak builds (since it will be broken: bubblewrap sandboxes do not nest).
Created attachment 363324 [details] Patch
Comment on attachment 363324 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=363324&action=review I don’t know a lot about GTK or WPE, but felt this patch was sane. I do know about bubble wrap (the packaging stuff not the bwrap executable). > Source/WebKit/PlatformWPE.cmake:19 > +add_definitions(-DLIBDIR="${LIB_INSTALL_DIR}") A “why”? for the ignorant in the change log would be nice :) > Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp:671 > +#if ENABLE(NETSCAPE_PLUGIN_API) There’s something about this launcher’s name I really like... could it be BUBBLEWRAP! :D Yes, that’s what it is. > Source/cmake/BubblewrapSandboxChecks.cmake:4 > + message(FATAL_ERROR "bwrap executable is needed for ENABLE_BUBBLEWRAP_SANDBOX") Bubblewrap sandbox <— I like it
Comment on attachment 363324 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=363324&action=review >> Source/WebKit/PlatformWPE.cmake:19 >> +add_definitions(-DLIBDIR="${LIB_INSTALL_DIR}") > > A “why”? for the ignorant in the change log would be nice :) It's used in BubblewrapLauncher.cpp. >> Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp:671 >> +#if ENABLE(NETSCAPE_PLUGIN_API) > > There’s something about this launcher’s name I really like... could it be BUBBLEWRAP! :D Yes, that’s what it is. Imagine a bubblewrap cannon. Hire an artist.
Comment on attachment 363324 [details] Patch Clearing flags on attachment: 363324 Committed r242354: <https://trac.webkit.org/changeset/242354>
All reviewed patches have been landed. Closing bug.
Re-opened since this is blocked by bug 195402
Hm, I could either land it disabled by default, or leave it for someone else to try to get layout tests working (probably by adding C API to whitelist directories for the TestRunner).
(In reply to Michael Catanzaro from comment #17) > Hm, I could either land it disabled by default Let's do this.
Committed r242709: <https://trac.webkit.org/changeset/242709>