[Windows][Host] Cannot connect to Windows 10 host in curtain mode. |
|||||||||||
Issue descriptionThis is a recent regression in M56 and needs to be fixed. I've seen two occurrences of this and was able to get a repro. It looks like the remoting_desktop process is not launched correctly (it exists with error code 0xC0000142 (STATUS_DLL_INIT_FAILED)).
,
Nov 22 2016
This does look to be related to my previous change (https://codereview.chromium.org/2370293002/). I added the uiAccess flag as the old GYP builds specified it and it was missing in GN. We are using a manifest fragment instead of command line argument so perhaps something is different there. I would like to revert the manifest change and then test whether it is actually required.
,
Nov 29 2016
After a bit more investigation, this problem was actually introduced by this change: Use ChannelMojo between the remoting daemon and desktop processes. https://codereview.chromium.org/2446053002 If I sync to the CL just before it, the UiAccess enabled desktop binary works fine. When I roll forward to include this change, I can no longer connect. I am investigating now to see what the root cause is.
,
Nov 29 2016
,
Nov 30 2016
I am going to check in my change to unblock M56 (need to verify whether removing UiAccess causes any problems). I will continue investigating the problems from the Mojo conversion to see if we can fix them in M56 or if they will need to resolved in M57.
,
Nov 30 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fdd9968f262f1fba0d9b2f3e0261226732e2ee67 commit fdd9968f262f1fba0d9b2f3e0261226732e2ee67 Author: joedow <joedow@chromium.org> Date: Wed Nov 30 15:59:24 2016 Fixing a problem where curtain mode connections were failing on Windows 8+ I tracked this down to a change I made where I added the uiAccess flag to the remoting_desktop binary (https://codereview.chromium.org/2370293002/). Adding this manifest flag causes CreateProcess to fail with STATUS_ELEVATION_REQUIRED when run in curtain mode. I suspect it has something to do with the process being started in the logon session. I am reverting this flag to unblock this scenario (M56 needs this fix) and will verify whether the flag is actually needed (it was originally added for parity with GYP builds but may not be necessary). BUG= 666992 Review-Url: https://codereview.chromium.org/2521453005 Cr-Commit-Position: refs/heads/master@{#435272} [modify] https://crrev.com/fdd9968f262f1fba0d9b2f3e0261226732e2ee67/remoting/host/win/BUILD.gn
,
Nov 30 2016
,
Dec 1 2016
Your change meets the bar and is auto-approved for M56 (branch: 2924)
,
Dec 2 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ffa8fec145e25ac6b34ee56c65d6b11eb9369c76 commit ffa8fec145e25ac6b34ee56c65d6b11eb9369c76 Author: Joe Downing <joedow@google.com> Date: Fri Dec 02 02:47:40 2016 Fixing a problem where curtain mode connections were failing on Windows 8+ I tracked this down to a change I made where I added the uiAccess flag to the remoting_desktop binary (https://codereview.chromium.org/2370293002/). Adding this manifest flag causes CreateProcess to fail with STATUS_ELEVATION_REQUIRED when run in curtain mode. I suspect it has something to do with the process being started in the logon session. I am reverting this flag to unblock this scenario (M56 needs this fix) and will verify whether the flag is actually needed (it was originally added for parity with GYP builds but may not be necessary). BUG= 666992 Review-Url: https://codereview.chromium.org/2521453005 Cr-Commit-Position: refs/heads/master@{#435272} (cherry picked from commit fdd9968f262f1fba0d9b2f3e0261226732e2ee67) Review URL: https://codereview.chromium.org/2543133002 . Cr-Commit-Position: refs/branch-heads/2924@{#281} Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059} [modify] https://crrev.com/ffa8fec145e25ac6b34ee56c65d6b11eb9369c76/remoting/host/win/BUILD.gn
,
Dec 7 2016
Here is the CL which originally added the UiAccess flag: https://chromiumcodereview.appspot.com/10836224 The intention was to enable usage of SendSAS(). I will need to do some testing to see if this is still necessary as based on the MSDN docs, the manifest flag is not needed if the process is running as LocalSystem.
,
Dec 13 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1cdb04810de9689ba0ea5ab7e70bd35ce5477063 commit 1cdb04810de9689ba0ea5ab7e70bd35ce5477063 Author: joedow <joedow@chromium.org> Date: Tue Dec 13 06:11:15 2016 Fixing CRD curtain mode connection failures on Win8+ official builds I've root caused this and the problem was introduced in this CL: https://codereview.chromium.org/2446053002 That CL moved the logic which reported the worker process launch from the IPC Channel Connected method to an IO completion port listener. This works fine on un-official builds, however it breaks on official builds. The reason for the failure is that the remoting_desktop process on official builds are signed and include 'UiAccess' in their manifest. When the launcher process uses ShellExecuteEx to launch the worker process in this scenario, we actually see two processes get launched sequentially. The first starts and exists with error code 'STATUS_ELEVATION_REQUIRED' and the second launches with the correct permissions. This behavior worked fine before as we listened for the connection to the IPC channel which was done by the second, successful process launch. With the new code, we observed the first process launch, set up the Mojo channel for it, and tried waiting on its process handle which exits immediately. The second process then starts and fails to connect to the Mojo channel. I investigated whether UiAccess is truly required for the desktop binary and I think that it is. For the Ctrl+Alt+Del scenario, there are registry keys that can be set which will require that flag. For Alt+Tab, it is possible that some windows might not be accessible if they have a high high enough integrity level (+ UiAccess themselves). So instead of removing the UiAccess flag, my approach is to listen for the worker process creation and exit events. I store the value of the last seen worker process id and use that in our process launch detection code once the launcher process exits. This allows both un-official builds (which do not require the extra permissions hop) and official builds will work consistently. BUG= 666992 Review-Url: https://codereview.chromium.org/2568983004 Cr-Commit-Position: refs/heads/master@{#438077} [modify] https://crrev.com/1cdb04810de9689ba0ea5ab7e70bd35ce5477063/remoting/host/desktop_session_win.cc [modify] https://crrev.com/1cdb04810de9689ba0ea5ab7e70bd35ce5477063/remoting/host/win/BUILD.gn [modify] https://crrev.com/1cdb04810de9689ba0ea5ab7e70bd35ce5477063/remoting/host/win/wts_session_process_delegate.cc
,
Dec 13 2016
After some additional investigation, I have an additional fix I would like to apply to M56 for this problem. The first fix unblocked our tester to look for additional problems, however this second fix is the right change to release. Removing previous labels and requesting merge for M56.
,
Dec 14 2016
Your change meets the bar and is auto-approved for M56 (branch: 2924)
,
Dec 14 2016
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 14 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/286e4172c0f1d07fa50d101ff46d20ad42aefb3e commit 286e4172c0f1d07fa50d101ff46d20ad42aefb3e Author: Joe Downing <joedow@google.com> Date: Wed Dec 14 16:58:33 2016 Fixing CRD curtain mode connection failures on Win8+ official builds I've root caused this and the problem was introduced in this CL: https://codereview.chromium.org/2446053002 That CL moved the logic which reported the worker process launch from the IPC Channel Connected method to an IO completion port listener. This works fine on un-official builds, however it breaks on official builds. The reason for the failure is that the remoting_desktop process on official builds are signed and include 'UiAccess' in their manifest. When the launcher process uses ShellExecuteEx to launch the worker process in this scenario, we actually see two processes get launched sequentially. The first starts and exists with error code 'STATUS_ELEVATION_REQUIRED' and the second launches with the correct permissions. This behavior worked fine before as we listened for the connection to the IPC channel which was done by the second, successful process launch. With the new code, we observed the first process launch, set up the Mojo channel for it, and tried waiting on its process handle which exits immediately. The second process then starts and fails to connect to the Mojo channel. I investigated whether UiAccess is truly required for the desktop binary and I think that it is. For the Ctrl+Alt+Del scenario, there are registry keys that can be set which will require that flag. For Alt+Tab, it is possible that some windows might not be accessible if they have a high high enough integrity level (+ UiAccess themselves). So instead of removing the UiAccess flag, my approach is to listen for the worker process creation and exit events. I store the value of the last seen worker process id and use that in our process launch detection code once the launcher process exits. This allows both un-official builds (which do not require the extra permissions hop) and official builds will work consistently. BUG= 666992 Review-Url: https://codereview.chromium.org/2568983004 Cr-Commit-Position: refs/heads/master@{#438077} (cherry picked from commit 1cdb04810de9689ba0ea5ab7e70bd35ce5477063) Review-Url: https://codereview.chromium.org/2575103002 . Cr-Commit-Position: refs/branch-heads/2924@{#490} Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059} [modify] https://crrev.com/286e4172c0f1d07fa50d101ff46d20ad42aefb3e/remoting/host/desktop_session_win.cc [modify] https://crrev.com/286e4172c0f1d07fa50d101ff46d20ad42aefb3e/remoting/host/win/BUILD.gn [modify] https://crrev.com/286e4172c0f1d07fa50d101ff46d20ad42aefb3e/remoting/host/win/wts_session_process_delegate.cc
,
Dec 14 2016
AJ, this change should now exist in both M56 and M57 as of tonight's build. Can you take a look at Win7 and Win10 and verify it is working as expected now?
,
Dec 14 2016
,
Dec 16 2016
Verified Fixed in 57.0.2952.0 on Windows 10. Looks good on Windows 7 as well. |
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by joedow@chromium.org
, Nov 22 2016