Closed Bug 282945 Opened 20 years ago Closed 11 years ago

Use better strings to identify a CRL in the prefs.js file

Categories

(Core :: Security: PSM, defect, P2)

Other Branch
defect

Tracking

()

RESOLVED INVALID
mozilla1.9alpha8

People

(Reporter: dave127001, Unassigned)

References

Details

(Whiteboard: [kerh-coa][psm-crl])

Attachments

(2 files, 1 obsolete file)

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20050211 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.5) Gecko/20050211 Mozilla will crash when attempting to exit from the suite if auto-update is enabled in the CRL manager and one or more of the CRLs have an empty value under "Organizational Unit" (in my experience, this includes about half of all CRLs). Please note that although this can be bug can be reliably reproduced if the above conditions are met, I have not verified that all of the above conditions are required for the "crash-on-exit" to occur. I.e., it may happen even after disabling auto-update. It is unlikely though, because the browser must remain open for some length of time prior to exiting in order for the crash to happen. There is a problem with the way in which Mozilla handles the CRL specific preferences. Mozilla appends the CA's OU (Organization Unit) name to the preference key string in its internal hash table, but many of the CRLs have empty OU fields, which results in the problem of multiple CRLs attempting to share the same key. E.g., this is how the key-value pair is supposed to look in the prefs.js file: user_pref("security.crl.autoupdate.enable.Certification Services Division", true); If there is no OU name, the above preference looks like this: user_pref("security.crl.autoupdate.enable.", true); I have generated numerous crash dump analyses and they all point to the same place. Someone whom is intimately familiar with the Mozilla source tree will certainly be able to zoom in on the problem. Reproducible: Always Steps to Reproduce: 1. Download the followinng CRLs and enable auto-updating: http://crl.verisign.com/ThawtePersonalFreemailIssuingCA.crl http://crl.verisign.com/ThawteSSLDomainCA.crl 2. Wait a while (may something to do with the auto-update timer) 3. Exit from the Mozilla suite. FOLLOWUP_IP: xpcom!clearHashEntry+12 [j:\mozilla\xpcom\ds\nshashtable.cpp @ 81] 10006072 83660400 and dword ptr [esi+0x4],0x0 SYMBOL_STACK_INDEX: 1 SYMBOL_NAME: xpcom!clearHashEntry+12 MODULE_NAME: xpcom IMAGE_NAME: xpcom.dll DEBUG_FLR_IMAGE_TIMESTAMP: 420ce883 STACK_COMMAND: .ecxr ; kb FAILURE_BUCKET_ID: ACCESS_VIOLATION_xpcom!clearHashEntry+12 BUCKET_ID: ACCESS_VIOLATION_xpcom!clearHashEntry+12
Note: This bug report details the origin of the problem reported in bug 200119, but I thought it warranted filing a separate bug because that thread has become rather diluted.
(In reply to comment #0) > 2. Wait a while (may something to do with the auto-update timer) > 3. Exit from the Mozilla suite. How long was "a while" for you roughly? I tried reproducing this bug here, but couldn't crash.
Product: PSM → Core
(In reply to comment #3) > (In reply to comment #0) > > 2. Wait a while (may something to do with the auto-update timer) > > 3. Exit from the Mozilla suite. > > How long was "a while" for you roughly? I tried reproducing this bug here, but > couldn't crash. Unfortunately, I don't know. I just know from testing that the crash did not occur if I immediately exited from the browser aftering startup. However, it wasn't *days*. A few hours should be enough time. I'm sorry that I can't be more specific. Installing either one of those CRLs in the original bug report also reveals a concrete bug in the way that the preference is handled. Specifically, one *cannot* enable auto updating of both CRLs for the reason that I described in my original report. See the paragraph in the initial bug report that begins: "[t]here is a problem with the way in which Mozilla handles the CRL specific preferences. . . ." The preference string bug is easily exposed by typing "about:config" in the location bar and then filtering on string "crl". I will attach a screenshot from "about:config".
Attached image Duplicate screen shot attachment (obsolete) (deleted) —
Comment on attachment 180231 [details] Screen shot from "about:config" exposing the preference string bug. Please delete the duplicate attachment. My connection was hung for about 5 minutes after the first submission. I should have verified that it had not been transmitted (sorry about that).
Attachment #180232 - Attachment description: Screen shot from "about:config" exposing the preference string bug. → Duplicate screen shot attachment
Attachment #180232 - Attachment is obsolete: true
Is there something else I should be doing to expedite the handling of this bug? I'm concerned that this bug is being dismissed either because others have been unable to reproduce the crash, or not enough people have attempted to reproduce it. However, the crash I reported was only a consequence of the real problem, which is the handling of the preferences by the certificate manager component. A Bugzilla search revealed that there are quite a number of reports related to this issue: https://bugzilla.mozilla.org/buglist.cgi?query_format=specific&order=relevance+desc&bug_status=__open__&product=&content=CRL+certficate+manager Here is one just for example: https://bugzilla.mozilla.org/show_bug.cgi?id=150589 It should be very easy to fix. 1.) Choose a field other than the Organizational Unit (OU) for the formation of the preference name (i.e., a field which must be present in every CRL or certificate). 2.) Validate that the preference string is not an empty value. CRLs without an OU present are not an exception to the rule. IIRC, probably at least half of the CRLs that I've downloaded in the past have not had an OU value present (unless Mozilla is failing to parse the OU, which I have not checked but I doubt is the case), and they were from Verisign or Thawte.
OS: Windows XP → All
Hardware: PC → All
Dave, this is a PSM bug. It's not being dismissed any more than any other PSM bugs are. There simply are no developers actively working on PSM. Recruiting PSM developers is about all you can do to expedite it.
Component: Security: UI → Security: PSM
(In reply to comment #9) > Dave, this is a PSM bug. It's not being dismissed any more than any other > PSM bugs are. There simply are no developers actively working on PSM. > Recruiting PSM developers is about all you can do to expedite it. OK. Noted. I'm personally unaffected by the bug as I don't even use CRLs anymore, and I suspect that it's a non-issue for most users as well. I was just startled by the number of related bugs that I saw filed in Bugzilla (spanning several years), and wanted to at least make sure that people were aware of the relative simplicity of it. Perhaps a cautionary note somewhere warning users of the potential danger of attempting to manage CRLs without an associated OU would be appropriate until the issue is resolved.
I see this crash too. Only I don't agree that the problem discussed here is the cause. I have one CRL defined, and it does have a OU name (and the prefs reflect that name). It seems more like a double-free. On shutdown this runs: nsNSSComponent::StopCRLUpdateTimer() { if(mUpdateTimerInitialized == PR_TRUE){ if(crlsScheduledForDownload != nsnull){ crlsScheduledForDownload->Enumerate(crlHashTable_clearEntry); crlsScheduledForDownload->Reset(); delete crlsScheduledForDownload; crlsScheduledForDownload = nsnull; } (...) the call to nsHashtable::Enumerate() |delete|'s each nsStringKey (the hash entry's key) within nsNSSComponent::crlHashTable_clearEntry, but doesn't do anything to mark it as deleted. The call to nsHashtable::Reset() then does a pldhash::PL_DHashTableEnumerate(&mHashtable, hashEnumerateRemove, NULL), which ultimately calls nsHashtable::clearHashEntry() on each entry, again |delete|'ing the entry's key (nsHashKey). That all said, this is the first time I really have looked at Mozilla code, so I may have missed something. Can someone more familiar take a look? What approach would be desired to fix this? It seems easiest just to remove the Enumerate() call, and rely on Reset() to clean up memory.
dupe of bug 200119 ?
Sounds to me like the same issue as Bug 200119
Let's make this bug dependent on 200119, so we can retest, once 200119 gets fixed.
Depends on: 200119
Josh, I agree with your statement about the double free, see bug 200119. A potential fix for the crash bug was checked in to the trunk last week. Is anyone on the CC list of this bug able to try out a nightly trunk build and test if you still crash?
I would like to change the intention of this bug to "use better strings to identify a CRL in the prefs.js file". I confirm we use bad strings.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Access violation caused by inappropriate handling of CRLs in nsHashtable → CRL update feature uses bad preference strings
Whiteboard: [kerh-coa]
*** Bug 198371 has been marked as a duplicate of this bug. ***
Priority: -- → P2
Target Milestone: --- → mozilla1.9beta
QA Contact: psm
Use of OU as a unique pref key has another problem: It has recently been reported that many CAs do not verify the names that their subjects request for OU names AT ALL. Consequently, OU names need to be treated as unverified, and no security decisions by the user or by the software should depend on their values.
So to get the key for the ff userpref.js unique there are a few options: SHA-1 Hash of combinations/permutations of the o the CLRs download URL provided by the user o CRL issuer DN contained in CRL o subject key identifier of the CRL signer certificate o modulus of the key of the CRL signer certificate The "Manage CRL" gui window should display the full CRL issuer DN and the download URL as well as the other stuff that is already there.
Updating subject as proposed in comment 16
Assignee: kaie → nobody
Summary: CRL update feature uses bad preference strings → Use better strings to identify a CRL in the prefs.js file
Whiteboard: [kerh-coa] → [kerh-coa][psm-crl]
The "Revocation Lists" feature was removed in bug 867465.
Status: NEW → RESOLVED
Closed: 11 years ago
Depends on: 867465
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: