380091 - PSM clobbers sender's SMIME capabilities when sending encrypted mail

Status: RESOLVED WONTFIX

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

Description

[Nelson Bolyard (seldom reads bugmail)]

As reported in bug 379753, using a recent trunk build of SeaMonkey,
when I try to send an encrypted message, pipnss calls CERT_SaveSMimeProfile() 
for my cert (sender's cert) with NULL pointers for emailProfile and profileTime. 
 
That causes the SMIME capabilities for my cert in the SMIME profile record in
my cert DB to be erased (set to zero length).  

THAT appears to be the reason why my cert DB doesn't have a valid set of SMIME 
capabilities in the SMIME profile record for my user certs.  And that, in turn, 
is why I cannot send encrypted emails with any ciphers other than the "default" ones.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Further investigation shows that there are exactly two invocations of  
CERT_SaveSMimeProfile in PSM, and BOTH of them pass NULLs for the 
emailProfile and profileTime.   

Those calls are in 
nsNSSCertificate::SaveSMimeProfile() and 
nsNSSCertificateDB::ImportEmailCertificate

So, at the moment, it's unclear to me how valid SMIME capabilities EVER get 
stored in an SMIME profile in the cert DB.

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

There are two invocations of CERT_SaveSMimeProfile in NSS itself.  One in 
sec_pkcs7_verify_signature (which is part of the old PKCS7 decoder that is
no longer used in SMIME, IINM), and the other is in  
NSS_SMIMESignerInfo_SaveSMIMEProfile which is part of the new(er) CMS 
decoder in libSMIME.  This code apparently works OK because we get valid
SMIME capabilities when we receive emails from others, but not for our own
"user" certs.  

The question is: why does PSM want to clobber the SMIME capabilities for
"user" certs?

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

I read a signed email that I had sent to myself.
PSM called NSS_SMIMESignerInfo_SaveSMIMEProfile to save my SMIME
capabilities, and it appeared to do so correctly.
NSS_SMIMESignerInfo_SaveSMIMEProfile called CERT_SaveSMimeProfile
with emailProfile and profileTime pointers that were non-NULL, and
those SECItems had non-zero lengths and non-NULL data pointers.
However, I did not verify the contents of those buffers.

Then I replied.  The compose window came up, and before I could do anything,
PSM called NSS_SMIMESignerInfo_SaveSMIMEProfile again.  Looked good this
time, too.

I selected Security -> Encrypt this message
Then I sent the email.
Immediately, PSM called CERT_SaveSMimeProfile with NULL values for
emailProfile and profileTime.

As an experiment, I shortcut that call, so that it returned without
doing anything, but returned success.
Then PSM called NSS_SMIMEUtil_FindBulkAlgForRecipients.
As a consequence of this one experimental change, alone, I was able
to send an AES_128 encrypted message without further fiddling.

This leads me to contemplate a patch to CERT_SaveSMimeProfile, to have
it simply ignore requests with NULL capabilities.  Not a good solution,
but perhaps an OK stop gap measure until the real problem that causes 
PSM to do this can be fixed.

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

Severity: Major
This is a major problem for S/MIME.  SMIME users cannot send messages 
encrypted with any cipher other than the default ones until this is fixed.

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Attachment: NSS patch to workaround this bug - don't clobber user certs (Checked in on trunk)

I don't really mean for this to be checked in, which is why I'm not requesting
review.  But this patch does workaround the problem enough that I am able to 
send AES_128 encrypted messages.

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 264202 [details] [diff] [review]
NSS patch to workaround this bug - don't clobber user certs (Checked in on trunk)

I committed this workaround on the trunk.  
When this bug is really fixed, the workaround should be backed out.

Comment 7

[Wayne Mery (:wsmwk)]

(In reply to Nelson Bolyard (seldom reads bugmail) from comment #6)
> When this bug is really fixed, the workaround should be backed out.

which bug (#) is that?
has it been fixed yet?

Comment 8

[Wayne Mery (:wsmwk)]

based on Nelson's bug 379753 comment 12, Nelson believes this patch to be a permanent solution, not a workaround, so this should not be backed out, and this bug is no longer needed.