134992 - Recipient encryption certs not found by mail client

Status: VERIFIED FIXED

(MailNews Core :: Security: S/MIME, defect, P1)

Description

[John Unruh]

1.) Get a signing only cert from 
https://lab212sun.mcom.com:444/DirUserEnrollSign.html
2.) Get an encrypting only cert from 
https://lab212sun.mcom.com:444/DirUserEnrollEncrypt.html
3.) Configure the client to send a signed mail. 
4.) Recieve a signed reply, and reply to it with a signed/encrypted reply.
What happens: Sending of mail fails.
Also: Click OK and try again.
What happens: Crash. Incident ID 4750745

Comment 1

[Charles Rosendahl]

Attachment: Cert For JUnruh (Black Helo Cert) is not found within S/MIME

Comment 2

[Charles Rosendahl]

Attachment: Cert For JUnruh (Black Helo Cert) is now found within S/MIME

Comment 3

[Charles Rosendahl]

Old Summary:
Cannot send encrypted email with sign only + encrypt only certs

additional Incident ID 4833325 (identical to that reported by John)

There is no real way to reproduce the problem.  The cert7.db and key3.db files 
attached in either zip files have additional certificates that are not displayed 
in the personal security manager.  I will add more information as I can get it.

It does not appear to have anything to do with the issuer of the certificate or 
the privileges associated with the certificate.  Rather, the cert db appears to 
get garbled, or the mail client is confused trying to read the database.

Comment 4

[John Unruh]

crash

Comment 5

[Stephane Saux]

If the database is corrupted, this is not a good test case.
Charles, can you reproduce the crash using the same methodology (i.e., get
yourself certs from these URLs.)
I have dual key certs and I don't crash.

Comment 6

[Stephane Saux]

In particular, you may want to try and reproduce it using a brand new profile.

Comment 7

[Kai Engert [:KaiE:]]

Charles, why do you think the cert database files are garbled?

Comment 8

[John Unruh]

Adding topcrash keyword - 
http://climate/reports/searchstacksignature.cfm?stacksig=smime3.dll

Comment 9

[Stephane Saux]

top crash. Nominating adt

Comment 10

[Charles Rosendahl]

Word of caution - there are several open defects that generate crashes.  It is
possible that if Bug 136937 is corrected first before these crashes are isolated
and corrected, that the fix for 136937 may mask the underlying problem which
might manifest in some other form during some a different execution path.

Comment 11

[Kai Engert [:KaiE:]]

I looked at the stack trace in the incident ID.

I'm confused:
- why is it that for all stack levels, we have file names and line numbers, but
not for the crash location inside smime3.dll?
- the level below the crash in smime3.dll is
mailnews/compose/src/nsMsgCompose.cpp line 840. But I don't find code in that
area that could do a direct call to smime3.dll.

Anyway, I'll try to reproduce next.

Comment 12

[Kai Engert [:KaiE:]]

I'm unable to reproduce.
Here is what I did:

- use a fresh profile (to simplify things, I usually use a profile, where I have
everything configured already, but I delete all *.db files in that profile
directory before I start the profile

- import the CA cert from https://lab212sun.mcom.com:444/GetCAChain.html
  using the workaround (waiting) as described in bug 137874

- I did what John describes in steps 1, 2 and 3

- I sent the signed message to [e-mail], because my profile was
configured to use that email account

- I received the signed message. It was invalid, because the profile was from
sectest, but the message header said kaie

- I tried to reply encrypted and signed, and sent. This did not work, because
the recipient was listed as [e-mail], but of course I did not have a
certificate for that address.

- I changed the recipient to [e-mail], and sent the message. This
worked. I look in my sent folder, and the message was encrypted and signed.

I can try that multiple times (failure, sending, etc). I don't crash.

Tested with current 1.0.0 branch.

John, in your initial report, you said sending failed. Do you remember why it
failed?

Can you still reproduce?

Comment 13

[Kai Engert [:KaiE:]]

John helped me to reproduce.

The cause of the crash is the fact that nsCMS is not failsafe enough. If it
fails to create a CMS encoder, it will pass NULL pointers to other CMS
functions, and that will crash.

I will attach a patch to fix the crash.

But this does not solve the problem why sending encrypted is not possible, we
should file a separate bug on that, I already have some info for the NSS team.

Comment 14

[Kai Engert [:KaiE:]]

Attachment: Suggested fix for the crash

Comment 15

[Kai Engert [:KaiE:]]

Ok, at least a quick hint why encrypting does not succeed:

I traced the code until the following line, after the code tried to create a
bulk key, but failed. Because that failed, creating a NSSCMS encoder fails.


#0  NSS_CMSEnvelopedData_Encode_BeforeStart (envd=0x8b36080) at cmsenvdata.c:222
	version = 0
	recipientinfos = (NSSCMSRecipientInfo **) 0x8b343e8
	cinfo = (NSSCMSContentInfo *) 0x8b36094
	bulkkey = (PK11SymKey *) 0x0
	bulkalgtag = SEC_OID_RC2_CBC
	type = 258
	slot = (PK11SlotInfo *) 0x881e370
	rv = 1159216884
	dummy = (SECItem *) 0x8b36080
	poolp = (PLArenaPool *) 0x8b5f510
	mark = (void *) 0x0
	i = 2
#1  0x450a504c in NSS_CMSEncoder_Start (cmsg=0x8b36010, outputfn=0x456d8718
<mime_crypto_write_base64(void *, char const *, unsigned long)>,
outputarg=0x8b49cc8, dest=0x0, destpoolp=0x0, pwfn=0, pwfn_arg=0x897e690,
decrypt_key_cb=0, decrypt_key_cb_arg=0x0, detached_digestalgs=0x0,
detached_digests=0x0) at cmsencode.c:552
	p7ecx = (NSSCMSEncoderContext *) 0x8b53340
	rv = 144172688
	cinfo = (NSSCMSContentInfo *) 0x8b36010
