130692 - Find matching certificate using case insensitive email address

Status: VERIFIED FIXED

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

Description

[Kai Engert [:KaiE:]]

A user reported on the newsgroups:

"I tried sending email with the first letter caps ([e-mail]) and it says I
dont have a cert .. but if I put [e-mail] ... it works just fine .. I
wouldn't have thought that case would have to do anything with it .. ????"


Are we allowed to treat email addresses as case insensitive?

I looked at 
  http://www.ietf.org/rfc/rfc2822.txt

It doesn't seem to say that one is allowed to ignore the case.

In addition, I searched Google
  http://www.google.com/search?q=%22case+sensitive%22+%22email+addresses%22&safe=off

The pages I find say, the local part of an email address is allowed to be case
sensitive.

So, I fear, there might be systems in the world, where an exact case match is
required.

We have the choice, whether we want to be picky and case sensitive, or whether
we behave user friendly.

I wonder whether it is a situation that only exists in theory:
If there were a domain, where two different email addresses user@domain and
USER@domain existed, we risk the chance of encrypting for the wrong recipient.

Comment 1

[Yannick Koehler]

The suggestion could be to ask the end-user with the 'possibility'.  Also I
found a option called Client Certificate Selection which ask the user to decide
of the cert selection should be done auto or manual (it seems to be only for web
sites but you could extend to email).  If auto, then either prompt or pick the
case-insensitive.

Comment 2

[Kai Engert [:KaiE:]]

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

Comment 3

[Stephane Saux]

Kai.

Comment 4

[Stephane Saux]

In rfc822, the only part talking about case-insensitivity is the "postmater"
reserverd local-part, which must be parsed in a case-insensitive manner. This
implies that "normal" local-parts are not case-insensitive.

However, it appears that the widespread usage is to make the local part of the
name case insensitive. The domain part of the name is always case-insensitive.

cc jgmyers, thayes.
I propose that we do two things:
When looking for a cert in the local db, we make a case insensitive search.
When matching an email address in a cert to the email address from the Header
fields, we also use a case-insensitive match (for signature validation).
(see bug 112640)

Comment 5

[Daniele Raffo]

The username part of an email address is always case sensitive. I think this
come from the fact that on a Unix machine, the email address of an user is
userlogin@machine, and Unix OS is case-sensitive.  

Comment 6

[Lapo Luchini]

I agree that it must be case-sensitive but partly it isn't: it says a
certificate is valid also with non-matching-case.
Either doesn't count that as valid or also take that as valid for encrypting ;)

Comment 7

[Charles Rosendahl]

My recommendation:  
Make it case-insensitive.  Fact is most IS departments and corporate policies
dictate that the name must be unique, regardless of case (AOL for instance - I
couldn't get Crosen, cRosen, cROSEN, etc. as a account name because crosen was
already taken.)

This is a technical issue that has little bearing on the practicality of use.

Comment 8

[Charles Rosendahl]

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

Comment 9

[Ilan Finci]

Some more info (look for the full report in my bug report 139107):

I have a certificate for the address [e-mail] (this is the exact
address appear in the certificate when I view it).

When I try to send encrypted mail to this address, written exactly as above - it
fails finding the certificate.

If I send to [e-mail] (all lower case) - it finds the certificate.

So it seems there is something strange in the matching (maybe it make the
certificate address all lower case and only then compare, without doing the same
to the address from the header).

Comment 10

[Charles Rosendahl]

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

Comment 11

[Stephane Saux]

We'll implement as per comment #4

Comment 12

[Dave Roberts]

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

Comment 13

[Kai Engert [:KaiE:]]

Crypto in Mozilla depends on the lower level crypto library NSS. Whatever we
decide to be approriate, the lower level library must support if first.

NSS has the hard coded behaviour to convert email addresses in certs to lower
case. Seen in the implementation of CERT_GetCertificateEmailAddress in file
alg1485.c and nsslowcert_FixupEmailAddr in lowcert.c.

As long as we rely on NSS' internal functionatity for looking up certs, I think
we need to stick to this behaviour.

For now this means, whenever we try to lookup certs, we should convert the email
address to lowercase before querying NSS, I think.

This strategy looks like it will work with all cases reported in the duplicate
bugs of this one.

Comment 14

[Kai Engert [:KaiE:]]

Attachment: Suggested fix

Lowercase conversion of recipient's email addresses before lookup up in NSS
databases.

Comment 15

[Kai Engert [:KaiE:]]

Javi, can you please review?

Comment 16

[Javier Delgadillo]

Comment on attachment 81696 [details] [diff] [review]
Suggested fix

looks good.

r=javi

Comment 17

[Kai Engert [:KaiE:]]

Jag, can you please sr= the string logic code?

Comment 18

[jag (Peter Annema)]

Comment on attachment 81696 [details] [diff] [review]
Suggested fix

nsCString mailbox_lowercase;
ToLowerCase(nsDependentCString(mailbox), mailbox_lowercase);

Avoids one copy.

Same for the other file.

sr=jag with that change

Comment 19

[Kai Engert [:KaiE:]]

Jag, when I use your alternative code, it does not work for me. It looks like
the ToLowerCase function taking two arguments is broken.

When I dump the contents after the call, the target string has random contents,
while the source string is correct.

In addition, I don't understand why your alternative suggestion avoids one copy.
I would have expected that ToLowerCase taking one argument does not make another
copy, but just modifies the string in place?

Comment 20

[Kai Engert [:KaiE:]]

Ok, found the problem, this is blocked by 141317.

Comment 21

[Kai Engert [:KaiE:]]

Attachment: Updated fix as requested by Jag

Comment 22

[Kai Engert [:KaiE:]]

Comment on attachment 81761 [details] [diff] [review]
Updated fix as requested by Jag

moving over reviews

Comment 23

[Kai Engert [:KaiE:]]

Checked in to trunk, fixed.
Nominating for ADT.

Comment 24

[Charles Rosendahl]

Verified - recipients and sender addresses are not dependent on the content of
the certificate email fields.

values used:
Certificate values:  Upper, lower, mixed
Sender values: Upper, lower, mixed
Recipient values: Upper, lower, mixed

Cert values and Sender values were combined in some cases (upper/mixed,
mixed/lower, lower,upper)

Please mark as fixed1.0.0 when on the branch.

Not sure, but in the process, did I verify bug 141317?

Comment 25

[Kai Engert [:KaiE:]]

> Not sure, but in the process, did I verify bug 141317?

That bug is in a different component, broser/string, and my assumption is that
the QA contact listed in bug 141317 should verify it.

Comment 26

[Jaime Rodriguez, Jr.]

let's get this one in after rc2. adt1.0.0- [adt2 rtm]

Comment 27

[Charles Rosendahl]

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

Comment 28

[Kai Engert [:KaiE:]]

Re-nominating.

Comment 29

[Ilan Finci]

Is it getting into the 1.0 branch before the final version? I've checked RC3 and
it still missing.

Comment 30

[Kai Engert [:KaiE:]]

Raising priority to ADT 1, because I think when using S/Mime, it is very likely
to run into a situation where a recipient cert use mixed case in the email address.

Comment 31

[Jaime Rodriguez, Jr.]

adt1.0.1+ (on ADT's behalf) approval for checkin to the 1.0 branch, pending
Driver's approval. Pls check this in tonight, and add the fixed1.0.1 keyword.

Comment 32

[Judson Valeski]

please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.

Comment 33

[Kai Engert [:KaiE:]]

Checked in to branch.

Comment 34

[Kai Engert [:KaiE:]]

correcting keyword

Comment 35

[Charles Rosendahl]

Verified 20020610 Branch builds

Comment 36

[Kai Engert [:KaiE:]]

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

Comment 37

[Charles Rosendahl]

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