[Mac] Input Injection fails on Mojave |
||||||||
Issue descriptionThis is due to new security restrictions in Mojave. We can no longer inject input w/o being added as an accessibility app in the security applet. The problem with this is: 1.) We don't use the restricted API until a user connects (so they cannot take action) 2.) The dialog appears to only show up once (regardless of approve/deny status) 3.) Users connecting to the lock screen will never see the dialog
,
Nov 1
,
Nov 1
,
Nov 1
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/19b1b85cd5c3227697d348a39cec89ddf5980690 commit 19b1b85cd5c3227697d348a39cec89ddf5980690 Author: Joe Downing <joedow@google.com> Date: Thu Nov 01 22:13:58 2018 [MacOs Host] Provide user with a prompt to enable input injection on Mojave This change is required due to new security restrictions in Mojave. We can no longer inject input w/o being added as an accessibility app in the security applet. While this sounds like a usefulk speedbump, it causes remote access applications quite a bit of trouble: 1.) We don't use the restricted API until a user connects so they cannot approve remotely 2.) The dialog appears to only show up once (regardless of approve/deny status) 3.) Users connecting to a locked machine will never see the dialog This is affecting quite a few CRD users, basically everyone who upgrades to Mojave will experience this one way or another. This is the simplest fix (and easiest to merge) that I could think of to unblock users. The prompt will only be shown on 10.14+ platforms and the request is only shown if the app has not been approved. I'd like to look at the user feedback after releasing this change to see if more work is needed. One problem I anticipate is that the dialog shown doesn't have a lot of context and it refers to the wrapper script (org.chromium.chromoting.me2me.sh) instead of Chrome Remote Desktop. If this is confusing, we can wrap the prompt request in a dialog where we control the text. My concern with checking in the feature first is that the new strings won't be available for merging. Another behavior to call about this impl is that the prompt will be displayed in two instances: 1.) When the host is first started (choosing enable via app/website) 2.) When the user signs in and the host service is started Scenario #2 will have less context but that is the only way to ask for permission for users who upgraded and had CRD installed previously. The dialog is not displayed at the login screen (i.e. when no one is signed in). One last note, there is no way that I can see to specify this permission in the manifest or set up via a script / at install time. It requires a user action to complete. Bug: 901021 Change-Id: I9dd1b24b6d4d083e7e019af32a0da816f6060a86 Reviewed-on: https://chromium-review.googlesource.com/c/1313170 Commit-Queue: Joe Downing <joedow@chromium.org> Reviewed-by: Jamie Walch <jamiewalch@chromium.org> Cr-Commit-Position: refs/heads/master@{#604723} [modify] https://crrev.com/19b1b85cd5c3227697d348a39cec89ddf5980690/remoting/host/mac/BUILD.gn [add] https://crrev.com/19b1b85cd5c3227697d348a39cec89ddf5980690/remoting/host/mac/permission_utils.h [add] https://crrev.com/19b1b85cd5c3227697d348a39cec89ddf5980690/remoting/host/mac/permission_utils.mm [modify] https://crrev.com/19b1b85cd5c3227697d348a39cec89ddf5980690/remoting/host/remoting_me2me_host.cc
,
Nov 1
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7ee7bac4b03590e4c7904e5270ec8264d8f09df1 commit 7ee7bac4b03590e4c7904e5270ec8264d8f09df1 Author: Mike Wittman <wittman@chromium.org> Date: Thu Nov 01 23:39:10 2018 Revert "[MacOs Host] Provide user with a prompt to enable input injection on Mojave" This reverts commit 19b1b85cd5c3227697d348a39cec89ddf5980690. Reason for revert: breaks mac-dbg build https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/mac-dbg/1574 In file included from ../../remoting/host/remoting_me2me_host.cc:15: In file included from ../../base/bind.h:10: In file included from ../../base/bind_internal.h:13: In file included from ../../base/callback_internal.h:14: In file included from ../../base/memory/ref_counted.h:16: ../../base/logging.h:786:26: error: comparison of integers of different signs: 'const unsigned int' and 'const int' [-Werror,-Wsign-compare] DEFINE_CHECK_OP_IMPL(NE, !=) ~~~~~~~~~~~~~~~~~~~~~~~~~^~~ ../../base/logging.h:774:33: note: expanded from macro 'DEFINE_CHECK_OP_IMPL' if (ANALYZER_ASSUME_TRUE(v1 op v2)) \ ~~ ^ ~~ ../../base/logging.h:338:36: note: expanded from macro 'ANALYZER_ASSUME_TRUE' #define ANALYZER_ASSUME_TRUE(arg) (arg) ^~~ ../../remoting/host/remoting_me2me_host.cc:1576:3: note: in instantiation of function template specialization 'logging::CheckNEImpl<unsigned int, int>' requested here DCHECK_NE(getuid(), 0); ^ ../../base/logging.h:960:31: note: expanded from macro 'DCHECK_NE' #define DCHECK_NE(val1, val2) DCHECK_OP(NE, !=, val1, val2) ^ ../../base/logging.h:913:18: note: expanded from macro 'DCHECK_OP' ::logging::Check##name##Impl((val1), (val2), \ ^ <scratch space>:21:1: note: expanded from here CheckNEImpl ^ 1 error generated. Original change's description: > [MacOs Host] Provide user with a prompt to enable input injection on Mojave > > This change is required due to new security restrictions in Mojave. We can no longer > inject input w/o being added as an accessibility app in the security applet. > > While this sounds like a usefulk speedbump, it causes remote access applications quite > a bit of trouble: > 1.) We don't use the restricted API until a user connects so they cannot approve remotely > 2.) The dialog appears to only show up once (regardless of approve/deny status) > 3.) Users connecting to a locked machine will never see the dialog > > This is affecting quite a few CRD users, basically everyone who upgrades to Mojave > will experience this one way or another. This is the simplest fix (and easiest to merge) > that I could think of to unblock users. The prompt will only be shown on 10.14+ platforms > and the request is only shown if the app has not been approved. I'd like to look at the > user feedback after releasing this change to see if more work is needed. > > One problem I anticipate is that the dialog shown doesn't have a lot of context and it > refers to the wrapper script (org.chromium.chromoting.me2me.sh) instead of Chrome Remote > Desktop. If this is confusing, we can wrap the prompt request in a dialog where we control > the text. My concern with checking in the feature first is that the new strings won't be > available for merging. > > Another behavior to call about this impl is that the prompt will be displayed in two instances: > 1.) When the host is first started (choosing enable via app/website) > 2.) When the user signs in and the host service is started > > Scenario #2 will have less context but that is the only way to ask for permission for > users who upgraded and had CRD installed previously. The dialog is not displayed at the login > screen (i.e. when no one is signed in). > > One last note, there is no way that I can see to specify this permission in the manifest or > set up via a script / at install time. It requires a user action to complete. > > Bug: 901021 > Change-Id: I9dd1b24b6d4d083e7e019af32a0da816f6060a86 > Reviewed-on: https://chromium-review.googlesource.com/c/1313170 > Commit-Queue: Joe Downing <joedow@chromium.org> > Reviewed-by: Jamie Walch <jamiewalch@chromium.org> > Cr-Commit-Position: refs/heads/master@{#604723} TBR=jamiewalch@chromium.org,joedow@chromium.org Change-Id: I7c948b26c00f6c6fc7c4e0a3ec4175dcae17e459 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 901021 Reviewed-on: https://chromium-review.googlesource.com/c/1313747 Reviewed-by: Mike Wittman <wittman@chromium.org> Commit-Queue: Mike Wittman <wittman@chromium.org> Cr-Commit-Position: refs/heads/master@{#604747} [modify] https://crrev.com/7ee7bac4b03590e4c7904e5270ec8264d8f09df1/remoting/host/mac/BUILD.gn [delete] https://crrev.com/0564739b836ebcec0fd43ef603d3282557d2bf2f/remoting/host/mac/permission_utils.h [delete] https://crrev.com/0564739b836ebcec0fd43ef603d3282557d2bf2f/remoting/host/mac/permission_utils.mm [modify] https://crrev.com/7ee7bac4b03590e4c7904e5270ec8264d8f09df1/remoting/host/remoting_me2me_host.cc
,
Nov 2
The revert was due to a comparison issue in a DCHECK, that has been fixed and landed: https://chromium-review.googlesource.com/c/chromium/src/+/1315468 ChangeID: 19e52db3f4a744f7343a8e8f21339bc50709c11f
,
Nov 2
Requesting a merge for this change to M71. It only affects the Mac Chrome Remote Desktop Host and addresses a problem for CRD users on Mojave. Thanks!
,
Nov 2
Approving merge to M71 branch 3578 based on comment #7. Pls merge ASAP. Thank you.
,
Nov 2
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/118c5611f1ab02c92c792a74bbcaf43fe8bbdc60 commit 118c5611f1ab02c92c792a74bbcaf43fe8bbdc60 Author: Joe Downing <joedow@google.com> Date: Fri Nov 02 21:31:51 2018 [MacOs Host] Provide user with a prompt to enable input injection on Mojave This change is required due to new security restrictions in Mojave. We can no longer inject input w/o being added as an accessibility app in the security applet. While this sounds like a usefulk speedbump, it causes remote access applications quite a bit of trouble: 1.) We don't use the restricted API until a user connects so they cannot approve remotely 2.) The dialog appears to only show up once (regardless of approve/deny status) 3.) Users connecting to a locked machine will never see the dialog This is affecting quite a few CRD users, basically everyone who upgrades to Mojave will experience this one way or another. This is the simplest fix (and easiest to merge) that I could think of to unblock users. The prompt will only be shown on 10.14+ platforms and the request is only shown if the app has not been approved. I'd like to look at the user feedback after releasing this change to see if more work is needed. One problem I anticipate is that the dialog shown doesn't have a lot of context and it refers to the wrapper script (org.chromium.chromoting.me2me.sh) instead of Chrome Remote Desktop. If this is confusing, we can wrap the prompt request in a dialog where we control the text. My concern with checking in the feature first is that the new strings won't be available for merging. Another behavior to call about this impl is that the prompt will be displayed in two instances: 1.) When the host is first started (choosing enable via app/website) 2.) When the user signs in and the host service is started Scenario #2 will have less context but that is the only way to ask for permission for users who upgraded and had CRD installed previously. The dialog is not displayed at the login screen (i.e. when no one is signed in). One last note, there is no way that I can see to specify this permission in the manifest or set up via a script / at install time. It requires a user action to complete. Bug: 901021 Change-Id: I54247dd56111feee9608e1b50387cb3501a659a4 Reviewed-on: https://chromium-review.googlesource.com/c/1315468 Reviewed-by: Gary Kacmarcik <garykac@chromium.org> Commit-Queue: Joe Downing <joedow@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#604947}(cherry picked from commit 19e52db3f4a744f7343a8e8f21339bc50709c11f) Reviewed-on: https://chromium-review.googlesource.com/c/1315706 Reviewed-by: Joe Downing <joedow@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#478} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [modify] https://crrev.com/118c5611f1ab02c92c792a74bbcaf43fe8bbdc60/remoting/host/mac/BUILD.gn [add] https://crrev.com/118c5611f1ab02c92c792a74bbcaf43fe8bbdc60/remoting/host/mac/permission_utils.h [add] https://crrev.com/118c5611f1ab02c92c792a74bbcaf43fe8bbdc60/remoting/host/mac/permission_utils.mm [modify] https://crrev.com/118c5611f1ab02c92c792a74bbcaf43fe8bbdc60/remoting/host/remoting_me2me_host.cc
,
Nov 2
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/118c5611f1ab02c92c792a74bbcaf43fe8bbdc60 Commit: 118c5611f1ab02c92c792a74bbcaf43fe8bbdc60 Author: joedow@google.com Commiter: joedow@chromium.org Date: 2018-11-02 21:31:51 +0000 UTC [MacOs Host] Provide user with a prompt to enable input injection on Mojave This change is required due to new security restrictions in Mojave. We can no longer inject input w/o being added as an accessibility app in the security applet. While this sounds like a usefulk speedbump, it causes remote access applications quite a bit of trouble: 1.) We don't use the restricted API until a user connects so they cannot approve remotely 2.) The dialog appears to only show up once (regardless of approve/deny status) 3.) Users connecting to a locked machine will never see the dialog This is affecting quite a few CRD users, basically everyone who upgrades to Mojave will experience this one way or another. This is the simplest fix (and easiest to merge) that I could think of to unblock users. The prompt will only be shown on 10.14+ platforms and the request is only shown if the app has not been approved. I'd like to look at the user feedback after releasing this change to see if more work is needed. One problem I anticipate is that the dialog shown doesn't have a lot of context and it refers to the wrapper script (org.chromium.chromoting.me2me.sh) instead of Chrome Remote Desktop. If this is confusing, we can wrap the prompt request in a dialog where we control the text. My concern with checking in the feature first is that the new strings won't be available for merging. Another behavior to call about this impl is that the prompt will be displayed in two instances: 1.) When the host is first started (choosing enable via app/website) 2.) When the user signs in and the host service is started Scenario #2 will have less context but that is the only way to ask for permission for users who upgraded and had CRD installed previously. The dialog is not displayed at the login screen (i.e. when no one is signed in). One last note, there is no way that I can see to specify this permission in the manifest or set up via a script / at install time. It requires a user action to complete. Bug: 901021 Change-Id: I54247dd56111feee9608e1b50387cb3501a659a4 Reviewed-on: https://chromium-review.googlesource.com/c/1315468 Reviewed-by: Gary Kacmarcik <garykac@chromium.org> Commit-Queue: Joe Downing <joedow@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#604947}(cherry picked from commit 19e52db3f4a744f7343a8e8f21339bc50709c11f) Reviewed-on: https://chromium-review.googlesource.com/c/1315706 Reviewed-by: Joe Downing <joedow@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#478} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
,
Nov 5
Issue has been merged and is being included as part of our M71 host rollout. I will follow-up with a separate bug to track UI changes needed to make the experience a little better (we currently rely on the Mac AX dialog which is pretty terse, it would be nice to provide more context to the user). |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by joedow@chromium.org
, Nov 1Labels: -Pri-3 OS-Mac Pri-1
Owner: joedow@chromium.org
Status: Assigned (was: Untriaged)