74339 - Support import of SSL, S/MIME, and CA certs

Status: VERIFIED FIXED

(Core Graveyard :: Security: UI, enhancement, P3)

Description

[Bob Lord]

PSM should allow the user to import and export certs of various sorts.  Note
that this is more than just PKCS#12 support.  Users should be able to export,
say, a friend's S/MIME encryption cert to a file.

Comment 1

[Bob Lord]

Target -> 2.1

Comment 2

[Stephane Saux]

-> enhancement
-> p3
-> move to future, as such a feature is most likely to be use in conjuction with
s/mime.

Comment 3

[Bob Lord]

You should be able to drag a cert from the Cert Manager (and possibly the Cert
Viewer) to a Mail Compose window, or to the desktop.

Comment 4

[Stephane Saux]

moving to t->2.1:
New feature candidate.

Comment 5

[Stephane Saux]

Moving all P3 and P4 bugs targetted to 2.1 to future.

Comment 6

[Stephane Saux]

removing nsenterprise keyword from PSM bugs with target milestone of future.

Comment 7

[John Unruh]

Mass assigning QA to ckritzer.

Comment 8

[John Unruh]

OS > all

Comment 9

[Aleksander Adamowski]

Any hope that this will be done before Mozilla 1.0?
This bug is the last significant feature that needs to be fixed to give usable
S/MIME support in Mozilla (that is, comparable with current OE and old Netscape 4).
I currently can't use my old certificates exported from other mail clients - I
need to generate a new one from scratch at my CA, and do the whole process in
Mozilla...

I vote for raising priority and setting the target to something in the nearest
future.

Comment 10

[David P. Drinan]

Re-assigning to Kai.

Comment 11

[Aleksander Adamowski]

Kai, I think this bug is quite important and its priority and severity needs to
be raised.
It's not only an enhancement, this functionality is a must.

For example, let's imagine I've received my friend's public key certificate in a
 .cer file on a floppy disk. Currently I'm unable to send encrypted mail to him
because I cannot import that certificate into Other People's cert store - he
needs to send signed mail to me first, that's the only option.

Another example: there's a new internal CA in our company. I can get its root
cert as a .crt file from a network fileserver share, but it isn't published on
any HTTP server. If I want to setup trust for that new CA, I need to access it
by HTTP, I cannot just point to a file on my disk and import it. So I need to
put it on the web server only for the purpose of importing it in Mozilla and
setting up trust. That's quite ridiculous.

There just need to be "Import" buttons on all tabs in the cert manager, and
import of various formats of certificate files must be possible (DER, PEM, p12
files and all).

Comment 12

[Kai Engert [:KaiE:]]

Attachment: Patch v1

I still think this is an enhancement, but I agree it is a very reasonable
enhancement.

As it is not difficult to to, I created a patch. I tested with email recipient
certificates and CA certificates, both in DER and PEM format.

I added the ability to import server certificates, too, although I'm not sure
whether anybody would ever need that.


I found other small problems.
- Our existing import routine to import a CA cert always returns failure.
- our cert tree view needed a fix to ensure the contents get refreshed if we
add new entries
- we reload the contents of the certificate manager user tab, even if the user
only tried to import but pressed cancel.

Comment 13

[Kai Engert [:KaiE:]]

Javi, can you please review?

Comment 14

[Kai Engert [:KaiE:]]

Attachment: Patch v2

Actually, I want to take back the statement about SSL web site certificates.

While PSM already has appropriate logic to import CA and Email certificates, it
does not for Web Site certificates.

Is it really needed?

Attaching a new patch that leaves out importing web sites certs for now.
Only adds support to import CA and Email certificates.

Comment 15

[Aleksander Adamowski]

The ability to import web site certificates is needed for at least 2 reasons:
1. UI Consistency - from the prespective of cert manager user interface, the
only difference between server certificates and the remaining 3 types (personal,
other people's, CAs') is the type of entities they identify.
2. There are scenarios where such functionality is required (e.g. there's a
secure website we intend to connect to, and we recieved its cert through a
trusted courier on a physical medium - think e.g. Federal Government procedures.
At the same time we don't want to trust the signer CA as we know it has been
compromised recently and we can expect forged certificates from it).

But this feature isn't as important to hold back fixing the bug for the
remaining cert types. So it may be wise to spawn a follow-up bug.

Comment 16

[Kai Engert [:KaiE:]]

Attachment: Patch v3

Ok, this patch imports server certificates, too.
I tested that server certificates actually show up in cert manager.

Note that none of the imported certificates are trusted initially.
You need to manually add the trust.


Note to reviewers:
- function defaultServerNickname is only being moved from one file to another,
it remains unchanged
- ImportServerCertificate is mostly a clone of existing function
ImportEmailCertificate

Comment 17

[Javier Delgadillo]

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

r=javi

Comment 18

[Kai Engert [:KaiE:]]

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

marking patch as has-review

Comment 19

[Kai Engert [:KaiE:]]

Alec, can you please review?

See also comment 16.

Comment 20

[Alec Flett]

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

I'm not font of your use of PR_smprintf - seems like you could use the string
classes and avoid the extra allocation.. 

something like
nsCAutoString nickname(servername);
if (count != 1) {
  nickname += " #";
  nickname.AppendInt(count);
}

That saves you a whole bunch of allocations.

the JS is another good place for string bundle stuff. 

sr=alecf for the branch (and landing testing on the trunk) but if this is
trunk-only I'd really prefer the string classes to simplify the code and reduce
allocations.

Comment 21

[Kai Engert [:KaiE:]]

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

Comment 22

[Kai Engert [:KaiE:]]

Alec, I agree that PR_smprintf is not optimal.

But note that I did not write this function. I was only required to move that
function around. I did not change its implementation.

I hoped to get around having to rewrite that function.

Comment 23

[Alec Flett]

ok ok :)
but if you aren't going to be cleaning it up now, and we both recognize that it
is a problem, please file a bug against cleaning up this code - especially if
there is other code like this in the file(s).. the sr= stands then.

Comment 24

[Kai Engert [:KaiE:]]

Thanks, bug 160112 filed.

Comment 25

[Aleksander Adamowski]

You mean bug 160122 ?

Comment 26

[Kai Engert [:KaiE:]]

oops, thanks.

Comment 27

[Aleksander Adamowski]

As far as I can understand, the patch is ok for trunk landing?

Comment 28

[Kai Engert [:KaiE:]]

The trunk is currently in the final phase for the 1.1 release. I don't want to
ask for allowance to check in new features at this time. I'd need additional
approval to check in.

The mozilla.org team scheduled to create branch in 2 days, and after that the
trunk will open for normal business. I will check in as soon as that happens.

Comment 29

[Kai Engert [:KaiE:]]

Also note, this patch only supports importing. We can either leave this bug open
for further development or spawn an export patch.

Comment 30

[Kai Engert [:KaiE:]]

Importing feature added on trunk. Marking fixed.

Export feature request now tracked in bug 161275.

Comment 31

[Jean-Marc Desperrier]

Just an additional remark. 

It's really nice to have this feature, but there was the same limitation in
Netscape 4.x and there is a work around, I'll describe it here in case that
would be useful to someone.

If you make sure there's a MIME association between the extension you give to
the file with the certificat and the type required for importation (should be
the case by default in Windows for a CA cert if you give it the extension
.cer/.crt, you can do it through Preference/Navigator/Helper Application in
Linux, the type is either application/x-x509-ca-cert or
application/x-x509-user-cert) you just have to open the file from a navigator
windows to import it.

Comment 32

[John Unruh]

Verified.