Closed Bug 1267229 Opened 4 years ago Closed 3 years ago

Use the build date to determine wrong computer clock on expired certificate errors

Categories

(Firefox :: Security, defect, P3)

defect

Tracking

()

RESOLVED FIXED
Firefox 53
Tracking Status
firefox53 --- fixed

People

(Reporter: johannh, Assigned: johannh)

References

Details

(Whiteboard: [fxprivacy])

Attachments

(2 files, 5 obsolete files)

In Bug 712612 we implemented a message on the NetError page that informs the user that their system time might be wrong in case of expired certificate (or similar) errors. We use the Kinto clock skew pref for that. This pref likely exists, however, it is not guaranteed to be there.

To cover these cases we could use Services.appinfo.appBuildID to determine if the system time is behind the current build date and show an error then. Note that the message from Bug 712612 includes the correct server date, which we don't have in this case. So we might want a slightly different message here.
Assignee: nobody → heejongahn
As suggested by Johann, I used Services.appinfo.appBuildID to determine the what would be the actual local date for the user. As build date might be few days to even few months before actual date, I used a year difference as an threshold. If the time difference exceeds the threshold, a corresponding error message(wrongSystemTimeWithoutReferencePanel) is shown.

Unfortunately, I couldn't provide a test for this. I tried to add a case in mozilla-central/browser/base/content/test/generalbrowser_aboutCertError.js file, but to test what this, I needed to explicitly set Services.appinfo.appBuildID. I tried it using createAppInfo function for that, but it actually didn't behave as I expected and I couldn't think of other workaround.
Thanks for picking this up! It looks already pretty good to me, but maybe Gijs can give you a better review?

Panos, it seems we didn't triage this but since you suggested it yourself in Bug 712612 I'm assuming we can go ahead here. It's a simple change and makes a lot of sense IMO.
Comment on attachment 8746719 [details] [diff] [review]
Handle wrong system time when kinto clock skew is not available

Ok, let's try MattN if Gijs is on PTO :)
Attachment #8746719 - Flags: review?(MattN+bmo)
Comment on attachment 8746719 [details] [diff] [review]
Handle wrong system time when kinto clock skew is not available

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

::: browser/base/content/content.js
@@ +225,1 @@
>  const MOZILLA_PKIX_ERROR_BASE = Ci.nsINSSErrorsService.MOZILLA_PKIX_ERROR_BASE;

Ugh… content.js became a dumping ground like I warned about… I don't understand why AboutNetAndCertErrorListener isn't in its own file but that's for a separate bug…