#2  0x45058568 in nsCMSEncoder::Start (this=0x8b4aa48, aMsg=0x8acca28,
cb=0x456d8718 <mime_crypto_write_base64(void *, char const *, unsigned long)>,
arg=0x8b49cc8) at /home/kaie/100/mozilla/security/manager/ssl/src/nsCMS.cpp:606
	this = (nsCMSEncoder *) 0x8b4aa48
	cmsMsg = (nsCMSMessage *) 0x8acca28
#3  0x456d6f83 in nsMsgComposeSecure::MimeInitEncryption (this=0x8a6fbf0,
aSign=1, sendReport=0x8a88d40) at
/home/kaie/100/mozilla/mailnews/extensions/smime/src/nsMsgComposeSecure.cpp:643
	rv = 0
	s = 0x0
	L = 197

I'll trace into PK11_KeyGen next, to see where it fails.

Comment 16

[Kai Engert [:KaiE:]]

The code calls nsc_SetupBulkKeyGen with a key length of zero, which leads to
error code 208.

The call comes from CK_RV NSC_GenerateKey, where pMechanism->mechanism is 256.

That value comes from PK11_GetKeyGen(258).

bulkalgtag is SEC_OID_RC2_CBC


#0  nsc_SetupBulkKeyGen (mechanism=256, key_type=0xbfffe780,
key_length=0xbfffe784) at pkcs11c.c:2688
	crv = 208
#1  0x451adcaa in NSC_GenerateKey (hSession=41, pMechanism=0xbfffe7d0,
pTemplate=0xbfffe7e8, ulCount=2, phKey=0x8b2bd4c) at pkcs11c.c:2876
	key = (PK11Object *) 0x8b4c040
	session = (PK11Session *) 0x45131721
	checkWeak = 0
	key_length = 0
	key_type = 17
	objclass = 4
	crv = 0
	cktrue = 1 '\001'
	i = 2
	slot = (PK11Slot *) 0x876e960
	buf =
"Ðæÿ¿°Ê5@\020\000\000\000¬\2362@àæÿ¿\000\000\000\000Ø'8@`:¯\bÈ\204\005E°æÿ¿Ì]7@x\b\n\b\002\000\000\000¼æÿ¿\214]7@<n7@°Ê5@Äæÿ¿\\S4@h\b\n\b°Ê5@\035{2@°Ê5@äèÿ¿\a{2@¬k\tE`:¯\bàÿ\t\b#\000\000\0001024[809ffe0]:
nsCMSEncoder::Start\n\000â\2107@â\2107@
çÿ¿Fä4@~\2127@~\2127@àÿ\t\b8çÿ¿\001Ì2@\\e0@\034èÿ¿\000\000\000\000"...
	key_gen_type = nsc_bulk
	pbe_param = (NSSPKCS5PBEParameter *) 0x40378a7e
	rsa_pms = (SSL3RSAPreMasterSecret *) 0x4032a00e
	version = (CK_VERSION *) 0xbfffe690
	faultyPBE3DES = 0
#2  0x4513413c in PK11_TokenKeyGen (slot=0x881e370, type=258, param=0x0,
keySize=0, keyid=0x0, isToken=0, wincx=0x8b21180) at pk11skey.c:1410
	symKey = (PK11SymKey *) 0x8b2bd48
	genTemplate = {{type = 260, pValue = 0xbfffe7c7, ulValueLen = 1}, {type = 264,
pValue = 0xbfffe7c7, ulValueLen = 1}, {type = 1158843540, pValue = 0x8670b20,
ulValueLen = 1159216884}, {type = 0, pValue = 0xbfffe844, ulValueLen =
1158861094}, {type = 1159219236, pValue = 0x852f9a0, ulValueLen = 1159216884}}
	attrs = (CK_ATTRIBUTE *) 0xbfffe800
	count = 2
	session = 41
	mechanism = {mechanism = 256, pParameter = 0x0, ulParameterLen = 0}
	crv = 1077370316
	weird = 0
	cktrue = 1 '\001'
	ck_key_size = 145701472
