Closed Bug 336335 Opened 19 years ago Closed 19 years ago

memory leaks in freebl with ECC and RSA cipher suites (CVE-2006-3127)

Categories

(NSS :: Libraries, defect, P1)

Product:
NSS
Component:
Libraries
Version:
3.11
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
3.11.1

People

(Reporter: julien.pierre, Assigned: julien.pierre)

Details

(Keywords: regression)

Attachments

(5 files, 2 obsolete files)

Purify data file
278.92 KB, application/octet-stream
Details
patch for long-standing ECC leak, including Wan-Teh's suggestions
4.17 KB, patch
wtc
: review+
Details | Diff | Splinter Review
new purify leak data
267.83 KB, application/octet-stream
Details
patch for RSA 3.11 regression, as checked in
2.47 KB, patch
wtc
: review+
Details | Diff | Splinter Review
20060505 purify data file
258.96 KB, application/octet-stream
Details
Jason found major memory leaks while running Purify on selfserv. I will attach his purify data file. Here is one of the most common stacks : [W] MLK: Memory leak of 2556928 bytes from 9988 blocks allocated in s_mp_alloc [freebl3.dll] Distribution of leaked blocks 2556928 bytes from 9988 blocks of 256 bytes (first block: 0x003e9d60) Allocation location calloc [C:\WINDOWS\System32\MSVCRT.dll] s_mp_alloc [E:/security/securitytip/builds/20060425.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/mpi/mpi.c:2877] mp_init_size [E:/security/securitytip/builds/20060425.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/mpi/mpi.c:148] mp_init [E:/security/securitytip/builds/20060425.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/mpi/mpi.c:127] ECGroup_new [E:/security/securitytip/builds/20060425.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/ecl/ecl.c:65] ECGroup_consGFp [E:/security/securitytip/builds/20060425.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/ecl/ecl.c:93] ecgroup_fromNameAndHex [E:/security/securitytip/builds/20060425.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/ecl/ecl.c:292] ECGroup_fromName [E:/security/securitytip/builds/20060425.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/ecl/ecl.c:370] EC_ValidatePublicKey [E:/security/securitytip/builds/20060425.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/ec.c:492] EC_ValidatePublicKey [E:/security/securitytip/builds/20060425.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/loader.c:1365] NSC_DeriveKey [E:/security/securitytip/builds/20060425.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/softoken/pkcs11c.c:5508] PK11_PubDeriveWithKDF [E:/security/securitytip/builds/20060425.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/pk11wrap/pk11skey.c:1733] ssl3_HandleECDHClientKeyExchange [E:/security/securitytip/builds/20060425.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/ssl3ecc.c:450] ssl3_HandleClientKeyExchange [E:/security/securitytip/builds/20060425.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/ssl3con.c:6611] ssl3_HandleHandshakeMessage [E:/security/securitytip/builds/20060425.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/ssl3con.c:7686] ssl3_HandleHandshake [E:/security/securitytip/builds/20060425.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/ssl3con.c:7757] ssl3_HandleRecord [E:/security/securitytip/builds/20060425.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/ssl3con.c:8020] ssl3_GatherCompleteHandshake [E:/security/securitytip/builds/20060425.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/ssl3gthr.c:206] ssl_GatherRecord1stHandshake [E:/security/securitytip/builds/20060425.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/sslcon.c:1258] ssl_Do1stHandshake [E:/security/securitytip/builds/20060425.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/sslsecur.c:149] ssl_SecureRecv [E:/security/securitytip/builds/20060425.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/sslsecur.c:1032] ssl_SecureRead [E:/security/securitytip/builds/20060425.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/sslsecur.c:1051] ssl_Read [E:/security/securitytip/builds/20060425.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/sslsock.c:1393] PR_Read [E:/security/securitytip/builds/20060425.1/blowfish_NT4.0_Win95/mozilla/nsprpub/WINNT4.0_DBG.OBJ/pr/src/io/../../../../pr/src/io/priometh.c:141] handle_connection [E:/security/securitytip/builds/20060425.1/blowfish_NT4.0_Win95/mozilla/security/nss/cmd/selfserv/selfserv.c:960] jobLoop [E:/security/securitytip/builds/20060425.1/blowfish_NT4.0_Win95/mozilla/security/nss/cmd/selfserv/selfserv.c:518] thread_wrapper [E:/security/securitytip/builds/20060425.1/blowfish_NT4.0_Win95/mozilla/security/nss/cmd/selfserv/selfserv.c:486] PR_NativeRunThread [E:/security/securitytip/builds/20060425.1/blowfish_NT4.0_Win95/mozilla/nsprpub/WINNT4.0_DBG.OBJ/pr/src/threads/combined/../../../../../pr/src/threads/combined/pruthr.c:436] pr_root [E:/security/securitytip/builds/20060425.1/blowfish_NT4.0_Win95/mozilla/nsprpub/WINNT4.0_DBG.OBJ/pr/src/md/windows/../../../../../pr/src/md/windows/ntthread.c:198]
Attached file Purify data file —
I have seen leaks on Solaris as well. We can't ship any release with ECC unless those memory leaks are resolved.
Severity: normal → blocker
Priority: -- → P1
Target Milestone: --- → 3.11.1
Here is a report from dbx check leaks, for one full handshake, using cipher suite C00E : Checking for memory leaks... Actual leaks report (actual leaks: 12 total size: 3072 bytes) Total Num of Leaked Allocation call stack Size Blocks Block Address ========== ====== =========== ======================================= 256 1 0x81c2588 calloc < s_mp_alloc < mp_init_copy < mp_div < mp_mod < ec_GFp_enc_mont < ECGroup_consGFp_mont < ecgroup_fromNameAndHex < ECGroup_fromName < ec_points_mul < ECDH_Derive < ECDH_Derive < NSC_DeriveKey < PK11_PubDeriveWithKDF < ssl3_HandleECDHClientKeyExchange < ssl3_HandleClientKeyExchange 256 1 0x81c1640 calloc < s_mp_alloc < mp_init_copy < mp_div < mp_mod < ec_GFp_enc_mont < ECGroup_consGFp_mont < ecgroup_fromNameAndHex < ECGroup_fromName < EC_ValidatePublicKey < EC_ValidatePublicKey < NSC_DeriveKey < PK11_PubDeriveWithKDF < ssl3_HandleECDHClientKeyExchange < ssl3_HandleClientKeyExchange < ssl3_HandleHandshakeMessage 256 1 0x81c1b40 calloc < s_mp_alloc < mp_init_copy < mp_div < mp_mod < ec_GFp_enc_mont < ECGroup_consGFp_mont < ecgroup_fromNameAndHex < ECGroup_fromName < EC_ValidatePublicKey < EC_ValidatePublicKey < NSC_DeriveKey < PK11_PubDeriveWithKDF < ssl3_HandleECDHClientKeyExchange < ssl3_HandleClientKeyExchange < ssl3_HandleHandshakeMessage 256 1 0x81c0808 calloc < s_mp_alloc < mp_init_size < mp_init < GFMethod_new < GFMethod_consGFp < GFMethod_consGFp_mont < ECGroup_consGFp_mont < ecgroup_fromNameAndHex < ECGroup_fromName < EC_ValidatePublicKey < EC_ValidatePublicKey < NSC_DeriveKey < PK11_PubDeriveWithKDF < ssl3_HandleECDHClientKeyExchange < ssl3_HandleClientKeyExchange 256 1 0x81c0920 calloc < s_mp_alloc < mp_init_copy < mp_div < mp_mod < ec_GFp_enc_mont < ECGroup_consGFp_mont < ecgroup_fromNameAndHex < ECGroup_fromName < EC_ValidatePublicKey < EC_ValidatePublicKey < NSC_DeriveKey < PK11_PubDeriveWithKDF < ssl3_HandleECDHClientKeyExchange < ssl3_HandleClientKeyExchange < ssl3_HandleHandshakeMessage 256 1 0x81c0e98 calloc < s_mp_alloc < mp_init_size < mp_init < ECGroup_new < ECGroup_consGFp_mont < ecgroup_fromNameAndHex < ECGroup_fromName < EC_ValidatePublicKey < EC_ValidatePublicKey < NSC_DeriveKey < PK11_PubDeriveWithKDF < ssl3_HandleECDHClientKeyExchange < ssl3_HandleClientKeyExchange < ssl3_HandleHandshakeMessage < ssl3_HandleHandshake 256 1 0x81c26a0 calloc < s_mp_alloc < mp_init_copy < mp_div < mp_mod < ec_GFp_enc_mont < ECGroup_consGFp_mont < ecgroup_fromNameAndHex < ECGroup_fromName < ec_points_mul < ECDH_Derive < ECDH_Derive < NSC_DeriveKey < PK11_PubDeriveWithKDF < ssl3_HandleECDHClientKeyExchange < ssl3_HandleClientKeyExchange 256 1 0x81c2c18 calloc < s_mp_alloc < mp_init_copy < mp_div < mp_mod < ec_GFp_enc_mont < ECGroup_consGFp_mont < ecgroup_fromNameAndHex < ECGroup_fromName < ec_points_mul < ECDH_Derive < ECDH_Derive < NSC_DeriveKey < PK11_PubDeriveWithKDF < ssl3_HandleECDHClientKeyExchange < ssl3_HandleClientKeyExchange 256 1 0x81c29e8 calloc < s_mp_alloc < mp_init_size < mp_init < ECGroup_new < ECGroup_consGFp_mont < ecgroup_fromNameAndHex < ECGroup_fromName < ec_points_mul < ECDH_Derive < ECDH_Derive < NSC_DeriveKey < PK11_PubDeriveWithKDF < ssl3_HandleECDHClientKeyExchange < ssl3_HandleClientKeyExchange < ssl3_HandleHandshakeMessage 256 1 0x81c2b00 calloc < s_mp_alloc < mp_init_size < mp_init < GFMethod_new < GFMethod_consGFp < GFMethod_consGFp_mont < ECGroup_consGFp_mont < ecgroup_fromNameAndHex < ECGroup_fromName < ec_points_mul < ECDH_Derive < ECDH_Derive < NSC_DeriveKey < PK11_PubDeriveWithKDF < ssl3_HandleECDHClientKeyExchange 256 1 0x81ca198 calloc < s_mp_alloc < mp_init_copy < mp_div < mp_mod < ec_GFp_enc_mont < ECGroup_consGFp_mont < ecgroup_fromNameAndHex < ECGroup_fromName < EC_ValidatePublicKey < EC_ValidatePublicKey < NSC_DeriveKey < PK11_PubDeriveWithKDF < ssl3_HandleECDHClientKeyExchange < ssl3_HandleClientKeyExchange < ssl3_HandleHandshakeMessage 256 1 0x81c5930 calloc < s_mp_alloc < mp_init_copy < mp_div < mp_mod < ec_GFp_enc_mont < ECGroup_consGFp_mont < ecgroup_fromNameAndHex < ECGroup_fromName < ec_points_mul < ECDH_Derive < ECDH_Derive < NSC_DeriveKey < PK11_PubDeriveWithKDF < ssl3_HandleECDHClientKeyExchange < ssl3_HandleClientKeyExchange Possible leaks report (possible leaks: 6 total size: 4406 bytes) Total Num of Leaked Allocation call stack Size Blocks Block Address ========== ====== =========== ======================================= 2071 1 0x81c7fe8 PR_Malloc < PL_ArenaAllocate < PORT_ArenaAlloc < CERT_DupCertList < ssl_DupSocket < ssl_Accept < PR_Accept < do_accepts < server_main < main 2071 1 0x81c4ec0 PR_Malloc < PL_ArenaAllocate < PORT_ArenaAlloc < CERT_DupCertList < ssl_DupSocket < ssl_Accept < PR_Accept < do_accepts < server_main < main 88 1 0x81b6c48 calloc < PR_Calloc < PR_NewLock < PORT_NewArena < CERT_DupCertList < ssl_DupSocket < ssl_Accept < PR_Accept < do_accepts < server_main < main 88 1 0x81a0908 calloc < PR_Calloc < PR_NewLock < PORT_NewArena < CERT_DupCertList < ssl_DupSocket < ssl_Accept < PR_Accept < do_accepts < server_main < main 88 2 - calloc < PR_Calloc < PORT_ZAlloc < PORT_NewArena < CERT_DupCertList < ssl_DupSocket < ssl_Accept < PR_Accept < do_accepts < server_main < main
It looks like the leaks are all within softoken, in NSC_DeriveKey doing ECDH_Derive to get the SSL pre-master secret. The problem does not appear to be the SSL code. libssl calls PK11_PubDeriveWithKDF, then calls PK11_FreeSymKey to destroy it once it's done with the value. But that call does not free the softoken/freebl memory associated with the object.
Attached patch fix (obsolete) — — Splinter Review
This patch fixes at least the leaks I have observed running cipher suite C00E manually in dbx .I will review the other leak stacks in the purify data file to find if other fixes are necessary.
Assignee: nobody → julien.pierre.bugs
Status: NEW → ASSIGNED
Attachment #220715 - Flags: superreview?(wtchang)
Attachment #220715 - Flags: review?(douglas)
I have reviewed all the suspect leaking stacks from the purify data file, and they all appear related to the bugs fixed by this patch. However, Jason said that he didn't run all the cipher suites once he ran into the leak with C004. I have verified that both C004 and C00E have no leak with the patch. I would like to check it in tonight so that Jason can run purify on tomorrow's build. Wan-Teh, could you please review it ? Thanks.
Comment on attachment 220715 [details] [diff] [review] fix r=wtc. I have a question for Douglas and two suggested changes. 1. Douglas, do we ever set group->constructed or meth->constructed to MP_NO? If not, I suggest that we remove these two fields to simplify our code. For example, in ECGroup_free we have: void ECGroup_free(ECGroup *group) { if (group == NULL) return; GFMethod_free(group->meth); if (group->constructed == MP_NO) return; I don't know if the GFMethod_free call should be moved after the test for group->constructed == MP_NO. Compare this code to GFMethod_free, which tests for meth->constructed == MP_NO first. 2. In ECGroup_new, we need to initialize group->meth and group->extra_free to NULL before the first MP_CHECKOK call. Otherwise we may go to CLEANUP with uninitialized group->meth and group->extra_free. I suggest you do this as follows: - add "group->meth = NULL;" before "group->text = NULL;". - Move all the MP_CHECKOK calls after "group->extra_free = NULL;". 3. In GFMethod_new, change MP_CHECKOK(mp_init(&meth->irr)); meth->extra_free = NULL; to MP_DIGITS(&meth->irr) = 0; meth->extra_free = NULL; MP_CHECKOK(mp_init(&meth->irr));
Attachment #220715 - Flags: superreview?(wtchang) → superreview+
As checked in to the tip. Checking in ecl.c; /cvsroot/mozilla/security/nss/lib/freebl/ecl/ecl.c,v <-- ecl.c new revision: 1.10; previous revision: 1.9 done Checking in ecl_curve.c; /cvsroot/mozilla/security/nss/lib/freebl/ecl/ecl_curve.c,v <-- ecl_curve.c new revision: 1.3; previous revision: 1.2 done Checking in ecl_gf.c; /cvsroot/mozilla/security/nss/lib/freebl/ecl/ecl_gf.c,v <-- ecl_gf.c new revision: 1.6; previous revision: 1.5 done 3.11 branch : Checking in ecl.c; /cvsroot/mozilla/security/nss/lib/freebl/ecl/ecl.c,v <-- ecl.c new revision: 1.5.2.4; previous revision: 1.5.2.3 done Checking in ecl_curve.c; /cvsroot/mozilla/security/nss/lib/freebl/ecl/ecl_curve.c,v <-- ecl_curve.c new revision: 1.2.28.2; previous revision: 1.2.28.1 done Checking in ecl_gf.c; /cvsroot/mozilla/security/nss/lib/freebl/ecl/ecl_gf.c,v <-- ecl_gf.c new revision: 1.2.28.2; previous revision: 1.2.28.1 done 3.11.1 branch : Checking in ecl.c; /cvsroot/mozilla/security/nss/lib/freebl/ecl/ecl.c,v <-- ecl.c new revision: 1.5.2.2.2.1; previous revision: 1.5.2.2 done Checking in ecl_curve.c; /cvsroot/mozilla/security/nss/lib/freebl/ecl/ecl_curve.c,v <-- ecl_curve.c new revision: 1.2.48.1; previous revision: 1.2 done Checking in ecl_gf.c; /cvsroot/mozilla/security/nss/lib/freebl/ecl/ecl_gf.c,v <-- ecl_gf.c new revision: 1.2.28.1.2.1; previous revision: 1.2.28.1 done
Attachment #220715 - Attachment is obsolete: true
Attachment #220729 - Flags: review?(douglas)
Attachment #220715 - Flags: review?(douglas)
Attachment #220729 - Flags: review+
Attached file new purify leak data —
Jason reran the tests. I looked at the new purify result. Here is still a suspect stack which is not in ECC but in RSA signature : [W] MLK: Memory leak of 1024 bytes from 2 blocks allocated in s_mp_alloc [freebl3.dll] Distribution of leaked blocks Allocation location calloc [C:\WINDOWS\System32\MSVCRT.dll] s_mp_alloc [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/mpi/mpi.c:2877] mp_init_size [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/mpi/mpi.c:148] mp_exptmod_safe_i [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/mpi/mpmontg.c:949] mp_exptmod [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/mpi/mpmontg.c:1196] rsa_PrivateKeyOpCRTCheckedPubKey [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/rsa.c:500] rsa_PrivateKeyOp [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/rsa.c:739] RSA_PrivateKeyOpDoubleChecked [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/rsa.c:778] RSA_PrivateKeyOpDoubleChecked [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/loader.c:394] RSA_Sign [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/softoken/rsawrapr.c:465] NSC_Sign [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/softoken/pkcs11c.c:2067] PK11_Sign [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/pk11wrap/pk11obj.c:766] ssl3_SignHashes [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/ssl3con.c:818] ssl3_SendECDHServerKeyExchange [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/ssl3ecc.c:832] ssl3_SendServerKeyExchange [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/ssl3con.c:6150] ssl3_SendServerHelloSequence [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/ssl3con.c:5338] ssl3_HandleV2ClientHello [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/ssl3con.c:5947] ssl2_HandleClientHelloMessage [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/sslcon.c:3472] ssl_Do1stHandshake [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/sslsecur.c:149] ssl_SecureRecv [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/sslsecur.c:1032] ssl_SecureRead [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/sslsecur.c:1051] ssl_Read [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/sslsock.c:1393] PR_Read [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/nsprpub/WINNT4.0_DBG.OBJ/pr/src/io/../../../../pr/src/io/priometh.c:141] handle_connection [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/cmd/selfserv/selfserv.c:960] jobLoop [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/cmd/selfserv/selfserv.c:518] thread_wrapper [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/cmd/selfserv/selfserv.c:486] PR_NativeRunThread [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/nsprpub/WINNT4.0_DBG.OBJ/pr/src/threads/combined/../../../../../pr/src/threads/combined/pruthr.c:436] pr_root [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/nsprpub/WINNT4.0_DBG.OBJ/pr/src/md/windows/../../../../../pr/src/md/windows/ntthread.c:198] It seems odd that there are only 2 blocks. Here is another suspicious stack, with just one block : [W] MLK: Memory leak of 512 bytes from 1 block allocated in s_mp_alloc [freebl3.dll] Distribution of leaked blocks Allocation location calloc [C:\WINDOWS\System32\MSVCRT.dll] s_mp_alloc [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/mpi/mpi.c:2877] mp_init_size [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/mpi/mpi.c:148] mp_exptmod_safe_i [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/mpi/mpmontg.c:949] mp_exptmod [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/mpi/mpmontg.c:1196] generate_blinding_params [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/rsa.c:552] init_blinding_params [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/rsa.c:582] get_blinding_params [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/rsa.c:654] rsa_PrivateKeyOp [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/rsa.c:727] RSA_PrivateKeyOpDoubleChecked [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/rsa.c:778] RSA_PrivateKeyOpDoubleChecked [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/loader.c:394] RSA_Sign [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/softoken/rsawrapr.c:465] NSC_Sign [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/softoken/pkcs11c.c:2067] PK11_Sign [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/pk11wrap/pk11obj.c:766] ssl3_SignHashes [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/ssl3con.c:818] ssl3_SendECDHServerKeyExchange [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/ssl3ecc.c:832] ssl3_SendServerKeyExchange [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/ssl3con.c:6150] ssl3_SendServerHelloSequence [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/ssl3con.c:5338] ssl3_HandleV2ClientHello [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/ssl3con.c:5947] ssl2_HandleClientHelloMessage [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/sslcon.c:3472] ssl_Do1stHandshake [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/sslsecur.c:149] ssl_SecureRecv [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/sslsecur.c:1032] ssl_SecureRead [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/sslsecur.c:1051] ssl_Read [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/sslsock.c:1393] PR_Read [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/nsprpub/WINNT4.0_DBG.OBJ/pr/src/io/../../../../pr/src/io/priometh.c:141] handle_connection [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/cmd/selfserv/selfserv.c:960] jobLoop [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/cmd/selfserv/selfserv.c:518] thread_wrapper [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/cmd/selfserv/selfserv.c:486] PR_NativeRunThread [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/nsprpub/WINNT4.0_DBG.OBJ/pr/src/threads/combined/../../../../../pr/src/threads/combined/pruthr.c:436] pr_root [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/nsprpub/WINNT4.0_DBG.OBJ/pr/src/md/windows/../../../../../pr/src/md/windows/ntthread.c:198] Another: [W] MLK: Memory leak of 512 bytes from 2 blocks allocated in s_mp_alloc [freebl3.dll] Distribution of leaked blocks Allocation location calloc [C:\WINDOWS\System32\MSVCRT.dll] s_mp_alloc [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/mpi/mpi.c:2877] mp_init_size [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/mpi/mpi.c:148] mp_exptmod_safe_i [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/mpi/mpmontg.c:949] mp_exptmod [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/mpi/mpmontg.c:1196] rsa_PrivateKeyOpCRTNoCheck [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/rsa.c:451] rsa_PrivateKeyOpCRTCheckedPubKey [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/rsa.c:496] rsa_PrivateKeyOp [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/rsa.c:739] RSA_PrivateKeyOpDoubleChecked [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/rsa.c:778] RSA_PrivateKeyOpDoubleChecked [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/freebl/loader.c:394] RSA_Sign [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/softoken/rsawrapr.c:465] NSC_Sign [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/softoken/pkcs11c.c:2067] PK11_Sign [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/pk11wrap/pk11obj.c:766] ssl3_SignHashes [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/ssl3con.c:818] ssl3_SendECDHServerKeyExchange [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/ssl3ecc.c:832] ssl3_SendServerKeyExchange [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/ssl3con.c:6150] ssl3_SendServerHelloSequence [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/ssl3con.c:5338] ssl3_HandleV2ClientHello [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/ssl3con.c:5947] ssl2_HandleClientHelloMessage [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/sslcon.c:3472] ssl_Do1stHandshake [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/sslsecur.c:149] ssl_SecureRecv [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/sslsecur.c:1032] ssl_SecureRead [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/sslsecur.c:1051] ssl_Read [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/lib/ssl/sslsock.c:1393] PR_Read [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/nsprpub/WINNT4.0_DBG.OBJ/pr/src/io/../../../../pr/src/io/priometh.c:141] handle_connection [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/cmd/selfserv/selfserv.c:960] jobLoop [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/cmd/selfserv/selfserv.c:518] thread_wrapper [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/security/nss/cmd/selfserv/selfserv.c:486] PR_NativeRunThread [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/nsprpub/WINNT4.0_DBG.OBJ/pr/src/threads/combined/../../../../../pr/src/threads/combined/pruthr.c:436] pr_root [E:/security/securityjes5/builds/20060504.1/blowfish_NT4.0_Win95/mozilla/nsprpub/WINNT4.0_DBG.OBJ/pr/src/md/windows/../../../../../pr/src/md/windows/ntthread.c:198] These leak stacks were not showing in yesterday's report. To me, they may indicate that some SSL connections were still not closed at the time the leak report was taken. Jason, do you know if that might have been the case ? All the other leak stacks are in the builtins (which have known unfixed leaks :() or in 1-time initialization allocation that don't have corresponding frees, such as the RSA stepdown keygen, but those are OK.
All the strsclnt commands had finished. The tstclnt connection may have still been open when selfserv terminated due to the "GET /stop HTTP/1.0". I noticed that I have to kill the tstclnts with a ctrl-c.
Jason, Were you doing session reuse with strsclnt or not during the test ? Perhaps the small number of leaks is explained if a small number of full handshakes occurred. The RSA signing leak looks legit. But it would happen every time the function is executed. A temporary bignum is created, and never freed. I will create a patch.
Attached patch free temporary bignum (obsolete) — — Splinter Review
Attachment #220862 - Flags: review?(wtchang)
Comment on attachment 220862 [details] [diff] [review] free temporary bignum r=wtc. This patch is correct. But more changes are needed to make it safe to goto CLEANUP on error. Please move the following seven "MP_DIGITS(&xxx) = 0;" statements to the beginning of the function: MP_DIGITS(&accum1) = 0; MP_DIGITS(&accum2) = 0; MP_DIGITS(&accum[0]) = 0; MP_DIGITS(&accum[1]) = 0; MP_DIGITS(&accum[2]) = 0; MP_DIGITS(&accum[3]) = 0; and MP_DIGITS(&tmp) = 0; They need to be before the code that allocates powersArray.
Attachment #220862 - Flags: review?(wtchang) → review+
Thanks for the quick review, Wan-Teh . I have checked in the fix to NSS_3_11_1_BRANCH : Checking in mpi/mpmontg.c; /cvsroot/mozilla/security/nss/lib/freebl/mpi/mpmontg.c,v <-- mpmontg.c new revision: 1.17.2.1.2.1; previous revision: 1.17.2.1 done NSS_3_11_BRANCH : Checking in mpi/mpmontg.c; /cvsroot/mozilla/security/nss/lib/freebl/mpi/mpmontg.c,v <-- mpmontg.c new revision: 1.17.2.2; previous revision: 1.17.2.1 done And the tip : Checking in mpi/mpmontg.c; /cvsroot/mozilla/security/nss/lib/freebl/mpi/mpmontg.c,v <-- mpmontg.c new revision: 1.19; previous revision: 1.18 done
Attachment #220862 - Attachment is obsolete: true
Attachment #220867 - Flags: review+
Jason, Please provide more information about how you are running the leak tests. I'm concerned because the RSA leak showed in thursday's purify report but not in wednesday's report. Unlike the ECC leak, I don't actually have a test case for this leak, although I did not make any effort to create one. I would like to know which exact selfserv and strsclnt commands you ran yesterday that led to finding this RSA leak. Thanks.
Here are the details of my recent purify work. The first time I did the tests, I only ran one strsclnt. Today and yesterday's work match the following writeup. whitetiger.sfbay.sun.com is running Windows XP Professional and has the Cygwin environment and Purify installed. Windows XP Professional version is 5.1.2600 Service Pack 2 Build 2600. The machine has K8T Master2-FAR motherboard, AMD Opteron processor at roughly 1.6 gigahertz, and 1024 megabytes of memory. Cygwin version is 1.5.19. Purify version is 2003.06.13.402.000 Build 5352. Builds taken from: /share/builds/mccrel3/security/securityjes5/ \ builds/<date>.1/blowfish_NT4.0_Win95. Build is placed into a tar file generated on mace.red.iplanet.com then extracted onto whitetiger into C:\<release> aka /cygdrive/c/<release> with release being m2 for the 20060503 build, m3 for the 20060504 build, and m4 for the 20060505 build The initial environment was generated with the 20060503 build with common.sh edited so all.sh would run. All the tests passed. For the subsequent builds, the whitetiger.20 test directory was copied over to each build's test_results area. The same set of certificates was used for all the Purify runs. Tests are run with two Cygwin shell windows with the following environment enviroment variables: PWD is /cygdrive/c/<release>/mozilla/tests_results/security/whitetiger.20 e.g. /cygdrive/c/m4/mozilla/tests_results/security/whitetiger.20 NSS_DISABLE_FREE_ARENA_LIST=1 DOMSUF=sfbay.sun.com NSS_ENABLE_ECC=1 NSS_STRICT_SHUTDOWN=1 PATH is setup to have <release>/mozilla/dist/WINNT4.0_DBG.OBJ/bin and <release>/mozilla/dist/WINNT4.0_DBG.OBJ/lib in it. Window one runs both purify and selfserv. purify selfserv -D -p 8443 -d ext_server -e whitetiger.sfbay.sun.com-ec \ -n whitetiger.sfbay.sun.com -w nss -c :C004:C009:C013:C00E -S Window two runs all the strsclnts and the finishing tstclnt. strsclnt -p 8443 -d ext_client -w nss -c 10000 -C :C009 -T -N whitetiger.sfbay.sun.com strsclnt -p 8443 -d ext_client -w nss -c 10000 -C :C013 -T whitetiger.sfbay.sun.com strsclnt -p 8443 -d ext_client -w nss -c 10000 -C :C004 -N whitetiger.sfbay.sun.com strsclnt -p 8443 -d ext_client -w nss -c 10000 -C :C013 whitetiger.sfbay.sun.comtstclnt -p 8443 -d ext_client -w nss -c :C004 -h whitetiger.sfbay.sun.com GET /stop HTTP/1.0 <two carriage returns> <ctrl-c> (Needed for tstclnt to exit.) Afterwards, the pfy file is copied via ftp to my workstation for general dispersal.
Thanks, Jason. You are running cipher suite C013 with session reuse (no -N argument on strsclnt to force full handshakes), twice. That is cipher suite TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA . That means you have 2 full handshake requests with that cipher suites, and 19998 restart handshakes. The 2 full handshake are probably the ones that showed the leak. I will try to verify that between yesterday and today's build, using dbx check leaks. I reviewed the latest purify data file. Everything looks good. Only the nssckbi (built-ins) known leaks are showing up. I was somewhat surprised not to see the leak for the RSA stepdown keys on this run. The stepdown keygen can only be disabled with the -E argument to selfserv, which you weren't using. Perhaps the stepdown keys actually got freed during shutdown in today's run and not the previous 2 days, but I can't really explain the discrepancy. That is somewhat strange, but lack of this leak on today's run is not a showstopper. I think we are finally good to go for 3.11.1 as far as memory leaks are concerned.
> tstclnt -p 8443 -d ext_client -w nss -c :C004 -h whitetiger.sfbay.sun.com tstclnt command should use the -f option for https client simulation, especially on Windows. > GET /stop HTTP/1.0 > <two carriage returns> > <ctrl-c> (Needed for tstclnt to exit.) control-C will kill tstclnt. You want to send it an "end of file", so that it will terminate normally. That's ctrl-D on Unix or ctrl-Z on windows.
Comment on attachment 220867 [details] [diff] [review] patch for RSA 3.11 regression, as checked in This bug reports a large leak that is seen only with ECC cipher suites. But this patch fixes a leak in an integer (bignum) modular exponentiation routine that is used by RSA (on some platforms only) and *might* also be used for ECC (not sure). I'd expect this modular exponentiation leak to occur equally for all users of this function. And I'd expect the patch to cure that particular leak for all users of this function. So, if this patch really does fix a leak that is seen only in ECC tests, I'd like to understand why the leak is not also seen in RSA tests.
Nelson, Unfortunately, the memory leak checks were not run on all the RSA cipher suites, only the ECC ones, as Jason reported. We were under pressure to release, and we had to draw the line somewhere. We had much more leak coverage for 3.11.1 than for 3.11, but we have a long way to go before we have proper leak testing. This is something we have to address for our next point release. You are correct that all callers of this function are affected equally by the memory leak. All RSA cipher suites are affected on the server-side, since RSA_PrivateKeyOp ends up calling this leaking function. Keygen is also affected.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Summary: memory leaks in selfserv with ECC cipher suites → memory leaks in selfserv with ECC and RSA cipher suites
The memory leak in the RSA code that Julien reported in comment 9 is a regression introduced in NSS 3.11 (rev. 1.16 of mpi/mpmontg.c) by Bob's "weaving patch" (bug 298630).
Keywords: regression
Summary: memory leaks in selfserv with ECC and RSA cipher suites → memory leaks in freebl with ECC and RSA cipher suites
Attachment #220867 - Attachment description: patch as checked in → patch for RSA 3.11 regression, as checked in
Attachment #220729 - Attachment description: fix including Wan-Teh's suggestions → patch for long-standing ECC leak, including Wan-Teh's suggestions
Attachment #220729 - Flags: review?(douglas)
Summary: memory leaks in freebl with ECC and RSA cipher suites → memory leaks in freebl with ECC and RSA cipher suites (CVE-2006-3127)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: