[Bug]: Rapid requests to same domain cause session.setCertificateVerifyProc to break requests (maybe session related?)

[CaseyLeeMurphy]

Preflight Checklist

• I have read the Contributing Guidelines for this project.
• I agree to follow the Code of Conduct that this project adheres to.
• I have searched the issue tracker for a feature request that matches the one I want to file, without success.

Electron Version

11.3.0

What operating system are you using?

macOS

Operating System Version

macOS 11.2.2

What arch are you using?

x64

Last Known Working Electron version

Maybe never?

Expected Behavior

When rapid requests are made to the same domain, they are routed to the setCertificateVerifyProc's registered proc callback function. That callback function should then be able to call the resolution callback with the appropriate verificationResult (0, -2, -3) without causing an http request failures or session dumping

Actual Behavior

When rapid requests are made to the same domain, they are routed to the setCertificateVerifyProc's registered proc callback function. When calling the resolution callback function on a domain that has already been resolved using the callback function with 0 or -2, something in electron breaks. Not sure if it is session related, or something running the http requests

For example, 2 requests to foo.bar.com are made one right after another. Both requests then are sent to the setCertificateVerifyProc's registered callback function because the cert for that domain hasn't been verified yet. The first request is processed, and the callback is called with a 2 or 0. The second request then gets processed by the same callback function, and again the resolution callback is called with 2 or 0. When the second callback is executed, something with the session or something related to the way http requests gets broken, and causes https requests to fail

Testcase Gist URL

https://gist.github.com/9235bb45412c6f55fba6b5703f467b87

[ckerr]

Tested on Ubuntu 20.10 w / Electron 11.3.0. @CaseyLeeMurphy's testcase gist https://gist.github.com/9235bb45412c6f55fba6b5703f467b87 behaved exactly as described on first run + on reload-without-exit.

Possibly something is being cached between runs, because I was unable to reproduce the issue after the first run, whether testing with 11.3.0 or other versions of Electron.

[nornagon]

Looks like this is a crash in the network process:

Crash reason:  EXC_BAD_ACCESS / KERN_INVALID_ADDRESS
Crash address: 0x30
Process uptime: 1 seconds

Thread 7 (crashed)
 0  Electron Framework!SSL_do_handshake [ssl_lib.cc : 204 + 0x0]
    rax = 0x0000000000000000   rdx = 0x0000000116c7c8d2
    rcx = 0x000000000000037f   rbx = 0x0000000000000000
    rsi = 0x00000001111b1b30   rdi = 0x0000000000000000
    rbp = 0x0000700009760780   rsp = 0x0000700009760760
     r8 = 0x0000000000000000    r9 = 0x0000000000000000
    r10 = 0x00000001144d8270   r11 = 0x00007fa77ec71430
    r12 = 0xaaaaaaaaaaaaaaaa   r13 = 0xaaaaaaaaaaaaaaaa
    r14 = 0x00007fa781850d00   r15 = 0x0000700009760790
    rip = 0x0000000111200e9d
    Found by: given as instruction pointer in context
 1  Electron Framework!net::SSLClientSocketImpl::DoHandshake() [ssl_client_socket_impl.cc : 897 + 0x5]
    rbp = 0x0000700009760930   rsp = 0x0000700009760790
    rip = 0x00000001113322a7
    Found by: previous frame's frame pointer
 2  Electron Framework!net::SSLClientSocketImpl::DoHandshakeLoop(int) [ssl_client_socket_impl.cc : 1335 + 0x8]
    rbp = 0x0000700009760980   rsp = 0x0000700009760940
    rip = 0x000000011133120b
    Found by: previous frame's frame pointer
 3  Electron Framework!net::SSLClientSocketImpl::OnVerifyComplete(int) [ssl_client_socket_impl.cc : 1310 + 0x7]
    rbp = 0x00007000097609b0   rsp = 0x0000700009760990
    rip = 0x00000001144c76ab
    Found by: previous frame's frame pointer
 4  Electron Framework!net::CachingCertVerifier::OnRequestFinished(unsigned int, net::CertVerifier::RequestParams const&, base::Time, base::OnceCallback<void (int)>, net::CertVerifyResult*, int) [callback.h : 101 + 0x6]
    rbp = 0x00007000097609e0   rsp = 0x00007000097609c0
    rip = 0x00000001144d82a3
    Found by: previous frame's frame pointer
 5  Electron Framework!base::internal::Invoker<base::internal::BindState<void (net::CachingCertVerifier::*)(unsigned int, net::CertVerifier::RequestParams const&, base::Time, base::OnceCallback<void (int)>, net::CertVerifyResult*, int), base::internal::UnretainedWrapper<net::CachingCertVerifier>, unsigned int, net::CertVerifier::RequestParams, base::Time, base::OnceCallback<void (int)>, net::CertVerifyResult*>, void (int)>::RunOnce(base::internal::BindStateBase*, int) [bind_internal.h : 498 + 0xc]
    rbp = 0x0000700009760a10   rsp = 0x00007000097609f0
    rip = 0x00000001144d8697
    Found by: previous frame's frame pointer
 6  Electron Framework!network::RemoteCertVerifier::OnRemoteResponse(net::CertVerifier::RequestParams const&, net::CertVerifyResult*, int, base::OnceCallback<void (int)>, int, net::CertVerifyResult const&) [network_context.cc : 0 + 0x3]
    rbp = 0x0000700009760a50   rsp = 0x0000700009760a20
    rip = 0x00000001119dbd87
    Found by: previous frame's frame pointer
[...]

346a62fd-790a-4493-bacc-2a85538f8ce9.dmp.zip

[nornagon]

This base::Unretained looks maybe suspicious?

electron/patches/chromium/network_service_allow_remote_certificate_verification_logic.patch

Line 52 in 665ac6f

+ &RemoteCertVerifier::OnRequestFinished, base::Unretained(this),

[nornagon]

Here's a stack trace from a debug build which indicates something different, looks like maybe a threading issue?

[56229:0322/161451.247347:FATAL:lock_impl.h(111)] Check failed: rv == 0 || rv == EBUSY. . Invalid argument
0   Electron Framework                  0x00000001100254b9 base::debug::CollectStackTrace(void**, unsigned long) + 9
1   Electron Framework                  0x000000010ff414a3 base::debug::StackTrace::StackTrace() + 19
2   Electron Framework                  0x000000010ff5afbf logging::LogMessage::~LogMessage() + 175
3   Electron Framework                  0x000000010ff5beae logging::LogMessage::~LogMessage() + 14
4   Electron Framework                  0x000000010b534920 base::internal::BasicAutoLock<base::Lock>::BasicAutoLock(base::Lock&) + 144
5   Electron Framework                  0x000000010ff9c7ff base::SequenceCheckerImpl::CalledOnValidSequence(std::__1::unique_ptr<base::debug::StackTrace, std::__1::default_delete<base::debug::StackTrace> >*) const + 47
6   Electron Framework                  0x000000010ffb6099 base::SupportsUserData::ClearAllUserData() + 25
7   Electron Framework                  0x0000000110439deb net::CertVerifyResult::Reset() + 203
8   Electron Framework                  0x0000000111dcd4df network::RemoteCertVerifier::OnRemoteResponse(net::CertVerifier::RequestParams const&, net::CertVerifyResult*, int, base::OnceCallback<void (int)>, int, net::CertVerifyResult const&) + 127
9   Electron Framework                  0x0000000111dcd6c4 base::internal::Invoker<base::internal::BindState<void (network::RemoteCertVerifier::*)(net::CertVerifier::RequestParams const&, net::CertVerifyResult*, int, base::OnceCallback<void (int)>, int, net::CertVerifyResult const&), base::internal::UnretainedWrapper<network::RemoteCertVerifier>, net::CertVerifier::RequestParams, net::CertVerifyResult*, int, base::OnceCallback<void (int)> >, void (int, net::CertVerifyResult const&)>::RunOnce(base::internal::BindStateBase*, int, net::CertVerifyResult const&) + 100
[...]