114893 - Can't send encrypted mail if encryption cert has not been imported.

Status: VERIFIED FIXED

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

Description

[Stephane Saux]

Currently, the browser stores "other people" certs when receiving a signed or
encrypted email from them (or they were cc'ed) or by imported it from a url with
the proper mime type.

So if you have certs of you own, but have never read an email signed with these
certs or received an encrypted email using these certs, and you try to send
yourself an encrypted email it will fail. The code will not find your encryption
cert.
nsNSSCertificateDB::GetCertByEmailAddress
doesn't do the right thing.
It may have to also check the user certs to see if there's a match.

To reproduce:
New profile configured with email account, and certs installed.
Do not read any mail.
Compose an email to self.
Get the dialog that you can't send because you don't have the cert for yourself.

Note: using a hardware device (ActivCard).  Not sure whether this has an effect.

Comment 1

[Stephane Saux]

S/MIME bugs are automatically nsbeta1 candidates. (this is a bulk update - there
may be some adjustment of the list).

Comment 2

[Stephane Saux]

kai

Comment 3

[Kai Engert [:KaiE:]]

Stephane, could you please try that again?

I saw that some other bugs are no longer reproducable, where recipient certs
were not found, although they were stored in the database somewhere.

I just tried your exact steps with a new profile, and it worked, although I did
not use a hardware device.

Comment 4

[Stephane Saux]

Tested again using Kai 0510 build, with activeCard. This is very much of a
problem. The encryption cert isnot found when I try and encrypt and email to myself.

Comment 5

[Julien Pierre]

Stephane,

Can you further detail what type of cert you have in your ActivCard ? 
Specifically is it a dual-key cert ? And do you see it show up in both 
the "Yours" and "Other people" tab ?

What I have found in my experiments with activcard is that I could only import 
one cert/key from a p12 into it. This may be a matter of space in the card. In 
any case, I have not had any luck importing both my signing and encryption 
certs.

But this problem does not seem to be specific to a hardware device.
Using the software device, I was able to do the following :
1) start with a fresh db
2) import both my signing/key/certs from a p12
3) don't read any e-mail
4) go to mail preferences and select signing and encryption certs
5) send mail to [e-mail] and set "require encryption"
6) got the pop-up that the application failed to find an encryption certificate 
for [e-mail]

Comment 6

[Julien Pierre]

I set a breakpoint in CERT_FindCertByNicknameOrEmailAddr in mozilla. It was 
looking for the cert for [e-mail] . The function returned one. Yet, 
mozilla still brought up the warning .

I then took the DBs from the browser and tried them with the S/MIME toolkit from 
the command-line. I had no problem sending myself an encrypted e-mail that way. 
I suspect that something is wrong in the mail client code here.

Comment 7

[Julien Pierre]

Stephane and I are hitting different problems. For Stephane the 
CERT_FindCertByNicknameOrEmailAddr returns NULL. For me it succeeds.
Stephane showed me the PSM code following it - it builds a subject list and then 
tries to filter it by usage. This is what fails and causes the browser to 
complain about not having my cert.
The smime toolkit cmsutil command-line tool does not have that code so it does 
not have that problem.
I think the bug may be in the creation of the subject list, because I tried to 
skip over the filtering, and the list was still empty afterwards.

Comment 8

[Julien Pierre]

I added the following code to my build of cmsutil to do the same thing as in the 
client : 

        certlist = CERT_CreateSubjectCertList(
            NULL, CERT_GetDefaultCertDB(), &recipientcerts[i]->derSubject, 
PR_Now(), PR_TRUE);
        if (!certlist)
            goto loser;  

        if (SECSuccess != CERT_FilterCertListByUsage(certlist, 
certUsageEmailRecipient, PR_FALSE))
            goto loser;
  
        if (CERT_LIST_END(CERT_LIST_HEAD(certlist), certlist))
            goto loser;

It turns out that in my case, the CERT_CreateSubjectCertList is returning an 
empty list. This is apparently because I have a mix of expired and the latest 
certificate. It should still return a list containing the valid one, but it does 
not. If I removed all expired certs from the db, it works as expected.
This is however a different bug than originally discovered here, so I will open 
a separate defect for here.

The concern for Stephane case is CERT_FindCertByNicknameOrEmailAddr's failure to 
find the recipient certificate. I will have to take a further look at the 
content of the card and the pkcs#11 code to make a better determination of the 
problem.

Comment 9

[Julien Pierre]

FYI, the NSS bug I filed for the CERT_CreateSubjectCertList problem is 145376 .

Comment 10

[Julien Pierre]

I have reproduced Stephane's environment. I was able to stick my encryption cert 
into my ActivCard (I can only get the encryption cert or the signing cert to 
fit, but never both). If I send mail to myself, I get the same error, until I 
click on an email. Then it works and sends the encrypted message using the card.
The failure is the same as on Stephane's machine, 
CERT_FindCertByNicknameOrEmailAddr returns NULL.

I traced the code and here is why.
The only code that can possibly search other tokens is PK11_FindCertByNickname. 
It actually gets passed the email address. Since that does not contain a colon, 
only the softoken is searched. The recipient cert is not present in the 
database, therefore it is not found.

The solution is probably to search all tokens. However, I notice that the search 
code uses CKA_NETSCAPE_EMAIL in the template to find recipient certs by e-mail. 
This sounds like a Netscape extension to PKCS#11. I hope that the third party 
tokens are able to respond positively to that sort of search. Otherwise it is 
pointless to search them for e-mail certs ...

Comment 11

[Julien Pierre]

I tried it just to make sure. I added some code to search explicitly in all 
other tokens for the email address. Eg. it would do a PK11_FindCertFromNickname 
looking for "ActivCard Gold:[e-mail]" which ends up setting 
[e-mail] in CKA_NETSCAPE_EMAIL in the search template when searching 
the card. Unfortunately that didn't return any object, just as I thought.

The workaround is probably to implement e-mail searches on third party tokens 
via enumerations.

I think it's a little unfortunate that PK11_FindCertByNickname actually searches 
not just by nickname but also by e-mail address. This does not play well with 
other tokens. There should be a PK11_FindCertByEmailAddress . Then 
CERT_FindCertByNicknameOrEmailAddr could try both searches.

Comment 12

[Julien Pierre]

Talked to Bob.

He said we shouldn't look by enumeration because the PKCS#11 token could have a 
large number of certs - in case it's not a smartcard but some other device.

Since there is no other way to do a lookup, he suggested an application-level 
workaround :

At the time when the user selects his e-mail certificate in PSM with the 
smartcard, the application could add an S/MIME record for the e-mail address to 
the softoken .

Comment 13

[Stephane Saux]

adding [adt2 RTM]
Kai, tell me how difficult it is to do that when the cert is selected in the
security pane of the mail/news account preferences.
If it's not hard, I'd like this to go in.

Comment 14

[Kai Engert [:KaiE:]]

1.) You say that you are unable to place both encryption and signing cert on the
smartcard. Julien, is that a problem only with your particular card? Or does
Stephane have the same problem? (Just curious)

2.) What you describe in comment 5 works does not fail for me. Maybe there are
differences between Mozilla branch and Mozilla trunk builds, I'll have to retry.

Comment 15

[Kai Engert [:KaiE:]]

If I understand you correctly, you suggest the following:

- when the user has selected a cert, check whether there is already a S/Mime
profile available for it or not.
- if there is none, add one.

This indeed sounds simple to do.

It should be the same NSS call that we are already using on receiving
certificates in S/Mime messages.

Comment 16

[Julien Pierre]

1) the problem is specific to my card

2) This is also specific to my email certs, because I have certs with 
multiple subjects matching [e-mail] . The S/MIME code in NSS will 
stop looking once it finds a cert with any subject matching the e-mail address. 
If I only import my latest unexpred cert with the current subject, then it 
works. See bug http://bugzilla.mozilla.org/show_bug.cgi?id=145376

As far as your latest comment, you should always replace the S/MIME entry for 
the given e-mail address when the user selects a cert for himself, because that 
cert is always expected to be good / unexpired. The issue with the existing 
entry is that it could be any of the certs in the database matching the user 
email address, and not necessarily a good one, as I have found.

