Closed Bug 1590970 Opened 5 years ago Closed 5 years ago

SSL_SetTimeFunc has incomplete coverage

Categories

(NSS :: Libraries, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mt, Assigned: mt)

Details

Attachments

(2 files)

https://searchfox.org/nss/search?q=PR_Now&case=false&regexp=false&path=lib%2Fssl shows a few places where the internal function is not being called. We need to correct that if those functions are to be used with an external clock.

I missed a few places that used PR_Now() before.

Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.48

It seems this may be causing the intermittent Esni test failures on taskcluster. On this revision, the following command produces random failures with a MacOS debug build:

for ((n=0;n<20;n++)); do ../dist/Debug/bin/ssl_gtest -d ~<path to testdb>/ssl_gtests/ -w --gtest_filter=*Esni*; done

On the revision before it, I get no failures.

Flags: needinfo?(mt)

The ESNI tests were using time() rather than PR_Now(), so they slipped
the net when I went looking for bad time functions. Now they do the right thing
again.

What we were probably seeing in the intermittents was the case where we set the
time for most of the SSL functions to PR_Now(), and that was just before a
second rollover. Then, when time() was called, it returned t+1 so the ESNI keys
that were being generated in the ESNI tests were given a notBefore time that was
in the future relative to the time being given to the TLS stack. Had the ESNI
keys generation been given time() - 1 for notBefore, as I have done here, this
would never have turned up.

It took me a lot more than 20 goes on my machine, but I guess that's because I have a faster box and the time between PR_Now() on the connection setup and time() on the ESNI setup was not spanning the rollover from one second to the next.

I've run the fixed tests hundreds of times now and they seem good.

Flags: needinfo?(mt)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: