Closed Bug 1342358 Opened 7 years ago Closed 7 years ago

Crash in tls13_DestroyKeyShares

Categories

(NSS :: Libraries, defect)

defect
Not set
critical

Tracking

(firefox51 wontfix, firefox52 wontfix, firefox-esr52 fixed, firefox53 fixed, firefox54 fixed)

RESOLVED FIXED
Tracking Status
firefox51 --- wontfix
firefox52 --- wontfix
firefox-esr52 --- fixed
firefox53 --- fixed
firefox54 --- fixed

People

(Reporter: philipp, Assigned: ttaubert)

References

Details

(Keywords: crash, regression)

Crash Data

This bug was filed from the Socorro interface and is 
report bp-b91ad495-1384-421c-8209-3c9332170224.
=============================================================
Crashing Thread (11)
Frame 	Module 	Signature 	Source
0 	nss3.dll 	tls13_DestroyKeyShares 	security/nss/lib/ssl/tls13con.c:2859
1 	nss3.dll 	ssl3_ResetExtensionData 	security/nss/lib/ssl/ssl3ext.c:468
2 	nss3.dll 	ssl3_DestroySSL3Info 	security/nss/lib/ssl/ssl3con.c:13249
3 	nss3.dll 	ssl_DestroySocketContents 	security/nss/lib/ssl/sslsock.c:404
4 	nss3.dll 	ssl_NewSocket 	security/nss/lib/ssl/sslsock.c:3722
5 	nss3.dll 	ssl_ImportFD 	security/nss/lib/ssl/sslsock.c:1808
6 	nss3.dll 	SSL_ImportFD 	security/nss/lib/ssl/sslsock.c:1840
7 	xul.dll 	nsSSLIOLayerImportFD 	security/manager/ssl/nsNSSIOLayer.cpp:2394
8 	xul.dll 	nsSSLIOLayerAddToSocket(int, char const*, int, nsIProxyInfo*, PRFileDesc*, nsISupports**, bool, unsigned int) 	security/manager/ssl/nsNSSIOLayer.cpp:2584
9 	xul.dll 	nsSSLIOLayerNewSocket(int, char const*, int, nsIProxyInfo*, PRFileDesc**, nsISupports**, bool, unsigned int) 	security/manager/ssl/nsNSSIOLayer.cpp:1861
10 	xul.dll 	nsSSLSocketProvider::NewSocket(int, char const*, int, nsIProxyInfo*, unsigned int, PRFileDesc**, nsISupports**) 	security/manager/ssl/nsSSLSocketProvider.cpp:30
11 	xul.dll 	mozilla::net::nsSocketTransport::BuildSocket(PRFileDesc*&, bool&, bool&) 	netwerk/base/nsSocketTransport2.cpp:1172
12 	xul.dll 	mozilla::net::nsSocketTransport::InitiateSocket() 	netwerk/base/nsSocketTransport2.cpp:1337
13 	xul.dll 	mozilla::net::nsSocketTransport::OnSocketEvent(unsigned int, nsresult, nsISupports*) 	netwerk/base/nsSocketTransport2.cpp:1898
14 	xul.dll 	mozilla::net::nsSocketEvent::Run() 	netwerk/base/nsSocketTransport2.cpp:85
15 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp:1067
16 	xul.dll 	NS_ProcessNextEvent(nsIThread*, bool) 	xpcom/glue/nsThreadUtils.cpp:311
17 	xul.dll 	mozilla::net::nsSocketTransportService::Run() 	netwerk/base/nsSocketTransportService2.cpp:939
18 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp:1067
19 	xul.dll 	NS_ProcessNextEvent(nsIThread*, bool) 	xpcom/glue/nsThreadUtils.cpp:311
20 	xul.dll 	mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp:338
21 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc:225
22 	xul.dll 	MessageLoop::Run() 	ipc/chromium/src/base/message_loop.cc:205
23 	xul.dll 	nsThread::ThreadFunc(void*) 	xpcom/threads/nsThread.cpp:465
24 	nss3.dll 	_PR_NativeRunThread 	nsprpub/pr/src/threads/combined/pruthr.c:397
25 	nss3.dll 	pr_root 	nsprpub/pr/src/md/windows/w95thred.c:95
26 	ucrtbase.dll 	thread_start<unsigned int (__stdcall*)(void*)> 	
27 	kernel32.dll 	BaseThreadInitThunk 	
28 	ntdll.dll 	__RtlUserThreadStart 	
29 	ntdll.dll 	_RtlUserThreadStart

this is a cross-platform crash signature that seems to be regressing in volume since the 51 cycle.
Assignee: nobody → nobody
Component: Security: PSM → Libraries
Product: Core → NSS
Version: 51 Branch → trunk
We seem to fail before we call ssl3_InitExtensionData() [1], probably due to being OOM?

[1] https://searchfox.org/nss/rev/2ddb4fc4cedab461d0464c0fc6a09ecad43b2784/lib/ssl/sslsock.c#3718
The volume of cash for 51 is low and we don't have the plan to have dot release for 51. Mark 51 won't fix.
https://hg.mozilla.org/projects/nss/rev/4e9aee9c5343
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.30
Assignee: nobody → ttaubert
Are we shipping TLS 1.3 in Fx52/ESR52? I'm guessing we'll at least want to backport this to NSS 3.29.x for Gecko 53, but I'm not sure about 3.28.x for Gecko 52.
Flags: needinfo?(ttaubert)
We've disabled TLS 1.3 for Fx52 in bug 1342082, so I think we don't need it in 3.28. We should probably uplift it to 3.29.
Flags: needinfo?(ttaubert)
I'm going to assume that if/when we ever want to try enabling TLS 1.3 on ESR52, we'd end up just taking a newer major version of NSS to support it rather than attempting to do so on 3.28, so calling this wontfix there as well.
We won't enable it on ESR52, but we are definitely going to run experiments on regular 52 with 3.28.

So, we should probably uplift it.
(In reply to Tim Taubert [:ttaubert] from comment #6)
> We've disabled TLS 1.3 for Fx52 in bug 1342082, so I think we don't need it
> in 3.28. We should probably uplift it to 3.29.

Turns out I was wrong. The crash is in code added for TLS 1.3 but we can hit that with TLS 1.3 enabled or not. I agree with Ekr that we should uplift it to 52, it's a simple fix.
Blocks: 1344368
What if NSS 3.28.x is built with NSS_DISABLE_TLS_1_3=1, can that crash, too?
(In reply to Kai Engert (:kaie) from comment #12)
> What if NSS 3.28.x is built with NSS_DISABLE_TLS_1_3=1, can that crash, too?

Yup. As I said above in comment #11. NSS_DISABLE_TLS mostly just affects SSL_LIBRARY_VERSION_MAX_SUPPORTED.
You need to log in before you can comment on or make changes to this bug.