Closed Bug 1488439 Opened Last year Closed Last year

Comcast video won't play (plugin sandbox level 3 issue)

Categories

(Core :: Plug-ins, defect, P1)

All
Windows
defect

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
geckoview62 --- unaffected
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox61 --- unaffected
firefox62 - disabled
firefox63 --- disabled
firefox64 --- fixed

People

(Reporter: jimm, Assigned: handyman)

References

Details

(Whiteboard: sb+)

Attachments

(3 files)

The situation in nightly is bad but things are working in release.  I don't _think_ any of the nightly issues are about content but I'm not sure.  The plugin nightly sandbox is tighter than release ("restricting SIDs") so that could be responsible.  The door hanger in the awesome bar says "No blockable content detected on this page."

In release (in a new profile) I can:

* Go to https://www.xfinity.com/stream .  Click "Sign In" and go through the signin process.
* The page shows a spinner and says "Adding Device", then the main page comes up.
* Click a recording.  It switches to the video HUD.
* The video starts playing.

In today's nightly:

* Go to https://www.xfinity.com/stream .  Click "Sign In" and go through the signin process.
* The page shows a spinner and says "Adding Device", then a "Lets get your Flash player up and running" dialog requesting to allow Flash.  After clicking to allow Flash, it just sits on that same dialog screen.  Clicking reload repeats the process (starting from "Adding Device")
* From there, I closed Nightly, opened release with the same profile, and was able to move past the Flash page.
* Switching back to Nightly, I can now get into XFinity.  So I tried clicking a recording.  It switches to the video HUD.
* The video never starts playing.
See Also: → 1425828
[Tracking Requested - why for this release]:
Blocks: 1366256
Whiteboard: sb?
(In reply to Tom Kloos from comment #28)
> Changing dom.ipc.plugins.sandbox-level.flash from default 3 to 2 fixes the
> observed "problems" in the current FF63 nightly.  After this config change,
> 63 nightly performs the same as 61 and 62, including a functioning sign-on. 
> The default value in the released FF61 is 2, but in the release candidate
> for FF62 it's 3.  Is this expected given that FF62 works?
Flags: needinfo?(davidp99)
CC'ing Tom and Paul because they commented on the Xfinity video issue in bug 1425828.
I'm assuming we can flip this pref pretty easily using Normandy if we need to. Question is do we want to. It's surprising to me it took so long for someone to report this.
I'm just getting started with this but I'd mention that I don't see what Chris said in bug 1425828 comment 29 -- that this is broken in 62.0rc2 (the one from Aug 30).  It looks like Paul says the same thing in bug 1425828 comment 31 -- that 62 is unaffected.

As said in comment 0, the nightly sandbox is tighter than release... but also tighter than beta.  The restricting SIDs component of the sandbox is turned off everywhere but nightly _using #ifdefs_ so, if they are responsible, that bug would always be nightly-only until we remove them (which we plan to do soon).  But i still need to confirm the issue is with the SIDs.
Flags: needinfo?(davidp99)
Assignee: nobody → davidp99
I've confirmed that the issue is with the restricting SIDs in the nightly build.

The relevant code is here:
https://searchfox.org/mozilla-central/rev/c3fef66a5b211ea8038c1c132706d02db408093a/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp#701

Minor note: in bug 1425828 comment 31, Paul added

> The only minor problem I have is an initial stuttering which last for about 10 to 15 seconds before the content plays smoothly.

I didn't see this issue in the release candidate like he did but I did see it in nightly when I removed the restricting SIDs for this test.  (Actually, it only stuttered for 3-4 seconds in my case.)
Doesn't sound like this needs to be tracked for 62 given the comments about the RC builds not being affected.
Priority: P1 → P2
Block restrict SIDs bug 1426733 since David confirmed that is why sandbox level 3 breaks Comcast.

status-firefox63=disabled because the restricting SIDs are currently disabled in non-Nightly channels.
Priority: P2 → P1
There are two issues here: 1) the plugin process doesn't provide a useful temp folder for GetTempFileName, and 2) Flash does some analysis of the entire path to the AppData\Roaming\Adobe\Flash Player folder using GetFileAttributes -- we provide special access to that folder [1] but GetFileAttributes isn't intercepted by the sandbox and Flash is also looking at parents of that folder.  Our restricting SIDs make it return an error for a number of the subfolders in the path.

Issue 1 is easily fixed -- I'm basically using the mechanism we have in place for setting up the temp folder in content.  I'm passing the temp folder path as a command line arg to the plugin process, unlike the content process' procedure, because content's procedure uses the nsDirectoryService, which isn't available in plugin.

Issue 2 is tougher to fix in Firefox.  A clean solution would be to ask Adobe to change the process of looking at the folder parents (at least to ignore the security error) but we want to get this in ASAP so trying to align our release schedules is probably a bad idea.  Instead, I'm intercepting GetFileAttributesW with the FunctionBroker and proxying calls that refer to temp folder parents.  So this isn't really much of a weakening of the sandbox -- it doesn't provide any new file access or anything -- it just returns something meaningful for these parent folders.

I'm looking into changing my solution for 2) to use FunctionHook and just return dummy results for the parent folders instead of actually brokering them.  This would be even less of a hole in the sandbox (in fact, it would be no hole at all).  I am mildly concerned about how all this will affect running remotely/on network drives/etc.  I still need to test against that.

----

[1] https://searchfox.org/mozilla-central/rev/819cd31a93fd50b7167979607371878c4d6f18e8/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp#760
Turns out the concern about running from a network drive was legitimate.  I doubt this is anything more than a hiccup tho.
...on further investigation, that last comment wasn't true.  The streaming works from a networked drive.

Build:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d7ee032999f960d20ab62f1f958f05f9e71e023e
The sandbox blocks GetTempFileName's prior response, causing the system to end up searching a number of (inaccessible) folders to use as a replacement for the temp folder.  This patch provides a path to a new folder on the command line for the plugin process.  This new temp folder, specific to this plugin process instance, is then communicated to the system via the TEMP/TMP environment variables.  This is similar to what is done for the content process but avoids nsDirectoryService, which doesn't exist in plugin processes.
Flash analyzes the parents of the path to its appdata folder on Windows using GetFileAttributesW.  If it runs into an error, it makes some internal decisions that cause it to break DRM video.  Our new sandbox hardening causes GetFileAttributesW to return an error for some components of the path.  This patch alters the behavior of GetFileAttributesW so that it always reports FILE_ATTRIBUTE_DIRECTORY for any path that both 1) would otherwise return an error and 2) is an ancestor of the appdata folder.  This may not always be 100% accurate (for instance, if the folder is a reparse point) but restores video functionality.

Depends on D7532
Addressed patch reviews and added erahm as reviewer for part 1 as suggested (the review of the content sandbox portion was formerly bsmedberg).
QA Contact: jmathies
...added jimm as reviewer for part 2, as suggested, to be safe.
QA Contact: jmathies
Pushed by davidp99@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/6af35f6a050f
Part 1 - Replace Windows temp folder in sandboxed plugin process (r=bobowen,erahm)
Whiteboard: sb? → sb+
Pushed by davidp99@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/85575fc37555
Part 1 - Replace Windows temp folder in sandboxed plugin process (r=bobowen,erahm)
Pushed by davidp99@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/74b2087ee696
Part 2 - Patch GetFileAttributesW for appdata parent folders in sandboxed plugin process (r=bobowen,jmathies)
Backed out 2 changesets (bug 1488439) for build bustage at /build/src/dom/plugins/ipc/PluginProcessParent.cpp 

Backout: https://hg.mozilla.org/integration/autoland/rev/32af88ce76dab548742417a74be9a13043540143

Failure push:
https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=85575fc37555213a204b8565bbadef7270edd19e

https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=74b2087ee696eb7369b65727b81fc67121789f7d

Failure log: 
https://treeherder.mozilla.org/logviewer.html#?job_id=205126705&repo=autoland&lineNumber=25623

https://treeherder.mozilla.org/logviewer.html#?job_id=205125995&repo=autoland&lineNumber=20814

