search

chromiumembedded / cef

Chromium Embedded Framework (CEF). A simple framework for embedding Chromium-based browsers in other applications.
https://bitbucket.org/chromiumembedded/cef/
Other
3.38k stars 467 forks source link

Certificate Transparency Verifier Crash on cef3.2883.1548 #2080

Closed magreenblatt closed 7 years ago

magreenblatt commented 7 years ago

Original report by GaRUi (Bitbucket: GaRUi, GitHub: GaRUi).

  1. What steps will reproduce the problem?

    • Initiate a WebRTC call with secure TURN server in iceConfiguration with cefclient.app with --enable-media-stream command line option.
    • https://appear.in/ website can be used for testing. Just join a room and try Chrome to join the same room. It looks like WebRTC will not be initiated until a second person joins the room.

  2. What is the expected output? What do you see instead?

    • Expected : Call should be initiated and ICE collection process should complete successfully.
    • Result : ICE collection process can not be completed and cefclient.app crashes with "Check failed: cert_transparencyverifier." error log.

  3. What version of the product are you using? On what operating system?

  4. Does the problem reproduce with the cefclient or cefsimple sample application at the same version? How about with a newer or older version?

    • Issue has seen on cefclient sample application. cefsimple has not been tested. Issue is not reproducible with older cef3, based on Chromium 47.

  5. Does the problem reproduce with Google Chrome at the same version? How about with a newer or older version?

    • Google Chrome 55.0.2883.95 (64-bit) works as expected.
magreenblatt commented 7 years ago

Original comment by GaRUi (Bitbucket: GaRUi, GitHub: GaRUi).

I have added below line to url_request_context_proxy.cc in order to fix the issue:

#!c++

set_cert_transparency_verifier(parent->cert_transparency_verifier()); 
set_ct_policy_enforcer(parent->ct_policy_enforcer());

That way SSLClientSocketImpl CHECKs will not crash the cefclient. There are four CHECKs in SSLClientSocketImpl constructor :

#!c++

CHECK(cert_verifier_);
CHECK(transport_security_state_);
CHECK(cert_transparency_verifier_);
CHECK(policy_enforcer_);

I am not much experienced with cefclient's libcef static dll code. Could you tell me if this is the right approach ?

I have also tried using DoNothingCTVerifier in chromium code with some modifications (DoNothingCTVerifier is not available in Cef2883) , but it also failed because of the bug in url_request_context_proxy.cc . But maybe it is a good way to implement it in Cef3 with a command line flag or preferences for enabling and disabling CT Logs verification.

BTW, I have not yet removed the DoNothingCTVerifier implementation. Maybe it is also helping to workaround this crash issue. I need to remove it and compile it again to be sure. Because right now I am just pushing DoNothingCTVerifier for every request.

Link to DoNothingCTVerifier

DoNothingCTVerifier commit message give some information Provide a concrete implementation of a CTVerifier that allows consumers to opt-out of verifying CT information. While this is not generally desirable for consumers of "the Web PKI" (e.g.: websites), it's acceptable and potentially desirable for things like peer-to-peer [D]TLS sockets (like WebRTC or Remoting), or for cases where the overall application is not ready to support Certificate Transparency (for example: may lack any form of update path).

magreenblatt commented 7 years ago

Original comment by GaRUi (Bitbucket: GaRUi, GitHub: GaRUi).

I have removed DoNothingCTVerifier. The only thing I have done is to add below lines to url_request_context_proxy.cc in order to fix the issue.

#!c++

set_cert_transparency_verifier(parent->cert_transparency_verifier()); 
set_ct_policy_enforcer(parent->ct_policy_enforcer());
magreenblatt commented 7 years ago

Thanks, the above fix appears to resolve the browser process crash. However, in current CEF master I'm also getting this renderer process crash due to the RTC_DCHECK(serveraddress.proto == PROTO_TCP); assertion in TurnPort::OnSocketConnect:

    libcef.dll!issue_debug_notification(const wchar_t * const message) Line 125 C++
    libcef.dll!__acrt_report_runtime_error(const wchar_t * message) Line 142    C++
    libcef.dll!abort() Line 51  C++
    libcef.dll!rtc::FatalMessage::~FatalMessage() Line 110  C++
>   libcef.dll!cricket::TurnPort::OnSocketConnect(rtc::AsyncPacketSocket * socket) Line 390 C++
    libcef.dll!sigslot::_connection1<cricket::TurnPort,rtc::AsyncPacketSocket *,sigslot::single_threaded>::emit(rtc::AsyncPacketSocket * a1) Line 1807  C++
    libcef.dll!sigslot::signal1<rtc::AsyncPacketSocket *,sigslot::single_threaded>::operator()(rtc::AsyncPacketSocket * a1) Line 2302   C++
    libcef.dll!content::`anonymous namespace'::IpcPacketSocket::OnOpen(const net::IPEndPoint & local_address, const net::IPEndPoint & remote_address) Line 584  C++
    libcef.dll!content::P2PSocketClientImpl::DeliverOnSocketCreated(const net::IPEndPoint & local_address, const net::IPEndPoint & remote_address) Line 170 C++
    libcef.dll!base::internal::FunctorTraits<void (__thiscall content::P2PSocketClientImpl::*)(net::IPEndPoint const &,net::IPEndPoint const &),void>::Invoke<scoped_refptr<content::P2PSocketClientImpl> const &,net::IPEndPoint const &,net::IPEndPoint const &>(void(content::P2PSocketClientImpl::*)(const net::IPEndPoint &, const net::IPEndPoint &) method, const scoped_refptr<content::P2PSocketClientImpl> & receiver_ptr, const net::IPEndPoint & <args_0>, const net::IPEndPoint & <args_1>) Line 215   C++
    libcef.dll!base::internal::InvokeHelper<0,void>::MakeItSo<void (__thiscall content::P2PSocketClientImpl::*const &)(net::IPEndPoint const &,net::IPEndPoint const &),scoped_refptr<content::P2PSocketClientImpl> const &,net::IPEndPoint const &,net::IPEndPoint const &>(void(content::P2PSocketClientImpl::*)(const net::IPEndPoint &, const net::IPEndPoint &) & functor, const scoped_refptr<content::P2PSocketClientImpl> & <args_0>, const net::IPEndPoint & <args_1>, const net::IPEndPoint & <args_2>) Line 285  C++
    libcef.dll!base::internal::Invoker<base::internal::BindState<void (__thiscall content::P2PSocketClientImpl::*)(net::IPEndPoint const &,net::IPEndPoint const &),scoped_refptr<content::P2PSocketClientImpl>,net::IPEndPoint,net::IPEndPoint>,void __cdecl(void)>::RunImpl<void (__thiscall content::P2PSocketClientImpl::*const &)(net::IPEndPoint const &,net::IPEndPoint const &),std::tuple<scoped_refptr<content::P2PSocketClientImpl>,net::IPEndPoint,net::IPEndPoint> const &,0,1,2>(void(content::P2PSocketClientImpl::*)(const net::IPEndPoint &, const net::IPEndPoint &) & functor, const std::tuple<scoped_refptr<content::P2PSocketClientImpl>,net::IPEndPoint,net::IPEndPoint> & bound, base::IndexSequence<0,1,2> __formal) Line 361  C++
    libcef.dll!base::internal::Invoker<base::internal::BindState<void (__thiscall content::P2PSocketClientImpl::*)(net::IPEndPoint const &,net::IPEndPoint const &),scoped_refptr<content::P2PSocketClientImpl>,net::IPEndPoint,net::IPEndPoint>,void __cdecl(void)>::Run(base::internal::BindStateBase * base) Line 339    C++
    libcef.dll!base::internal::RunMixin<base::Callback<void __cdecl(void),0,0> >::Run() Line 68 C++
    libcef.dll!base::debug::TaskAnnotator::RunTask(const char * queue_function, base::PendingTask * pending_task) Line 54   C++
    libcef.dll!base::MessageLoop::RunTask(base::PendingTask * pending_task) Line 419    C++
    libcef.dll!base::MessageLoop::DeferOrRunPendingTask(base::PendingTask pending_task) Line 430    C++
    libcef.dll!base::MessageLoop::DoWork() Line 520 C++
    libcef.dll!base::MessagePumpDefault::Run(base::MessagePump::Delegate * delegate) Line 33    C++
    libcef.dll!base::MessageLoop::RunHandler() Line 384 C++
    libcef.dll!base::RunLoop::Run() Line 38 C++
    libcef.dll!base::Thread::Run(base::RunLoop * run_loop) Line 246 C++
    libcef.dll!base::Thread::ThreadMain() Line 331  C++
    libcef.dll!base::`anonymous namespace'::ThreadFunc(void * params) Line 86   C++
    [External Code]

Filed with Chromium as https://bugs.chromium.org/p/chromium/issues/detail?id=687282

magreenblatt commented 7 years ago

Browser process crash fixed in master revision 30451b7 (bb), 2987 branch revision cd266e8 (bb) and 2924 branch revision 1758b74 (bb).

magreenblatt commented 7 years ago

Original comment by GaRUi (Bitbucket: GaRUi, GitHub: GaRUi).

Thanks Marshall. I have never faced with the issue, you have mentioned above. Maybe the crash you have seen in RTC_DCHECK(serveraddress.proto == PROTO_TCP); has been introduced with latest chromium. Thanks for reporting it to chromium.

Just one thing about your patch. Maybe you can remove one of the set_transport_security_state(parent->transport_security_state()); line, because it has been introduced twice in url_request_context_proxy.cc

magreenblatt commented 7 years ago

@GaRUi : Thanks for pointing out the duplicate line.

magreenblatt commented 7 years ago

Original changes by GaRUi (Bitbucket: GaRUi, GitHub: GaRUi).

magreenblatt commented 7 years ago