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

Fix handling of notBefore/notAfter in issued certs #103

Closed andygabby closed 5 years ago

andygabby commented 5 years ago

If windowStart/windowEnd were defined, an issued cert would be created with notBefore/notAfter set close to those same values. This would cause problems issuing to a shard that wouldn't accept certs dated in the future (which is Trillian's behavior with "reject_expired: true").

Now certs issued for sharded logs, notBefore is set to now and notAfter set to ~windowEnd, except when windowEnd is in the past. In that case the cert notBefore is backdated 90 days from windowEnd.

Fixes #99

coveralls commented 5 years ago

Pull Request Test Coverage Report for Build 761


Totals Coverage Status
Change from base Build 757: 0.08%
Covered Lines: 848
Relevant Lines: 1230

💛 - Coveralls
jsha commented 5 years ago

Other options we considered:

Are we certain that the Trillian folks won't accept a PR that would allow future dated certificates? Is there an issue with the upstream to discuss?

I pinged them in chat but haven't heard back. I'll turn that into an issue.

Having certificates with lifetimes that stretch across every shard is a bit clumsy and I'm worried we're picking this fix without having considered others first.

We definitely spent a bunch of time thinking about alternatives, but figured since the team had talked about wanting to enable reject_expired, we should at least try to support that.

Note that "lifetimes that stretch across every shard" isn't really that clumsy. We expect to receive certificates of many different lifetimes, put into whichever shard matches their expiration date. It will be quite common for certificates' NotBefore to lie outside the shard they are submitted to.

cpu commented 5 years ago

I pinged them in chat but haven't heard back. I'll turn that into an issue.

Thanks :+1:

We expect to receive certificates of many different lifetimes

True, but nothing with a validity period close to 825 days...

jsha commented 5 years ago

Update: Trillian made a change so reject_expired only rejects expired certificates, not future-dated ones. So we can revamp this PR to have a constant validity period, even if that means we create future-dated certificates.

cpu commented 5 years ago

@jsha @andygabby Can this PR be closed until its ready to be reviewed? It makes it easier to keep track of outstanding work.

jsha commented 5 years ago

Sure, sounds good.