-
Notifications
You must be signed in to change notification settings - Fork 55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Validate OCSP response ProducedAt. #515
Conversation
Don't accept an OCSP response if its ProducedAt is outside the leaf cert's NotBefore/NotAfter validity window.
To make it easier to review, you may want to look at this commit separately from the vendor update. |
@@ -429,9 +429,9 @@ func (this *CertCache) readOCSP(allowRetries bool) ([]byte, time.Time, error) { | |||
|
|||
// Print # of retries, wait for specified time and returned updated wait time. | |||
func waitForSpecifiedTime(waitTimeInMinutes int, numRetries int) int { | |||
log.Printf("Retrying OCSP server: retry #%d\n", numRetries) | |||
log.Printf("Retrying OCSP server: retry #%d", numRetries) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you remove the \n on purpose? Did you mean to change it to Println then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the \n is redundant. I saw this done on webpkg. It's a difference between fmt
and log
: https://golang.org/pkg/log/#pkg-overview:~:text=Every%20log%20message%20is%20output%20on,newline%2C%20the%20logger%20will%20add%20one.
packager/certcache/certcache_test.go
Outdated
var err error | ||
this.fakeOCSP, err = FakeOCSPResponse(this.fakeClock.Now()) | ||
this.fakeOCSP, err = FakeOCSPResponse(now, now.Add(1*time.Minute)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a comment here as to why a minute was added?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea. Decided to extract it into the FakeOCSPResponse function so there was only one place to document it.
Don't accept an OCSP response if its ProducedAt is outside the leaf cert's
NotBefore/NotAfter validity window.
Fixes #514.