IT2Me host should be shut down when incoming connection is invalid |
|||||||||
Issue descriptionVersion: Windows Host 55.0.2867.0 or later; CrOS Version 55.0.2869.0 dev (64-bit) Platform 8838.0.0 (Official Build) dev-channel samus ARC Version 3294522 Client app version 52.0.2743.48 OS: Windows What steps will reproduce the problem? (1)Install Host Version 55.0.2867.0 or later (2)In IT2Me click Share on the Host (3)On the client, enter the code (4)On the Host, do not click the option to "share" to allow the client to see the host desktop (5)On the client, let it time out, then click OK (6)On the client, click Access again, the code should be pre-entered. Click Connect. (7)Host Crashes Since this bug requires the new IT2Me sharing confirmation dialog, this does appear to be related to bug 645540 Host Sawbuck log only logged two errors: ERROR 28180 24880 16:07:37-369 c:\b\build\slave\win\build\src\net\proxy\proxy_config_service_win.cc 135 WinHttpGetIEProxyConfigForCurrentUser failed: 2 ERROR 24868 3276 16:09:17-374 c:\b\build\slave\win\build\src\remoting\host\native_messaging\native_messaging_reader.cc 100 Failed to read message header, read returned -1 Client log contains the following: Connecting as ajnolley@google.com client_session.js:448 [2016-10-03T22:55:59.289Z]iq receive id=12885200690639227019 result google:jingleinfo stun 'stun.l.google.com' udp:19302; 'alt2.stun.l.google.com' udp:19302; 'alt4.stun.l.google.com' udp:19302; 'alt3.stun.l.google.com' udp:19302; 'alt1.stun.l.google.com' udp:19302; relay 'relay.google.com' udp:19305 tcp:19305 tcpssl:443; token: CAESHAoTYWpub2xsZXlAZ29vZ2xlLmNvbRD3gJX0+CoaEGFMSFCVPBd82HF/1zJHPSw= client_session.js:448 [2016-10-03T22:55:59.497Z]Receiving Iq: <iq xmlns="jabber:client" id="4228751752688902366" type="result" to="ajnolley@google.com/chromotingE4565563" from="ajnolley@google.com/chromotingECCDE13B"><jingle xmlns="urn:xmpp:jingle:1"/></iq> console_wrapper.js:115 Failed to receive from XMPP socket: -101 xmpp_connection.js:328 remoting.ConsoleWrapper.recordAndLog_ @ console_wrapper.js:115 fallback_signal_strategy.js:339 FallbackSignalStrategy progress: xmpp failed-late ui_mode.js:122 App mode: home client_session.js:415 [2016-10-03T23:01:18.198Z]iq send id=session-terminate set session-terminate sid=346048331714772081 reason=success console_wrapper.js:115 Missing translation for "" l10n.js:22 remoting.ConsoleWrapper.recordAndLog_ @ console_wrapper.js:115 ui_mode.js:122 App mode: home.client.connect-failed.it2me desktop_remoting_activity.js:84 Disconnected. console_wrapper.js:115 Failed to receive from XMPP socket: -101 xmpp_connection.js:328 remoting.ConsoleWrapper.recordAndLog_ @ console_wrapper.js:115 fallback_signal_strategy.js:339 FallbackSignalStrategy progress: xmpp failed-late console_wrapper.js:115 Failed to receive from XMPP socket: -101 xmpp_connection.js:328 remoting.ConsoleWrapper.recordAndLog_ @ console_wrapper.js:115 fallback_signal_strategy.js:339 FallbackSignalStrategy progress: xmpp failed-late console_wrapper.js:115 Failed to receive from XMPP socket: -101 xmpp_connection.js:328 remoting.ConsoleWrapper.recordAndLog_ @ console_wrapper.js:115 fallback_signal_strategy.js:339 FallbackSignalStrategy progress: xml
,
Oct 4 2016
I'm pretty sure this is due to my change to remove the It2meConfirmationDialogFactory (cleanup CL after my initial dialog change). I'd like to address this in the current milestone so bumping to P1 and marking M55.
,
Oct 5 2016
Confirmed this is due to the removal of the It2meConfirmationDialogFactory. This change affects all of the It2Me hosts so I want to fix it in this milestone. Also, you can repro this by cancelling the connection and then re-trying, no need to wait for the timeout.
,
Oct 5 2016
,
Oct 5 2016
Per discussion with Sergey, we should not continue running the It2Me host if an aspect of the incoming connection is invalid (i.e. the domain does not match the policy or the user rejects it via the dialog). Updated bug title to reflect that. Note that the fix for this issue will prevent the crash that AJ reported.
,
Oct 5 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bd36c87b70a3c88b3969b95208c4142e58c90ea3 commit bd36c87b70a3c88b3969b95208c4142e58c90ea3 Author: joedow <joedow@chromium.org> Date: Wed Oct 05 22:55:57 2016 Disconnect It2Me session if the incoming connection is invalid In a previous CL, I removed the It2MeConfirmationDialogFactory and instead held on to an instance of the dialog which was then used, and destroyed, when validating the incoming connection. The assumption was that the It2Me host would be destroyed when the user rejected the incoming connection. This is not actually what happens, the host remains running and will continue to receive these invalid requests until the web host times out and kills the native messaging host process. Due to this behavior, we now see a crash when the remote user attempts to connect again. Ideally we should stop the It2Me connection process completely if the incoming request is rejected. This change does that by calling Disconnect() when a validation error occurs. It also adds some DCHECKs to ensure the validation code is called on the correct thread (even for success cases) and updates the unit tests to work with these stricter thread affinity requirements. BUG= 652490 Review-Url: https://codereview.chromium.org/2398803002 Cr-Commit-Position: refs/heads/master@{#423325} [modify] https://crrev.com/bd36c87b70a3c88b3969b95208c4142e58c90ea3/remoting/host/it2me/it2me_host.cc [modify] https://crrev.com/bd36c87b70a3c88b3969b95208c4142e58c90ea3/remoting/host/it2me/it2me_host_unittest.cc
,
Oct 6 2016
,
Oct 6 2016
Verified Fixed in 55.0.2882.0. It will now time on either end with "An unexpected error..."
,
Oct 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bd36c87b70a3c88b3969b95208c4142e58c90ea3 commit bd36c87b70a3c88b3969b95208c4142e58c90ea3 Author: joedow <joedow@chromium.org> Date: Wed Oct 05 22:55:57 2016 Disconnect It2Me session if the incoming connection is invalid In a previous CL, I removed the It2MeConfirmationDialogFactory and instead held on to an instance of the dialog which was then used, and destroyed, when validating the incoming connection. The assumption was that the It2Me host would be destroyed when the user rejected the incoming connection. This is not actually what happens, the host remains running and will continue to receive these invalid requests until the web host times out and kills the native messaging host process. Due to this behavior, we now see a crash when the remote user attempts to connect again. Ideally we should stop the It2Me connection process completely if the incoming request is rejected. This change does that by calling Disconnect() when a validation error occurs. It also adds some DCHECKs to ensure the validation code is called on the correct thread (even for success cases) and updates the unit tests to work with these stricter thread affinity requirements. BUG= 652490 Review-Url: https://codereview.chromium.org/2398803002 Cr-Commit-Position: refs/heads/master@{#423325} [modify] https://crrev.com/bd36c87b70a3c88b3969b95208c4142e58c90ea3/remoting/host/it2me/it2me_host.cc [modify] https://crrev.com/bd36c87b70a3c88b3969b95208c4142e58c90ea3/remoting/host/it2me/it2me_host_unittest.cc
,
Nov 2 2016
,
Nov 4 2016
[Automated comment] removing mislabelled merge-merged-2840 |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 Deleted