Closed Bug 391588 Opened 17 years ago Closed 17 years ago

CRLs and its entries are created with local time instead of universal

Categories

(NSS :: Test, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: sfk1, Assigned: julien.pierre)

References

(Depends on 1 open bug)

Details

Attachments

(2 files)

User-Agent:       bot
Build Identifier: nss-3.11.7-with-nspr-4.6.7.tar.gz


Tested on Ubuntu and LinuxFromScratch:

cert.sh creates the revocation lists using the local time instead of UTC.
This causes 130 of 800 tests to fail. Using "date -u" instead of "date"
fixes the problem (patch attached).

Reproducible: Always

Steps to Reproduce:

You should only be able to reproduce the problem if your local time
runs *ahead* of UTC.
Attached patch cert.sh.diffSplinter Review
The problem you're encountering is that certificates are NOT YET VALID 
when they are issued, right?

NSS has a feature, known as "slop time" that is supposed to handle this.  
The existing test scripts are supposed to test that feature.  It sounds 
like you are finding that the feature is not working as intended.  
If that is accurate, then the fix is not to change the scripts to stop 
testing the feature, but rather is to fix the broken feature.

I wonder also if there is an issue with your TZ environment variable not
being set correctly, or with NSPR not recognizing it.  
What is your TZ set to?
I'll answer in reverse order:

The TZ variable is not set on either system - the glibc docs say that it
is not necessary. /etc/localtime is configured correctly on both systems
and points to /usr/share/zoneinfo/Europe/Amsterdam. gettimeofday() returns
the correct UTC time.


I can now confirm my speculations above:

The bug does *not* appear if I run the tests with /etc/localtime linked to
/usr/share/zoneinfo/US/Pacific. Conversely, this means that the bug should
be reproducible if you link /etc/localtime to /usr/share/zoneinfo/Europe/Amsterdam.


Finally, more details about what happens:

The certificates are not the problem. The problem is that selfserv does not
regard the *revocation lists* as valid (yet), since they are generated with
local time.


Example:
========

TestUser40's certificate is revoked with a *local* timestamp:

Entry (7):
    Serial Number: 40 (0x28)
    Revocation Date: Fri Aug 10 11:13:00 2007
    Entry Extensions:
        Name: CRL reason code


selfserv regards this date to be UTC, does not consider the cert to be revoked
(yet), so this test fails:

TLS Request don't require client auth (client auth) (cert TestUser40 - revoked)


If the slop time feature is supposed to handle revocation lists, then this
looks like a bug in selfserv or the libraries.


If not, then setting CRL_GRP_DATE with "date -u" fixes this problem (but
might break other tests). CRLUPDATE can in fact be set with "date", the
patch I posted was overly broad.


Excellent research, Stefan.  Your results certainly appear to show that
slop-time doesn't work for CRLs.  But it should.  
So I am changing the subject of this bug to indicated that problem.

I'll assign this bug to Mr. CRL himself. :)
Assignee: nobody → julien.pierre.boogz
Component: Test → Libraries
OS: Linux → All
Priority: -- → P2
Hardware: PC → All
Summary: cert.sh should use UTC to create CRLs → Slop-time doesn't work on CRLs
Target Milestone: --- → 3.12
Version: unspecified → 3.9
Hmm.  On second thought, maybe the problem is that the time zone isn't
factored into the revocation date for CRLs?  
In either case, Julien is the right person for this bug.
cc'ing the author of the CRL tests as well.
Stefan,

You say selfserv doesn't regard the CRL as valid yet. Can you be more specific ?

Are the CERT_VerifyCRL / CERT_VerifySignedData call failing when verifying the CRL signature ?

Or is it a problem with the CRL's lastUpdate / nextUpdate fields that is causing your issue here ?

I would guess it has to do with the signature. I'll try to reproduce this.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment on attachment 276035 [details] [diff] [review]
cert.sh.diff

Next time, please attach a universal diff (cvs diff -u).
Attachment #276035 - Flags: review-
The "Z" syntax in the GeneralizedTime indicates that the time must be universal. But we were getting the local time. Thus, date -u is required. Thanks for the patch. I will attach an updated one for the trunk. AFAIK, there is no slop-time for CRL verification like there is for OCSP.
Assignee: julien.pierre.boogz → nobody
Component: Libraries → Test
Summary: Slop-time doesn't work on CRLs → CRLs are created with local time instead of universal
Assignee: nobody → julien.pierre.boogz
Attachment #276215 - Flags: review?(nelson)
Attachment #276215 - Attachment is patch: true
Attachment #276215 - Attachment mime type: application/octet-stream → text/plain
Comment on attachment 276215 [details] [diff] [review]
create CRL with universal times

We need a separate bug about the slop time issue.  Slop time is very important for browsers, whose time zones are often set incorrectly.
Attachment #276215 - Flags: review?(nelson) → review+
If we fix this bug, then we need to add a separate test case to all.sh
(er, cert.sh) to test that slop time works for CRLs.  Julien,, let me 
suggest that you do that as another patch for this same bug report.
Nelson,

an additional point that isn't yet clear to me:

The *certificates* are created using UTC with or without the patch. So I don't quite see where the scripts test if slop-time is working for certs (slop-time is implemented for certs, right?).


Separating test cases sounds like a good idea. Wouldn't it be possible
to forget about local time and have three cases:

UTC, UTC + SLOP_TIME_MAX - TEST_RUNTIME, UTC - SLOP_TIME_MAX + TEST_RUNTIME

