Conversation
If there's discrepencies between the PSL and Zlint TLD list, this will take the most conservative option of rejecting if either list doesn't have a TLD.
|
Tests fail because a domain was blocked for the wrong reason, I'll look at an update there in a bit. |
This domain is now rejected earlier by not treating .arpa as a valid TLD, so use a different one to get the expected error.
If there's any disagreement over what "The TLD" is, IsInTLDMap is not the correct thing to use. zlint has add/remove dates for TLD, which we should use time.Now() for issuance. I don't think we need to plumb in a fake clock here.
This reverts commit f5f7613.
|
I was incorrect about the reason for the failure. The "TLD" returned from PSL is a string like "co.uk" or "in-addr.arpa", but IsInTLDMap only takes the final component "uk". So I have switched to use HasValidTLD on the full name instead. zlint has add/remove dates for TLD, which we should use time.Now() for issuance. cert-checker could inadvertently alert here if a TLD is removed between issuance and cert-checker running. I'm not sure if that's worth fixing, or if it is, I don't think I want to take on plumbing fake clocks around. |
aarongable
left a comment
aarongable
left a comment
There was a problem hiding this comment.
The use of bare time.Now() does make me slightly concerned. I'm going to take some more time to think about this and whether there are viable alternatives.
|
I think the best path forward is passing the certificate's issuance time (or time.NotBefore) in, but that's a deeper slice across the codebase. |
3 participants
If there's discrepencies between the PSL and Zlint TLD list, this will take the
most conservative option of rejecting if either list doesn't have a TLD.