712612 - When complaining about bad certificate dates (expired etc.), tell user if their clock is wrong

Status: RESOLVED FIXED

(Core Graveyard :: Security: UI, defect, P1)

Description

[Gervase Markham [:gerv]]

There are many sources of correct time and date out there on the internet. We don't have to implement an NTP library - we could take the timestamp from the HTTP headers in the response to the daily blocklist ping and store an offset between that and the local time. That must be 10 lines of code or less, surely.

Then, if a user gets a "certificate expired" or "certificate not valid yet" error, we could say something like: "Your computer's clock is out by X days/years/hours. Please fix it and try again." 

I don't have stats, but I've seen people argue that wrong clocks are a common problem, and that breaks various cert and security things. Well, let's try and make it less of a problem :-)

Related: bug 471723.

Gerv

Comment 2

[Brent]

Ran into this problem yesterday.  During boot, the PC displayed an error message about the CMOS battery, and set the date to sometime in 2006, which is about when it was manufactured.  Windows 7 didn't adjust the date.  When I visited duckduckgo via the little search box on the right side of the URL bar, I got the scary invalid security certificate error.  Reading the fine print (which I wonder if most users would skip), the message stated DDG's certificate wouldn't become valid until Nov 2013.

The error message isn't appropriate for the real problem.  In this case, there is no security issue at all, nothing wrong with the web site.  The PC has the wrong date, that's all.  Certainly don't want to add exceptions to the security for a problem like that, or be presented with an option to do so.

Comment 3

[Craig Francis]

This feature has just been added to Chrome 40 (currently alpha).

On error it will check the date/time it receives from Google's servers and will show a much more appropriate error.

For reference I have had a few customers go onto my website, ignore the certificate expired warning (yes, they get into a pattern), then on the checkout page complain that the certificate has expired... every one of them was due to their clock being out (sometimes by a few years).

You can see more at:

https://code.google.com/p/chromium/issues/detail?id=414843
https://groups.google.com/a/chromium.org/forum/#!topic/security-dev/oj2xXq3CF0E

Comment 5

[Lawrence Mandel [:lmandel] (use needinfo)]

This bit me yesterday (see bug 1234995) when installing a new, out of the box, MS Surface laptop. My initial thought was Firefox was broken as firstrun gave an error. I couldn't even report the error with the "report this error" link. Only after I noticed that other sites were broken did I think that this might be a system error.

rbarnes - Can you please comment on feasibility and priority of an improved error message in the case where the system clock is off?

Comment 6

[Hector Zhao [:hectorz]]