Or even two further cases to test that slop-time isn't too generous:

UTC + SLOP_TIME_MAX + 3600, UTC - SLOP_TIME_MAX - 3600


Otherwise, you'd have to run the official tests on two machines with one TZ > UTC and the other < UTC (or tinker with /etc/localtime instead).













Nelson,

As far as I remember, the reason there is no slop time for CRLs is that there is presently no need to as NSS doesn't fetch CRLs. The current code is just supposed to use the CRL with the newest thisUpdate that passes signature verification, and never reject any CRL based on date. The nextUpdate for CRLs is not an expiration date, only an indication there may be another one available. At least that's the way I remember writing it in the CRL cache. I need to reproduce this bug first in order to figure out why there was an issue.
Right, the issue is presumably with the thisUpdate date, and not with 
the nextUpdate date.  Mozilla has a CRL fetcher that can fetch them 
daily or at other intervals.  So, if a system had the classic problem
of being in the eastern hemisphere, and having its timezone set to 
California (Pacific) time, it could download a CRL within hours of its
issuance, and find that it appears to not yet be valid.
I reproduced this problem. The issue is not that the CRL is being rejected as
conjectured in comment 3. If that was the case, CRL cache would consider every
single certificate to be revoked for security reasons.

The issue is the revocationDate field in the CRL entry for that certificate
serial number. The revocation date is later than the local time, which is what
selfserv always checks at, so CERT_CheckCRL considers the certificate not to be
revoked yet.

I don't think we should introduce a slop time there, especially one that would
allow for intervals as long as one time zone difference. IMO, the use of any
revocation mechanism, real-time or not, requires clocks to be relatively
closely in sync.

OCSP has a slop time, but it is used only for acceptance of the response for
the producedAt field in the response, and hardcoded to 5 minutes.

CRLs do not have a producedAt field, and we just use the latest one we have and
always accept it, so the same slop time cannot and shouldn't be applied.

If we want to introduce a slop-time for CRL entry revocationDate and OCSP
revocationTime, I suggest that it should be a short one, in order not to impair
systems that are time synchronized and actually want to do real-time revocation
checking. This wouldn't be the answer to a timezone problem such as is the case
here.
Timezone problems are rampant among browser users.  This problem means that 
a browser user whose timezone is set wrong (farther west than it should be)
will see newly revoked certs as unrevoked for up to 23 hours (18 hours is 
common).  

Unless we believe that CAs intentionally issue CRLs with future revocations,
effectively pre-announcing that some cert will be revoked in the future, 
then I think it is reasonable to apply slop time to revocation dates.
Nelson,

No, I don't think CAs are pre-announcing cert revocations, and even if they are, that would probably be outside the standards.

The problem is that our cert verification code allows checking certs at dates in the past and not just for the current date. This is useful for example when verifying old e-mail messages. This is why we need to use revocationDate at all. If the checking time was always the current time, we could just ignore the date completely, since it would have to be in the past (something we should always do for OCSP revocationTime but we don't, and I consider that a bug).

At the level of the code where the revocationDate check currently happens, we don't know if the check date is the current time. Only the very highest levels, CERT_VerifyCertificateNow / CERT_VerifyCertNow really know that they are doing checks at the current time and could thus potentially pass the information all the way down the stack and instruct the code to use a slop time for potentially erroneous dates.

It should be noted that currently doing revocation checks in the past is outside the standards, since expired certs are allowed to be dropped from CRLs, unless some CRL archive extension is present, which we don't currently handle.

Even if we added support for that archive extension and knew for a fact that the CRL had the revocation information for the (possibly expired) cert we are interested in, we would need to make a decision about how to handle the date. When viewing an old email message for example, if the cert has been revoked, at a close date, say 5 minutes before or 5 minutes after, how would you want to apply the slop time ? What about 23 hours ? IMO, when we are talking about verifying at times other than the current time, the use of the slop time is questionable. Of course we don't know that those message times are correct for sure either ...

One way to settle the matter for good would be to just ignore the revocationDate, on the grounds that once a certificate has been revoked, all its uses, including in the past are suspect. That would be arbitrary but something I would be agreeable to, rather than trying to apply a slop time.

This is all very arbitrary in the absence of trusted time sources, both for the local time and for message times.
(In reply to comment #16)

Julien,


> I reproduced this problem. The issue is not that the CRL is being rejected as
> conjectured in comment 3. If that was the case, CRL cache would consider every
> single certificate to be revoked for security reasons.
>
> The issue is the revocationDate field in the CRL entry for that certificate
> serial number. The revocation date is later than the local time, which is what
> selfserv always checks at, so CERT_CheckCRL considers the certificate not
> to be revoked yet.

While the sentence preceding the example in <a href="show_bug.cgi?id=391588#c3">comment 3</a> is not accurate, I would think that the example itself makes it clear that the issue is the revocation date. Sorry for not responding to your questions in <a href="show_bug.cgi?id=391588#c3">comment 7</a>, I assumed the whole matter was resolved after <a href="show_bug.cgi?id=391588#c3">comment 9</a>.

FWIW, on my machine the revocation date is *exactly* the local time. 


Thanks for all the work you are doing on this! 


Summary: CRLs are created with local time instead of universal → CRLs and its entries are created with local time instead of universal
Checking in cert.sh;
/cvsroot/mozilla/security/nss/tests/cert/cert.sh,v  <--  cert.sh
new revision: 1.45; previous revision: 1.44
done
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: