Disable some session creation time tests on Windows.

[prbprbprb]

Seems to be an off-by-one issue with the creation time versus system time making things appear they were
created a second in the future.

But only on Windows and only ever 1 second, so disabling the test for now as no real app risk.

[prbprbprb]

@davidben what API does BoringSSL use to populate SSL_SESSION->time on Windows? I'm wondering if it can round to the nearest second rather than measuring elapsed seconds? Which could explain the test flakes on Windows.

[davidben]

@davidben what API does BoringSSL use to populate SSL_SESSION->time on Windows? I'm wondering if it can round to the nearest second rather than measuring elapsed seconds? Which could explain the test flakes on Windows.

We use _ftime, which does report milliseconds, but an SSL_SESSION only stores things at second granularity, so everything is rounded down to seconds. But we round down, not to the nearest second, so it should all be fine? Unless there are multiple subtly inconsistent clocks on Windows.

I'm guessing you don't have the offending times themselves? Is there a way to get junit to print them on error? (Custom message or something? I couldn't find an assertLessThan.)

[davidben]

it should all be fine?

I mean, I might replace time < System.currentTimeMillis() with time <= System.currentTimeMillis() just in case your test runs really really fast. But test_SSLSession_getCreationTime failing is puzzling.

[prbprbprb]

Unless there are multiple subtly inconsistent clocks on Windows

I was wondering about some kind of thread local issue - maybe Java's currentTimeMillis() can lag. It does seem to always be Java 8 as well, so maybe a bona fide Java bug.

replace time < System.currentTimeMillis()

Oops, missed that one, thanks! That's definitely a bug as Github CI seems to run on really fast boxes.

The other test was already using <= and getting times a second apart ... example failure in https://github.com/google/conscrypt/actions/runs/10763531850/job/29845169929

org.conscrypt.ConscryptOpenJdkSuite > org.conscrypt.javax.net.ssl.SSLSessionTest.test_SSLSession_getCreationTime FAILED
    java.lang.AssertionError: 1725834495 <= 1725834494
        at org.junit.Assert.fail(Assert.java:89)
        at org.junit.Assert.assertTrue(Assert.java:42)
        at org.conscrypt.javax.net.ssl.SSLSessionTest.test_SSLSession_getCreationTime(SSLSessionTest.java:96)

[davidben]

(Whoah, how does that print the values? Magical reflection?)

Weird. Yeah, maybe clocks are goofy on Windows. I know Chrome has some goofy things I don't understand here, so I wouldn't be too surprised. Or maybe you're racing with NTP or something, I dunno.

What to do given that, I'm not sure. BoringSSL does have an API to swap out the clock if you want to consistently use one clock. Though that API is currently not Y2038-clean on Windows, so I should replace it before you all do that.

This claims _ftime is using GetSystemTimePreciseAsFileTime under the hood. (Huh, do we need the precise one? We're just rounding to a second anyway. We do use higher granularity for DTLS retransmits, but only millisecond resolution.)
https://developercommunity.visualstudio.com/t/ucrtbases-implementation-of-time-unnecessarily-use/184829

Anyway, if Java is using GetSystemTimeAsFileTime, maybe the two clocks can shift from each other? I don't understand Windows.

[davidben]

Wow. Okay, well, this is probably okay for session creation and seems not okay for the DTLS clock. Also yeeesh. But I suppose this means ideally we'd pass a bool precise into the callback if we're redoing it.

// The default windows timer, GetSystemTimeAsFileTime is not very precise.
// It is only good to ~15.5ms.

https://source.chromium.org/chromium/chromium/src/+/main:base/time/time_win.cc;drc=c8d6b16f02bc9013db2354b8649d2e0d97829a1a;l=11

[davidben]

Although the DTLS retransmit clock really wants to be the monotonic clock anyway. Yeah, all this probably warrants significant reworking on the BoringSSL side. (But it probably won't help this test. No idea what to do here.)

[prbprbprb]

(Whoah, how does that print the values? Magical reflection?)

printf (effectively :)
Assertions can have custom messages, so you can just compose the relevant values into the message. For common assertions like assertEquals the default message prints values etc, but for assertTrue you need to roll your own.

As far as this goes, it sounds like it's not a problem for session creation so maybe we'll just add a margin to the time checks on Windows - other platforms should pick up any real regressions.

And we don't do DTLS so not gonna worry about that (sorry).