Use help Method beforeoron instead of #717
Conversation
This reverts commit 6c23670.
util: gtld_map autopull updates for 2021-10-21T07:25:20 UTC
…ificates that cover the edge cases
| @@ -42,7 +42,7 @@ func (l *subCertValidTimeLongerThan39Months) CheckApplies(c *x509.Certificate) b | |||
| } | |||
|
|
|||
| func (l *subCertValidTimeLongerThan39Months) Execute(c *x509.Certificate) *lint.LintResult { | |||
| if c.NotBefore.AddDate(0, 39, 0).Before(c.NotAfter) { | |||
| if util.BeforeOrOn(c.NotBefore.AddDate(0, 39, 0), c.NotAfter) { | |||
There was a problem hiding this comment.
Is this actually correct to make this change? The Description of the lint says that the lint validity period must be "no greater than 39 months". A certificate that is valid for exactly 39 months doesn't have a validity period greater than 39 months so I think the original code is the correct version here.
| @@ -47,7 +47,7 @@ func (l *evValidTooLong) CheckApplies(c *x509.Certificate) bool { | |||
| } | |||
|
|
|||
| func (l *evValidTooLong) Execute(c *x509.Certificate) *lint.LintResult { | |||
| if c.NotBefore.AddDate(0, 27, 0).Before(c.NotAfter) { | |||
| if util.BeforeOrOn(c.NotBefore.AddDate(0, 27, 0), c.NotAfter) { | |||
There was a problem hiding this comment.
Same comment here. The lint description says the validity period must be "27 months... or less", a 27 month validity would therefore be acceptable and yet this change makes that an error case.
|
The implementation in the pull request and jzlint was motivated by the following discussions/pull requests: Certificate eeServerCertValidOver398.pem Accordingly, a certificate which has a notBefore "Jan 1 00:00:00 2023" and a notAfter "Feb 1 00:00:00 2023" has a validity of two months, which I believe is correct when the granularity of the duration is expressed in months. |
|
The definition of Validity Period in the BRs was not aligned with the RFC 5280 definition until SC31. Changing the validity period calculation to align with 5280 for certificates issued prior to the SC31 effective date will yield false positive findings/errors. Given this, I agree with @robplee that the changes proposed in this PR do not align with the BRs as written. |
|
Ok. Then, let us keep the certificates and change the assertion in the test to have an explicit test for this behaviour (or simply close this PR). I will commit this soon. |
…on in the pull request
|
I think it's fine, as there appears to be no actual logic change introduced as part of this PR. It appears to merely add test cases of existing lints. |
Two lints use the before method to the calculate the validity of certificates, while the BeforeOrOn help method should be used. Two certificates that test the edge case are also added.