task 2018-10-12T17:49:05.871Z] 17:49:05     INFO -  /builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/clang/bin/clang++ -target x86_64-apple-darwin11 -B /builds/worker/workspace/build/src/cctools/bin -isysroot /builds/worker/workspace/build/src/MacOSX10.11.sdk -o SkBitmapProcState_opts_SSSE3.o -c -flto=thin -fvisibility=hidden -fvisibility-inlines-hidden -DNDEBUG=1 -DTRIMMED=1 -DSKIA_IMPLEMENTATION=1 -DSK_PDF_USE_SFNTLY=1 -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/builds/worker/workspace/build/src/gfx/skia -I/builds/worker/workspace/build/src/obj-firefox/gfx/skia -I/builds/worker/workspace/build/src/gfx/skia/skia/include/c -I/builds/worker/workspace/build/src/gfx/skia/skia/include/codec -I/builds/worker/workspace/build/src/gfx/skia/skia/include/config -I/builds/worker/workspace/build/src/gfx/skia/skia/include/core -I/builds/worker/workspace/build/src/gfx/skia/skia/include/effects -I/builds/worker/workspace/build/src/gfx/skia/skia/include/encode -I/builds/worker/workspace/build/src/gfx/skia/skia/include/gpu -I/builds/worker/workspace/build/src/gfx/skia/skia/include/pathops -I/builds/worker/workspace/build/src/gfx/skia/skia/include/ports -I/builds/worker/workspace/build/src/gfx/skia/skia/include/private -I/builds/worker/workspace/build/src/gfx/skia/skia/include/utils -I/builds/worker/workspace/build/src/gfx/skia/skia/include/utils/mac -I/builds/worker/workspace/build/src/gfx/skia/skia/include/views -I/builds/worker/workspace/build/src/gfx/skia/skia/src/core -I/builds/worker/workspace/build/src/gfx/skia/skia/src/gpu -I/builds/worker/workspace/build/src/gfx/skia/skia/src/gpu/effects -I/builds/worker/workspace/build/src/gfx/skia/skia/src/gpu/gl -I/builds/worker/workspace/build/src/gfx/skia/skia/src/gpu/glsl -I/builds/worker/workspace/build/src/gfx/skia/skia/src/image -I/builds/worker/workspace/build/src/gfx/skia/skia/src/lazy -I/builds/worker/workspace/build/src/gfx/skia/skia/src/opts -I/builds/worker/workspace/build/src/gfx/skia/skia/src/sfnt -I/builds/worker/workspace/build/src/gfx/skia/skia/src/shaders -I/builds/worker/workspace/build/src/gfx/skia/skia/src/sksl -I/builds/worker/workspace/build/src/gfx/skia/skia/src/utils -I/builds/worker/workspace/build/src/gfx/skia/skia/src/utils/mac -I/builds/worker/workspace/build/src/gfx/skia/skia/src/utils/win -I/builds/worker/workspace/build/src/gfx/sfntly/cpp/src -I/builds/worker/workspace/build/src/obj-firefox/dist/include -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nspr -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/build/src/obj-firefox/mozilla-config.h -Qunused-arguments -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Qunused-arguments -Wall -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wc++1z-compat -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-unknown-warning-option -Wno-return-type-c-linkage -fno-sized-deallocation -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fno-exceptions -fno-strict-aliasing -stdlib=libc++ -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -Xclang -load -Xclang /builds/worker/workspace/build/src/obj-firefox/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -O3 -fno-omit-frame-pointer -funwind-tables -Wno-deprecated-declarations -Wno-overloaded-virtual -Wno-shadow -Wno-sign-compare -Wno-unreachable-code -Wno-unused-function -Wno-implicit-fallthrough -Wno-inconsistent-missing-override -Wno-macro-redefined -Wno-unused-private-field  -MD -MP -MF .deps/SkBitmapProcState_opts_SSSE3.o.pp  -O3 -mssse3 /builds/worker/workspace/build/src/gfx/skia/skia/src/opts/SkBitmapProcState_opts_SSSE3.cpp
[task 2018-10-12T17:49:05.872Z] 17:49:05     INFO -  make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/gfx/skia'
[task 2018-10-12T17:49:05.872Z] 17:49:05     INFO -  make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/gfx/skia'
[task 2018-10-12T17:49:05.873Z] 17:49:05     INFO -  make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/gfx/skia'
[task 2018-10-12T17:49:05.873Z] 17:49:05     INFO -  make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/gfx/skia'
[task 2018-10-12T17:49:05.874Z] 17:49:05     INFO -  make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/gfx/skia'
[task 2018-10-12T17:49:05.874Z] 17:49:05     INFO -  make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/gfx/skia'
[task 2018-10-12T17:49:05.874Z] 17:49:05     INFO -  make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/gfx/skia'
[task 2018-10-12T17:49:05.875Z] 17:49:05     INFO -  make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/gfx/skia'
[task 2018-10-12T17:49:05.875Z] 17:49:05     INFO -  make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/gfx/skia'
[task 2018-10-12T17:49:06.138Z] 17:49:06     INFO -  make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/dom/plugins/ipc'
[task 2018-10-12T17:49:06.138Z] 17:49:06     INFO -  /builds/worker/workspace/build/src/sccache2/sccache /builds/worker/workspace/build/src/clang/bin/clang++ -target x86_64-apple-darwin11 -B /builds/worker/workspace/build/src/cctools/bin -isysroot /builds/worker/workspace/build/src/MacOSX10.11.sdk -o Unified_cpp_dom_plugins_ipc0.o -c -flto=thin -fvisibility=hidden -fvisibility-inlines-hidden -DNDEBUG=1 -DTRIMMED=1 -DOS_POSIX=1 -DOS_MACOSX=1 -DFORCE_PR_LOG -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/builds/worker/workspace/build/src/dom/plugins/ipc -I/builds/worker/workspace/build/src/obj-firefox/dom/plugins/ipc -I/builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/_ipdlheaders -I/builds/worker/workspace/build/src/ipc/chromium/src -I/builds/worker/workspace/build/src/ipc/glue -I/builds/worker/workspace/build/src/dom/plugins/base -I/builds/worker/workspace/build/src/xpcom/base -I/builds/worker/workspace/build/src/xpcom/threads -I/builds/worker/workspace/build/src/obj-firefox/dist/include -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nspr -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/build/src/obj-firefox/mozilla-config.h -Qunused-arguments -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Qunused-arguments -Wall -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wc++1z-compat -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-unknown-warning-option -Wno-return-type-c-linkage -fno-sized-deallocation -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fno-exceptions -fno-strict-aliasing -stdlib=libc++ -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -Xclang -load -Xclang /builds/worker/workspace/build/src/obj-firefox/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -O3 -fno-omit-frame-pointer -funwind-tables -Werror -I/builds/worker/workspace/build/src/obj-firefox/dist/include/cairo -Wno-error=shadow  -MD -MP -MF .deps/Unified_cpp_dom_plugins_ipc0.o.pp   /builds/worker/workspace/build/src/obj-firefox/dom/plugins/ipc/Unified_cpp_dom_plugins_ipc0.cpp
[task 2018-10-12T17:49:06.138Z] 17:49:06     INFO -  In file included from /builds/worker/workspace/build/src/obj-firefox/dom/plugins/ipc/Unified_cpp_dom_plugins_ipc0.cpp:128:
[task 2018-10-12T17:49:06.138Z] 17:49:06     INFO -  /builds/worker/workspace/build/src/dom/plugins/ipc/PluginProcessParent.cpp:96:2: error: invalid preprocessing directive
[task 2018-10-12T17:49:06.138Z] 17:49:06     INFO -  #elseif defined(XP_WIN) && defined(MOZ_SANDBOX)
[task 2018-10-12T17:49:06.139Z] 17:49:06     INFO -   ^
[task 2018-10-12T17:49:06.139Z] 17:49:06     INFO -  1 error generated.
[task 2018-10-12T17:49:06.139Z] 17:49:06     INFO -  /builds/worker/workspace/build/src/config/rules.mk:1118: recipe for target 'Unified_cpp_dom_plugins_ipc0.o' failed
[task 2018-10-12T17:49:06.139Z] 17:49:06     INFO -  make[4]: *** [Unified_cpp_dom_plugins_ipc0.o] Error 1
[task 2018-10-12T17:49:06.140Z] 17:49:06     INFO -  make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/dom/plugins/ipc'
[task 2018-10-12T17:49:06.140Z] 17:49:06     INFO -  make[4]: *** Waiting for unfinished jobs....
[task 2018-10-12T17:49:06.140Z] 17:49:06     INFO -  make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/gfx/skia'
Mingw failures: https://treeherder.mozilla.org/logviewer.html#?job_id=205126705&repo=autoland&lineNumber=25623

