The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/822fd5f5b3ab71603d34a376928bf05323abfd0b

commit 822fd5f5b3ab71603d34a376928bf05323abfd0b
Author: estark <estark@chromium.org>
Date: Sat Dec 10 02:11:42 2016

Send crash dumps for odd ERR_CERT_DATE_INVALID cert errors

Safe Browsing Extended Reporting reports show a number of certificate
errors from Windows users where the certificate error is
ERR_CERT_DATE_INVALID but the certificate chain appears to have
perfectly valid dates.

To get some diagnostic data from Canary, this CL calls
DumpWithoutCrashing() when we observe such a chain. This CL is intended
to be reverted after obtaining a few days of data from Canary.

BUG= 672906 

Review-Url: https://codereview.chromium.org/2567643003
Cr-Commit-Position: refs/heads/master@{#437726}

[modify] https://crrev.com/822fd5f5b3ab71603d34a376928bf05323abfd0b/net/cert/cert_verify_proc_win.cc

Turns out this is trivial to reproduce on a Windows machine:
1. set your clock to 2010, visit https://facebook.com and observe bad clock interstitial
2. set your clock to the correct date and click Refresh on the interstitial
3. observe a generic SSL interstitial for an ERR_CERT_DATE_INVALID error, feel sad.

It looks plausible that this could be due to the synchronization logic in time_win.cc which I don't fully understand yet (e.g. [1]). NowFromSystemTime(), which is used to decide to trigger the bad clock interstitial based on the build time, does a "// Force resync", whereas Now() only does that if 60 seconds has elapsed.

[1] https://cs.chromium.org/chromium/src/base/time/time_win.cc?q=time_win.c&sq=package:chromium&l=80

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ba0293aaab7ce154481fdca25e2eceff210da905

commit ba0293aaab7ce154481fdca25e2eceff210da905
Author: estark <estark@chromium.org>
Date: Mon Dec 12 21:29:02 2016

Revert of Send crash dumps for odd ERR_CERT_DATE_INVALID cert errors (patchset #5 id:80001 of https://codereview.chromium.org/2567643003/ )

Reason for revert:
Turns out this issue is trivial to reproduce locally (I probably should have tried that first...) so diagnostic data from crash dumps isn't needed.

Original issue's description:
> Send crash dumps for odd ERR_CERT_DATE_INVALID cert errors
>
> Safe Browsing Extended Reporting reports show a number of certificate
> errors from Windows users where the certificate error is
> ERR_CERT_DATE_INVALID but the certificate chain appears to have
> perfectly valid dates.
>
> To get some diagnostic data from Canary, this CL calls
> DumpWithoutCrashing() when we observe such a chain. This CL is intended
> to be reverted after obtaining a few days of data from Canary.
>
> BUG= 672906 
>
> Committed: https://crrev.com/822fd5f5b3ab71603d34a376928bf05323abfd0b
> Cr-Commit-Position: refs/heads/master@{#437726}

TBR=rsleevi@chromium.org,wfh@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 672906 

Review-Url: https://codereview.chromium.org/2572553002
Cr-Commit-Position: refs/heads/master@{#437916}

[modify] https://crrev.com/ba0293aaab7ce154481fdca25e2eceff210da905/net/cert/cert_verify_proc_win.cc

Pretty sure it's because of kMaxMillisecondsToAvoidDrift check in base::Time::Now:
https://cs.chromium.org/chromium/src/base/time/time_win.cc?rcl=0&l=152

- When the first SSL error is encountered, assume the clock is correctly obtained (e.g. because InitializeClock is called). Call this time t0. |initial_ticks| in Time::Now equals t0.
- The user changes their clock to a future time t1 (e.g. +1 year).
- Assume Time::Now is called after 5 seconds after the clock change. |ticks| in Time::Now will be 5 seconds after t0 so |elapsed| is 5 seconds. This is less than kMaxMillisecondsToAvoidDrift (60 seconds), so InitializeClock is not called.
- The function happily returns elapsed + initial_time (5 + t0) instead of t1.

Adding miu as a base/time owner. The tl;dr of this bug is that Chrome warns the user to fix their clock in order to be able to make a secure connection, but then (on Windows) the system clock change doesn't really take effect for 60 seconds, meaning that users frequently follow the warning's advice and then refresh just to see another warning.

Mustafa and I have been discussing two possible approaches for fixing this:

1. Maybe the net stack should be using base::Time::NowFromSystemTime() in places where it's currently using base::Time::Now()? Or NowFromSystemTime() should be renamed to Now() and Now() should be renamed to... something. (HighResolutionSortOfNow?) In other words, maybe the net stack should be using real system time instead of the "cached" system time that base::Time::Now returns on Windows.

2. Or, maybe the bad clock interstitial page is the only use case where we actually care about this problem, and so we should try to fix this from the interstitial code. From reading the base::Time code, I think that we could make the problem go away by calling NowFromSystemTime() when the bad clock interstitial is dismissed (which would cause base::Time to read an updated system time that will get used on subsequent calls to Now). But that seems kinda hacky because base::Time doesn't really have a documented guarantee that calling NowFromSystemTime() will cause subsequent Now() calls to use an updated system time.

sleevi or miu, do you have any thoughts?

I'm going to premptively CC brucedawson@ on this, because I'm not sure what, if any, impact on power these changes might have - and I know timers & power usage are the peanut butter and nutella of Windows.

It does feel 'wrong' that base::Time::Now() is so... weird... on Windows. I'd also be concerned with //net switching to NowFromSystemTime becoming the new cargo cult - and the nuance between precision and accuracy being lost.

Should we be watching for WM_TIMECHANGE events and forcing a base::Time::NowFromSystemTime on Windows to reinitialize the timer? I don't see why not.

Windows really hates it if you try to have time that is both precise and accurate.

> Should we be watching for WM_TIMECHANGE events and forcing a
> base::Time::NowFromSystemTime on Windows to reinitialize the timer?

That seems reasonable. Such a change should have no effect on power unless those messages show up way more frequently than I would have expected.

Although, I'm not sure why calling InitializeClock occasionally actually works. It seems racy to be writing to two global variables at unpredictable times. There must be some risk of other threads seeing anomalous results during that time. Even if there isn't a race then 60 s of drift could still cause time to go backwards slightly. But, I guess that works.

Regarding raciness, there is bug 259944 with a longer explanation and it has similar conclusions (time going backwards slightly).

I always worry about using the system clock to determine the current calendar date and time under an assumption of trust. Especially so for these cert validation use cases. Instead of base::Time, perhaps Chrome should ask a known-reliable time server (i.e., NOT the user's system) for the current time?

Regarding using alternative time sources, "trusted time" is a quite complex problem, we're aware of and exploring the space (e.g. https://www.imperialviolet.org/2016/09/19/roughtime.html / https://roughtime.googlesource.com/roughtime ), but that shouldn't distract from base::Time being a critical component of trust already, and needing to be reliable for purpose.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/06007208e1229de1a3f7ee70b5a6f2c49589951c

commit 06007208e1229de1a3f7ee70b5a6f2c49589951c
Author: estark <estark@chromium.org>
Date: Wed Jan 18 01:29:07 2017

Call base::Time::NowFromSystemTime() on WM_TIMECHANGE

On Windows, base::Time only consults the system clock once every 60
seconds. Thus, prior to this CL, if the system clock changed,
base::Time::Now() would continue to reflect the old system clock for up
to 60 seconds. This was somewhat weird and caused problems for UI that
instructed the user to fix their clock. (A warning would tell the user
to fix their clock in order to make a secure connection, but after
fixing their clock and refreshing, they still would not be able to
establish a connection.)

To fix this, we now listen for WM_TIMECHANGE messages and call
NowFromSystemTime() upon receiving them. NowFromSystemTime()
reinitializes base::Time from the current system clock.

BUG= 672906 
TEST=Set system clock to the year 2010 and load https://google.com.
Observe an interstitial page prompting you to fix your clock. Fix your
system clock back to the true current date, and refresh. Observe that
the page loads as normal.

Review-Url: https://codereview.chromium.org/2628423002
Cr-Commit-Position: refs/heads/master@{#444228}

[modify] https://crrev.com/06007208e1229de1a3f7ee70b5a6f2c49589951c/ui/views/win/hwnd_message_handler.cc
[modify] https://crrev.com/06007208e1229de1a3f7ee70b5a6f2c49589951c/ui/views/win/hwnd_message_handler.h

Verified on 57.0.2985.0