(In reply to Gervase Markham [:gerv] from comment #0)
> We don't have to implement an NTP library

FYI, I've seen this sntp implementation currently used by FxOS:
https://dxr.mozilla.org/mozilla-central/source/toolkit/modules/Sntp.jsm

Comment 7

[Richard Barnes [:rbarnes]]

lmandel: Thanks for the ping.  This has been on our radar for a bit, but I don't think we've come up with a concrete solution strategy.  In particular, the source for the "authoritative rough time" that we would compare with the local clock to determine whether it's plausible.  Chrome uses a build timestamp as a coarse correction; do you know if we have that information in Firefox?

tarek, mgoodwin: Could we have Kinto / Balrog pings store the time they get back from the server?  That would give us a much finer check.

Comment 8

[Gervase Markham [:gerv]]

(In reply to Richard Barnes [:rbarnes] from comment #7)
> tarek, mgoodwin: Could we have Kinto / Balrog pings store the time they get
> back from the server?  That would give us a much finer check.

This is the obvious fix. It must be, like, 2 lines of code to store the time received and calculated offset in a pref somewhere, and then a few lines to pull it out to improve the error message.

Gerv

Comment 9

[Lawrence Mandel [:lmandel] (use needinfo)]

(In reply to Richard Barnes [:rbarnes] from comment #7)
> Chrome uses a build
> timestamp as a coarse correction; do you know if we have that information in
> Firefox?

There are several prefs with the buildid. For ex, see gecko.buildID.

Comment 10

[Mark Goodwin [:mgoodwin]]

(In reply to Richard Barnes [:rbarnes] from comment #7)
> tarek, mgoodwin: Could we have Kinto / Balrog pings store the time they get
> back from the server?  That would give us a much finer check.

Yes. I was planning to do this.

Comment 12

[Panos Astithas (he/him) [:past] (please ni?)]

I filed bug 1238519 before coming across this bug. What is proposed in comment 7 is I think what Google does. An alternative approach was suggested in this other bug:

---------------------

What we could do is treat these[*] errors  differently and use a heuristic to figure out if the local time is wrong and update the error message accordingly.

The heuristic would work like this: in these cases Firefox performs a HEAD request to http://www.mozilla.org and parses the Date HTTP response header. If the certificate is valid according to that time but not the system time, then the message is updated to say that the system clock *is* wrong. Otherwise, Firefox reports that the server is misconfigured. Bug 1057120 contains a patch with a similar approach for Fennec.

Potential further enhancements to the above include:

1) Display a link to a non-secure URL with instructions for updating the local time in the local platform.
2) Displaying a button to open the system dialog for date and time settings[**].
3) Use a local page for the Learn More link instead of opening SUMO, which could be unreachable in such cases.

[*]: SEC_ERROR_EXPIRED_CERTIFICATE, SEC_ERROR_EXPIRED_ISSUER_CERTIFICATE and MOZILLA_PKIX_ERROR_NOT_YET_VALID_CERTIFICATE
[**]: this should be fairly easy on OS X; you just build and execute an NSAppleScript object with the source:

 tell application "System Preferences"
   activate
   set the current pane to pane id "com.apple.preference.datetime"
 end tell

Comment 13

[Justin Dolske [:Dolske]]

(In reply to Panos Astithas [:past] from comment #12)

> The heuristic would work like this: in these cases Firefox performs a HEAD
> request to http://www.mozilla.org and parses the Date HTTP response header.

Might want to use a non-standard header, I can easily imagine there being all kinds of weird caching proxies and malware (sorry, anti-virus programs) that might cause old/broken dates to come though.

For that matter, is there any worry about an active MITM using this to try and trick a user into changing their clock to a wrong time and then doing evil things? One would hope that this would be ineffective due to multiple steps and users knowing that it's no longer 1999, but figured I'd note it. :)

Actually, some of that could be avoided by treating dates older than the build-date as bogus. So an attacker could only try to trick the user a limited amount into the past, or into the future.

> 2) Displaying a button to open the system dialog for date and time
> settings[**].
> 3) Use a local page for the Learn More link instead of opening SUMO, which
> could be unreachable in such cases.

It would be mildly interesting to learn why the clock is bad -- did they disable their OS's time-syncing feature? Is it an old OS without such support? Is their network blocking NTP? etc.

Also, we could add UITour-type functionality to allow a SUMO page to trigger opening the relevant OS dialog.

Comment 14

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to Justin Dolske [:Dolske] from comment #13)
> (In reply to Panos Astithas [:past] from comment #12)
> Might want to use a non-standard header, I can easily imagine there being
> all kinds of weird caching proxies and malware (sorry, anti-virus programs)
> that might cause old/broken dates to come though.

Interesting, I wonder if there are actual proxies out there that mess with Date.

> For that matter, is there any worry about an active MITM using this to try
> and trick a user into changing their clock to a wrong time and then doing
> evil things? One would hope that this would be ineffective due to multiple
> steps and users knowing that it's no longer 1999, but figured I'd note it. :)

I've thought about this, but I couldn't think of a plausible attack scenario apart from a DoS. It seems that messing with the system clock breaks many things and in a very in-your-face way, which is the opposite of what an attacker usually wants.

One other idea that mgoodwin mentioned is to use HTTPS+HPKP, but also have the platform treat all pinned certs as valid, regardless of the expiry date. More work, but avoids the above.

> Actually, some of that could be avoided by treating dates older than the
> build-date as bogus. So an attacker could only try to trick the user a
> limited amount into the past, or into the future.

Agreed.

Comment 17

[Johann Hofmann [:johannh]]

Attachment: Display a warning on about:certerror if the system time is wrong

Comment 18

[Johann Hofmann [:johannh]]

Comment on attachment 8741409 [details] [diff] [review]
Display a warning on about:certerror if the system time is wrong

Gijs, would you mind reviewing this? I tried to stay similar to the existing code. One thing I wasn't sure about is how much time skew should be allowed for this, maybe you have an opinion?

Bram, Panos told me to ni? you on the text. I took it from the cert warning document with small modifications, now's your chance to get it changed! :)

Comment 19

[[:philipp]]

would such a warning also make sense for timing-related OCSP errors (sec_error_ocsp_future_response, sec_error_ocsp_old_response)?

and another suggestion (depending on how sensitive the time skew detection will be implemented): maybe we could squeeze into the error text to fix date, time & _timezone_ since sometimes we come across cases in sumo, where the latter part plays a role as well

Comment 20

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

(In reply to philipp from comment #19)
> would such a warning also make sense for timing-related OCSP errors
> (sec_error_ocsp_future_response, sec_error_ocsp_old_response)?

Yes - good call.

Comment 21

[:Gijs (he/him)]

Comment on attachment 8741409 [details] [diff] [review]
Display a warning on about:certerror if the system time is wrong

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

::: browser/base/content/aboutNetError.xhtml
@@ +219,5 @@
>        {
>          var err = getErrorCode();
>          gIsCertError = (err == "nssBadCert");
>  
> +        addEventListener("AboutCertErrorWrongTime", function (event) {

Generally the rest of the code uses events to communicate from page to content script, not the other way around - that way around, content.js can just manipulate the DOM directly. Let's do that here, too (ie, please do the manipulations below directly from content.js ).

::: browser/base/content/content.js
@@ +269,5 @@
> +        if (Services.prefs.getPrefType(PREF_KINTO_CLOCK_SKEW_SECONDS)) {
> +          let difference = Services.prefs.getIntPref(PREF_KINTO_CLOCK_SKEW_SECONDS);
> +
> +          // XXX what's a good heuristic for that?
> +          if (difference > 10000) {

Anything more than a day (difference is in seconds, so 60 * 60 * 24) seems too much of a difference.

On the other hand, your initial suggestion of 10000 is quite small (under 3 hours), and worse, you're displaying dates to the user (at least on my machine, new Intl.DateTimeFormat().format(new Date()); produces "14/04/2016". So you're now liable to end up with a string that says:


Your computer thinks today is 14/04/2016, while it appears to actually be 14/04/2016 .

which is not very helpful at all. :-)

So I would suggest (a) taking a full day as the minimum difference, and/or (b) making sure that we list the time of day as well as the date in both formats (probably only hours::minutes).

@@ +270,5 @@
> +          let difference = Services.prefs.getIntPref(PREF_KINTO_CLOCK_SKEW_SECONDS);
> +
> +          // XXX what's a good heuristic for that?
> +          if (difference > 10000) {
> +            let actualTime = Services.prefs.getIntPref(PREF_KINTO_LAST_UPDATE);

Halp. No, I hope not?

The claim, throughout this patch, is that this is a value in seconds. But to keep that up-to-date we would be phoning home every second (unless we're updating it locally as well, which I checked, and we're not, AFAICT). I'm sure the kinto code is not that daft, but I can't (through a quick dxr search) figure out how often https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/nsBlocklistService.js#516 gets called.

Even if we phone home every 10 minutes, we're still going to be liable to showing the wrong date right around midnight etc. I would hope we phone home even less often than that, but there we are...

It seems like the correct way to compute this would be to take the current time (Date.now()) and alter by clock skew (assuming that that preference is signed, ie has both positive and negative values so we can use it that way).

::: browser/locales/en-US/chrome/overrides/netError.dtd
@@ +191,5 @@
>  <!ENTITY weakCryptoAdvanced.override "(Not secure) Try loading <span class='hostname'></span> using outdated security">
>  
> +<!-- LOCALIZATION NOTE (certerror.wrongSystemTime) - The <span id='..' /> tags will be injected with actual values,
> +     please leave them unchanged. -->
> +<!ENTITY certerror.wrongSystemTime "<p>We can't create a secure connection to <span id='wrongSystemTime_URL'/> because your computer's clock appears to be off.</p> <p>Your computer thinks today is <span id='wrongSystemTime_systemDate'/>, while it appears to actually be <span id='wrongSystemTime_actualDate'/>. Fixing your date and time may solve this problem.</p>">

Who came up with this text? I think it needs some love, in particular, "Fixing your date and time" - I don't have a time machine, so my date and time are not alterable. My computer's date and time settings, on the other hand... could be fixed. :-)

Normally for things like this someone from UX like Matej suggests phrasing (usually much better than anything I come out with). Did that already happen? I looked through the deps and didn't see anything, but it's totally possible I missed it...

Comment 22

[Bram Pitoyo [:bram]]

Hi Gijs, I came up with this text as part of a larger set that includes all other common error messages. I agree that it’s a bit unclear.

The alteration I would make is something like this:

> We can't create a secure connection to [wrongSystemTime_URL] because your computer's clock appears to show an incorrect time.
> 
> Your computer thinks that today is [wrongSystemTime_systemDate], but the correct time should be [wrongSystemTime_actualDate]. > 
> To fix this problem, change your computer's date and time settings to match the correct time.

Let’s also ask Matej for phrasing help, who I’m needinfo-ing here.

Comment 23

[Johann Hofmann [:johannh]]

Sorry, I should've commented on this more precisely. Thanks for everyone's input!

(In reply to :Gijs Kruitbosch from comment #21)
> Comment on attachment 8741409 [details] [diff] [review]
> Display a warning on about:certerror if the system time is wrong
> 
> 
> Anything more than a day (difference is in seconds, so 60 * 60 * 24) seems
> too much of a difference.
> 
> On the other hand, your initial suggestion of 10000 is quite small (under 3
> hours), and worse, you're displaying dates to the user (at least on my
> machine, new Intl.DateTimeFormat().format(new Date()); produces
> "14/04/2016". So you're now liable to end up with a string that says:
> 
> 
> Your computer thinks today is 14/04/2016, while it appears to actually be
> 14/04/2016 .
> 
> which is not very helpful at all. :-)
> 
> So I would suggest (a) taking a full day as the minimum difference, and/or
> (b) making sure that we list the time of day as well as the date in both
> formats (probably only hours::minutes).
> 

You're absolutely right, either we make it a small value and show time or we make it a day when going for dates. I would like to opt for only showing dates, because

- I think the most common use case will be about system time being vastly off, so the date is really the central piece of information
- I assume any user will have easy access to an accurate clock
- We can not make any guarantee that the time is actually correct, it will be incorrect after a minute for sure and I didn't expect the user to set their system time by our reference but rather get a quick impression of "oh wow my computer thinks it's 1980".

Are these assumptions valid? If so, I'll set the difference to 60 * 60 * 24.

> 
> It seems like the correct way to compute this would be to take the current
> time (Date.now()) and alter by clock skew (assuming that that preference is
> signed, ie has both positive and negative values so we can use it that way).
> 

It is not signed unfortunately, however, we could use the last_update_time to determine that. I agree that this is the better solution and will update it. :)


I believe that making an HTTP request to some server and parsing the Date header is a better way to determine the time. It would also cover cases where the Kinto preferences are not set, which is not unlikely with HTTPS not working. This comes with the drawback of being asynchronous (and potentially failing) and I'd like to avoid flickering on that page. We might opt for a hybrid solution, but that could be solved in a follow-up bug.

Comment 24

[Johann Hofmann [:johannh]]

Attachment: Display a warning on about:certerror if the system time is wrong

Comment 25

[Johann Hofmann [:johannh]]

Comment on attachment 8741699 [details] [diff] [review]
Display a warning on about:certerror if the system time is wrong

Gijs, I tried to improve things based on your suggestions. Maybe you can take a look while we're figuring out the exact wording.

I had wanted to test that the dates are actually displayed correctly, but I'm not yet sure how to match them without risking intermittents from different system date settings. I might use Intl from mochitest, I'll think about it over the weekend.

Comment 26

[:Gijs (he/him)]

Comment on attachment 8741699 [details] [diff] [review]
Display a warning on about:certerror if the system time is wrong

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

Sorry, but I'm not quite convinced - see below. :-)


Another thought... ideally we shouldn't show this warning if, say, your system clock is 1 day out but the certificate isn't valid after/before 5 months in the past/future, or something like that. That is, we should only tell you if we think changing the date will help!

Do we have the detailed certificate information in here? Could be a followup, I guess, it already seems complicated enough. :-)

::: browser/base/content/content.js
@@ +281,5 @@
> +
> +            // difference is absolute, so we need to find out if it's positive or negative
> +            if (lastUpdate * 1000 < Date.now()) {
> +              // if the system date is in the future we subtract from it
> +              difference *= -1;

Hm. So how often does kinto update?

The thing is... given kinto time A and system time B, if

Monday: I get a kinto update which gets me a clock skew of a day and a half, with A > B;
Wednesday: I get an SSL error that produces this warning, but now B has increased by 2, and so B > A

this logic doesn't quite work. Even if kinto is meant to update more frequently than that, the machine might have been asleep, or ... well, there are a lot of reasons why the update might not have happened in the meantime.


However, in the kinto updater code, we have all this information, presumably. Why don't we save it when we get it, instead of trying to reverse engineer it here? :-)
We can either use a separate preference if too much already depends on the clock skew thing being unsigned - but the preference system can cope with signed ints, so it shouldn't really be a problem to use them here.

@@ +287,5 @@
> +
> +            let systemDate = new Intl.DateTimeFormat()
> +              .format(new Date());
> +            let actualDate = new Intl.DateTimeFormat()
> +              .format(new Date(Date.now() + difference * 1000));

Nit:

let formatter = new Intl.DateTimeFormat();
let systemDate = formatter.format(new Date());
let actualDate = formatter.format(new Date(Date.now() + difference * 1000));

@@ +289,5 @@
> +              .format(new Date());
> +            let actualDate = new Intl.DateTimeFormat()
> +              .format(new Date(Date.now() + difference * 1000));
> +
> +            // hacky interpolation/injection of our variables

FWIW, we do this all the time, it's really not that hacky ;-)

Comment 27

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 8741699 [details] [diff] [review]
Display a warning on about:certerror if the system time is wrong

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

::: browser/base/content/content.js
@@ +271,5 @@
> +      case SEC_ERROR_OCSP_OLD_RESPONSE:
> +      case MOZILLA_PKIX_ERROR_NOT_YET_VALID_CERTIFICATE:
> +
> +        // use Kinto stats if available
> +        if (Services.prefs.getPrefType(PREF_KINTO_CLOCK_SKEW_SECONDS)) {

You can fall back to using the build date (Services.appinfo.appBuildID) if no blocklist ping has been sent yet.

Comment 28

[Matej Novak [:matej]]

This is how I would reword the copy in comment 22:

> A secure connection to [wrongSystemTime_URL] isn't possible because your clock appears to show the wrong time.
> 
> Your computer thinks it is [wrongSystemTime_systemDate], when it should be [wrongSystemTime_actualDate].
> To fix this problem, change your date and time settings to match the correct time.

Let me know if you have any questions.

Comment 29

[Johann Hofmann [:johannh]]

(In reply to Panos Astithas [:past] from comment #27)
> Comment on attachment 8741699 [details] [diff] [review]
> Display a warning on about:certerror if the system time is wrong
> 
> Review of attachment 8741699 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/base/content/content.js
> @@ +271,5 @@
> > +      case SEC_ERROR_OCSP_OLD_RESPONSE:
> > +      case MOZILLA_PKIX_ERROR_NOT_YET_VALID_CERTIFICATE:
> > +
> > +        // use Kinto stats if available
> > +        if (Services.prefs.getPrefType(PREF_KINTO_CLOCK_SKEW_SECONDS)) {
> 
> You can fall back to using the build date (Services.appinfo.appBuildID) if
> no blocklist ping has been sent yet.

I could, but that would be a date that might be several days or even weeks off the actual date, right? I think it's a good idea, but we might make a follow-up bug out of it because it would need some additional love to make it nice.

Comment 30

[Johann Hofmann [:johannh]]

Attachment: Display a warning on about:certerror if the system time is wrong

Comment 31

[:Gijs (he/him)]

Comment on attachment 8744362 [details] [diff] [review]
Display a warning on about:certerror if the system time is wrong

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

This generally looks good to me, thanks for pursuing this!

Note that the nits I gave below are all for the test, and that they almost all apply multiple times throughout the test - please fix throughout. :-)

::: browser/base/content/test/general/browser_aboutCertError.js
@@ +126,5 @@
> +      let systemDateDiv = doc.getElementById("wrongSystemTime_systemDate");
> +      let actualDateDiv = doc.getElementById("wrongSystemTime_actualDate");
> +
> +      return {
> +        divDisplay: div.style.display,

Please use content.getComputedStyle(div).display instead, so that if we start using CSS to hide this, the test doesn't break.

@@ +137,5 @@
> +
> +  let formatter = new Intl.DateTimeFormat();
> +
> +  // pretend we have a positively skewed (ahead) system time
> +  let serverDate = new Date("10/27/2015");

Nit: Please use international date format - "2015/10/27". :-)

@@ +142,5 @@
> +  let serverDateFmt = formatter.format(serverDate);
> +  let localDateFmt = formatter.format(new Date());
> +
> +  Services.prefs.setIntPref(PREF_KINTO_CLOCK_SKEW_SECONDS,
> +                            Math.floor((Date.now() - serverDate.getTime()) / 1000));

If you use:

yield new Promise(r => SpecialPowers.pushPrefEnv({set: [[PREF_KINTO_CLOCK_SKEW_SECONDS, ...]], r);

then mochitest will clean up after us and reset that pref when our test finishes.

Probably worth moving the pref value into a temporary.

@@ +147,5 @@
> +
> +  info("Loading a bad cert page with a skewed clock");
> +  let message = yield Task.spawn(setUpPage);
> +
> +  is(message.divDisplay, "block", "Wrong time message information is visible");

Nit: use isnot(display, "none", "..."); so that if we use flexbox, inline-block or something else to make this element show up, this still works.

Comment 32

[Johann Hofmann [:johannh]]

Attachment: Display a warning on about:certerror if the system time is wrong

Comment 33

[Johann Hofmann [:johannh]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=496b388ce9af

Comment 34

[Johann Hofmann [:johannh]]

Attachment: Display a warning on about:certerror if the system time is wrong

Comment 35

[Johann Hofmann [:johannh]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=498cb9644614

Comment 36

[Johann Hofmann [:johannh]]

(Fixed try failures due to use of non-unicode apostrophes)

Comment 37

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/d65b48650a68

Comment 38

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/d65b48650a68