task 2018-10-12T17:53:25.384Z] 17:53:25     INFO -  In file included from /builds/worker/workspace/build/src/obj-firefox/dist/include/nsWindowsDllInterceptor.h:30:
[task 2018-10-12T17:53:25.385Z] 17:53:25     INFO -  /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/interceptor/VMSharingPolicies.h:156:12: warning: moving a local object in a return statement prevents copy elision [-Wpessimizing-move]
[task 2018-10-12T17:53:25.385Z] 17:53:25     INFO -      return std::move(items);
[task 2018-10-12T17:53:25.385Z] 17:53:25     INFO -             ^
[task 2018-10-12T17:53:25.386Z] 17:53:25     INFO -  /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/interceptor/VMSharingPolicies.h:156:12: note: remove std::move call here
[task 2018-10-12T17:53:25.386Z] 17:53:25     INFO -      return std::move(items);
[task 2018-10-12T17:53:25.386Z] 17:53:25     INFO -             ^~~~~~~~~~     ~
[task 2018-10-12T17:53:25.387Z] 17:53:25     INFO -  In file included from /builds/worker/workspace/build/src/obj-firefox/dom/plugins/ipc/Unified_cpp_dom_plugins_ipc0.cpp:38:
[task 2018-10-12T17:53:25.387Z] 17:53:25     INFO -  /builds/worker/workspace/build/src/dom/plugins/ipc/FunctionBroker.cpp:832:19: warning: comparison of integers of different signs: 'DWORD' (aka 'unsigned long') and 'long' [-Wsign-compare]
[task 2018-10-12T17:53:25.387Z] 17:53:25     INFO -      if (ncHeadLen == -1L) {
[task 2018-10-12T17:53:25.388Z] 17:53:25     INFO -          ~~~~~~~~~ ^  ~~~
[task 2018-10-12T17:53:25.388Z] 17:53:25     INFO -  /builds/worker/workspace/build/src/dom/plugins/ipc/FunctionBroker.cpp:1068:49: warning: ISO C++11 does not allow conversion from string literal to 'const LPSTR' (aka 'char *const') [-Wwritable-strings]
[task 2018-10-12T17:53:25.388Z] 17:53:25     INFO -  const LPSTR ACHAReqInfo::FixedValue<1>::value = UNISP_NAME_A;
[task 2018-10-12T17:53:25.388Z] 17:53:25     INFO -                                                  ^
[task 2018-10-12T17:53:25.389Z] 17:53:25     INFO -  /builds/worker/workspace/build/src/clang/bin/../i686-w64-mingw32/include/schannel.h:12:22: note: expanded from macro 'UNISP_NAME_A'
[task 2018-10-12T17:53:25.389Z] 17:53:25     INFO -  #define UNISP_NAME_A "Microsoft Unified Security Protocol Provider"
[task 2018-10-12T17:53:25.389Z] 17:53:25     INFO -                       ^
[task 2018-10-12T17:53:25.389Z] 17:53:25     INFO -  In file included from /builds/worker/workspace/build/src/obj-firefox/dom/plugins/ipc/Unified_cpp_dom_plugins_ipc0.cpp:74:
[task 2018-10-12T17:53:25.390Z] 17:53:25     INFO -  /builds/worker/workspace/build/src/dom/plugins/ipc/FunctionHook.cpp:334:49: error: no member named 'GetFlashRoamingPath' in 'mozilla::plugins::PluginModuleChild'
[task 2018-10-12T17:53:25.390Z] 17:53:25     INFO -    std::wstring roamingPath = PluginModuleChild::GetFlashRoamingPath();
[task 2018-10-12T17:53:25.390Z] 17:53:25     INFO -                               ~~~~~~~~~~~~~~~~~~~^
[task 2018-10-12T17:53:25.390Z] 17:53:25     INFO -  4 warnings and 1 error generated.
[task 2018-10-12T17:53:25.391Z] 17:53:25     INFO -  /builds/worker/workspace/build/src/config/rules.mk:1118: recipe for target 'Unified_cpp_dom_plugins_ipc0.o' failed
[task 2018-10-12T17:53:25.391Z] 17:53:25     INFO -  make[4]: *** [Unified_cpp_dom_plugins_ipc0.o] Error 1
[task 2018-10-12T17:53:25.391Z] 17:53:25     INFO -  make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/dom/plugins/ipc'
[task 2018-10-12T17:53:25.391Z] 17:53:25     INFO -  /builds/worker/workspace/build/src/config/recurse.mk:74: recipe for target 'dom/plugins/ipc/target' failed
[task 2018-10-12T17:53:25.392Z] 17:53:25     INFO -  make[3]: *** [dom/plugins/ipc/target] Error 2
[task 2018-10-12T17:53:25.392Z] 17:53:25     INFO -  make[3]: *** Waiting for unfinished jobs....
Pushed by davidp99@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/a5d9e6a7242d
Part 1 - Replace Windows temp folder in sandboxed plugin process (r=bobowen,erahm)
Pushed by davidp99@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/e63e2e4c0484
Part 2 - Patch GetFileAttributesW for appdata parent folders in sandboxed plugin process (r=bobowen,jmathies)
https://hg.mozilla.org/mozilla-central/rev/a5d9e6a7242d
https://hg.mozilla.org/mozilla-central/rev/e63e2e4c0484
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Comment on attachment 9013804 [details]
Bug 1488439: Part 1 - Replace Windows temp folder in sandboxed plugin process (r?bobowen)

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: N/A

User impact if declined: This failure, which breaks Comcast streaming video, is due to the restricting SIDs work that is currently active only in trunk.  These patches (and the subsequent touch-up in bug 1498831) fix the issue.  This is being uplifted so that we can (later this week) uplift that work from bug 1426733.  We want to uplift since there is a low likelihood of stressing these patches in nightly.

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: Bug 1498831

Risk to taking this patch: Medium

Why is the change risky/not risky? (and alternatives if risky): These patches do separate things.  The first replaces the Flash temp folder in plugin processes.  This should not be controversial.  The second hijacks a Win32 API to replace functionality that is blocked by the sandbox.  This could potentially fail to work on unexpected configurations (like enterprise setups).  I would not anticipate anything worse than plugin process stalls in that case.

String changes made/needed: N/A
Attachment #9013804 - Flags: approval-mozilla-beta?
Comment on attachment 9013805 [details]
Bug 1488439: Part 2 - Patch GetFileAttributesW for appdata parent folders in sandboxed plugin process (r?bobowen)

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: N/A

User impact if declined: see previous comment

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: Bug 1498831

Risk to taking this patch: Medium

Why is the change risky/not risky? (and alternatives if risky): See previous comment

String changes made/needed: N/A
Attachment #9013805 - Flags: approval-mozilla-beta?
Comment on attachment 9013804 [details]
Bug 1488439: Part 1 - Replace Windows temp folder in sandboxed plugin process (r?bobowen)

This is already on Beta.
Attachment #9013804 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
Attachment #9013805 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
I'd add that the rest SIDs stuff (bug 1426733) is also in 64.  So there is nothing to uplift.  Good timing.
On my HP laptop (Core 2 Duo P8600 2.4Ghz with NVIDIA 9600M video card 8GB memory) I am getting a choppy video display when I play Xfinity live material.  The sound is OK but the video appears to skip several frames.  Firefox 64 b5 64bit. When using Chrome the video plays smoothly.  On my desktop (AMD FX8350 and AMD FX580 video card 24GB memory) Firefox Xfinity video plays smoothly.  So it appears Firefox has a problem playing Xfinity video on low end computers while Chrome can do it.
(In reply to Paul Goldberg from comment #30)
> On my HP laptop (Core 2 Duo P8600 2.4Ghz with NVIDIA 9600M video card 8GB
> memory) I am getting a choppy video display when I play Xfinity live
> material.  The sound is OK but the video appears to skip several frames. 
> Firefox 64 b5 64bit. When using Chrome the video plays smoothly.  On my
> desktop (AMD FX8350 and AMD FX580 video card 24GB memory) Firefox Xfinity
> video plays smoothly.  So it appears Firefox has a problem playing Xfinity
> video on low end computers while Chrome can do it.

Hi Paul, I opened a new bug report (bug 1503497) to track your issue because it has different symptoms than this bug.

Is this a new regression in Firefox 64? Did Xfinity live video playback acceptably in Firefox 63?
See Also: → 1503497
I'm not sure why this bug is marked fixed.  I still can't play Comcast video with either a nightly 65 or 64b5 unless I set dom.ipc.plugins.sandbox-level.flash to 2 (from the default 3) (tested with a clean profile). When I set that pref I can finally login and play video (plays fine on a Dell I5-3230M stuck at 1.2 GHz due to wrong power adapter).  It's the same problem as bug 1425828 which is still open.
(In reply to Tom Kloos from comment #32)
> I'm not sure why this bug is marked fixed.  I still can't play Comcast video
> with either a nightly 65 or 64b5 unless I set
> dom.ipc.plugins.sandbox-level.flash to 2 (from the default 3) (tested with a
> clean profile). When I set that pref I can finally login and play video
> (plays fine on a Dell I5-3230M stuck at 1.2 GHz due to wrong power adapter).
> It's the same problem as bug 1425828 which is still open.

David, this bug is marked as fixed in 64. Are there any other fixes needed to make Comcast video work with dom.ipc.plugins.sandbox-level.flash = 3?
Flags: needinfo?(davidp99)
(In reply to Tom Kloos from comment #32)
> I'm not sure why this bug is marked fixed.  I still can't play Comcast video
> with either a nightly 65 or 64b5 unless I set
> dom.ipc.plugins.sandbox-level.flash to 2 (from the default 3) (tested with a
> clean profile). When I set that pref I can finally login and play video
> (plays fine on a Dell I5-3230M stuck at 1.2 GHz due to wrong power adapter).
> It's the same problem as bug 1425828 which is still open.

Bug 1425828 is different, it appears to be related to a dom security feature tied to:

security.mixed_content.block_object_subrequest

which is off by default. Do you have this set on the affected device? If so, lets move this discussion over there.
Flags: needinfo?(davidp99)
Flags: needinfo?(tkloos)
Hi Jim,  security.mixed_content.block_object_subrequest is "false" according to about:config and not present in prefs.js.
The OS is Windows 7 Ultimate or Windows 7 Home Premium.  I wonder if this is a problem that doesn't show up on Windows 10?  I recently encountered an issue with Thunderbird related to text display with Windows 7 that wasn't seen in Windows 10 which the developers all seem to be using.
Flags: needinfo?(tkloos)
Hey Tom, can you tell me exactly what happens when you try to use Comcast with the sandbox on?  Also, is there anything strange about your setup that might make this run different than usual?  Like running from the network or a custom Flash install location or something?
Flags: needinfo?(tkloos)
Hi David.  The problem shows up in two stages of accessing the Comcast/Xfinity schedule then video.

First, starting with a clean profile and hence having to login to Comcast, after supplying login info and allowing Flash, FF stalls on "https://www.xfinity.com/stream/adding-device" eventually showing "Let’s get your Adobe Flash Player up and running."
The only way past that is to change dom.ipc.plugins.sandbox-level.flash = 2.  Then set it back to 3 after which the Xfinity schedule can be accessed.

Second, as long as dom.ipc.plugins.sandbox-level.flash = 3, trying to play any video simply ends up with the Xfinity throbber sitting on the screen forever (see attachment).  Clicking on the screen's "X" generates the error:
  TypeError: this.player.getCurrentStatus is not a function[Learn More] PlayerPlatformAPI.js:2337:4025
  TypeError: The expression cannot be converted to return the specified type.
... suggesting that some Flash stuff is blocked?

There's nothing special, at least then I use a clean default profile, and all are run on the local drive.  All three 64 bit Win 7 platforms with 31.0.0.122 flash and with Home Premium, Ultimate, and Pro OS versions -- patched to current levels -- exhibit the same problem.  The only difference is that my test versions come from the current .ZIP instead of the installer.  So, I just flushed Firefox totally from a system and installed 64b6 from the installer.   The same problem still exists.
Flags: needinfo?(tkloos)
Blocks: 1505482
This is an issue with the way Windows 7 implements GetFileAttributesW -- the DLL interceptor failed to register, making the fix for this fail.

I've created bug 1505482 for this.  I have a rough implementation that fixes the issue.
Confirmed now fixed in firefox-65.0b8_20190103150357 win64 under Windows 7.
You need to log in before you can comment on or make changes to this bug.