zlint icon indicating copy to clipboard operation
zlint copied to clipboard

Use help Method beforeoron instead of

Open mtgag opened this issue 2 years ago • 5 comments

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.

mtgag avatar May 17 '23 05:05 mtgag

The implementation in the pull request and jzlint was motivated by the following discussions/pull requests:

https://github.com/zmap/zlint/issues/467 https://github.com/zmap/zlint/pull/469

Certificate eeServerCertValidOver398.pem (https://github.com/zmap/zlint/blob/master/v3/testdata/eeServerCertValidOver398.pem) from the testdata, as the name suggests, is valid over 398 days and the test lint_e_server_cert_valid_time_longer_than_398_days_test.go treats it as an error for the corresponding lint.

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.

mtgag avatar May 19 '23 04:05 mtgag

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.

CBonnell avatar May 19 '23 13:05 CBonnell

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.

mtgag avatar May 19 '23 13:05 mtgag

@CBonnell, are we in better shape after the latest change from @mtgag?

zakird avatar Feb 25 '24 20:02 zakird

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.

CBonnell avatar Feb 26 '24 06:02 CBonnell