Last Comment Bug 371522 - Auto-Update of CRLs stops after first update
: Auto-Update of CRLs stops after first update
Status: RESOLVED FIXED
:
Product: NSS
Classification: Components
Component: Libraries (show other bugs)
: 3.4
: All All
: P2 normal with 7 votes (vote)
: 3.12.3
Assigned To: Raphaël Fairise
:
:
Mentors:
: 417182 (view as bug list)
Depends on:
Blocks: 477244
  Show dependency treegraph
 
Reported: 2007-02-24 08:00 PST by Ketteltasche
Modified: 2011-08-26 03:26 PDT (History)
19 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Copy url value in new CERTSignedCrl object (505 bytes, patch)
2009-01-15 01:58 PST, Raphaël Fairise
no flags Details | Diff | Splinter Review
Raphaël's patch converted to a CVS diff for trunk (724 bytes, patch)
2009-01-15 12:13 PST, Nelson Bolyard (seldom reads bugmail)
julien.pierre: review-
Details | Diff | Splinter Review
patch v2 (1.22 KB, patch)
2009-01-28 22:11 PST, Nelson Bolyard (seldom reads bugmail)
julien.pierre: review+
Details | Diff | Splinter Review
screenshot of the CRL not updated bug in Firefox 6.0 beta3 (45.67 KB, image/png)
2011-08-05 14:47 PDT, bjoern
no flags Details

Description Ketteltasche 2007-02-24 08:00:04 PST
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.8.1.2) Gecko/20070219 Firefox/2.0.0.2
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.8.1.2) Gecko/20070219 Firefox/2.0.0.2

The Auto-Update of Revocation Lists (CRLs) doesn’t work as I would expect it to do.
I found this bug in the newest version of Firefox (also Netscape and Seamonkey) under Linux (Suse 10.0) and under Windows XP Pro.
Instead of updating the CRLs and showing the current update states the fields “last update” and “next update” remain unchanged and the field “Auto-Update status” is left blank.

Reproducible: Always

Steps to Reproduce:
1. Go to your “Manage Revocation List” option in your favourite browser and import a CRL from http://crl.verisign.com or http://www.trustcenter.de/cgi-bin/CRL.cgi 
You can also go directly to these pages in order to import the CRLs just by left-clicking them.
When you import CRLs have a look at the date when the CRL is updated next time so you won’t have to wait too long.
2. After importing is done activate the Auto-Update in the upcoming menu or in the settings of the CRL under “Manage Revocation List”.
3. Come back a day or few days later when the update should have been done. (Perhaps go back to the site where you’ve imported the CRL from and check that there is a new issue date.)
Actual Results:  
You will find the fields “last update” and “next update” with the same unchanged dates as you left them in the past. The field “Auto-Update status” is still left blank.

Expected Results:  
The CRLs should have been updated including an update of the fields “last update”, “next update” and “Auto-Update status”.

I’m not sure whether this is a display bug or a bug that the CRLs is indeed not updated.
I also tested it with an apache server and self created CRLs with the result that the CRLs are not updated but this may be a fault of my CRLs. (But I don’t think so.)

May anybody test it and confirm that this is a bug?

Good to know: I think that all CRLs I’ve imported so far are version 2 CRLs.
Comment 1 Ketteltasche 2007-03-01 04:16:18 PST
edit: I have tested my self created CRLs with the Internet Explorer 7 in order to check whether they work as they should. IE7 downloads the newest CRL from the Distribution Point whenever the time for the next update has come. So I guess it isn’t the fault of my CRLs that Auto-Update of Firefox doesn’t work.
Comment 2 ms 2007-07-04 00:50:35 PDT
i can confirm this behavior in thunderbird and firefox.
i tried to do the same, as german speaking friends, but only one can reproduce this behavior.
http://www.thunderbird-mail.de/forum/viewtopic.php?t=28216
Comment 3 Thomas Schaefer 2007-07-22 02:27:11 PDT
i can confirm this behavior too. in firefox and thundebird (1.x and 2.x).
At least the GUI does not update the fields “last update”, “next update” and “Auto-Update status”. 
Not sure if autoupdate for CRLs works at all.
Manual upate works and updates the fields "last update" and "next update".
But: i selected "auto every 1 day" but the "next update" field is set to a date 7 days in the future after a manual update.
Further information:
I'm using http://www.cacert.org/revoke.crl als CRL
My Thunderbird runs continuously (only stopped for updates)
Comment 4 Andreas 2007-07-22 02:49:35 PDT
I can confirm this for Thunderbird 1.5 on WinXP and Thunderbird 2.0.0.5 on Win2000.

There are no problems updating CRLs manually. Also no problems are reportet for the last update. So I suppose, they never have been.
Comment 5 Ketteltasche 2007-07-23 00:47:05 PDT
(In reply to comment #3)
> i can confirm this behavior too. in firefox and thundebird (1.x and 2.x).
> At least the GUI does not update the fields “last update”, “next
> update” and “Auto-Update status”. 
> Not sure if autoupdate for CRLs works at all.
> Manual upate works and updates the fields "last update" and "next update".
> But: i selected "auto every 1 day" but the "next update" field is set to a date
> 7 days in the future after a manual update.
> Further information:
> I'm using http://www.cacert.org/revoke.crl als CRL
> My Thunderbird runs continuously (only stopped for updates)
> 

In the meanwhile I have tested the new versions of each browser (Firefox 2.0.0.5, SeaMonkey 1.1.3 and Netscape 8.1.3) and they still don’t work.

But I think that I can help Thomas Schaefer: The CRL includes the fields „this update“ „next update“. When you manually update your CRL the browser takes a look into the „next update“ of the CRL and sets its field according to that.

For everybody who must use CRLs: Take a look at The Internet Explorer 7. It downloads automatically the newest CRL as the „next update“-date is reached.

Comment 6 Jan Tomasek 2008-06-08 08:14:31 PDT
I'm having similar problem. I want to have my CRL updated every 1 day. But Firefox/Thunderbird never do it. 

I tested and reproduced this problem in:
   Thunderbird version 2.0.0.12 (20080213)
   Iceweasel/2.0.0.12 (Debian-2.0.0.12-1)
   Firefox 3.0 RC1 and RC2

I would realy like to have this feature working. Especialy after so many certificates were revoked after weak random number generator discovery in Debian.
Comment 7 Nelson Bolyard (seldom reads bugmail) 2008-06-24 00:19:08 PDT
Months ago, I manually downloaded numerous CRLs from Thawte, Verisign, GTE 
& Apple, and enabled auto update for them all.  A recent check showed they
were all past their expected next update date, and had not updated.  
Manually updating them succeeded for every one.  

In reply to comment 3:
> But: i selected "auto every 1 day" but the "next update" field is set to 
> a date 7 days in the future after a manual update.

That field shows the value of the "next update" field in the CRL itself.
Comment 8 Nelson Bolyard (seldom reads bugmail) 2008-06-24 00:19:42 PDT
*** Bug 417182 has been marked as a duplicate of this bug. ***
Comment 9 michael helm 2008-12-15 15:28:31 PST
Checked on this based on a comment by a colleague who runs Grid CAs.

My Vista Firefox 3.0.4 and Thunderbird 2.0.0.18 instances are not updating,
nor are some Firefox 2.x on other platforms.  Configuration like
comment #6 (https://bugzilla.mozilla.org/show_bug.cgi?id=371522#c6)
I've been watching these configurations for about a month and 
I have not detected the watched hosts connecting to the CRL publishing
points and asking for updated CRLs.

This really should be fixed.  This flaw undermines the trust
in these products in certain areas, since the UI, such as it is,
gives the misleading impression to the user that he can trust it
to keep revocation information up to date.
Comment 10 Raphaël Fairise 2009-01-15 01:57:10 PST
I tried to study the problem with CRL auto-update.
1) An entry "security.crl.autoupdate.url.NAME", set by the CRL "Settings" dialog when OK is pressed, contains the URL of the CRL.
(e.g.: "security.crl.autoupdate.url.VeriSign Trust Network")
2) CRL Manager (C++) reads "security.crl.autoupdate.*" in about:config db.

Warning: UI of "Manage CRL" dialog is not refreshed when a CRL is imported or settings are changed. You'll need to close the window and reopen it. Bug 104137

Warning: the NAME value which is appended to all about:config settings for CRL is taken from the Organisation Unit of the CRL, so there are obvious problems when several CRL have the same OU. The NAME should be unique. Bug 282945

Here is what happens:
The CRL Manager succeeds in fetching the CRL the first time but, just after fetching the CRL, it overwrites "security.crl.autoupdate.url.NAME" preference entry with an updated value of the URL. (in nsCRLManager::ImportCrl)
The problem I found is that the URL has not been correctly copied to the CERTSignedCrl object used to handle the CRL in crl_storeCRL(). The URL is left empty after the operation and therefore the value of "security.crl.autoupdate.url.NAME" became an empty string.
This happens with automatic update by CRL Manager AND manual update (by pressing Update button in Manager CRL window).

I don't know if this following patch of security/nss/lib/certdb/crl.c is enough and if other members of CERTSignedCrl have been forgotten in the copy but it seems to do the job.
Comment 11 Raphaël Fairise 2009-01-15 01:58:59 PST
Created attachment 357119 [details] [diff] [review]
Copy url value in new CERTSignedCrl object
Comment 12 Nelson Bolyard (seldom reads bugmail) 2009-01-15 12:13:58 PST
Created attachment 357208 [details] [diff] [review]
Raphaël's patch converted to a CVS diff for trunk

I've converted this patch to a CVS diff to facilitate review.

Raphaël, does this patch solve the problem for you?
Comment 13 Nelson Bolyard (seldom reads bugmail) 2009-01-15 12:28:40 PST
I gather that the stack for the problematic call was:

crl_storeCRL                          nss/lib/certdb/crl.c        line  734
PK11_ImportCRL                        nss/lib/pk11wrap/pk11nobj.c line  805
SEC_NewCrl                            nss/lib/certdb/crl.c        line  802
nsCRLManager::ImportCrl               nsCRLManager.cpp            line  137
PSMContentDownloader::OnStopRequest   nsNSSComponent.cpp          line 2946

Yes ?

I'm not sure what preceded that.
Comment 14 Raphaël Fairise 2009-01-16 02:54:16 PST
(In reply to comment #12)
> Raphaël, does this patch solve the problem for you?

Yes, the patch solves auto-update of CRL. They are now downloaded automatically and regularly.

(Tested with Thunderbird 2.0.0.19, with a small CRL auto-update interval in order not to have to wait days)


(In reply to comment #13)
> I gather that the stack for the problematic call was:
> [...]
> PSMContentDownloader::OnStopRequest   nsNSSComponent.cpp          line 2946
> 
> Yes ?

Yes, that's what I found too. OnStopRequest is called when all data have been received by PSMContentDownloader.
Comment 15 Nelson Bolyard (seldom reads bugmail) 2009-01-16 07:29:03 PST
OK, then this is an NSS bug, not PSM.
Thanks very much for tracking this down!
Comment 16 Julien Pierre 2009-01-23 17:43:43 PST
Comment on attachment 357208 [details] [diff] [review]
Raphaël's patch converted to a CVS diff for trunk

I think this is not correct. The url needs to be copied into the arena using PORT_ArenaStrdup.

I think there is at least one other instance where the arena is not used during an assignment, and should be. That's a pre-existing bug, not a problem with the patch.

I am also not certain why we give priority to the oldCrl->url, as opposed to the one passed in to crl_storeCRL.

Even if that's what we want to do, it seems to me that if a url argument was passed in, and oldCrl->url is NULL, then we should use the argument.

So, the new assignment should probably be something like this :

if (oldCrl->url && !url)
   url = oldCrl->url;

crl->url = PORT_ArenaStrdup(crl->arena, url);
Comment 17 Nelson Bolyard (seldom reads bugmail) 2009-01-28 22:03:31 PST
(In reply to comment #16)

> I think there is at least one other instance where the arena is not used 
> during an assignment, and should be. That's a pre-existing bug, not a 
> problem with the patch.

Julien, Is that in this same function?  What line of code is it? 
Let's fix it while we're in here.

> I am also not certain why we give priority to the oldCrl->url, as opposed to
> the one passed in to crl_storeCRL.

I think you're acknowledging that the new code mimics the existing code path
for the case where the new cert will replace an existing old cert.  In that
case, the old code always prefers the value of oldCrl->url to the passed in 
url value.  I agree that I'm not sure that's the best idea, but again, if 
we're going to change it, we should probably change it in both places.

> Even if that's what we want to do, it seems to me that if a url argument was
> passed in, and oldCrl->url is NULL, then we should use the argument.

How could oldCrl->url be NULL?  I guess that would have been a bug.  

> So, the new assignment should probably be something like this :
> 
> if (oldCrl->url && !url)
>    url = oldCrl->url;
> 
> crl->url = PORT_ArenaStrdup(crl->arena, url);

I will write a new patch, and ask Raphaël Fairise to test it.
But if there's another bug in this function, please advise ASAP.
Comment 18 Nelson Bolyard (seldom reads bugmail) 2009-01-28 22:11:40 PST
Created attachment 359476 [details] [diff] [review]
patch v2

Julien, please review
Comment 19 Nelson Bolyard (seldom reads bugmail) 2009-01-28 22:46:17 PST
Comment on attachment 359476 [details] [diff] [review]
patch v2

Raphaël, please test this patch and advise if it works for you. Thanks.
Comment 20 Raphaël Fairise 2009-01-29 03:36:21 PST
(In reply to comment #19)
> (From update of attachment 359476 [details] [diff] [review])
> Raphaël, please test this patch and advise if it works for you. Thanks.

Yes Nelson, it works fine.
Comment 21 Julien Pierre 2009-02-04 15:06:02 PST
Comment on attachment 359476 [details] [diff] [review]
patch v2

This patch looks fine.

In response to comment 17, I think I was wrong and the only assignment of crl->url used PORT_ArenaStrdup, so it was OK before the patch.

You did the right thing in the new patch WRT the priority of the url input argument and oldCrl->url.

Lastly, I think oldCrl->url could legitimately be NULL, if the old CRL was stored without a url, without there being any bug. But that shouldn't be a problem.
Comment 22 Nelson Bolyard (seldom reads bugmail) 2009-02-05 12:32:00 PST
Checking in crl.c; new revision: 1.62; previous revision: 1.61
Comment 23 bjoern 2011-08-05 14:47:21 PDT
Created attachment 551168 [details]
screenshot of the CRL not updated bug in Firefox 6.0 beta3

I wonder why this bug is in state "closed fixed". I saw this bug since ages. Here is a screenshot of my two CRLs in Firefox 6.0 beta3. Both of the CRLs are only updated when I do that manually. As you can see it *should* have been updated one day before expiration, that means on 2011-07-15 (today is 2011-08-05).
Comment 24 Norman L. Smith 2011-08-06 06:39:09 PDT
This bug continues to be present in 5.0. Bjoern Jacke's attachment tells the story.  What does it take to re-open this bug?
Comment 25 bjoern 2011-08-26 03:26:10 PDT
as this bug status seems to be not changebale, I filed a new one: bug #682244

Note You need to log in before you can comment on or make changes to this bug.