Closed
Bug 495357
Opened 15 years ago
Closed 8 years ago
Update some documentation concerning SaveIntermediateCerts()
Categories
(Core :: Security: PSM, defect, P1)
Core
Security: PSM
Tracking
()
RESOLVED
FIXED
mozilla52
| Tracking | Status | |
|---|---|---|
| firefox52 | --- | fixed |
People
(Reporter: wtc, Assigned: Cykesiopka)
Details
(Whiteboard: [psm-logic][psm-assigned])
Attachments
(1 file, 3 obsolete files)
|
3.47 KB,
patch
|
Cykesiopka
:
review+
|
Details | Diff | Splinter Review |
The code in question was originally added in bug 100215 and later modified in bug 399045. http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/manager/ssl/src/nsNSSCallbacks.cpp&rev=1.73&mark=960-962,982-1019#953 There are three problems: 1. The patch in bug 399045 changed PSM to remember the intermediate CA certs in the permanent db, but it didn't update the comments, so we have some stale comments that still refer to the "temp db". 2. The code also remembers root CA certs even though the intention (according the the comments) is to remember only the intermediate CA certs. 3. With the patch in bug 399045, I believe nsNSSComponent::RememberCert and the hashTableCerts hashtable are now dead code. The attached patch fixes problems 1 and 2. Please improve the comments I wrote about why we want to remember the intermediate CA certs in the permanent db.
Attachment #380314 -
Flags: review?(kaie)
|
||
Updated•14 years ago
|
Assignee: kaie → nobody
Whiteboard: [psm-logic]
|
|
||
Comment 1•12 years ago
|
||
|
|
||
Comment 2•12 years ago
|
||
Comment on attachment 652024 [details] [diff] [review] merged patch Wan-Teh, I'm OK with your patch (do not remember root certs), r=kaie But I'm worried that Brian will probably argue that this patch should be rejected because it doesn't come with a test? :)
Attachment #652024 -
Flags: review+
|
|
||
Comment 3•12 years ago
|
||
Leaving reviewer undefined. I'm just cleaning up my old review requests, I don't want to work on this further.
Attachment #652025 -
Flags: review?
|
|
||
Updated•12 years ago
|
Attachment #380314 -
Flags: review?(kaie)
|
||
Updated•10 years ago
|
Attachment #652025 -
Flags: review?
Whiteboard: [psm-logic] → [psm-logic][psm-cleanup]
|
Assignee | |
Comment 4•8 years ago
|
||
Problem 3 was addressed in Bug 879135: https://hg.mozilla.org/mozilla-central/rev/3c033a95ef26#l1.159 So we only need to care about updating the documentation now.
Assignee: nobody → cykesiopka.bmo
Status: NEW → ASSIGNED
Priority: -- → P1
Summary: The code in AuthCertificateCallback that remembers intermediate CA certs has some minor problems → Update some documentation concerning SaveIntermediateCerts()
Whiteboard: [psm-logic][psm-cleanup] → [psm-logic][psm-assigned]
|
|
Assignee | |
Comment 5•8 years ago
|
||
Oops, forgot to mention Problem 2 was addressed in Bug 1277240.
|
|
Assignee | |
Comment 6•8 years ago
|
||
This is an updated patch for problem 1.
Attachment #380314 -
Attachment is obsolete: true
Attachment #652024 -
Attachment is obsolete: true
Attachment #652025 -
Attachment is obsolete: true
Attachment #8800039 -
Flags: review+
|
|
Assignee | |
Comment 7•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=25a84225955c5ad3b50e43a5f5cd7041fd267081
Keywords: checkin-needed
Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/5efc720972a9 Update some documentation concerning SaveIntermediateCerts(). r=kaie,me
Keywords: checkin-needed
|
||
Comment 9•8 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/5efc720972a9
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in
before you can comment on or make changes to this bug.
Description
•