letsencrypt / ct-woodpecker

A tool to monitor a certificate transparency log for operational problems
Mozilla Public License 2.0
182 stars 19 forks source link

windowStart shouldn't set NotBefore #99

Closed andygabby closed 5 years ago

andygabby commented 5 years ago

In pki/certs.go IssueTestCertificate(): If windowStart is defined, the NotBefore in issueLeafCert is set to the same value as windowStart. In most use cases we probably don't want to backdate or future date the NotBefore to the beginning of the temporal shard.

Instead we could have issueLeafCert use now or now -1h for NotBefore, but set NotAfter to sometime within the temporal shard window range.

Also, in both windowStart/windowEnd if statements there is an attempt to perform an AddDate() but is not actually used because the AddDate() return value is not assigned back to the earliest/latest vars (NotAfter in issueLeafCert could then have AddDate(0, 0, -1) removed from the latest var as well).

andygabby commented 5 years ago

To quick fix our ct-woodpecker instance that had trouble submitting certs to a CT temporal shard that wouldn't accept certs that were not valid yet, I rebuilt the binary with this quick patch.

changes.txt

cpu commented 5 years ago

@jsha Can this be closed? It seems like the linked PR (https://github.com/letsencrypt/ct-woodpecker/pull/103) was abandoned.

jsha commented 5 years ago

Trillian upstream changed the definition of reject_expired so it only rejects expired, and doesn't reject backdated certificates. So the problem this was causing us is gone, and we probably won't fix it.

It would be slightly nice to tweak this just so the test certs are a little more "realistic" but I don't think it's worth keeping an issue around for.