81250 - No warning that CA cert already exists.

Status: VERIFIED FIXED

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

Description

[John Unruh]

1.) Visit the above URL (AOL access only)
2.) Import the CA chain.
3.) Click submit again.
What happens: I am shown the same dialog box as before.
What is expected: I should get a dialog box stating that the CA is already in 
the Cert Database.

Comment 1

[John Unruh]

URL changed to an external URL.

Comment 2

[David P. Drinan]

Target Milestone --- -> 2.0.

Comment 3

[Stephane Saux]

Mass reassigning target to 2.1

Comment 4

[John Unruh]

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

Comment 5

[Rangan Sen]

->rangansen

Comment 6

[Rangan Sen]

Attachment: patch for bug# 81250

Comment 7

[Rangan Sen]

Attachment: xul file

Comment 8

[Rangan Sen]

Attachment: java script file

Comment 9

[Rangan Sen]

Here is a suggested patch for the bug (attached above)
There are 2 new files :
mozilla\security\manager\pki\resources\content\cacertexists.xul
mozilla\security\manager\pki\resources\content\cacertexists.js

I have attached these separately from the patch.
David, Javier - please review.

Comment 10

[Rangan Sen]

Attachment: updated patch - up tabs and replaced by spaces

Comment 11

[Stephane Saux]

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

Comment 12

[Stephane Saux]

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

Comment 13

[David P. Drinan]

r=ddrinan.

Comment 14

[Joe Hewitt (gone)]

sr=hewitt for the xul/js parts of the patch only.  You should have someone more
familiar with the psm c++ code sr that portion.

Comment 15

[Christopher Blizzard (:blizzard)]

+      cert->GetRawDER((char **)&der.data, &der.len);

Is there any chance that this will fail and der will be used later uninitialized?

+
  //Added to check if cert exists
+
  if (tmpCert->isperm) 
+
  {
+         dialogs->CACertExists(ctx, &canceled);

There's still some funky tab and whitespace problems in there.

Other than that, sr=blizzard.

Comment 16

[John Unruh]

Mass assigning QA to ckritzer.

Comment 17

[Rangan Sen]

Incorporating brizzard's suggestions. Also uploading the .js and the .xul files 
again, just to make sure that we do not miss them when we apply the patch ....

Comment 18

[Rangan Sen]

Attachment: patch

Comment 19

[Rangan Sen]

Attachment: manager\pki\resources\content\cacertexists.js - WE SHOULD NOT MISS THIS WHEN APPLYING PATCH

Comment 20

[Rangan Sen]

Attachment: security\manager\pki\resources\content\cacertexists.xul - WE SHOULD NOT MISS THIS WHEN APPLYING PATCH

Comment 21

[Rangan Sen]

Checked patch into cvs with Javi's help

Comment 22

[ckritzer (gone)]

I get the warning on:
 - MacOS91
 - MacOS_X
 - Win98SE
 - LinRH62

But when I remove the root.cacert from the Authorities db on Win98SE, and access
the url above again, I don't get the "There is a problem with the certificate"
dialogue, I get the "This Certificate already exists" dialogue.

Also, when I look for the GlobalSign root cacert, it doesn't exist in the Certs
db under the Authorities tab in the Certificate Manager.

Comment 23

[Rangan Sen]

To me it looks like there is some problem with deleting these kind of
certificates. This might be another bug - though not directly related to this
one. Not very sure at this point - but shall look into this issue...

Comment 24

[Rangan Sen]

Possibly not a case of regression - looks like the trouble existed.
I checked this out with build of 08/02 - once I delete the GlobalSign Root CA
and try to import that again, it fails to do so - it does not show any warnings
[the warnings dialog was not added at that point of time] - but just does not
import the certificate. In fact, Globalsign Root CA is on of the special cases
where this happens...
I believe this is related to the way certificate deletion is handled ... I
suggest we open a separate bug on this issue

Comment 25

[Stephane Saux]

Change the target of bugs with state 'RESOLVED' and target 'Future' to target
'2.1' since they were fixed for the 2.1 release.

Comment 26

[John Unruh]

Verified.