1274313 - NSS trunk fails when both NSS_ENABLE_TLS_1_3=1 and NSS_NO_PKCS11_BYPASS=1 are defined

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Kai Engert [:KaiE:]]

When building NSS trunk with both 
  NSS_ENABLE_TLS_1_3=1
  NSS_NO_PKCS11_BYPASS=1
the ssl_gtest crashes.

It works fine when only one of them is defined (regardless which one).

I tried to investigate the crash, but my initial attempts have failed.

I see results which are difficult to explain, and might suggest a memory error.

I've used the following patch for tracing:
https://paste.fedoraproject.org/368505/

I've got these results prior to the crash:
https://paste.fedoraproject.org/368507/

Compare the first line containing ==== with the last one:

===== in ssl3_InitState calling PR_INIT_CLIST, ss:0xbff5f0 &ss->ssl3:0xbff9e8  &cipherSpecs:0xbffd60

===== in SSLInt_CountTls13CipherSpecs, ss:0xbff5f0 &ss->ssl3:0xbff9e8 &cipherSpecs:0xc00090

How could &ss->ssl3.hs.cipherSpecs possibly change, 
if cipherSpecs is a member of hs, which is a member of ssl3, 
and both ss and &ss->ssl3 haven't changed?

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Kai Engert (:kaie) from comment #0)
> How could &ss->ssl3.hs.cipherSpecs possibly change, 
> if cipherSpecs is a member of hs, which is a member of ssl3, 
> and both ss and &ss->ssl3 haven't changed?

The SSL3HandshakeState struct looks different with NO_PKCS11_BYPASS=1:

https://hg.mozilla.org/projects/nss/annotate/c4e46ec959d3/lib/ssl/sslimpl.h#l868

libssl is properly built with the smaller version of the struct, ssl_gtests however doesn't know about that define, includes sslimpl.h, and assumes the bigger version of the struct.

Comment 2

[Tim Taubert [:ttaubert] (inactive)]

Attachment: 0001-Bug-1274313-Ensure-NSS_NO_PKCS11_BYPASS-1-is-propaga.patch

Comment 3

[Eric Rescorla (:ekr)]

Comment on attachment 8759974 [details] [diff] [review]
0001-Bug-1274313-Ensure-NSS_NO_PKCS11_BYPASS-1-is-propaga.patch

Review of attachment 8759974 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM. You should be able to commit it without two more reviews :)

::: coreconf/config.mk
@@ +211,5 @@
>  
> +# Allow disabling PKCS11 bypass.
> +ifdef NSS_NO_PKCS11_BYPASS
> +DEFINES += -DNO_PKCS11_BYPASS
> +endif

LGTM

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Eric Rescorla (:ekr) from comment #3)
> LGTM. You should be able to commit it without two more reviews :)

Yes, that was the plan :) Thanks!

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/projects/nss/rev/a1850f518e18