@@ +283,3 @@
>  
>          // use Kinto stats if available
>          if (Services.prefs.getPrefType(PREF_KINTO_CLOCK_SKEW_SECONDS)) {

Nit: this is assuming that the desired type is truthy but this should be using the const's instead

@@ +308,5 @@
> +          let appBuildID = Services.appinfo.appBuildID;
> +
> +          let year = parseInt(appBuildID.substr(0,4));
> +          let month = parseInt(appBuildID.substr(4,2)) - 1;
> +          let day = parseInt(appBuildID.substr(6,2));

I thought I was involved in a bug years ago where it was mentioned that we shouldn't assume the build ID is a date in the future. We should double-check with releng about this and definitely have a test that ensures that it is a date that can be parsed and is in the correct century or something.

@@ +314,5 @@
> +          let buildDate = new Date(year, month, day);
> +
> +          let difference = Date.now() - buildDate;
> +
> +          if (Math.abs(difference) > 1000 * 60 * 60 * 24 * 365) {

Shouldn't we show a message if the current time is ever less than the build date as that shouldn't be possible without a time machine?

@@ +320,5 @@
> +
> +            let systemDate = formatter.format(new Date());
> +
> +            content.document.getElementById("wrongSystemTimeWithoutReference_URL")
> +              .textContent = content.document.location.hostname;

This should be .host (or .origin) so it includes the non-default port as different ports could be using different certificates. The same issue should be fixed in the kinto code above.
Attachment #8746719 - Flags: review?(MattN+bmo) → feedback+
(In reply to Matthew N. [:MattN] (behind on reviews) from comment #5)
> Comment on attachment 8746719 [details] [diff] [review]
> Handle wrong system time when kinto clock skew is not available
> 
> Review of attachment 8746719 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/base/content/content.js
> @@ +225,1 @@
> >  const MOZILLA_PKIX_ERROR_BASE = Ci.nsINSSErrorsService.MOZILLA_PKIX_ERROR_BASE;
> 
> Ugh… content.js became a dumping ground like I warned about… I don't
> understand why AboutNetAndCertErrorListener isn't in its own file but that's
> for a separate bug…
> 

AboutNetAndCertErrorListener has been growing only recently. Yeah, we should open a bug for extracting it.

> @@ +314,5 @@
> > +          let buildDate = new Date(year, month, day);
> > +
> > +          let difference = Date.now() - buildDate;
> > +
> > +          if (Math.abs(difference) > 1000 * 60 * 60 * 24 * 365) {
> 
> Shouldn't we show a message if the current time is ever less than the build
> date as that shouldn't be possible without a time machine?

I agree.

> @@ +320,5 @@
> > +
> > +            let systemDate = formatter.format(new Date());
> > +
> > +            content.document.getElementById("wrongSystemTimeWithoutReference_URL")
> > +              .textContent = content.document.location.hostname;
> 
> This should be .host (or .origin) so it includes the non-default port as
> different ports could be using different certificates. The same issue should
> be fixed in the kinto code above.

We use location.hostname for all other errors on about:neterror, including other certificate errors. We'd need to change all of them for consistency.
> I thought I was involved in a bug years ago where it was mentioned that we shouldn't assume the build ID is a date in the future. We should double-check with releng about this and definitely have a test that ensures that it is a date that can be parsed and is in the correct century or something.

Rok, can you check for us if the build ID is guaranteed to be a date in the future? Thank you!! :)
Flags: needinfo?(rgarbas)
(In reply to Matthew N. [:MattN] (behind on reviews) from comment #5)

> @@ +308,5 @@
> > +          let appBuildID = Services.appinfo.appBuildID;
> > +
> > +          let year = parseInt(appBuildID.substr(0,4));
> > +          let month = parseInt(appBuildID.substr(4,2)) - 1;
> > +          let day = parseInt(appBuildID.substr(6,2));
> 
> I thought I was involved in a bug years ago where it was mentioned that we
> shouldn't assume the build ID is a date in the future. We should
> double-check with releng about this and definitely have a test that ensures
> that it is a date that can be parsed and is in the correct century or
> something.

I assume you mean _past_ here? BuildID should always be a date in the past, in normal situations?

I'm not sure when it's not a date, maybe for 3rd party builds?

> > +          if (Math.abs(difference) > 1000 * 60 * 60 * 24 * 365) {
> 
> Shouldn't we show a message if the current time is ever less than the build
> date as that shouldn't be possible without a time machine?

Also, is there a particular reason 1 year was chosen?

I remember APF talked about some data from Chrome (https://twitter.com/__apf__/status/685172030887481344?lang=en, slide 15), saying that ~50% of clock errors were < 1 month.
(In reply to Justin Dolske [:Dolske] from comment #8)
> (In reply to Matthew N. [:MattN] (behind on reviews) from comment #5)
> 
> > @@ +308,5 @@
> > > +          let appBuildID = Services.appinfo.appBuildID;
> > > +
> > > +          let year = parseInt(appBuildID.substr(0,4));
> > > +          let month = parseInt(appBuildID.substr(4,2)) - 1;
> > > +          let day = parseInt(appBuildID.substr(6,2));
> > 
> > I thought I was involved in a bug years ago where it was mentioned that we
> > shouldn't assume the build ID is a date in the future. We should
> > double-check with releng about this and definitely have a test that ensures
> > that it is a date that can be parsed and is in the correct century or
> > something.
> 
> I assume you mean _past_ here? BuildID should always be a date in the past,
> in normal situations?

No, I mean that in the future the build "ID" (not labelled as a date) could be some arbitrary identifier.

> I'm not sure when it's not a date, maybe for 3rd party builds?

Me either so I don't think we should assume/guess and should enforce this with an automated test and get releng onboard if we want to abuse it for this.
There is discussion every few years about whether "buildid" should continue to be a date-like string, or some other identifier. Would introducing a new attribute to store the build date be feasible?

(In reply to Justin Dolske [:Dolske] from comment #8)
> > > +          if (Math.abs(difference) > 1000 * 60 * 60 * 24 * 365) {
> > 
> > Shouldn't we show a message if the current time is ever less than the build
> > date as that shouldn't be possible without a time machine?
> 
> Also, is there a particular reason 1 year was chosen?
> 
> I remember APF talked about some data from Chrome
> (https://twitter.com/__apf__/status/685172030887481344?lang=en, slide 15),
> saying that ~50% of clock errors were < 1 month.

Keep in mind that we have a fairly long tail of users who are not up-to-date. Also consider our ESR builds. Is it really a good idea to assume that if somebody has a build that is > 1yr old, that their clock is wrong?
(In reply to Chris AtLee [:catlee] from comment #10)
> There is discussion every few years about whether "buildid" should continue
> to be a date-like string, or some other identifier. Would introducing a new
> attribute to store the build date be feasible?

I think this is a good idea (if possible). I think having an attribute guaranteed to contain build date is better than hoping that buildID remains as current form and use regex to ensure it.

> Keep in mind that we have a fairly long tail of users who are not
> up-to-date. Also consider our ESR builds. Is it really a good idea to assume
> that if somebody has a build that is > 1yr old, that their clock is wrong?

There was no particular reason that I chose 1 year as the threshold value. There surely is tradeoff and I just thought most people will have Firefox built within past one year, but apparently there are some cases I couldn't think of... Still not sure about what would be the optimal value. Or should we show other message instead of just saying their clock might be wrong?
Flags: needinfo?(rgarbas)
(In reply to Chris AtLee [:catlee] from comment #10)
> There is discussion every few years about whether "buildid" should continue
> to be a date-like string, or some other identifier. Would introducing a new
> attribute to store the build date be feasible?
> 

While this feature by itself is very tiny, it seems that having a reliable date string might be something more people would want. Certainly has my vote.

> (In reply to Justin Dolske [:Dolske] from comment #8)
> > > > +          if (Math.abs(difference) > 1000 * 60 * 60 * 24 * 365) {
> > > 
> > > Shouldn't we show a message if the current time is ever less than the build
> > > date as that shouldn't be possible without a time machine?
> > 
> > Also, is there a particular reason 1 year was chosen?
> > 
> > I remember APF talked about some data from Chrome
> > (https://twitter.com/__apf__/status/685172030887481344?lang=en, slide 15),
> > saying that ~50% of clock errors were < 1 month.
> 
> Keep in mind that we have a fairly long tail of users who are not
> up-to-date. Also consider our ESR builds. Is it really a good idea to assume
> that if somebody has a build that is > 1yr old, that their clock is wrong?

We're only making these assumptions if you are on a page that failed to load because the certificate has expired and it's only to display a warning on that page. Personally, I'd rather show some false positives for this warning than not show it at all. Also, this is used as fallback in case the Kinto preference we normally use is not available. Any ESR user with a Kinto clock_skew preference would get the correct message.
(In reply to Johann Hofmann [:johannh] from comment #12)
> Also, this is used as fallback in case the Kinto
> preference we normally use is not available. Any ESR user with a Kinto
> clock_skew preference would get the correct message.

Taking a step back, once we enable Kinto, everyone should have that pref so why are we even bothering with this bug? Is it just for the short term until Kinto is enabled? If that's going to happen soon then it seems like we should WONTFIX.
(In reply to Johann Hofmann [:johannh] from comment #12)
> (In reply to Chris AtLee [:catlee] from comment #10)
> > Keep in mind that we have a fairly long tail of users who are not
> > up-to-date. Also consider our ESR builds. Is it really a good idea to assume
> > that if somebody has a build that is > 1yr old, that their clock is wrong?
> 
> We're only making these assumptions if you are on a page that failed to load
> because the certificate has expired and it's only to display a warning on
> that page. Personally, I'd rather show some false positives for this warning
> than not show it at all. Also, this is used as fallback in case the Kinto
> preference we normally use is not available. Any ESR user with a Kinto
> clock_skew preference would get the correct message.

The purpose of this bug is to show the warning message when the user system clock says that the current time is before the build was created. So if one is using say an old ESR build from 2014, then in order to show this particular error the system clock would have to say that it's not 2014 yet. So I don't think there is a way to have false positives here.

(In reply to Matthew N. [:MattN] (behind on reviews) from comment #13)
> Taking a step back, once we enable Kinto, everyone should have that pref so
> why are we even bothering with this bug? Is it just for the short term until
> Kinto is enabled? If that's going to happen soon then it seems like we
> should WONTFIX.

It is for that case, but also for the cases where TLS connections fail early in a browsing session, before the kinto pref has had a chance to be set (e.g. slow/failed connection to mozilla.org).
(In reply to Panos Astithas [:past] from comment #14)
> (In reply to Johann Hofmann [:johannh] from comment #12)
> > (In reply to Chris AtLee [:catlee] from comment #10)
> > > Keep in mind that we have a fairly long tail of users who are not
> > > up-to-date. Also consider our ESR builds. Is it really a good idea to assume
> > > that if somebody has a build that is > 1yr old, that their clock is wrong?
> > 
> > We're only making these assumptions if you are on a page that failed to load
> > because the certificate has expired and it's only to display a warning on
> > that page. Personally, I'd rather show some false positives for this warning
> > than not show it at all. Also, this is used as fallback in case the Kinto
> > preference we normally use is not available. Any ESR user with a Kinto
> > clock_skew preference would get the correct message.
> 
> The purpose of this bug is to show the warning message when the user system
> clock says that the current time is before the build was created. So if one
> is using say an old ESR build from 2014, then in order to show this
> particular error the system clock would have to say that it's not 2014 yet.
> So I don't think there is a way to have false positives here.
> 

That would cover certificates that are not yet valid because the system time is in the past. Don't we also want to warn if people's system time is in the future and the certificate is considered expired? If the build date is 2014 that would give a false positive for 2016 but a correct result for 2030. (Assuming we use a year as measure)

> (In reply to Matthew N. [:MattN] (behind on reviews) from comment #13)
> > Taking a step back, once we enable Kinto, everyone should have that pref so
> > why are we even bothering with this bug? Is it just for the short term until
> > Kinto is enabled? If that's going to happen soon then it seems like we
> > should WONTFIX.
> 
> It is for that case, but also for the cases where TLS connections fail early
> in a browsing session, before the kinto pref has had a chance to be set
> (e.g. slow/failed connection to mozilla.org).

Yes, as mentioned in the original description:

This pref likely exists, however, it is not guaranteed to be there.

If there was a successful Kinto ping, this pref is set. If there was no successful request yet, it's not yet set.

So I think this patch is definitely justified.
(In reply to Johann Hofmann [:johannh] from comment #15)
> That would cover certificates that are not yet valid because the system time
> is in the past. Don't we also want to warn if people's system time is in the
> future and the certificate is considered expired? If the build date is 2014
> that would give a false positive for 2016 but a correct result for 2030.
> (Assuming we use a year as measure)

I'm not sure the result would be correct even for 2030; as catlee suggested, people of the future may be trying ancient builds for some reason (or perhaps the Mars space station will have eaten all of their data plan and they won't be able to update the Firefox on the Curiosity Rover).

From what I've read, the most common case for a bogus system clock is a dead CMOS battery in the motherboard that results in a time of ~1970. That should be enough for this extra check IMO.
Ok, despite the discussion that evolved I'd really like to see this happening. :heejongahn would you still like to finish this bug? I think there are two main things to do:

- change the if condition to only check if the current date is before the build date
- add a test that the build date is actually a date so that we're notified if that ever changes

Matt, do you think this is enough? I would say the other issues can be left for follow-up bugs. Do you know a way to get around the problem with the tests from https://bugzilla.mozilla.org/show_bug.cgi?id=1267229#c2?
Flags: needinfo?(heejongahn)
Flags: needinfo?(MattN+bmo)
(In reply to Johann Hofmann [:johannh] from comment #15)
> If there was a successful Kinto ping, this pref is set. If there was no
> successful request yet, it's not yet set.
> 
> So I think this patch is definitely justified.

That seems quite strong considering how rare this should be but okay.

(In reply to Johann Hofmann [:johannh] from comment #17)
> Ok, despite the discussion that evolved I'd really like to see this
> happening. :heejongahn would you still like to finish this bug? I think
> there are two main things to do:
> 
> - change the if condition to only check if the current date is before the
> build date
> - add a test that the build date is actually a date so that we're notified
> if that ever changes
> 
> Matt, do you think this is enough? I would say the other issues can be left
> for follow-up bugs.

Okay, I don't know exactly what other issues you are referring to that would deserve follow-ups though.

> Do you know a way to get around the problem with the
> tests from https://bugzilla.mozilla.org/show_bug.cgi?id=1267229#c2?

It seems like simply a matter of modifying testing/modules/AppInfo.jsm, no?
Flags: needinfo?(MattN+bmo)
> Okay, I don't know exactly what other issues you are referring to that would deserve follow-ups though.

I was referring to

> > 
> > Ugh… content.js became a dumping ground like I warned about… I don't
> > understand why AboutNetAndCertErrorListener isn't in its own file but that's
> > for a separate bug…
> > 

and

> > 
> > This should be .host (or .origin) so it includes the non-default port as
> > different ports could be using different certificates. The same issue should
> > be fixed in the kinto code above.
> 
> We use location.hostname for all other errors on about:neterror, including
> other certificate errors. We'd need to change all of them for consistency.

That one could be a good first bug as well.
See Also: → 1281129
I made Bug 1281129 to deal with the problem of missing MOZILLA_PKIX_ERROR_NOT_YET_VALID_ISSUER_CERTIFICATE since it's a regression that should be dealt with and not the subject of this bug. :)
Component: Security: UI → Security
Product: Core → Firefox
Unassigning due to 3 months of inactivity. Feel free to post an update if you have time to work on this again.
Assignee: heejongahn → nobody
See Also: → 1314470
Priority: -- → P3
Whiteboard: [fxprivacy]
I added the suggested changes and transformed the if condition to always check against
the build date if the Kinto skew check failed.

I also added a test asserting that the buildID contains a valid date into the aboutCertError mochitest.
I know that an xpcshell test would be sufficient for this, but I feel like creating a new test file just for that
would actually be less efficient than adding it to the existing tests, where it also has some context.

I was not able to change the appinfo.appBuildID in mochitests, updateAppInfo only seems to work in xpcshell,
and I'd want to test UI so that isn't an option. Hence no UI test. I'll happily take suggestions.
Attachment #8812135 - Flags: review?(MattN+bmo)
Attachment #8746719 - Attachment is obsolete: true
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
Noticed that parseInt wasn't specifying a radix. :)
Attachment #8812135 - Attachment is obsolete: true
Attachment #8812135 - Flags: review?(MattN+bmo)
Flags: needinfo?(heejongahn)
Attachment #8812138 - Flags: review?(MattN+bmo)
Matt, feel free to forward or cancel review if you're too busy. Just wanted to give you a chance to comment on this since you originally reviewed it.
Comment on attachment 8812138 [details] [diff] [review]
Handle wrong system time when kinto clock skew is not available

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

Thanks

::: browser/base/content/content.js
@@ +319,5 @@
>  
> +        // If the difference is more than a day.
> +        if (Math.abs(difference) > 60 * 60 * 24) {
> +          let formatter = new Intl.DateTimeFormat();
> +          let systemDate = formatter.format(new Date());

Because of the indentation change this was much easier to review on mozreview… (I pushed it as my own draft)

::: browser/locales/en-US/chrome/overrides/netError.dtd
@@ +199,5 @@
>  
>  <!-- LOCALIZATION NOTE (certerror.wrongSystemTime) - The <span id='..' /> tags will be injected with actual values,
>       please leave them unchanged. -->
>  <!ENTITY certerror.wrongSystemTime "<p>A secure connection to <span id='wrongSystemTime_URL'/> isn’t possible because your clock appears to show the wrong time.</p> <p>Your computer thinks it is <span id='wrongSystemTime_systemDate'/>, when it should be <span id='wrongSystemTime_actualDate'/>. To fix this problem, change your date and time settings to match the correct time.</p>">
> +<!ENTITY certerror.wrongSystemTimeWithoutReference "<p>A secure connection to <span id='wrongSystemTimeWithoutReference_URL'/> isn’t possible because your clock appears to show the wrong time.</p> <p>Your computer thinks it is <span id='wrongSystemTimeWithoutReference_systemDate'/>. Are you sure that it is configured right? To fix this problem, change your date and time settings to match the correct time.</p>">

Did we get copywriting help with the variation to the string?

"Are you sure that it is configured right?" feels wrong in this context. "Right" can be used as an adverb but I think it's too informal for this case. I would prefer s/right/correctly/ but also in this case we know it's not correct since it's in the past (our the build machine's time was wrong which hopefully would be rare) so I'm not sure we should ask the question. To also avoid the two short sentences ("Your computer thinks it is …. Are you sure that it is configured right?") I would probably just remove the question.

i.e. please ping a copywriter on this.
Attachment #8812138 - Flags: review?(MattN+bmo) → review+
> Did we get copywriting help with the variation to the string?

Thanks for catching that, we did indeed not. I agree with your sentiment, but maybe mheubusch has a better idea...
Flags: needinfo?(mheubusch)
Michelle, as mentioned, the text right now is

"
A secure connection to ... isn’t possible because your clock appears to show the wrong time. Your computer thinks it is ... . Are you sure that it is configured right? To fix this problem, change your date and time settings to match the correct time.
"

Any suggestions?

I can upload a screenshot later :)

Thanks!
Attached image screenshot
See attached screenshot
Summary: Use the build date to to determine wrong computer clock on expired certificate errors → Use the build date to determine wrong computer clock on expired certificate errors
Thank you for the screenshot, :johannh.  Can you tell me where the Learn more link leads as well as the Go back and Advanced buttons (or if you can direct me to the about: page where this message lives. That will help me ensure the message is complete and actionable.
Flags: needinfo?(mheubusch) → needinfo?(jhofmann)
(In reply to mheubusch from comment #30)
> Can you tell me where the Learn more link leads

A URL like: https://support.mozilla.org/1/firefox/53.0a1/Darwin/en-US/time-errors

> as well as the Go back

"Go back" is like the browser back button, it takes the user back to their previous page (or about:home if this was the first load in a tab)

> and Advanced buttons (or if you can
> direct me to the about: page where this message lives. That will help me
> ensure the message is complete and actionable.

Advanced opens up the usual advanced details at the bottom with technical information.

You can test the current version of the page at https://expired.badssl.com/ though your clock would have to be incorrect to see the existing clock-related message.
Flags: needinfo?(jhofmann) → needinfo?(mheubusch)
Hm, it would be great to finally resolve this.

Michelle, ping? :)
For the main headline and body of the page, I recommend:

This connection is not secure

Firefox did not connect to <URL> because your computer’s clock appears to show the wrong time and this is preventing a secure connection. 

Your computer is set to <date and time>. Is this correct? If so, this website is not configured properly and may not protect your personal information. If not, updating your computer’s date and time settings should fix the problem.  

Learn more about secure connection errors and how they protect you online. 

[Go Back]  [Try to Connect]

Note that I am assuming that the Advanced button (now called Try to Connect) will allow you to add an exception in the case of clocks not matching.  I reset my clock and tried to get this error but just saw the expired certificate. But if this is not a valid assumption, and Firefox does not allow the user to add an exception in this case, the button should be called See Details.  
adding Bram to the thread just to validate this copy works with the structure he created for all error messages.
Flags: needinfo?(mheubusch) → needinfo?(bram)
> adding Bram to the thread just to validate this copy works with the structure he created for all error messages

Yes. This copy works with the structure.

Separate concern: is “Advanced” still the right wording for the error page button? Maybe “See Details” is the right one. This is a larger discussion that we should have (in person or in another bug!)
Flags: needinfo?(bram)
Thanks! The "Advanced" button doesn't let you directly add an exception but first shows cert error details. So "See Details" might indeed be more fitting, but that discussion should probably happen somewhere else, as it affects all cert error pages.
Michelle, while trying to integrate your copy I noticed a few things we might need to change.

(In reply to mheubusch from comment #33)
> For the main headline and body of the page, I recommend:
> 
> This connection is not secure
> 
> Firefox did not connect to <URL> because your computer’s clock appears to
> show the wrong time and this is preventing a secure connection. 
> 
> Your computer is set to <date and time>. Is this correct? If so, this
> website is not configured properly and may not protect your personal
> information. If not, updating your computer’s date and time settings should
> fix the problem. 

The question is not really necessary. Unlike our approach of comparing with the Kinto server date, this approach is guaranteed to show that the user's system clock is behind. So we're 99.9% confident that the system clock is really wrong, and there's no issue with the website. I would just leave it out, but then the other sentences don't make sense anymore either.

> 
> Learn more about secure connection errors and how they protect you online. 

The provided link is more talking about time related in issues specifically. I'll keep this as "Learn More", for now, also considering that the "Learn More" text has never been customized for any cert error page and we'd have to code that up first.

> 
> [Go Back]  [Try to Connect]
> 
> Note that I am assuming that the Advanced button (now called Try to Connect)
> will allow you to add an exception in the case of clocks not matching.  I
> reset my clock and tried to get this error but just saw the expired
> certificate. But if this is not a valid assumption, and Firefox does not
> allow the user to add an exception in this case, the button should be called
> See Details.  
> adding Bram to the thread just to validate this copy works with the
> structure he created for all error messages.
Flags: needinfo?(mheubusch)
MozReview-Commit-ID: Ko2Rq6PLfoX
Attachment #8812138 - Attachment is obsolete: true
Comment on attachment 8828760 [details] [diff] [review]
Handle wrong system time when kinto clock skew is not available

So I took out the parts of your suggestions that wouldn't really work right now and will land it in this rather minimal form. Since there seems to be ongoing efforts to rework the cert error copy anyway, I think this is fine.
Attachment #8828760 - Flags: review+
MozReview-Commit-ID: Ko2Rq6PLfoX
Attachment #8828760 - Attachment is obsolete: true
MozReview-Commit-ID: Ko2Rq6PLfoX
Attachment #8828770 - Attachment is obsolete: true
https://hg.mozilla.org/integration/mozilla-inbound/rev/645331fa032779e6324e32a11898a55e6c75ee24
Bug 1267229 - Handle wrong system time when kinto clock skew is not available. r=MattN
https://hg.mozilla.org/mozilla-central/rev/645331fa0327
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
I really hope someone is around over the week-end to fix this, since you just landed a string change without updating the ID.
https://developer.mozilla.org/en-US/docs/Mozilla/Localization/Localization_content_best_practices#Changing_existing_strings

Monday is merge day, this needs to be fixed *before* it moves to Aurora. 
https://hg.mozilla.org/mozilla-central/diff/645331fa0327/browser/locales/en-US/chrome/overrides/netError.dtd

That's the main reason I hate strings landing at the very end of the cycle
https://hg.mozilla.org/mozilla-central/diff/645331fa0327/browser/locales/en-US/chrome/overrides/netError.dtd
Flags: needinfo?(jhofmann)
Flags: needinfo?(MattN+bmo)
For the record, the patch I reviewed didn't have that problem.

I'll push a fix now.
Flags: needinfo?(jhofmann)
Flags: needinfo?(MattN+bmo)
Pushed by mozilla@noorenberghe.ca:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7fcfc41e319c
Follow-up to bump the certerror.wrongSystemTime string ID due to a change.
(In reply to Matthew N. [:MattN] (PM me if requests are blocking you) from comment #46)
> For the record, the patch I reviewed didn't have that problem.

Indeed :-\

Thank you very much for the quick fix, fixing it post merge day would have been extremely painful on all sides.
Flags: needinfo?(mheubusch)
Depends on: 1358904
You need to log in before you can comment on or make changes to this bug.