Intermittent crash in D2D Auth |
|||||||||
Issue descriptionI observed this during a Tether host scan (I can't tell if it's due to Tether or EasyUnlock though). This account has two Tether hosts, and one unlock_key. chrome logs attached as well. I've observed this happening in < 5% of host scans; it's quite rare. Tim/Kyle, did something change recently in cryptauth/EasyUnlock that might have caused this crash? Backtrace: Thread 1 "chrome" received signal SIGSEGV, Segmentation fault. 0x3542e28c in cryptauth::(anonymous namespace)::OnSessionSymmetricKeyDerived(cryptauth::(anonymous namespace)::ValidateResponderAuthMessageContext, std::string const&) () at ../../components/cryptauth/device_to_device_initiator_operations.cc:153 (gdb) bt #0 0x3542e28c in cryptauth::(anonymous namespace)::OnSessionSymmetricKeyDerived(cryptauth::(anonymous namespace)::ValidateResponderAuthMessageContext, std::string const&) () at ../../components/cryptauth/device_to_device_initiator_operations.cc:153 #1 0x3542e444 in base::internal::Invoker<base::internal::BindState<void (*)(cryptauth::(anonymous namespace)::ValidateResponderAuthMessageContext, std::string const&), cryptauth::(anonymous namespace)::ValidateResponderAuthMessageContext>, void (std::string const&)>::Run(base::internal::BindStateBase*, std::string const&) () at ../../base/bind_internal.h:164 #2 0x33166f86 in chromeos::(anonymous namespace)::EasyUnlockClientImpl::OnData(base::Callback<void (std::string const&), (base::internal::CopyMode)1, (base::internal::RepeatMode)1> const&, dbus::Response*) () at ../../base/callback.h:80 #3 0x332470ac in dbus::ObjectProxy::RunResponseCallback(base::Callback<void (dbus::Response*), (base::internal::CopyMode)1, (base::internal::RepeatMode)1>, base::Callback<void (dbus::ErrorResponse*), (base::internal::CopyMode)1, (base::internal::RepeatMode)1>, base::TimeTicks, DBusMessage*) () at ../../base/callback.h:80 #4 0x332495c0 in base::internal::Invoker<base::internal::BindState<void (dbus::ObjectProxy::*)(base::Callback<void (dbus::Response*), (base::internal::CopyMode)1, (base::internal::RepeatMode)1>, base::Callback<void (dbus::ErrorResponse*), (base::internal::CopyMode)1, (base::internal::RepeatMode)1>, base::TimeTicks, DBusMessage*), scoped_refptr<dbus::ObjectProxy>, base::Callback<void (dbus::Response*), (base::internal::CopyMode)1, (base::internal::RepeatMode)1>, base::Callback<void (dbus::ErrorResponse*), (base::internal::CopyMode)1, (base::internal::RepeatMode)1>, base::TimeTicks, DBusMessage*>, void ()>::Run(base::internal::BindStateBase*) () at ../../base/bind_internal.h:209 #5 0x32adf560 in base::debug::TaskAnnotator::RunTask(char const*, base::PendingTask*) () at ../../base/callback.h:91 #6 0x32a844dc in base::MessageLoop::RunTask(base::PendingTask*) () at ../../base/message_loop/message_loop.cc:422 #7 0x32a847b0 in base::MessageLoop::DeferOrRunPendingTask(base::PendingTask) () at ../../base/message_loop/message_loop.cc:433 #8 0x32a84a74 in base::MessageLoop::DoWork() () at ../../base/message_loop/message_loop.cc:540 #9 0x32a85d5c in base::MessagePumpLibevent::Run(base::MessagePump::Delegate*) () at ../../base/message_loop/message_pump_libevent.cc:219 #10 0x32a9f054 in base::RunLoop::Run() () at ../../base/run_loop.cc:111 #11 0x327f42ce in ChromeBrowserMainParts::MainMessageLoopRun(int*) () at ../../chrome/browser/chrome_browser_main.cc:1962 #12 0x31969e64 in content::BrowserMainLoop::RunMainMessageLoopParts() () at ../../content/browser/browser_main_loop.cc:1147 #13 0x3196bca4 in content::BrowserMainRunnerImpl::Run() () at ../../content/browser/browser_main_runner.cc:142 #14 0x31966976 in content::BrowserMain(content::MainFunctionParams const&) () at ../../content/browser/browser_main.cc:46 #15 0x327d42a4 in content::ContentMainRunnerImpl::Run() () at ../../content/app/content_main_runner.cc:686 #16 0x327ec050 in service_manager::Main(service_manager::MainParams const&) () at ../../services/service_manager/embedder/main.cc:469 #17 0x327d3688 in content::ContentMain(content::ContentMainParams const&) () at ../../content/app/content_main.cc:19 #18 0x31538376 in ChromeMain () at ../../chrome/app/chrome_main.cc:134 #19 0xaec448b8 in __libc_start_main (main=0x0, argc=-1090743560, argv=0x0, init=<optimized out>, fini=0x35f50bfd <__libc_csu_fini>, rtld_fini=0xaf2bbf41 <_dl_fini>, stack_end=0xbefc9454) at libc-start.c:289 #20 0x31538218 in _start ()
,
Jul 11 2017
Tim, this crash originates from EasyUnlockClient. Can you look into it?
,
Jul 11 2017
It seems the |secure_message_delegate| is nil. I don't know exactly why. This not necessary triggered by EasyUnlock, as the |secure_message_delegate| is used by both Magic Tether and EasyUnlock to establish the secure channel. The delegate performs the crypto operations.
,
Jul 11 2017
Both EasyUnlock and Tether utilize CryptAuthService to create a new SecureMessageDelegate, and here is the implementation: https://cs.chromium.org/chromium/src/chrome/browser/cryptauth/chrome_cryptauth_service.cc?q=CreateSecureMessageDelegate Can you determine how EasyUnlockClientImpl::OnData() passes the delegate along?
,
Jul 11 2017
The delegate is part of a |context| passed to the delegate itself on every crypto operation. The same |context| was just used before to derive the symmetric key, and the delegate was not nil then: https://cs.chromium.org/chromium/src/components/cryptauth/device_to_device_initiator_operations.cc?l=138
,
Jul 11 2017
From a brief look at the logs, it looks like this is caused by a race condition of timing out the connection attempt while authentication is in progress. I see the following in the logs: [12137:12137:0711/113711.473872:INFO:device_to_device_authenticator.cc(237)] Received [Responder Auth] message, payload_size=325 . . . [12137:12137:0711/113711.509895:INFO:ble_connection_manager.cc(462)] Connection attempt timeout - Device ID "CAESR...0qr4=" [12137:12137:0711/113711.509985:VERBOSE1:bluetooth_discovery_session.cc(45)] Stopping device discovery session. [12137:12137:0711/113711.510195:VERBOSE1:device_event_log_impl.cc(158)] [11:37:11.510] Bluetooth: EVENT: bluetooth_adapter_bluez.cc:1427 RemoveDiscoverySession [12137:12137:0711/113711.510365:INFO:ble_advertiser.cc(276)] Stopping advertising to device CAESR...0qr4= [12137:12137:0711/113711.510419:INFO:ble_advertiser.cc(72)] Calling Unregister [12137:12137:0711/113711.510537:VERBOSE1:bluetooth_le_advertisement_service_provider.cc(84)] Cleaning up Bluetooth Advertisement: /org/chromium/bluetooth_advertisement/dac3bacfdd6d47fbaa3f4d28188038a9 [12137:12137:0711/113711.510599:INFO:ble_advertiser.cc(283)] Remaining adverts: 0 [12137:12137:0711/113711.510693:INFO:ble_connection_manager.cc(499)] Status change - Device ID: "CAESR...0qr4=": [connecting] => [disconnected] This seems to correspond to the stack trace in ValidateResponderAuthMessage. The |secure_message_delegate| that Gustavo is referring to in #3 is owned by DeviceToDeviceAuthenticator, which would be destroyed when the session is invalidated. This crash occurs in the callback of a DBus message response (EasyUnlockClient talks to the easyunlock deamon in ChromeOS), which would call into the now invalidated SecureMessageContext. At least, this is what I think is happening.
,
Jul 21 2017
,
Jul 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2693aa8e650f3819cbf455d628622d694d9d35a3 commit 2693aa8e650f3819cbf455d628622d694d9d35a3 Author: Kyle Horimoto <khorimoto@google.com> Date: Thu Jul 27 17:08:01 2017 [CrOS Tether] Eliminate crash in D2D-Auth protocol. This fix makes it possible to instantiate DeviceToDeviceInitiatorOperations instances and uses WeakPtrs within the class to ensure that it is not possible to segfault by attempting to call a function on a deleted object. Bug: 672263, 741042 Change-Id: Idcb13aa4772d6e227a335556bd4bf6b3cb4d3f4c Reviewed-on: https://chromium-review.googlesource.com/583563 Reviewed-by: Tim Song <tengs@chromium.org> Commit-Queue: Kyle Horimoto <khorimoto@chromium.org> Cr-Commit-Position: refs/heads/master@{#489989} [modify] https://crrev.com/2693aa8e650f3819cbf455d628622d694d9d35a3/components/cryptauth/BUILD.gn [modify] https://crrev.com/2693aa8e650f3819cbf455d628622d694d9d35a3/components/cryptauth/device_to_device_authenticator.cc [modify] https://crrev.com/2693aa8e650f3819cbf455d628622d694d9d35a3/components/cryptauth/device_to_device_authenticator.h [modify] https://crrev.com/2693aa8e650f3819cbf455d628622d694d9d35a3/components/cryptauth/device_to_device_authenticator_unittest.cc [rename] https://crrev.com/2693aa8e650f3819cbf455d628622d694d9d35a3/components/cryptauth/device_to_device_initiator_helper.cc [add] https://crrev.com/2693aa8e650f3819cbf455d628622d694d9d35a3/components/cryptauth/device_to_device_initiator_helper.h [delete] https://crrev.com/9191b87bd02343147139fb3f3e320d8bec5b8999/components/cryptauth/device_to_device_initiator_operations.h [modify] https://crrev.com/2693aa8e650f3819cbf455d628622d694d9d35a3/components/cryptauth/device_to_device_operations_unittest.cc
,
Jul 27 2017
,
Jul 27 2017
Approving merge to M61 Chrome OS.
,
Jul 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3dbc8112cfdb0ff5a155b9a894d14b3d4813e089 commit 3dbc8112cfdb0ff5a155b9a894d14b3d4813e089 Author: Kyle Horimoto <khorimoto@google.com> Date: Thu Jul 27 21:34:30 2017 [CrOS Tether] Eliminate crash in D2D-Auth protocol. This fix makes it possible to instantiate DeviceToDeviceInitiatorOperations instances and uses WeakPtrs within the class to ensure that it is not possible to segfault by attempting to call a function on a deleted object. TBR=khorimoto@google.com (cherry picked from commit 2693aa8e650f3819cbf455d628622d694d9d35a3) Bug: 672263, 741042 Change-Id: Idcb13aa4772d6e227a335556bd4bf6b3cb4d3f4c Reviewed-on: https://chromium-review.googlesource.com/583563 Reviewed-by: Tim Song <tengs@chromium.org> Commit-Queue: Kyle Horimoto <khorimoto@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#489989} Reviewed-on: https://chromium-review.googlesource.com/590750 Reviewed-by: Kyle Horimoto <khorimoto@chromium.org> Cr-Commit-Position: refs/branch-heads/3163@{#94} Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528} [modify] https://crrev.com/3dbc8112cfdb0ff5a155b9a894d14b3d4813e089/components/cryptauth/BUILD.gn [modify] https://crrev.com/3dbc8112cfdb0ff5a155b9a894d14b3d4813e089/components/cryptauth/device_to_device_authenticator.cc [modify] https://crrev.com/3dbc8112cfdb0ff5a155b9a894d14b3d4813e089/components/cryptauth/device_to_device_authenticator.h [modify] https://crrev.com/3dbc8112cfdb0ff5a155b9a894d14b3d4813e089/components/cryptauth/device_to_device_authenticator_unittest.cc [rename] https://crrev.com/3dbc8112cfdb0ff5a155b9a894d14b3d4813e089/components/cryptauth/device_to_device_initiator_helper.cc [add] https://crrev.com/3dbc8112cfdb0ff5a155b9a894d14b3d4813e089/components/cryptauth/device_to_device_initiator_helper.h [delete] https://crrev.com/17ad12406183efb7d6ece31ff78f787aae97a3a5/components/cryptauth/device_to_device_initiator_operations.h [modify] https://crrev.com/3dbc8112cfdb0ff5a155b9a894d14b3d4813e089/components/cryptauth/device_to_device_operations_unittest.cc
,
Jul 27 2017
,
Jan 22 2018
|
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by hansberry@chromium.org
, Jul 11 2017560 KB
560 KB View Download