#3  0x451341dd in PK11_KeyGen (slot=0x881e370, type=258, param=0x0, keySize=0,
wincx=0x8b21180) at pk11skey.c:1433
No locals.
#4  0x450a57e6 in NSS_CMSEnvelopedData_Encode_BeforeStart (envd=0x8b71730) at
cmsenvdata.c:220
	version = 0
	recipientinfos = (NSSCMSRecipientInfo **) 0x899f7a8
	cinfo = (NSSCMSContentInfo *) 0x8b71744
	bulkkey = (PK11SymKey *) 0x0
	bulkalgtag = SEC_OID_RC2_CBC
	type = 258
	slot = (PK11SlotInfo *) 0x881e370
	rv = 1159216884
	dummy = (SECItem *) 0x8b71730
	poolp = (PLArenaPool *) 0x8b3dce8
	mark = (void *) 0x0
	i = 2

Comment 17

[Kai Engert [:KaiE:]]

1.) Javi, can you please review the crash patch?

2.) Bob, Julien, do you understand why this fails?

Comment 18

[Javier Delgadillo]

Comment on attachment 79835 [details] [diff] [review]
Suggested fix for the crash

r=javi

Comment 19

[Robert Relyea]

Well the failure sounds normal to me (at least at the PKCS #11 layer).

RC2 is a variable key length algorithm, so key length needs to be specified when
you are doing RC2. The key length should be passed into the GenerateKey() call
form the previous layer if the algorithm is RC2, RC5, or RC4 (RC5 currently
isn't supported and RC4 isn't legal for S/MIME).

An interesting question is to determine why we are trying to do RC2 instead of
Triple DES. If we are trying to do RC2-40, we should just fail. We should never
generate a 40 bit S/MIME message anymore.

bob

Comment 20

[Kai Engert [:KaiE:]]

Alec, can you please sr this small patch?

Comment 21

[Kai Engert [:KaiE:]]

Bob,

> RC2 is a variable key length algorithm, so key length needs to be specified when
> you are doing RC2. The key length should be passed into the GenerateKey() call
> form the previous layer if the algorithm is RC2, RC5, or RC4 (RC5 currently
> isn't supported and RC4 isn't legal for S/MIME).

> An interesting question is to determine why we are trying to do RC2 instead of
> Triple DES. If we are trying to do RC2-40, we should just fail. We should never
> generate a 40 bit S/MIME message anymore.


nsCMSMessage::CreateEncrypted calls NSS_SMIMEUtil_FindBulkAlgForRecipients

That function set bulkalgtag=SEC_OID_RC2_CBC and keysize=-1

smime_choose_cipher decides to use that cipher, because a recipient cert is not
> 512.

The keysize of -1 is given to NSS_CMSEnvelopedData_Create and remembers it in
the created data structure.

Then we arrive at the following line, which seems not to be prepared for a
keysize of -1:

bulkkey = PK11_KeyGen(slot, type, NULL, NSS_CMSContentInfo_GetBulkKeySize(cinfo)
/ 8, envd->cmsg->pwfn_arg);

keysize is -1, therefore and (-1/8) seems to get rounded to zero

Comment 22

[Charles Rosendahl]

*** Bug 136998 has been marked as a duplicate of this bug. ***

Comment 23

[Stephane Saux]

Comment on attachment 79835 [details] [diff] [review]
Suggested fix for the crash

Has review as per javi's comment

Comment 24

[Alec Flett]

Comment on attachment 79835 [details] [diff] [review]
Suggested fix for the crash

sr=alecf

Comment 25

[scottputterman]

adding adt1.0.0+.  Please check this into the branch as soon as possible and add
the fixed1.0.0 keyword.

Comment 26

[Kai Engert [:KaiE:]]

Checked into trunk, fixed.

Comment 27

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 79835 [details] [diff] [review]
Suggested fix for the crash

a=[e-mail] for branch checkin

Comment 28

[Charles Rosendahl]

Verified - Crash no longer occurs.  A separate bug has been filed related to the
inability to encrypt using 512 bit keys.

Comment 29

[Kai Engert [:KaiE:]]

Checked in to branch.

Comment 30

[Charles Rosendahl]

See Bug 139882 for problems related to encrypting w/ RC2&512bit keys