1458518 - Nicknames of existing certificates in NSS SQL DB should remain unchanged on repeated import attempts

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Kai Engert (:KaiE:)]

A functional difference between the DBM and SQL database was identified, that is considered a regression.

If a certificate already exists in an NSS DBM database with nickname1, and the same certificate is imported again with nickname2, then the nickname in the DBM database remains at nickname1.

Currently, with an SQL DBM database, the repeated nickname causes the nickname in the database to be changed to nickname2. This causes a regression in some NSS applications.

This bug suggests to adjust the NSS code, to ensure the nickname of the existing certificate remains at nickname1, in both DBM and SQL databases.

This issue has been originally reported at https://bugzilla.redhat.com/attachment.cgi?id=1428962

Comment 1

[Kai Engert (:KaiE:)]

Attachment: Bob's patch, option 3

Bob has already provided a patch, which I'm attaching for him.

Comment 2

[Kai Engert (:KaiE:)]

Attachment: test-1458518-v1.patch

This patch adds a test.

Comment 3

[Kai Engert (:KaiE:)]

I ran test builds for NSS:
  https://treeherder.mozilla.org/#/jobs?repo=nss-try&revision=3360eed0cc12b17fffac49cda58687167c85a959

and Firefox:
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=6053ae67cf3b659793abd7b3169c72c03d4cc741

and there were no regressions in existing tests.

Comment 4

[Kai Engert (:KaiE:)]

Comment on attachment 8972541 [details] [diff] [review]
Bob's patch, option 3

Fix checked in:
https://hg.mozilla.org/projects/nss/rev/2f1ee2b8f7a6

Leaving bug open until test is reviewed.

Comment 5

[Robert Relyea]

Comment on attachment 8972566 [details] [diff] [review]
test-1458518-v1.patch

Review of attachment 8972566 [details] [diff] [review]:
-----------------------------------------------------------------

Though not part of this bug, it would be good to verify the following:

Import a certificate using certutil -E and don't specify a nickname.
Import the same cert again using certutil -A -n and make sure the nickname get set. (may or may not work on dbm).

Comment 6

[Kai Engert (:KaiE:)]

Bob, I have verified that it works as you described. I tested this manually with both the old code, and the new code (with the fix from this bug), and it works the same with both dbm and sql, the behavior didn't change.

Comment 7

[Kai Engert (:KaiE:)]

Comment on attachment 8972566 [details] [diff] [review]
test-1458518-v1.patch

https://hg.mozilla.org/projects/nss/rev/5db9e969c74a

Comment 8

[Robert Relyea]

Thanks Kai. I was meaning that it would be good for the test case should check. Anyway thanks for improving the current testcase.

bob