Replacing the S/MIME entry should take care of both my problem with the multiple 
software certs with different subjects and Stephane's problem with not finding 
his e-mail cert with the smartcard.

What it won't solve is if you have a public e-mail cert in a token, as opposed 
to a user cert. That will never be found, as there is no way in PKCS#11 to 
search by e-mail address. But that case should never present itself when 
using Mozilla, since all public e-mail certs are stored in the database when you 
read e-mails.

Comment 17

[Kai Engert [:KaiE:]]

I looked into implementing the fix.

My understanding is: Whenever a user configures which encryption cert to use, we
should call

  SECStatus
  CERT_SaveSMimeProfile(CERTCertificate *cert, SECItem *emailProfile,
                        SECItem *profileTime)

passing in the configured cert as the first parameter. We seem to be allowed to
set both parameters emailProfile and profileTime to NULL.

Julien, do you agree?



But we are only allowed to do that, if the user actually confirms the new
selected certificate by closing the mail preferences with OK. We must not do
that immediately when the new cert gets selected, because the user might cancel
the pref changes, and we would end up with an inappropriately modified cert
database.

We must find a way to hook into the dialog closure event.

If the mail prefs were part of the global browser pref system, this would be
easy, because there is already a mechanism in place to suppport that. We could
register an OK callback function, as it is done e.g. in pref-history.xul.

Unfortunately, the mail pref window seems to work completely different.
One reason is that the global pref window is a static tree, while the account
manager uses a dynamic set of tree nodes, to allow managing a dynamic number of
mail accounts. A difference in behaviour of its implementation is: The
JavaScript contexts of the different pages do not live until the OK button is
hit. They go away as soon as a different page is selected in mail prefs. Because
of that, using a callback to a function within the security tab javascript is
not possible, because the JS context could already have gone away.

I had talked to jaggernaut, he suggested a backup approach, in case the OK
callback does not help me. He suggested we could use a "pref change listener".
However, doing that would require that the name of the pref is known. But for
mail prefs, with the possibility for having multiple accounts, the pref string
does contain a dynamic component, an identifier for a account. Because those
identifiers are not known in advance, and because there could be multiple
accounts, I think this approach does not help us either.


I have two different ideas to implement triggering a save action.

Idea 1)

We could extend the mail pref account manager to allow overlays to register for
executing OK actions.
We could define a new interface
  interface nsIMailConfigChangeListenerService
  {
     void configChanged(string mailAccountID);
  }

The S/Mime extension (and other's if required) would implement this interface.
During construction of the account manager, the S/Mime UI overlay would register
with the mail pref account manager, and inform it about the contract ID, which
implements the interface.

When a user confirms the new prefs using OK, the account manager could iterate
over all known mail accounts, and for each account, it would iterate over each
registered contract ID implementing nsIMailConfigChangeListenerService, and
calling the notification method.

The S/Mime extension would lookup the new confirmed pref for each notified
account, retrieve the configured nickname, lookup the cert, and finally call the
NSS function to save the new profile for that cert.


Idea 2)

In case we decide the above is overkill, or the mail team does not want it, or
does not think anybody else would ever require that, we could use a hack that is
much simpler to implement.

Besides having the configuration, storing the ID of the configured encrypted
cert, we could have another pref in addition (for each mail account), the
LastSavedSMimeProfileCert.

Because the entry in the security database is only required when preparing to
send a message, we could delay doing the actual NSS database change until a
message is sent.

Before doing the message encryption, we could compare the ID of the configured
cert with the new LastSavedSMimeProfileCert ID. When a difference is detected,
we call the NSS function and update the pref.

Comment 18

[Kai Engert [:KaiE:]]

Writing the previous comment helped me to find a simpler solution.

With my previous suggestion, I tried to be smart, and tried to find a solution,
where updating the S/Mime profile would be only be done, if there is indeed a
change detected.

But if there is no argument against it, maybe we should simply save the S/Mime
profile for the sender's encryption cert each time we prepare to send a S/Mime
message?

Comment 19

[Kai Engert [:KaiE:]]

Attachment: Patch v1

Julien, if you agree to my comments, this patch should be reasonable.


Stephane, could you please try this patch?

Comment 20

[Kai Engert [:KaiE:]]

Attachment: Patch v2

Slightly corrected patch, the interface implementation needs to return NS_OK.

Comment 21

[Julien Pierre]

I tried the patch. Unfortunately it does not solve the problem with the
smartcard. I spent quite a lot of time debugging this yesterday.

There is a problem with CERT_SaveSMimeProfile. I have opened a bug on this, 154212.

Comment 22

[Julien Pierre]

This bug is worse than previously thought. The workaround of reading an email to
add the recipient cert to the database only works if the smartcard is not
inserted into the reader. Otherwise the cert is not added to the database.

Comment 23

[Julien Pierre]

Narrowing down the problem :
It is possible to have add the cert to the database from the email while having
the card in the reader, as long as it is the first thing you do after starting
mozilla, before actually logging in to the smartcard.

If you don't have the cert yet in the database, login to the card first, then
click on the email, it fails to add the cert. Going to the PSM "security
devices" page and selecting "log out" in the ActivCard Gold token does not help.
Once you have logged in to the card in Mozilla, the only way to add the cert to
the database is to physically remove the card, then click on the e-mail, or to
exit Mozilla and click on the email.

Comment 24

[Julien Pierre]

Kai,

The conjunction of your latest patch and the NSS patch in 154212 resolves this
problem.

Comment 25

[Julien Pierre]

Kai, Stephane,

Even though this isn't going into mozilla 1.0.1, is this patch going to get
checked into the mozilla trunk ?

Comment 26

[Stephane Saux]

adding patch, review

Comment 27

[Kai Engert [:KaiE:]]

Javi, can you please review?

Comment 28

[Wan-Teh Chang]

Comment on attachment 88934 [details] [diff] [review]
Patch v2

>+NS_IMETHODIMP
>+nsNSSCertificate::SaveSMimeProfile()
>+{
>+  CERT_SaveSMimeProfile(mCert, nsnull, nsnull);
>+  return NS_OK;
>+}

Kai, do you think we should check the return status
of CERT_SaveSMimeProfile?

Comment 29

[Javier Delgadillo]

Comment on attachment 88934 [details] [diff] [review]
Patch v2

saveSMIMEProfile should return an rv.

other than that r=javi

Comment 30

[Kai Engert [:KaiE:]]

That's a reasonable suggestion, thanks. I will attach a new patch as requested,
where the implementation of nsNSSCertificate::SaveSMimeProfile will return
NS_ERROR_FAILURE, if the rv of the NSS function != SECStatus.

But I don't know whether we should have special logic on send, that checks for
that new possible failure - saving the profile might have worked in the past,
and sending the message might still work, so I think we can try to send and
report if that fails. (Unless there exists a good reason, why we should actually
report such a saving failure to the user.)

Comment 31

[Kai Engert [:KaiE:]]

Attachment: Patch v3

Has the discussed change.

Comment 32

[Kai Engert [:KaiE:]]

Comment on attachment 90628 [details] [diff] [review]
Patch v3

carrying forward r=javi

Comment 33

[Kai Engert [:KaiE:]]

Alec, can you please review?

Comment 34

[Alec Flett]

Comment on attachment 90628 [details] [diff] [review]
Patch v3

sr=alecf

Comment 35

[Asa Dotzler [:asa]]

Comment on attachment 90628 [details] [diff] [review]
Patch v3

a=asa (on behalf of drivers) for checkin to the 1.1 trunk.

Comment 36

[Kai Engert [:KaiE:]]

Checked in to trunk.

Comment 37

[Kai Engert [:KaiE:]]

Stephane, could you please verify with a trunk build whether this bug is fixed
for you?

Comment 38

[Charles Rosendahl]

20020808 Trunk Build

New Profiles:

- encrypting using a certificate on the software device works without problem.
- encrypting using a certificate from a hardware token works for me as well.

Verified.